cli: refactor collect-logs to the new approach

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

features/help.feature

@@ -0,0 +1,25 @@
+Feature: Pro Client help text
+
+  Scenario Outline: Help text for the Pro Client commands
+    Given a `<release>` `<machine_type>` machine with ubuntu-advantage-tools installed
+    When I run `pro collect-logs --help` as non-root
+    Then stdout matches regexp:
+      """
+      usage: pro collect-logs \[flags\]
+
+      Collect logs and relevant system information into a tarball.
+
+      (optional arguments|options):
+        -h, --help            show this help message and exit
+        -o OUTPUT, --output OUTPUT
+                              tarball where the logs will be stored. \(Defaults to
+                              ./pro_logs.tar.gz\)
+      """
+
+    Examples: ubuntu release
+      | release | machine_type  |
+      | xenial  | lxd-container |
+      | bionic  | lxd-container |
+      | focal   | lxd-container |
+      | jammy   | lxd-container |
+      | noble   | lxd-container |

uaclient/cli/__init__.py

@@ -4,8 +4,6 @@
 import json
 import logging
 import sys
-import tarfile
-import tempfile
 import time
 from typing import Optional
 
@@ -50,6 +48,7 @@
 )
 from uaclient.apt import AptProxyScope, setup_apt_proxy
 from uaclient.cli import cli_api, cli_util, disable, enable, fix
+from uaclient.cli.collect_logs import collect_logs_command
 from uaclient.cli.constants import NAME, USAGE_TMPL
 from uaclient.data_types import AttachActionsConfigFile, IncorrectTypeError
 from uaclient.entitlements import (
@@ -71,12 +70,10 @@
 
 STATUS_FORMATS = ["tabular", "json", "yaml"]
 
-UA_COLLECT_LOGS_FILE = "pro_logs.tar.gz"
-
 event = event_logger.get_event_logger()
 LOG = logging.getLogger(util.replace_top_level_logger_name(__name__))
 
-COMMANDS = []
+COMMANDS = [collect_logs_command]
 
 
 class UAArgumentParser(argparse.ArgumentParser):
@@ -106,19 +103,6 @@ def auto_attach_parser(parser):
     return parser
 
 
-def collect_logs_parser(parser):
-    """Build or extend an arg parser for 'collect-logs' subcommand."""
-    parser.prog = "collect-logs"
-    parser.description = messages.CLI_COLLECT_LOGS_DESC
-    parser.usage = USAGE_TMPL.format(name=NAME, command=parser.prog)
-    parser.add_argument(
-        "-o",
-        "--output",
-        help=messages.CLI_COLLECT_LOGS_OUTPUT,
-    )
-    return parser
-
-
 def config_show_parser(parser, parent_command: str):
     """Build or extend an arg parser for 'config show' subcommand."""
     parser.usage = USAGE_TMPL.format(
@@ -920,19 +904,6 @@ def action_attach(args, *, cfg, **kwargs):
         return ret
 
 
-def action_collect_logs(args, *, cfg: config.UAConfig, **kwargs):
-    output_file = args.output or UA_COLLECT_LOGS_FILE
-    with tempfile.TemporaryDirectory() as output_dir:
-        actions.collect_logs(cfg, output_dir)
-        try:
-            with tarfile.open(output_file, "w:gz") as results:
-                results.add(output_dir, arcname="logs/")
-        except PermissionError as e:
-            LOG.error(e)
-            return 1
-    return 0
-
-
 def get_parser(cfg: config.UAConfig):
     parser = UAArgumentParser(
         prog=NAME,
@@ -973,12 +944,6 @@ def get_parser(cfg: config.UAConfig):
     auto_attach_parser(parser_auto_attach)
     parser_auto_attach.set_defaults(action=action_auto_attach)
 
-    parser_collect_logs = subparsers.add_parser(
-        "collect-logs", help=messages.CLI_ROOT_COLLECT_LOGS
-    )
-    collect_logs_parser(parser_collect_logs)
-    parser_collect_logs.set_defaults(action=action_collect_logs)
-
     parser_config = subparsers.add_parser(
         "config", help=messages.CLI_ROOT_CONFIG
     )

uaclient/cli/collect_logs.py

@@ -0,0 +1,41 @@
+import argparse
+import logging
+import tarfile
+import tempfile
+
+from uaclient.actions import collect_logs
+from uaclient.cli.commands import ProArgument, ProCommand
+from uaclient.config import UAConfig
+from uaclient.messages import (
+    CLI_COLLECT_LOGS_DESC,
+    CLI_COLLECT_LOGS_OUTPUT,
+    CLI_ROOT_COLLECT_LOGS,
+)
+from uaclient.util import replace_top_level_logger_name
+
+PRO_COLLECT_LOGS_FILE = "pro_logs.tar.gz"
+LOG = logging.getLogger(replace_top_level_logger_name(__name__))
+
+
+def action_collect_logs(args: argparse.Namespace, cfg: UAConfig):
+    output_file = args.output or PRO_COLLECT_LOGS_FILE
+    with tempfile.TemporaryDirectory() as output_dir:
+        collect_logs(cfg, output_dir)
+        try:
+            with tarfile.open(output_file, "w:gz") as results:
+                results.add(output_dir, arcname="logs/")
+        except PermissionError as e:
+            LOG.error(e)
+            return 1
+    return 0
+
+
+collect_logs_command = ProCommand(
+    "collect-logs",
+    help=CLI_ROOT_COLLECT_LOGS,
+    description=CLI_COLLECT_LOGS_DESC,
+    action=action_collect_logs,
+    arguments=[
+        ProArgument("--output", short_name="-o", help=CLI_COLLECT_LOGS_OUTPUT)
+    ],
+)

uaclient/cli/tests/test_cli_collect_logs.py

@@ -1,52 +1,13 @@
-import re
-import textwrap
-
 import mock
 import pytest
 
-from uaclient.cli import (
-    action_collect_logs,
-    collect_logs_parser,
-    get_parser,
-    main,
-)
+from uaclient.cli.collect_logs import collect_logs_command
 from uaclient.defaults import APPARMOR_PROFILES
 
 M_PATH = "uaclient.cli."
 
-HELP_OUTPUT = textwrap.dedent(
-    """\
-usage: pro collect-logs \[flags\]
-
-Collect logs and relevant system information into a tarball.
-
-(optional arguments|options):
-  -h, --help            show this help message and exit
-  -o OUTPUT, --output OUTPUT
-                        tarball where the logs will be stored. \(Defaults to
-                        ./pro_logs.tar.gz\)
-"""  # noqa
-)
-
 
 class TestActionCollectLogs:
-    @mock.patch("uaclient.log.setup_cli_logging")
-    @mock.patch(M_PATH + "contract.get_available_resources")
-    def test_collect_logs_help(
-        self, _m_resources, _m_setup_logging, capsys, FakeConfig
-    ):
-        with pytest.raises(SystemExit):
-            with mock.patch(
-                "sys.argv", ["/usr/bin/ua", "collect-logs", "--help"]
-            ):
-                with mock.patch(
-                    "uaclient.config.UAConfig",
-                    return_value=FakeConfig(),
-                ):
-                    main()
-        out, _err = capsys.readouterr()
-        assert re.match(HELP_OUTPUT, out)
-
     @pytest.mark.parametrize(
         "series,extension", (("jammy", "list"), ("noble", "sources"))
     )
@@ -115,7 +76,7 @@ def test_collect_logs(
             ]
 
         cfg = FakeConfig()
-        action_collect_logs(mock.MagicMock(), cfg=cfg)
+        collect_logs_command.action(mock.MagicMock(), cfg=cfg)
 
         assert m_subp.call_args_list == [
             mock.call(["cloud-id"], rcs=None),
@@ -235,20 +196,3 @@ def test_collect_logs(
             APPARMOR_PROFILES
         )
         assert m_shutilcopy.call_count == len(APPARMOR_PROFILES)
-
-
-class TestParser:
-    @mock.patch(M_PATH + "contract.get_available_resources")
-    def test_collect_logs_parser_updates_parser_config(
-        self, _m_resources, FakeConfig
-    ):
-        """Update the parser configuration for 'collect-logs'."""
-        m_parser = collect_logs_parser(mock.Mock())
-        assert "pro collect-logs [flags]" == m_parser.usage
-        assert "collect-logs" == m_parser.prog
-
-        full_parser = get_parser(FakeConfig())
-        with mock.patch("sys.argv", ["pro", "collect-logs"]):
-            args = full_parser.parse_args()
-        assert "collect-logs" == args.command
-        assert "action_collect_logs" == args.action.__name__