cli: refactor disable to the new approach

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

features/help.feature

@@ -33,6 +33,27 @@ Feature: Pro Client help text
                               key=value
         --data DATA           arguments in JSON format to the API endpoint
       """
+    When I run `pro disable --help` as non-root
+    Then stdout matches regexp:
+      """
+      usage: pro disable \[flags\]
+
+      Disable an Ubuntu Pro service.
+
+      positional arguments:
+        service              the name\(s\) of the Ubuntu Pro services to disable. One
+                             of: anbox-cloud, cc-eal, cis, esm-apps, esm-infra,
+                             fips, fips-preview, fips-updates, landscape, livepatch,
+                             realtime-kernel, ros, ros-updates
+
+      (optional arguments|options):
+        -h, --help           show this help message and exit
+        --assume-yes         do not prompt for confirmation before performing the
+                             disable
+        --format \{cli,json\}  output in the specified format \(default: cli\)
+        --purge              disable the service and remove/downgrade related
+                             packages \(experimental\)
+      """
 
     Examples: ubuntu release
       | release | machine_type  |

uaclient/cli/__init__.py

@@ -29,7 +29,6 @@
     util,
     version,
 )
-from uaclient.api import ProgressWrapper
 from uaclient.api.u.pro.attach.auto.full_auto_attach.v1 import (
     FullAutoAttachOptions,
     _full_auto_attach,
@@ -47,10 +46,11 @@
     _reboot_required,
 )
 from uaclient.apt import AptProxyScope, setup_apt_proxy
-from uaclient.cli import cli_util, disable, enable, fix
+from uaclient.cli import cli_util, enable, fix
 from uaclient.cli.api import api_command
 from uaclient.cli.collect_logs import collect_logs_command
 from uaclient.cli.constants import NAME, USAGE_TMPL
+from uaclient.cli.disable import disable_command, perform_disable
 from uaclient.data_types import AttachActionsConfigFile, IncorrectTypeError
 from uaclient.entitlements import (
     create_enable_entitlements_not_found_error,
@@ -59,7 +59,6 @@
 )
 from uaclient.entitlements.entitlement_status import (
     ApplicationStatus,
-    CanDisableFailure,
     CanEnableFailure,
 )
 from uaclient.files import machine_token, state_files
@@ -74,7 +73,7 @@
 event = event_logger.get_event_logger()
 LOG = logging.getLogger(util.replace_top_level_logger_name(__name__))
 
-COMMANDS = [api_command, collect_logs_command]
+COMMANDS = [api_command, collect_logs_command, disable_command]
 
 
 class UAArgumentParser(argparse.ArgumentParser):
@@ -437,52 +436,6 @@ def _print_help_for_subcommand(
         )
 
 
-def _perform_disable(
-    entitlement, cfg, *, json_output, assume_yes, update_status=True
-):
-    """Perform the disable action on a named entitlement.
-
-    :param entitlement_name: the name of the entitlement to enable
-    :param cfg: the UAConfig to pass to the entitlement
-    :param json_output: output should be json only
-
-    @return: True on success, False otherwise
-    """
-    # Make sure we have the correct variant of the service
-    # This can affect what packages get uninstalled
-    variant = entitlement.enabled_variant
-    if variant is not None:
-        entitlement = variant
-
-    if json_output:
-        progress = ProgressWrapper()
-    else:
-        progress = ProgressWrapper(
-            cli_util.CLIEnableDisableProgress(assume_yes=assume_yes)
-        )
-
-    ret, reason = entitlement.disable(progress)
-
-    if not ret:
-        event.service_failed(entitlement.name)
-
-        if reason is not None and isinstance(reason, CanDisableFailure):
-            if reason.message is not None:
-                event.info(reason.message.msg)
-                event.error(
-                    error_msg=reason.message.msg,
-                    error_code=reason.message.name,
-                    service=entitlement.name,
-                )
-    else:
-        event.service_processed(entitlement.name)
-
-    if update_status:
-        status.status(cfg=cfg)  # Update the status cache
-
-    return ret
-
-
 def action_config(args, *, cfg, **kwargs):
     """Perform the config action.
 
@@ -749,7 +702,7 @@ def _detach(cfg: config.UAConfig, assume_yes: bool, json_output: bool) -> int:
     if not util.prompt_for_confirmation(assume_yes=assume_yes):
         return 1
     for ent in to_disable:
-        _perform_disable(
+        perform_disable(
             ent,
             cfg,
             json_output=json_output,
@@ -955,7 +908,6 @@ def get_parser(cfg: config.UAConfig):
     detach_parser(parser_detach)
     parser_detach.set_defaults(action=action_detach)
 
-    disable.add_parser(subparsers, cfg)
     enable.add_parser(subparsers, cfg)
     fix.add_parser(subparsers)
 

uaclient/cli/disable.py

@@ -4,7 +4,6 @@
 from typing import Dict, List  # noqa: F401
 
 from uaclient import (
-    api,
     config,
     contract,
     entitlements,
@@ -14,14 +13,17 @@
     status,
     util,
 )
+from uaclient.api import ProgressWrapper
 from uaclient.api.u.pro.services.dependencies.v1 import (
     ServiceWithDependencies,
     _dependencies,
 )
 from uaclient.api.u.pro.status.enabled_services.v1 import _enabled_services
-from uaclient.cli import cli_util, constants
+from uaclient.cli import cli_util
+from uaclient.cli.commands import ProArgument, ProCommand
 from uaclient.entitlements.entitlement_status import CanDisableFailure
 
+event = event_logger.get_event_logger()
 LOG = logging.getLogger(util.replace_top_level_logger_name(__name__))
 
 
@@ -56,6 +58,52 @@ def prompt_for_dependency_handling(
             )
 
 
+def perform_disable(
+    entitlement, cfg, *, json_output, assume_yes, update_status=True
+):
+    """Perform the disable action on a named entitlement.
+
+    :param entitlement_name: the name of the entitlement to enable
+    :param cfg: the UAConfig to pass to the entitlement
+    :param json_output: output should be json only
+
+    @return: True on success, False otherwise
+    """
+    # Make sure we have the correct variant of the service
+    # This can affect what packages get uninstalled
+    variant = entitlement.enabled_variant
+    if variant is not None:
+        entitlement = variant
+
+    if json_output:
+        progress = ProgressWrapper()
+    else:
+        progress = ProgressWrapper(
+            cli_util.CLIEnableDisableProgress(assume_yes=assume_yes)
+        )
+
+    ret, reason = entitlement.disable(progress)
+
+    if not ret:
+        event.service_failed(entitlement.name)
+
+        if reason is not None and isinstance(reason, CanDisableFailure):
+            if reason.message is not None:
+                event.info(reason.message.msg)
+                event.error(
+                    error_msg=reason.message.msg,
+                    error_code=reason.message.name,
+                    service=entitlement.name,
+                )
+    else:
+        event.service_processed(entitlement.name)
+
+    if update_status:
+        status.status(cfg=cfg)  # Update the status cache
+
+    return ret
+
+
 @cli_util.verify_json_format_args
 @cli_util.assert_root
 @cli_util.assert_attached(cli_util._raise_enable_disable_unattached_error)
@@ -135,9 +183,9 @@ def action_disable(args, *, cfg, **kwargs):
                 continue
 
         if json_output:
-            progress = api.ProgressWrapper()
+            progress = ProgressWrapper()
         else:
-            progress = api.ProgressWrapper(
+            progress = ProgressWrapper(
                 cli_util.CLIEnableDisableProgress(assume_yes=assume_yes)
             )
         progress.total_steps = ent.calculate_total_disable_steps()
@@ -241,43 +289,38 @@ def action_disable(args, *, cfg, **kwargs):
     return 0 if ret else 1
 
 
-def add_parser(subparsers, cfg: config.UAConfig):
-    """Build or extend an arg parser for disable subcommand."""
-    parser = subparsers.add_parser("disable", help=messages.CLI_ROOT_DISABLE)
-    parser.set_defaults(action=action_disable)
-    usage = constants.USAGE_TMPL.format(
-        name=constants.NAME, command="disable <service> [<service>]"
-    )
-    parser.description = messages.CLI_DISABLE_DESC
-    parser.usage = usage
-    parser.prog = "disable"
-    parser._positionals.title = messages.CLI_ARGS
-    parser._optionals.title = messages.CLI_FLAGS
-    parser.add_argument(
-        "service",
-        action="store",
-        nargs="+",
-        help=(
-            messages.CLI_DISABLE_SERVICE.format(
-                options=", ".join(entitlements.valid_services(cfg=cfg))
-            )
+disable_command = ProCommand(
+    "disable",
+    help=messages.CLI_ROOT_DISABLE,
+    description=messages.CLI_DISABLE_DESC,
+    action=action_disable,
+    arguments=[
+        ProArgument(
+            "service",
+            help=messages.CLI_DISABLE_SERVICE.format(
+                options=", ".join(
+                    entitlements.valid_services(cfg=config.UAConfig())
+                )
+            ),
+            action="store",
+            nargs="+",
         ),
-    )
-    parser.add_argument(
-        "--assume-yes",
-        action="store_true",
-        help=messages.CLI_ASSUME_YES.format(command="disable"),
-    )
-    parser.add_argument(
-        "--format",
-        action="store",
-        choices=["cli", "json"],
-        default="cli",
-        help=messages.CLI_FORMAT_DESC.format(default="cli"),
-    )
-    parser.add_argument(
-        "--purge",
-        action="store_true",
-        help=messages.CLI_PURGE,
-    )
-    return parser
+        ProArgument(
+            "--assume-yes",
+            help=messages.CLI_ASSUME_YES.format(command="disable"),
+            action="store_true",
+        ),
+        ProArgument(
+            "--format",
+            help=messages.CLI_FORMAT_DESC.format(default="cli"),
+            action="store",
+            choices=["cli", "json"],
+            default="cli",
+        ),
+        ProArgument(
+            "--purge",
+            help=messages.CLI_PURGE,
+            action="store_true",
+        ),
+    ],
+)