cli: refactor system to the new approach

Signed-off-by: Renan Rodrigo <[email protected]>

features/help.feature

@@ -246,6 +246,39 @@ Feature: Pro Client help text
       (optional arguments|options):
         -h, --help            show this help message and exit
       """
+    When I run `pro system --help` as non-root
+    Then stdout matches regexp:
+      """
+      usage: pro system \[flags\]
+
+      Output system related information related to Pro services
+
+      (optional arguments|options):
+        -h, --help       show this help message and exit
+
+      Available Commands:
+
+          reboot-required
+                         does the system need to be rebooted
+      """
+    When I run `pro system reboot-required --help` as non-root
+    Then stdout matches regexp:
+      """
+      usage: pro system reboot-required \[flags\]
+
+      Report the current reboot-required status for the machine.
+
+      This command will output one of the three following states
+      for the machine regarding reboot:
+
+      * no: The machine doesn't require a reboot
+      * yes: The machine requires a reboot
+      * yes-kernel-livepatches-applied: There are only kernel related
+        packages that require a reboot, but Livepatch has already provided
+        patches for the current running kernel. The machine still needs a
+        reboot, but you can assess if the reboot can be performed in the
+        nearest maintenance window.
+      """
 
     Examples: ubuntu release
       | release | machine_type  |

uaclient/cli/__init__.py

@@ -20,9 +20,6 @@
     util,
     version,
 )
-from uaclient.api.u.pro.security.status.reboot_required.v1 import (
-    _reboot_required,
-)
 from uaclient.apt import AptProxyScope, setup_apt_proxy
 from uaclient.cli import cli_util
 from uaclient.cli.api import api_command
@@ -38,6 +35,7 @@
 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.cli.system import system_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
@@ -58,6 +56,7 @@
     refresh_command,
     security_status_command,
     status_command,
+    system_command,
 ]
 
 
@@ -165,35 +164,6 @@ def config_parser(parser):
     return parser
 
 
-def system_parser(parser):
-    """Build or extend an arg parser for system subcommand."""
-    parser.usage = USAGE_TMPL.format(name=NAME, command="system <command>")
-    parser.description = messages.CLI_SYSTEM_DESC
-    parser.prog = "system"
-    parser._optionals.title = messages.CLI_FLAGS
-    subparsers = parser.add_subparsers(
-        title=messages.CLI_AVAILABLE_COMMANDS, dest="command", metavar=""
-    )
-    parser_reboot_required = subparsers.add_parser(
-        "reboot-required", help=messages.CLI_SYSTEM_REBOOT_REQUIRED
-    )
-    parser_reboot_required.set_defaults(action=action_system_reboot_required)
-    reboot_required_parser(parser_reboot_required)
-
-    return parser
-
-
-def reboot_required_parser(parser):
-    # This formatter_class ensures that our formatting below isn't lost
-    parser.usage = USAGE_TMPL.format(
-        name=NAME, command="system reboot-required"
-    )
-    parser.pro = "reboot-required"
-    parser.formatter_class = argparse.RawDescriptionHelpFormatter
-    parser.description = messages.CLI_SYSTEM_REBOOT_REQUIRED_DESC
-    return parser
-
-
 def _print_help_for_subcommand(
     cfg: config.UAConfig, cmd_name: str, subcmd_name: str
 ):
@@ -451,32 +421,9 @@ def get_parser(cfg: config.UAConfig):
     config_parser(parser_config)
     parser_config.set_defaults(action=action_config)
 
-    parser_system = subparsers.add_parser(
-        "system", help=messages.CLI_ROOT_SYSTEM
-    )
-    parser_system.set_defaults(action=action_system)
-    system_parser(parser_system)
-
     return parser
 
 
-def action_system(args, *, cfg, **kwargs):
-    """Perform the system action.
-
-    :return: 0 on success, 1 otherwise
-    """
-    _print_help_for_subcommand(
-        cfg, cmd_name="system", subcmd_name=args.command
-    )
-    return 0
-
-
-def action_system_reboot_required(args, *, cfg: config.UAConfig, **kwargs):
-    result = _reboot_required(cfg)
-    event.info(result.reboot_required)
-    return 0
-
-
 def configure_apt_proxy(
     cfg: config.UAConfig,
     scope: AptProxyScope,

uaclient/cli/system.py

@@ -0,0 +1,37 @@
+from uaclient import event_logger, messages
+from uaclient.api.u.pro.security.status.reboot_required.v1 import (
+    _reboot_required,
+)
+from uaclient.cli.commands import ProCommand
+
+event = event_logger.get_event_logger()
+
+
+def action_reboot_required(args, *, cfg, **kwargs):
+    result = _reboot_required(cfg)
+    event.info(result.reboot_required)
+    return 0
+
+
+def action_system(args, *, cfg, **kwargs):
+    # Avoiding a circular import
+    from uaclient.cli import get_parser
+
+    get_parser(cfg).parse_args(["system", "--help"])
+
+
+reboot_required_subcommand = ProCommand(
+    "reboot-required",
+    help=messages.CLI_SYSTEM_REBOOT_REQUIRED,
+    description=messages.CLI_SYSTEM_REBOOT_REQUIRED_DESC,
+    action=action_reboot_required,
+    preserve_description=True,
+)
+
+system_command = ProCommand(
+    "system",
+    help=messages.CLI_ROOT_SYSTEM,
+    description=messages.CLI_SYSTEM_DESC,
+    action=action_system,
+    subcommands=[reboot_required_subcommand],
+)

uaclient/cli/tests/test_cli_reboot_required.py

@@ -1,38 +1,18 @@
 import mock
-import pytest
 
-from uaclient.cli import main
+from uaclient.cli.system import reboot_required_subcommand
 
-HELP_OUTPUT = """\
-usage: pro system reboot-required [flags]
-
-Report the current reboot-required status for the machine.
-
-This command will output one of the three following states
-for the machine regarding reboot:
-
-* no: The machine doesn't require a reboot
-* yes: The machine requires a reboot
-* yes-kernel-livepatches-applied: There are only kernel related
-  packages that require a reboot, but Livepatch has already provided
-  patches for the current running kernel. The machine still needs a
-  reboot, but you can assess if the reboot can be performed in the
-  nearest maintenance window.
-"""
+M_PATH = "uaclient.cli.system."
 
 
 class TestActionRebootRequired:
-    @mock.patch("uaclient.log.setup_cli_logging")
-    def test_enable_help(self, _m_setup_logging, capsys, FakeConfig):
-        with pytest.raises(SystemExit):
-            with mock.patch(
-                "sys.argv",
-                ["/usr/bin/ua", "system", "reboot-required", "--help"],
-            ):
-                with mock.patch(
-                    "uaclient.config.UAConfig",
-                    return_value=FakeConfig(),
-                ):
-                    main()
-        out, _err = capsys.readouterr()
-        assert HELP_OUTPUT in out
+    @mock.patch(M_PATH + "_reboot_required")
+    def test_returns_the_api_response(self, m_api_reboot_required, event):
+        m_cfg = mock.MagicMock()
+        with mock.patch.object(event, "info") as m_event_info:
+            reboot_required_subcommand.action(mock.MagicMock(), cfg=m_cfg)
+
+        assert [mock.call(m_cfg)] == m_api_reboot_required.call_args_list
+        assert [
+            mock.call(m_api_reboot_required.return_value.reboot_required)
+        ] == m_event_info.call_args_list