Skip to content

Commit

Permalink
cfg: remove status cache from UAConfig
Browse files Browse the repository at this point in the history
The UAConfig class should not handle the status cache as it is not
tied to any Pro Client configuration. We have created a separate
file object for it that is being handled directly by the status
module now
  • Loading branch information
lucasmoura committed Feb 26, 2024
1 parent 7ef9901 commit a4b2615
Show file tree
Hide file tree
Showing 16 changed files with 123 additions and 93 deletions.
1 change: 1 addition & 0 deletions features/attach_validtoken.feature
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ Feature: Command behaviour when attaching a machine to an Ubuntu Pro
This machine is already attached to '.+'
To use a different subscription first run: sudo pro detach.
"""
And I verify that `/var/lib/ubuntu-advantage/status.json` is world readable

Examples: ubuntu release packages
| release | machine_type | downrev_pkg | cc_status | cis_or_usg | cis | fips | livepatch_desc |
Expand Down
16 changes: 15 additions & 1 deletion features/steps/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import yaml
from behave import then, when
from hamcrest import assert_that, matches_regexp
from hamcrest import assert_that, equal_to, matches_regexp

from features.steps.shell import when_i_run_command
from features.util import SUT, process_template_vars
Expand Down Expand Up @@ -160,3 +160,17 @@ def when_i_move_file_from_one_machine_to_another(
when_i_create_file_with_content(
context, dest_path, machine_name=dest_machine, text=content
)


@then("I verify that `{file_path}` is world readable")
def then_i_verify_that_file_is_world_readable(
context, file_path, machine_name=SUT
):
when_i_run_command(
context,
"stat -c '%a' {}".format(file_path),
"with sudo",
machine_name=machine_name,
)

assert_that(context.process.stdout.strip(), equal_to("644"))
2 changes: 2 additions & 0 deletions uaclient/cli/tests/test_cli_attach.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ def test_attach_config_enable_services(
),
),
)
@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.lock.check_lock_info", return_value=(-1, ""))
@mock.patch(
"uaclient.entitlements.check_entitlement_apt_directives_are_unique",
Expand All @@ -567,6 +568,7 @@ def test_attach_when_one_service_fails_to_enable(
_m_attachment_data_file_write,
_m_check_ent_apt_directives,
_m_check_lock_info,
_m_status_cache_file,
expected_exception,
expected_msg,
expected_outer_msg,
Expand Down
8 changes: 8 additions & 0 deletions uaclient/cli/tests/test_cli_enable.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ def test_unattached_invalid_and_valid_service_error_message(
assert expected == json.loads(fake_stdout.getvalue())

@pytest.mark.parametrize("assume_yes", (True, False))
@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.lock.check_lock_info", return_value=(-1, ""))
@mock.patch(
"uaclient.cli.contract.UAContractClient.update_activity_token",
Expand All @@ -444,6 +445,7 @@ def test_assume_yes_passed_to_service_init(
_m_get_available_resources,
_m_update_activity_token,
_m_check_lock_info,
_m_status_cache_file,
m_refresh,
assume_yes,
FakeConfig,
Expand Down Expand Up @@ -481,6 +483,7 @@ def test_assume_yes_passed_to_service_init(
)
] == m_entitlement_cls.call_args_list

@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.lock.check_lock_info", return_value=(-1, ""))
@mock.patch("uaclient.status.get_available_resources", return_value={})
@mock.patch("uaclient.entitlements.entitlement_factory")
Expand All @@ -492,6 +495,7 @@ def test_entitlements_not_found_disabled_and_enabled(
_m_get_available_resources,
_m_check_lock_info,
_m_refresh,
_m_status_cache_file,
event,
FakeConfig,
):
Expand Down Expand Up @@ -610,6 +614,7 @@ def factory_side_effect(cfg, name, variant):
assert expected == json.loads(fake_stdout.getvalue())

@pytest.mark.parametrize("beta_flag", ((False), (True)))
@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.lock.check_lock_info", return_value=(-1, ""))
@mock.patch("uaclient.status.get_available_resources", return_value={})
@mock.patch("uaclient.entitlements.entitlement_factory")
Expand All @@ -620,6 +625,7 @@ def test_entitlements_not_found_and_beta(
m_entitlement_factory,
_m_get_available_resources,
_m_check_lock_info,
_m_status_cache_file,
_m_refresh,
beta_flag,
event,
Expand Down Expand Up @@ -766,6 +772,7 @@ def valid_services_side_effect(cfg, allow_beta, all_names=False):
}
assert expected == json.loads(fake_stdout.getvalue())

@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.lock.check_lock_info", return_value=(-1, ""))
@mock.patch(
"uaclient.cli.contract.UAContractClient.update_activity_token",
Expand All @@ -776,6 +783,7 @@ def test_print_message_when_can_enable_fails(
_m_get_available_resources,
_m_update_activity_token,
_m_check_lock_info,
_m_status_cache_file,
_m_refresh,
event,
FakeConfig,
Expand Down
12 changes: 12 additions & 0 deletions uaclient/cli/tests/test_cli_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,12 @@ def test_status_help(
),
),
)
@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.status.format_expires", return_value="formatteddate")
def test_attached(
self,
_m_format_expires,
_m_status_cache_file,
_m_get_contract_information,
_m_get_avail_resources,
_m_should_reboot,
Expand Down Expand Up @@ -449,8 +451,10 @@ def test_attached(
(False),
),
)
@mock.patch("uaclient.files.state_files.status_cache_file.write")
def test_unattached(
self,
_m_status_cache_file,
_m_get_contract_information,
_m_get_avail_resources,
_m_should_reboot,
Expand Down Expand Up @@ -497,6 +501,7 @@ def test_simulated(
)
assert expected == capsys.readouterr()[0]

@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.lock.check_lock_info")
@mock.patch("uaclient.version.get_version", return_value="test_version")
@mock.patch("uaclient.system.subp")
Expand All @@ -507,6 +512,7 @@ def test_wait_blocks_until_lock_released(
_m_subp,
_m_get_version,
m_check_lock_info,
_m_status_cache_file,
_m_get_contract_information,
_m_get_avail_resources,
_m_should_reboot,
Expand Down Expand Up @@ -558,8 +564,10 @@ def fake_sleep(seconds):
},
),
)
@mock.patch("uaclient.files.state_files.status_cache_file.write")
def test_unattached_formats(
self,
_m_status_cache_file,
_m_get_contract_information,
_m_get_avail_resources,
_m_should_reboot,
Expand Down Expand Up @@ -668,8 +676,10 @@ def test_unattached_formats(
),
)
@pytest.mark.parametrize("use_all", (True, False))
@mock.patch("uaclient.files.state_files.status_cache_file.write")
def test_attached_formats(
self,
_m_status_cache_file,
_m_get_contract_information,
_m_get_avail_resources,
_m_should_reboot,
Expand Down Expand Up @@ -978,10 +988,12 @@ def test_error_on_connectivity_errors(
"encoding,expected_dash",
(("utf-8", "\u2014"), ("UTF-8", "\u2014"), ("ascii", "-")),
)
@mock.patch("uaclient.files.state_files.status_cache_file.write")
@mock.patch("uaclient.status.format_expires", return_value="formatteddate")
def test_unicode_dash_replacement_when_unprintable(
self,
_m_format_expires,
_m_status_cache_file,
_m_get_contract_information,
_m_get_avail_resources,
_m_should_reboot,
Expand Down
4 changes: 1 addition & 3 deletions uaclient/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@


class UAConfig:
data_paths = {
"status-cache": DataPath("status.json", False),
} # type: Dict[str, DataPath]
data_paths = {} # type: Dict[str, DataPath]

ua_scoped_proxy_options = ("ua_apt_http_proxy", "ua_apt_https_proxy")
global_scoped_proxy_options = (
Expand Down
1 change: 0 additions & 1 deletion uaclient/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ def for_attached_machine(

config = cls()
config.machine_token_file._machine_token = machine_token
config.write_cache("status-cache", status_cache)
return config

def override_features(self, features_override):
Expand Down
3 changes: 2 additions & 1 deletion uaclient/entitlements/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
ContractStatus,
UserFacingStatus,
)
from uaclient.files.state_files import status_cache_file
from uaclient.types import MessagingOperationsDict, StaticAffordance
from uaclient.util import is_config_value_true

Expand Down Expand Up @@ -1196,7 +1197,7 @@ def is_access_expired(self) -> bool:

def _check_application_status_on_cache(self) -> ApplicationStatus:
"""Check on the state of application on the status cache."""
status_cache = self.cfg.read_cache("status-cache")
status_cache = status_cache_file.read()

if status_cache is None:
return ApplicationStatus.DISABLED
Expand Down
3 changes: 2 additions & 1 deletion uaclient/entitlements/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
CanDisableFailure,
CanDisableFailureReason,
)
from uaclient.files.state_files import status_cache_file

event = event_logger.get_event_logger()
LOG = logging.getLogger(util.replace_top_level_logger_name(__name__))
Expand Down Expand Up @@ -395,7 +396,7 @@ def process_contract_deltas(
delta_directives = delta_entitlement.get("directives", {})
delta_apt_url = delta_directives.get("aptURL")
delta_packages = delta_directives.get("additionalPackages")
status_cache = self.cfg.read_cache("status-cache")
status_cache = status_cache_file.read()

if delta_directives and status_cache:
application_status = self._check_application_status_on_cache()
Expand Down
28 changes: 16 additions & 12 deletions uaclient/entitlements/tests/test_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ def test_not_allow_enable_logs_message_when_inactive_enable_by_default(
assert expected_msg in capsys.readouterr()[1]

@pytest.mark.parametrize("packages", ([], ["extremetuxracer"]))
@mock.patch(
"uaclient.files.state_files.status_cache_file.read", return_value=None
)
@mock.patch.object(RepoTestEntitlement, "install_packages")
@mock.patch(M_PATH + "apt.remove_auth_apt_repo")
@mock.patch.object(RepoTestEntitlement, "setup_apt_config")
Expand All @@ -182,6 +185,7 @@ def test_update_apt_config_and_install_packages_when_active(
m_setup_apt_config,
m_remove_auth_apt_repo,
m_install_packages,
_m_status_cache,
packages,
entitlement,
):
Expand Down Expand Up @@ -215,10 +219,10 @@ def test_update_apt_config_and_install_packages_when_active(
@pytest.mark.parametrize(
"series,file_extension", (("jammy", "list"), ("noble", "sources"))
)
@mock.patch("uaclient.files.state_files.status_cache_file.read")
@mock.patch(
"uaclient.entitlements.base.UAEntitlement.process_contract_deltas"
)
@mock.patch("uaclient.config.UAConfig.read_cache")
@mock.patch(M_PATH + "system.get_release_info")
@mock.patch(M_PATH + "apt.remove_auth_apt_repo")
@mock.patch.object(RepoTestEntitlement, "setup_apt_config")
Expand All @@ -231,16 +235,16 @@ def test_remove_old_auth_apt_repo_when_active_and_apt_url_delta(
m_setup_apt_config,
m_remove_auth_apt_repo,
m_release_info,
m_read_cache,
m_process_contract_deltas,
m_status_cache_read,
series,
file_extension,
entitlement,
):
"""Remove old apt url when aptURL delta occurs on active service."""
m_check_apt_url_applied.return_value = False
m_process_contract_deltas.return_value = False
m_read_cache.return_value = {
m_status_cache_read.return_value = {
"services": [{"name": "repotest", "status": "enabled"}]
}
m_release_info.return_value.series = series
Expand Down Expand Up @@ -271,9 +275,9 @@ def test_remove_old_auth_apt_repo_when_active_and_apt_url_delta(
]
assert apt_auth_remove_calls == m_remove_auth_apt_repo.call_args_list
assert [
mock.call("status-cache"),
mock.call("status-cache"),
] == m_read_cache.call_args_list
mock.call(),
mock.call(),
] == m_status_cache_read.call_args_list
assert 1 == m_process_contract_deltas.call_count

assert [
Expand All @@ -284,7 +288,7 @@ def test_remove_old_auth_apt_repo_when_active_and_apt_url_delta(
@mock.patch(
"uaclient.entitlements.base.UAEntitlement.process_contract_deltas"
)
@mock.patch("uaclient.config.UAConfig.read_cache")
@mock.patch("uaclient.files.state_files.status_cache_file.read")
@mock.patch(M_PATH + "system.get_release_info")
@mock.patch(M_PATH + "apt.remove_auth_apt_repo")
@mock.patch.object(RepoTestEntitlement, "setup_apt_config")
Expand All @@ -297,14 +301,14 @@ def test_system_does_not_change_when_apt_url_delta_already_applied(
m_setup_apt_config,
m_remove_auth_apt_repo,
m_release_info,
m_read_cache,
m_status_cache_read,
m_process_contract_deltas,
entitlement,
):
"""Do not change system if apt url delta is already applied."""
m_check_apt_url_applied.return_value = True
m_process_contract_deltas.return_value = False
m_read_cache.return_value = {
m_status_cache_read.return_value = {
"services": [{"name": "repotest", "status": "enabled"}]
}
assert entitlement.process_contract_deltas(
Expand All @@ -327,9 +331,9 @@ def test_system_does_not_change_when_apt_url_delta_already_applied(
assert m_remove_auth_apt_repo.call_count == 0

assert [
mock.call("status-cache"),
mock.call("status-cache"),
] == m_read_cache.call_args_list
mock.call(),
mock.call(),
] == m_status_cache_read.call_args_list
assert 1 == m_process_contract_deltas.call_count

assert [
Expand Down
Loading

0 comments on commit a4b2615

Please sign in to comment.