From b4292cf152fc0e7451cdaedd37221dff281fda6b Mon Sep 17 00:00:00 2001 From: Grant Orndorff Date: Thu, 4 Jan 2024 08:44:43 -0500 Subject: [PATCH] wip: enable progress --- features/api_enable.feature | 62 ++++++----------------- uaclient/actions.py | 3 +- uaclient/api/__init__.py | 56 +++++++++++++++++++++ uaclient/api/u/pro/services/enable/v1.py | 17 +++++-- uaclient/entitlements/base.py | 63 +++++++++++++++++++----- uaclient/entitlements/livepatch.py | 7 ++- uaclient/entitlements/repo.py | 42 ++++++++++++---- uaclient/entitlements/tests/test_repo.py | 2 +- uaclient/messages/__init__.py | 1 + 9 files changed, 177 insertions(+), 76 deletions(-) diff --git a/features/api_enable.feature b/features/api_enable.feature index d57d865cf9..c0860f1e9d 100644 --- a/features/api_enable.feature +++ b/features/api_enable.feature @@ -2,7 +2,6 @@ Feature: u.pro.services.enable Scenario Outline: u.pro.services.enable.v1 container services Given a `` `` machine with ubuntu-advantage-tools installed -<<<<<<< HEAD When I apt update And I apt install `jq` @@ -34,14 +33,6 @@ Feature: u.pro.services.enable # Basic enable When I run shell command `pro api u.pro.services.enable.v1 --args service=esm-infra | jq .data.attributes` with sudo -======= - When I run `apt-get update` with sudo - And I apt install `jq` - And I attach `contract_token` with sudo and options `--no-auto-enable` - - # Basic enable - And I run shell command `pro api u.pro.services.enable.v1 --args service=esm-infra | jq .data.attributes` with sudo ->>>>>>> 464f5f13 (api: add u.pro.services.enable.v1) Then I will see the following on stdout: """ { @@ -53,15 +44,7 @@ Feature: u.pro.services.enable "reboot_required": false } """ -<<<<<<< HEAD Then I verify that `esm-infra` is enabled -======= - When I run shell command `pro api u.pro.status.enabled_services.v1 | jq .data.attributes.enabled_services ` with sudo - Then stdout contains substring: - """ - esm-infra - """ ->>>>>>> 464f5f13 (api: add u.pro.services.enable.v1) # Enable already enabled service succeeds When I run shell command `pro api u.pro.services.enable.v1 --args service=esm-infra | jq .data.attributes` with sudo Then I will see the following on stdout: @@ -141,7 +124,6 @@ Feature: u.pro.services.enable | release | machine_type | | xenial | lxd-container | | bionic | lxd-container | -<<<<<<< HEAD Scenario Outline: u.pro.services.enable.v1 landscape Given a `` `` machine with ubuntu-advantage-tools installed @@ -166,15 +148,6 @@ Feature: u.pro.services.enable Scenario Outline: u.pro.services.enable.v1 vm services Given a `` `` machine with ubuntu-advantage-tools installed When I apt update -======= - | focal | lxd-container | - | jammy | lxd-container | - - - Scenario Outline: u.pro.services.enable.v1 vm services - Given a `` `` machine with ubuntu-advantage-tools installed - When I run `apt-get update` with sudo ->>>>>>> 464f5f13 (api: add u.pro.services.enable.v1) And I apt install `jq` And I attach `contract_token` with sudo and options `--no-auto-enable` @@ -206,11 +179,7 @@ Feature: u.pro.services.enable } """ # Enable with disable_incompatible works and variant works -<<<<<<< HEAD When I run shell command `pro api u.pro.services.enable.v1 --data \"{\\\"service\\\": \\\"realtime-kernel\\\", \\\"variant\\\": \\\"intel-iotg\\\"}\" | jq .data.attributes` with sudo -======= - When I run shell command `pro api u.pro.services.enable.v1 --data \"{\\\"service\\\": \\\"realtime-kernel\\\", \\\"variant\\\": "intel-iotg"}\" | jq .data.attributes` with sudo ->>>>>>> 464f5f13 (api: add u.pro.services.enable.v1) Then I will see the following on stdout: """ { @@ -221,7 +190,6 @@ Feature: u.pro.services.enable "realtime-kernel" ], "messages": [], -<<<<<<< HEAD "reboot_required": true } """ @@ -239,20 +207,22 @@ Feature: u.pro.services.enable Examples: | release | machine_type | | jammy | lxd-vm | -======= - "reboot_required": false - } + + Scenario Outline: u.pro.services.enable.v1 with progress + Given a `` `` machine with ubuntu-advantage-tools installed + When I run `apt-get update` with sudo + And I apt install `jq` + And I attach `contract_token` with sudo and options `--no-auto-enable` + + # Basic enable + And I run shell command `pro api u.pro.services.enable.v1 --show-progress --args service=esm-infra` with sudo + Then I will see the following on stdout: """ - When I run shell command `pro api u.pro.status.enabled_services.v1 | jq ".data.attributes.enabled_services | select(.name==\"realtime-kernel\")" ` with sudo - Then stdout contains substring: + {} """ - { - "name": "realtime-kernel", - "variant_enabled": true, - "variant_name": "intel-iotg" - } + # Enabling multiple services shows steps correctly + When I run shell command `pro api u.pro.services.enable.v1 --show-progress --args service=ros` with sudo + Then I will see the following on stdout: + """ + {} """ - Examples: - | release | machine_type | - | jammy | lxd-vm | ->>>>>>> 464f5f13 (api: add u.pro.services.enable.v1) diff --git a/uaclient/actions.py b/uaclient/actions.py index 9ee6c7c49b..adea4b5a5b 100644 --- a/uaclient/actions.py +++ b/uaclient/actions.py @@ -7,6 +7,7 @@ from typing import List, Optional # noqa: F401 from uaclient import ( + api, clouds, config, contract, @@ -151,7 +152,7 @@ def enable_entitlement_by_name( access_only=access_only, extra_args=extra_args, ) - return entitlement.enable() + return entitlement.enable(api.ProgressWrapper()) def status( diff --git a/uaclient/api/__init__.py b/uaclient/api/__init__.py index c8e5dacb08..8697c85e63 100644 --- a/uaclient/api/__init__.py +++ b/uaclient/api/__init__.py @@ -1,4 +1,60 @@ +import abc import logging +from typing import Optional # setup null handler for all API endpoints logging.getLogger("ubuntupro").addHandler(logging.NullHandler()) + + +class AbstractProgress(metaclass=abc.ABCMeta): + @abc.abstractmethod + def progress( + self, + *, + total_steps: int, + done_steps: int, + previous_step_message: Optional[str], + current_step_message: Optional[str] + ): + pass + + +class NullProgress(AbstractProgress): + def progress( + self, + *, + total_steps: int, + done_steps: int, + previous_step_message: Optional[str], + current_step_message: Optional[str] + ): + pass + + +class ProgressWrapper: + def __init__(self, progress_object: Optional[AbstractProgress] = None): + if progress_object is not None: + self.progress_object = progress_object + else: + self.progress_object = NullProgress() + self.done_steps = 0 + self.total_steps = -1 + self.previous_step_message = None # type: Optional[str] + + def progress(self, message: str): + self.progress_object.progress( + total_steps=self.total_steps, + done_steps=self.done_steps, + previous_step_message=self.previous_step_message, + current_step_message=message, + ) + self.previous_step_message = message + self.done_steps += 1 + + def start(self, total_steps: int, message: str): + self.total_steps = total_steps + self.progress(message) + + def finish(self, message: str): + self.done_steps = self.total_steps + self.progress(message) diff --git a/uaclient/api/u/pro/services/enable/v1.py b/uaclient/api/u/pro/services/enable/v1.py index 05abc3c45f..dcde4b095e 100644 --- a/uaclient/api/u/pro/services/enable/v1.py +++ b/uaclient/api/u/pro/services/enable/v1.py @@ -2,7 +2,7 @@ from typing import List, Optional from uaclient import entitlements, event_logger, lock, messages, util -from uaclient.api import exceptions +from uaclient.api import AbstractProgress, ProgressWrapper, exceptions from uaclient.api.api import APIEndpoint from uaclient.api.data_types import AdditionalInfo from uaclient.api.u.pro.status.enabled_services.v1 import _enabled_services @@ -71,14 +71,19 @@ def _enabled_services_names(cfg: UAConfig) -> List[str]: return [s.name for s in _enabled_services(cfg).enabled_services] -def enable(options: EnableOptions) -> EnableResult: - return _enable(options, UAConfig()) +def enable( + options: EnableOptions, progress_object: Optional[AbstractProgress] = None +) -> EnableResult: + return _enable(options, UAConfig(), progress_object=progress_object) def _enable( options: EnableOptions, cfg: UAConfig, + progress_object: Optional[AbstractProgress] = None, ) -> EnableResult: + progress = ProgressWrapper(progress_object) + event.set_event_mode(event_logger.EventLoggerMode.JSON) if not util.we_are_currently_root(): @@ -111,6 +116,11 @@ def _enable( access_only=options.access_only, ) + progress.start( + entitlement.calculate_total_enable_steps() + 1, + "Acquiring Ubuntu Pro lock file", + ) # TODO i18n + success = False fail_reason = None @@ -120,6 +130,7 @@ def _enable( lock_holder="u.pro.services.enable.v1", ): success, fail_reason = entitlement.enable( + progress, enable_required_services=options.enable_required_services, disable_incompatible_services=options.disable_incompatible_services, # noqa: E501 api=True, diff --git a/uaclient/entitlements/base.py b/uaclient/entitlements/base.py index d42e03a5dd..4e8397cfae 100644 --- a/uaclient/entitlements/base.py +++ b/uaclient/entitlements/base.py @@ -6,6 +6,7 @@ from typing import Any, Dict, List, Optional, Set, Tuple, Type, Union from uaclient import ( + api, apt, config, contract, @@ -336,6 +337,37 @@ def entitlement_cfg(self): return entitlement_cfg + @abc.abstractmethod + def enable_steps(self) -> int: + """ + The number of steps that are reported as progress while enabling + this specific entitlement that are not shared with other entitlements. + """ + pass + + def calculate_total_enable_steps(self) -> int: + total_steps = self.enable_steps() + required_snaps = ( + self.entitlement_cfg.get("entitlement", {}) + .get("directives", {}) + .get("requiredSnaps") + ) + if required_snaps is not None and len(required_snaps) > 0: + total_steps += 1 + required_packages = ( + self.entitlement_cfg.get("entitlement", {}) + .get("directives", {}) + .get("requiredPackages") + ) + if required_packages is not None and len(required_packages) > 0: + total_steps += 1 + for incompatible_service in self.incompatible_services: + # TODO: calculate disable steps when progress is added to disable + total_steps += 1 + for required_service in self.required_services: + total_steps += required_service().enable_steps() + return total_steps + def can_enable(self) -> Tuple[bool, Optional[CanEnableFailure]]: """ Report whether or not enabling is possible for the entitlement. @@ -420,6 +452,7 @@ def can_enable(self) -> Tuple[bool, Optional[CanEnableFailure]]: # functions). Then the CLI enable will be refactored to use the API. def enable( self, + progress: api.ProgressWrapper, silent: bool = False, enable_required_services: Optional[bool] = None, disable_incompatible_services: Optional[bool] = None, @@ -463,7 +496,7 @@ def enable( ): # Try to enable those services before proceeding with enable req_ret, error = self._enable_required_services( - api=api, allow=enable_required_services + progress, api=api, allow=enable_required_services ) if not req_ret: fail.message = error @@ -482,12 +515,12 @@ def enable( # to the base class. The additionalSnaps handling # is a step on that direction if not self.access_only: - if not self.handle_required_snaps(): + if not self.handle_required_snaps(progress): return False, None - if not self.handle_required_packages(): + if not self.handle_required_packages(progress): return False, None - ret = self._perform_enable(silent=silent) + ret = self._perform_enable(progress, silent=silent) if not ret: return False, None @@ -498,7 +531,7 @@ def enable( return True, None - def handle_required_snaps(self) -> bool: + def handle_required_snaps(self, progress: api.ProgressWrapper) -> bool: """ "install snaps necessary to enable a service.""" required_snaps = ( self.entitlement_cfg.get("entitlement", {}) @@ -544,6 +577,7 @@ def handle_required_snaps(self) -> bool: ) if required_snaps: + progress.progress(messages.INSTALLING_REQUIRED_SNAPS) event.info(messages.INSTALLING_REQUIRED_SNAPS) for snap_pkg in sorted(required_snaps, key=lambda x: x.get("name")): @@ -590,7 +624,7 @@ def are_required_packages_installed(self) -> bool: [required in installed_packages for required in package_names] ) - def handle_required_packages(self) -> bool: + def handle_required_packages(self, progress: api.ProgressWrapper) -> bool: """install packages necessary to enable a service.""" required_packages = ( self.entitlement_cfg.get("entitlement", {}) @@ -607,11 +641,11 @@ def handle_required_packages(self) -> bool: package_names = [package["name"] for package in required_packages] LOG.debug("Installing packages %r", package_names) - event.info( - messages.INSTALLING_PACKAGES.format( - packages=" ".join(package_names) - ) + installing_msg = messages.INSTALLING_PACKAGES.format( + packages=" ".join(package_names) ) + progress.progress(installing_msg) + event.info(installing_msg) apt.run_apt_install_command(package_names) return True @@ -654,7 +688,9 @@ def handle_removing_required_packages(self) -> bool: return True @abc.abstractmethod - def _perform_enable(self, silent: bool = False) -> bool: + def _perform_enable( + self, progress: api.ProgressWrapper, silent: bool = False + ) -> bool: """ Enable specific entitlement. This should be implemented by subclasses. This method does the actual enablement, and does not check can_enable @@ -784,6 +820,7 @@ def handle_incompatible_services( def _enable_required_services( self, + progress: api.ProgressWrapper, api: bool, allow: bool, ) -> Tuple[bool, Optional[messages.NamedMessage]]: @@ -827,7 +864,7 @@ def _enable_required_services( service=ent.title ) ) - ret, fail = ent.enable(silent=True) + ret, fail = ent.enable(progress, silent=True) if not ret: error_msg = "" if fail and fail.message and fail.message.msg: @@ -1342,7 +1379,7 @@ def process_contract_deltas( msg = messages.ENABLE_BY_DEFAULT_TMPL.format(name=self.name) event.info(msg, file_type=sys.stderr) - self.enable() + self.enable(api.ProgressWrapper()) else: msg = messages.ENABLE_BY_DEFAULT_MANUAL_TMPL.format( name=self.name diff --git a/uaclient/entitlements/livepatch.py b/uaclient/entitlements/livepatch.py index 31200d2f7d..8c2d6386ff 100644 --- a/uaclient/entitlements/livepatch.py +++ b/uaclient/entitlements/livepatch.py @@ -2,6 +2,7 @@ from typing import Any, Dict, Optional, Tuple from uaclient import ( + api, event_logger, exceptions, http, @@ -81,7 +82,9 @@ def static_affordances(self) -> Tuple[StaticAffordance, ...]: ), ) - def _perform_enable(self, silent: bool = False) -> bool: + def _perform_enable( + self, progress: api.ProgressWrapper, silent: bool = False + ) -> bool: """Enable specific entitlement. @return: True on success, False otherwise. @@ -300,7 +303,7 @@ def process_contract_deltas( ) if process_enable_default: - enable_success, _ = self.enable() + enable_success, _ = self.enable(api.ProgressWrapper()) return enable_success application_status, _ = self.application_status() diff --git a/uaclient/entitlements/repo.py b/uaclient/entitlements/repo.py index d73ba28ef0..de018ea40c 100644 --- a/uaclient/entitlements/repo.py +++ b/uaclient/entitlements/repo.py @@ -6,6 +6,7 @@ from typing import Any, Dict, List, Optional, Tuple, Union from uaclient import ( + api, apt, contract, event_logger, @@ -113,13 +114,25 @@ def can_disable( return result, reason - def _perform_enable(self, silent: bool = False) -> bool: + def enable_steps(self) -> int: + will_install = self.packages is not None and len(self.packages) > 0 + if self.access_only or not will_install: + return 2 + else: + return 3 + + def _perform_enable( + self, progress: api.ProgressWrapper, silent: bool = False + ) -> bool: """Enable specific entitlement. @return: True on success, False otherwise. @raises: UbuntuProError on failure to install suggested packages """ - self.setup_apt_config(silent=silent) + progress.progress( + messages.CONFIGURING_APT_ACCESS.format(service=self.name) + ) + self.setup_apt_config(progress, silent=silent) if self.supports_access_only and self.access_only: if len(self.packages) > 0: @@ -130,7 +143,7 @@ def _perform_enable(self, silent: bool = False) -> bool: ) event.info(messages.ACCESS_ENABLED_TMPL.format(title=self.title)) else: - self.install_packages() + self.install_packages(progress) event.info(messages.ENABLED_TMPL.format(title=self.title)) self._check_for_reboot_msg(operation="install") return True @@ -394,7 +407,7 @@ def process_contract_deltas( apt.remove_auth_apt_repo(self.repo_file, old_url) self.remove_apt_config() - self.setup_apt_config() + self.setup_apt_config(api.ProgressWrapper()) if delta_packages: LOG.info("New additionalPackages, installing %r", delta_packages) @@ -403,12 +416,15 @@ def process_contract_deltas( packages=", ".join(delta_packages) ) ) - self.install_packages(package_list=delta_packages) + self.install_packages( + api.ProgressWrapper(), package_list=delta_packages + ) return True def install_packages( self, + progress: api.ProgressWrapper, package_list: Optional[List[str]] = None, cleanup_on_failure: bool = True, verbose: bool = True, @@ -438,10 +454,12 @@ def install_packages( self.remove_apt_config() raise + installing_msg = messages.INSTALLING_SERVICE_PACKAGES.format( + title=self.title + ) + progress.progress(installing_msg) if verbose: - event.info( - messages.INSTALLING_SERVICE_PACKAGES.format(title=self.title) - ) + event.info(installing_msg) if self.apt_noninteractive: override_env_vars = {"DEBIAN_FRONTEND": "noninteractive"} @@ -466,7 +484,9 @@ def install_packages( self.remove_apt_config() raise - def setup_apt_config(self, silent: bool = False) -> None: + def setup_apt_config( + self, progress: api.ProgressWrapper, silent: bool = False + ) -> None: """Setup apt config based on the resourceToken and directives. Also sets up apt proxy if necessary. @@ -580,8 +600,10 @@ def setup_apt_config(self, silent: bool = False) -> None: # probably wants access to the repo that was just enabled. # Side-effect is that apt policy will now report the repo as accessible # which allows pro status to report correct info + updating_msg = messages.APT_UPDATING_LIST.format(name=self.title) + progress.progress(updating_msg) if not silent: - event.info(messages.APT_UPDATING_LIST.format(name=self.title)) + event.info(updating_msg) try: apt.update_sources_list(repo_filename) except exceptions.UbuntuProError: diff --git a/uaclient/entitlements/tests/test_repo.py b/uaclient/entitlements/tests/test_repo.py index b1e79b8e73..70ba085d2e 100644 --- a/uaclient/entitlements/tests/test_repo.py +++ b/uaclient/entitlements/tests/test_repo.py @@ -527,7 +527,7 @@ def test_enable_calls_adds_apt_repo_and_calls_apt_update( # We patch the type of entitlement because packages is a property with mock.patch.object(type(entitlement), "packages", packages): with messaging_patch: - entitlement.enable() + entitlement.enable(mock.MagicMock()) expected_calls = [ mock.call(apt.APT_METHOD_HTTPS_FILE), diff --git a/uaclient/messages/__init__.py b/uaclient/messages/__init__.py index fa86b64ec2..ac3ff69f93 100644 --- a/uaclient/messages/__init__.py +++ b/uaclient/messages/__init__.py @@ -339,6 +339,7 @@ class TxtColor: """\ A reboot is required to complete {operation}.""" ) +CONFIGURING_APT_ACCESS = t.gettext("Configuring APT access to {service}") # DISABLE DISABLE_FAILED_TMPL = t.gettext("Could not disable {title}.")