From 4b91fd80fcd9a1dd277b76e7b3fa3f1f591a8bdd Mon Sep 17 00:00:00 2001 From: Renan Rodrigo Date: Fri, 5 Jul 2024 15:48:53 -0300 Subject: [PATCH] cli: refactor refresh to the new approach Signed-off-by: Renan Rodrigo --- features/help.feature | 22 +++++ uaclient/cli/__init__.py | 79 +---------------- uaclient/cli/refresh.py | 79 +++++++++++++++++ uaclient/cli/tests/test_cli_config.py | 2 +- uaclient/cli/tests/test_cli_config_set.py | 4 +- uaclient/cli/tests/test_cli_config_show.py | 2 +- uaclient/cli/tests/test_cli_config_unset.py | 2 +- uaclient/cli/tests/test_cli_disable.py | 2 +- uaclient/cli/tests/test_cli_refresh.py | 86 +++++++------------ .../cli/tests/test_cli_security_status.py | 2 +- 10 files changed, 142 insertions(+), 138 deletions(-) create mode 100644 uaclient/cli/refresh.py diff --git a/features/help.feature b/features/help.feature index d41861e96b..7fc96631c1 100644 --- a/features/help.feature +++ b/features/help.feature @@ -224,6 +224,28 @@ Feature: Pro Client help text simulate the output status using a provided token --all Include unavailable and beta services """ + When I run `pro status --help` as non-root + Then stdout matches regexp: + """ + usage: pro refresh \[flags\] + + Refresh three distinct Ubuntu Pro related artifacts in the system: + + * contract: Update contract details from the server. + * config: Reload the config file. + * messages: Update APT and MOTD messages related to UA. + + You can individually target any of the three specific actions, + by passing the target name to the command. If no `target` + is specified, all targets are refreshed. + + positional arguments: + \{contract,config,messages\} + Target to refresh. + + (optional arguments|options): + -h, --help show this help message and exit + """ Examples: ubuntu release | release | machine_type | diff --git a/uaclient/cli/__init__.py b/uaclient/cli/__init__.py index 4d692c2a8e..844db90f45 100644 --- a/uaclient/cli/__init__.py +++ b/uaclient/cli/__init__.py @@ -11,7 +11,6 @@ apt, apt_news, config, - contract, defaults, entitlements, event_logger, @@ -37,12 +36,12 @@ from uaclient.cli.disable import disable_command from uaclient.cli.enable import enable_command from uaclient.cli.fix import fix_command +from uaclient.cli.refresh import refresh_command from uaclient.cli.security_status import security_status_command from uaclient.cli.status import status_command from uaclient.entitlements.entitlement_status import ApplicationStatus from uaclient.files import state_files from uaclient.log import get_user_or_root_log_file_path -from uaclient.timer.update_messaging import refresh_motd, update_motd_messages UA_AUTH_TOKEN_URL = "https://auth.contracts.canonical.com" @@ -60,6 +59,7 @@ disable_command, enable_command, fix_command, + refresh_command, security_status_command, status_command, ] @@ -169,26 +169,6 @@ def config_parser(parser): return parser -def refresh_parser(parser): - """Build or extend an arg parser for refresh subcommand.""" - parser.prog = "refresh" - parser.usage = USAGE_TMPL.format( - name=NAME, command="refresh [contract|config|messages]" - ) - - parser._optionals.title = messages.CLI_FLAGS - parser.formatter_class = argparse.RawDescriptionHelpFormatter - parser.description = messages.CLI_REFRESH_DESC - parser.add_argument( - "target", - choices=["contract", "config", "messages"], - nargs="?", - default=None, - help=messages.CLI_REFRESH_TARGET, - ) - return parser - - def help_parser(parser, cfg: config.UAConfig): """Build or extend an arg parser for help subcommand.""" usage = USAGE_TMPL.format(name=NAME, command="help [service]") @@ -510,12 +490,6 @@ def get_parser(cfg: config.UAConfig): help_parser(parser_help, cfg=cfg) parser_help.set_defaults(action=action_help) - parser_refresh = subparsers.add_parser( - "refresh", help=messages.CLI_ROOT_REFRESH - ) - parser_refresh.set_defaults(action=action_refresh) - refresh_parser(parser_refresh) - parser_version = subparsers.add_parser( "version", help=messages.CLI_ROOT_VERSION.format(name=NAME) ) @@ -551,55 +525,6 @@ def print_version(_args=None, cfg=None, **kwargs): print(version.get_version()) -def _action_refresh_config(args, cfg: config.UAConfig): - try: - cfg.process_config() - except RuntimeError as exc: - LOG.exception(exc) - raise exceptions.RefreshConfigFailure() - print(messages.REFRESH_CONFIG_SUCCESS) - - -@cli_util.assert_attached() -def _action_refresh_contract(_args, cfg: config.UAConfig): - try: - contract.refresh(cfg) - except exceptions.ConnectivityError: - raise exceptions.RefreshContractFailure() - print(messages.REFRESH_CONTRACT_SUCCESS) - - -def _action_refresh_messages(_args, cfg: config.UAConfig): - # Not performing any exception handling here since both of these - # functions should raise UbuntuProError exceptions, which are - # covered by the main_error_handler decorator - try: - update_motd_messages(cfg) - refresh_motd() - if cfg.apt_news: - apt_news.update_apt_news(cfg) - except Exception as exc: - LOG.exception(exc) - raise exceptions.RefreshMessagesFailure() - else: - print(messages.REFRESH_MESSAGES_SUCCESS) - - -@cli_util.assert_root -@cli_util.assert_lock_file("pro refresh") -def action_refresh(args, *, cfg: config.UAConfig, **kwargs): - if args.target is None or args.target == "config": - _action_refresh_config(args, cfg) - - if args.target is None or args.target == "contract": - _action_refresh_contract(args, cfg) - - if args.target is None or args.target == "messages": - _action_refresh_messages(args, cfg) - - return 0 - - def configure_apt_proxy( cfg: config.UAConfig, scope: AptProxyScope, diff --git a/uaclient/cli/refresh.py b/uaclient/cli/refresh.py new file mode 100644 index 0000000000..8116fbc50b --- /dev/null +++ b/uaclient/cli/refresh.py @@ -0,0 +1,79 @@ +import logging + +from uaclient import apt_news, config, contract, exceptions, messages, util +from uaclient.cli import cli_util +from uaclient.cli.commands import ProArgument, ProArgumentGroup, ProCommand +from uaclient.timer.update_messaging import refresh_motd, update_motd_messages + +LOG = logging.getLogger(util.replace_top_level_logger_name(__name__)) + + +def _action_refresh_config(args, cfg: config.UAConfig): + try: + cfg.process_config() + except RuntimeError as exc: + LOG.exception(exc) + raise exceptions.RefreshConfigFailure() + print(messages.REFRESH_CONFIG_SUCCESS) + + +@cli_util.assert_attached() +def _action_refresh_contract(_args, cfg: config.UAConfig): + try: + contract.refresh(cfg) + except exceptions.ConnectivityError: + raise exceptions.RefreshContractFailure() + print(messages.REFRESH_CONTRACT_SUCCESS) + + +def _action_refresh_messages(_args, cfg: config.UAConfig): + # Not performing any exception handling here since both of these + # functions should raise UbuntuProError exceptions, which are + # covered by the main_error_handler decorator + try: + update_motd_messages(cfg) + refresh_motd() + if cfg.apt_news: + apt_news.update_apt_news(cfg) + except Exception as exc: + LOG.exception(exc) + raise exceptions.RefreshMessagesFailure() + else: + print(messages.REFRESH_MESSAGES_SUCCESS) + + +@cli_util.assert_root +@cli_util.assert_lock_file("pro refresh") +def action_refresh(args, *, cfg: config.UAConfig, **kwargs): + if args.target is None or args.target == "config": + _action_refresh_config(args, cfg) + + if args.target is None or args.target == "contract": + _action_refresh_contract(args, cfg) + + if args.target is None or args.target == "messages": + _action_refresh_messages(args, cfg) + + return 0 + + +refresh_command = ProCommand( + "refresh", + help=messages.CLI_ROOT_REFRESH, + description=messages.CLI_REFRESH_DESC, + action=action_refresh, + preserve_description=True, + argument_groups=[ + ProArgumentGroup( + arguments=[ + ProArgument( + "target", + help=messages.CLI_REFRESH_TARGET, + nargs="?", + choices=["contract", "config", "messages"], + default=None, + ) + ] + ) + ], +) diff --git a/uaclient/cli/tests/test_cli_config.py b/uaclient/cli/tests/test_cli_config.py index cd6d050c02..4a611b205d 100644 --- a/uaclient/cli/tests/test_cli_config.py +++ b/uaclient/cli/tests/test_cli_config.py @@ -23,7 +23,7 @@ @mock.patch("uaclient.cli.LOG.error") @mock.patch("uaclient.log.setup_cli_logging") -@mock.patch(M_PATH + "contract.get_available_resources") +@mock.patch("uaclient.contract.get_available_resources") class TestMainConfig: @pytest.mark.parametrize("additional_params", ([], ["--help"])) def test_config_help( diff --git a/uaclient/cli/tests/test_cli_config_set.py b/uaclient/cli/tests/test_cli_config_set.py index c9fdc97baf..964a874090 100644 --- a/uaclient/cli/tests/test_cli_config_set.py +++ b/uaclient/cli/tests/test_cli_config_set.py @@ -65,7 +65,7 @@ class TestMainConfigSet: ), ), ) - @mock.patch("uaclient.cli.contract.get_available_resources") + @mock.patch("uaclient.contract.get_available_resources") def test_set_error_with_help_on_invalid_key_value_pair( self, _m_resources, @@ -92,7 +92,7 @@ def test_set_error_with_help_on_invalid_key_value_pair( @mock.patch("uaclient.config.user_config_file.user_config.write") -@mock.patch("uaclient.cli.contract.get_available_resources") +@mock.patch("uaclient.contract.get_available_resources") class TestActionConfigSet: @mock.patch("uaclient.util.we_are_currently_root", return_value=False) def test_set_error_on_non_root_user( diff --git a/uaclient/cli/tests/test_cli_config_show.py b/uaclient/cli/tests/test_cli_config_show.py index 291462f8ac..19e0de791a 100644 --- a/uaclient/cli/tests/test_cli_config_show.py +++ b/uaclient/cli/tests/test_cli_config_show.py @@ -18,7 +18,7 @@ @mock.patch("uaclient.cli.logging.error") @mock.patch("uaclient.log.setup_cli_logging") -@mock.patch(M_PATH + "contract.get_available_resources") +@mock.patch("uaclient.contract.get_available_resources") class TestMainConfigShow: def test_config_show_help( self, diff --git a/uaclient/cli/tests/test_cli_config_unset.py b/uaclient/cli/tests/test_cli_config_unset.py index cccf3d5346..5a51aa2103 100644 --- a/uaclient/cli/tests/test_cli_config_unset.py +++ b/uaclient/cli/tests/test_cli_config_unset.py @@ -25,7 +25,7 @@ @mock.patch("uaclient.log.setup_cli_logging") -@mock.patch("uaclient.cli.contract.get_available_resources") +@mock.patch("uaclient.contract.get_available_resources") class TestMainConfigUnSet: @pytest.mark.parametrize( "kv_pair,err_msg", diff --git a/uaclient/cli/tests/test_cli_disable.py b/uaclient/cli/tests/test_cli_disable.py index ba7d9ed865..35c25ef7c4 100644 --- a/uaclient/cli/tests/test_cli_disable.py +++ b/uaclient/cli/tests/test_cli_disable.py @@ -47,7 +47,7 @@ class TestDisable: @mock.patch("uaclient.cli.disable._enabled_services") @mock.patch("uaclient.lock.check_lock_info", return_value=(-1, "")) @mock.patch( - "uaclient.cli.contract.UAContractClient.update_activity_token", + "uaclient.cli.disable.contract.UAContractClient.update_activity_token", ) @mock.patch("uaclient.cli.entitlements.entitlement_factory") @mock.patch("uaclient.cli.entitlements.valid_services") diff --git a/uaclient/cli/tests/test_cli_refresh.py b/uaclient/cli/tests/test_cli_refresh.py index 11f47a5a49..98d766a7ce 100644 --- a/uaclient/cli/tests/test_cli_refresh.py +++ b/uaclient/cli/tests/test_cli_refresh.py @@ -2,47 +2,13 @@ import pytest from uaclient import exceptions, lock, messages -from uaclient.cli import action_refresh, main +from uaclient.cli.refresh import refresh_command from uaclient.files.notices import Notice -HELP_OUTPUT = """\ -usage: pro refresh [contract|config|messages] [flags] - -Refresh three distinct Ubuntu Pro related artifacts in the system: - -* contract: Update contract details from the server. -* config: Reload the config file. -* messages: Update APT and MOTD messages related to UA. - -You can individually target any of the three specific actions, -by passing the target name to the command. If no `target` -is specified, all targets are refreshed. - -positional arguments: - {contract,config,messages} - Target to refresh. - -Flags: - -h, --help show this help message and exit -""" +M_PATH = "uaclient.cli.refresh." class TestActionRefresh: - @mock.patch("uaclient.log.setup_cli_logging") - @mock.patch("uaclient.cli.contract.get_available_resources") - def test_refresh_help( - self, _m_resources, _m_setup_logging, capsys, FakeConfig - ): - with pytest.raises(SystemExit): - with mock.patch("sys.argv", ["/usr/bin/ua", "refresh", "--help"]): - with mock.patch( - "uaclient.config.UAConfig", - return_value=FakeConfig(), - ): - main() - out, _err = capsys.readouterr() - assert HELP_OUTPUT in out - @mock.patch("uaclient.util.we_are_currently_root", return_value=False) def test_non_root_users_are_rejected( self, we_are_currently_root, FakeConfig, fake_machine_token_file @@ -52,7 +18,7 @@ def test_non_root_users_are_rejected( cfg = FakeConfig() fake_machine_token_file.attached = True with pytest.raises(exceptions.NonRootUserError): - action_refresh(mock.MagicMock(), cfg=cfg) + refresh_command.action(mock.MagicMock(), cfg=cfg) @pytest.mark.parametrize( "target, expect_unattached_error", @@ -75,9 +41,11 @@ def test_not_attached_errors( with mock.patch.object(lock, "lock_data_file"): if expect_unattached_error: with pytest.raises(exceptions.UnattachedError): - action_refresh(mock.MagicMock(target=target), cfg=cfg) + refresh_command.action( + mock.MagicMock(target=target), cfg=cfg + ) else: - action_refresh(mock.MagicMock(target=target), cfg=cfg) + refresh_command.action(mock.MagicMock(target=target), cfg=cfg) @mock.patch("uaclient.lock.check_lock_info") @mock.patch("time.sleep") @@ -95,7 +63,7 @@ def test_lock_file_exists( fake_machine_token_file.attached = True m_check_lock_info.return_value = (123, "pro disable") with pytest.raises(exceptions.LockHeldError) as err: - action_refresh(mock.MagicMock(), cfg=cfg) + refresh_command.action(mock.MagicMock(), cfg=cfg) assert 12 == m_check_lock_info.call_count expected_msg = messages.E_LOCK_HELD_ERROR.format( lock_request="pro refresh", lock_holder="pro disable", pid=123 @@ -125,7 +93,9 @@ def test_refresh_contract_error_on_failure_to_update_contract( with pytest.raises(exceptions.UbuntuProError) as excinfo: with mock.patch.object(lock, "lock_data_file"): - action_refresh(mock.MagicMock(target="contract"), cfg=cfg) + refresh_command.action( + mock.MagicMock(target="contract"), cfg=cfg + ) assert messages.E_REFRESH_CONTRACT_FAILURE.msg == excinfo.value.msg assert [ @@ -150,7 +120,9 @@ def test_refresh_contract_happy_path( cfg = FakeConfig() fake_machine_token_file.attached = True with mock.patch.object(lock, "lock_data_file"): - ret = action_refresh(mock.MagicMock(target="contract"), cfg=cfg) + ret = refresh_command.action( + mock.MagicMock(target="contract"), cfg=cfg + ) assert 0 == ret assert messages.REFRESH_CONTRACT_SUCCESS in capsys.readouterr()[0] @@ -160,7 +132,7 @@ def test_refresh_contract_happy_path( ] == m_remove_notice.call_args_list @mock.patch("uaclient.lock.check_lock_info", return_value=(-1, "")) - @mock.patch("uaclient.cli.update_motd_messages") + @mock.patch(M_PATH + "update_motd_messages") def test_refresh_messages_error( self, m_update_motd, _m_check_lock_info, FakeConfig ): @@ -169,7 +141,7 @@ def test_refresh_messages_error( with pytest.raises(exceptions.UbuntuProError) as excinfo: with mock.patch.object(lock, "lock_data_file"): - action_refresh( + refresh_command.action( mock.MagicMock(target="messages"), cfg=FakeConfig() ) @@ -180,7 +152,7 @@ def test_refresh_messages_error( @mock.patch("uaclient.timer.update_messaging.exists", return_value=True) @mock.patch("uaclient.timer.update_messaging.LOG.exception") @mock.patch("uaclient.system.subp") - @mock.patch("uaclient.cli.update_motd_messages") + @mock.patch(M_PATH + "update_motd_messages") def test_refresh_messages_doesnt_fail_if_update_notifier_does( self, m_update_motd, @@ -196,7 +168,7 @@ def test_refresh_messages_doesnt_fail_if_update_notifier_does( m_subp.side_effect = [subp_exc, ""] with mock.patch.object(lock, "lock_data_file"): - ret = action_refresh( + ret = refresh_command.action( mock.MagicMock(target="messages"), cfg=FakeConfig() ) @@ -207,8 +179,8 @@ def test_refresh_messages_doesnt_fail_if_update_notifier_does( @mock.patch("uaclient.lock.check_lock_info", return_value=(-1, "")) @mock.patch("uaclient.apt_news.update_apt_news") - @mock.patch("uaclient.cli.refresh_motd") - @mock.patch("uaclient.cli.update_motd_messages") + @mock.patch(M_PATH + "refresh_motd") + @mock.patch(M_PATH + "update_motd_messages") def test_refresh_messages_happy_path( self, m_update_motd, @@ -221,7 +193,9 @@ def test_refresh_messages_happy_path( """On success from request_updates_contract root user can refresh.""" cfg = FakeConfig() with mock.patch.object(lock, "lock_data_file"): - ret = action_refresh(mock.MagicMock(target="messages"), cfg=cfg) + ret = refresh_command.action( + mock.MagicMock(target="messages"), cfg=cfg + ) assert 0 == ret assert messages.REFRESH_MESSAGES_SUCCESS in capsys.readouterr()[0] @@ -251,7 +225,9 @@ def test_refresh_config_error_on_failure_to_process_config( with pytest.raises(exceptions.UbuntuProError) as excinfo: with mock.patch.object(lock, "lock_data_file"): - action_refresh(mock.MagicMock(target="config"), cfg=cfg) + refresh_command.action( + mock.MagicMock(target="config"), cfg=cfg + ) assert messages.E_REFRESH_CONFIG_FAILURE.msg == excinfo.value.msg @@ -270,7 +246,9 @@ def test_refresh_config_happy_path( cfg = FakeConfig() fake_machine_token_file.attached = True with mock.patch.object(lock, "lock_data_file"): - ret = action_refresh(mock.MagicMock(target="config"), cfg=cfg) + ret = refresh_command.action( + mock.MagicMock(target="config"), cfg=cfg + ) assert 0 == ret assert messages.REFRESH_CONFIG_SUCCESS in capsys.readouterr()[0] @@ -278,8 +256,8 @@ def test_refresh_config_happy_path( @mock.patch("uaclient.lock.check_lock_info", return_value=(-1, "")) @mock.patch("uaclient.apt_news.update_apt_news") - @mock.patch("uaclient.cli.refresh_motd") - @mock.patch("uaclient.cli.update_motd_messages") + @mock.patch(M_PATH + "refresh_motd") + @mock.patch(M_PATH + "update_motd_messages") @mock.patch("uaclient.contract.refresh") @mock.patch("uaclient.config.UAConfig.process_config") @mock.patch("uaclient.files.notices.NoticesManager.remove") @@ -301,7 +279,7 @@ def test_refresh_all_happy_path( cfg = FakeConfig() fake_machine_token_file.attached = True with mock.patch.object(lock, "lock_data_file"): - ret = action_refresh(mock.MagicMock(target=None), cfg=cfg) + ret = refresh_command.action(mock.MagicMock(target=None), cfg=cfg) out, err = capsys.readouterr() diff --git a/uaclient/cli/tests/test_cli_security_status.py b/uaclient/cli/tests/test_cli_security_status.py index 99bbd68aca..1ee341a5ae 100644 --- a/uaclient/cli/tests/test_cli_security_status.py +++ b/uaclient/cli/tests/test_cli_security_status.py @@ -10,7 +10,7 @@ @mock.patch(M_PATH + "security_status.security_status") @mock.patch(M_PATH + "security_status.security_status_dict") -@mock.patch("uaclient.cli.contract.get_available_resources") +@mock.patch("uaclient.contract.get_available_resources") class TestActionSecurityStatus: @pytest.mark.parametrize("output_format", ("json", "yaml", "text")) @mock.patch(M_PATH + "safe_dump")