diff --git a/sonic_package_manager/constraint.py b/sonic_package_manager/constraint.py index a2cffe0cf0ec..6965ad9cb3e8 100644 --- a/sonic_package_manager/constraint.py +++ b/sonic_package_manager/constraint.py @@ -95,7 +95,7 @@ def parse(constraints: Dict) -> 'ComponentConstraints': """ components = {component: VersionConstraint.parse(version) - for component, version in constraints.items()} + for component, version in constraints.items()} return ComponentConstraints(components) def deparse(self) -> Dict[str, str]: diff --git a/sonic_package_manager/dockerapi.py b/sonic_package_manager/dockerapi.py index 926600d0bc19..7f051d2d7298 100644 --- a/sonic_package_manager/dockerapi.py +++ b/sonic_package_manager/dockerapi.py @@ -186,6 +186,15 @@ def rm(self, container: str, **kwargs): self.client.containers.get(container).remove(**kwargs) log.debug(f'removed container {container}') + def rm_by_ancestor(self, image_id: str, **kwargs): + """ Docker 'rm' command for running containers instantiated + from image passed to this function. """ + + # Clean containers based on the old image + containers = self.ps(filters={'ancestor': image_id}, all=True) + for container in containers: + self.rm(container.id, **kwargs) + def ps(self, **kwargs): """ Docker 'ps' command. """ diff --git a/sonic_package_manager/errors.py b/sonic_package_manager/errors.py index 17279c52c4ca..fe4de39a39b4 100644 --- a/sonic_package_manager/errors.py +++ b/sonic_package_manager/errors.py @@ -143,4 +143,3 @@ class PackageComponentConflictError(PackageInstallationError): def __str__(self): return (f'Package {self.name} conflicts with {self.component} {self.constraint} ' f'in package {self.dependency} but version {self.installed_ver} is installed') - diff --git a/sonic_package_manager/main.py b/sonic_package_manager/main.py index c0589ae5b5f0..dfb88bd58d94 100644 --- a/sonic_package_manager/main.py +++ b/sonic_package_manager/main.py @@ -361,7 +361,7 @@ def install(ctx, package_source = package_expr or from_repository or from_tarball if not package_source: - exit_cli(f'Package source is not specified', fg='red') + exit_cli('Package source is not specified', fg='red') if not yes and not force: click.confirm(f'{package_source} is going to be installed, ' @@ -386,7 +386,7 @@ def install(ctx, except Exception as err: exit_cli(f'Failed to install {package_source}: {err}', fg='red') except KeyboardInterrupt: - exit_cli(f'Operation canceled by user', fg='red') + exit_cli('Operation canceled by user', fg='red') @cli.command() @@ -409,7 +409,7 @@ def reset(ctx, name, force, yes, skip_host_plugins): except Exception as err: exit_cli(f'Failed to reset package {name}: {err}', fg='red') except KeyboardInterrupt: - exit_cli(f'Operation canceled by user', fg='red') + exit_cli('Operation canceled by user', fg='red') @cli.command() @@ -426,12 +426,16 @@ def uninstall(ctx, name, force, yes): click.confirm(f'Package {name} is going to be uninstalled, ' f'continue?', abort=True, show_default=True) + uninstall_opts = { + 'force': force, + } + try: - manager.uninstall(name, force) + manager.uninstall(name, **uninstall_opts) except Exception as err: exit_cli(f'Failed to uninstall package {name}: {err}', fg='red') except KeyboardInterrupt: - exit_cli(f'Operation canceled by user', fg='red') + exit_cli('Operation canceled by user', fg='red') @cli.command() @@ -453,7 +457,7 @@ def migrate(ctx, database, force, yes, dockerd_socket): except Exception as err: exit_cli(f'Failed to migrate packages {err}', fg='red') except KeyboardInterrupt: - exit_cli(f'Operation canceled by user', fg='red') + exit_cli('Operation canceled by user', fg='red') if __name__ == "__main__": diff --git a/sonic_package_manager/manager.py b/sonic_package_manager/manager.py index 41f95bd4bd03..3ecd1f560005 100644 --- a/sonic_package_manager/manager.py +++ b/sonic_package_manager/manager.py @@ -39,6 +39,7 @@ from sonic_package_manager.progress import ProgressManager from sonic_package_manager.reference import PackageReference from sonic_package_manager.registry import RegistryResolver +from sonic_package_manager.service_creator import SONIC_CLI_COMMANDS from sonic_package_manager.service_creator.creator import ( ServiceCreator, run_command @@ -52,7 +53,6 @@ RegistrySource, TarballSource ) -from sonic_package_manager.utils import DockerReference from sonic_package_manager.version import ( Version, version_to_tag, @@ -101,7 +101,7 @@ def wrapped_function(*args, **kwargs): return wrapped_function -def rollback(func, *args, **kwargs): +def rollback(func, *args, **kwargs) -> Callable: """ Used in rollback callbacks to ignore failure but proceed with rollback. Error will be printed but not fail the whole procedure of rollback. """ @@ -131,7 +131,7 @@ def package_constraint_to_reference(constraint: PackageConstraint) -> PackageRef return PackageReference(package_name, version_to_tag(version)) -def parse_reference_expression(expression): +def parse_reference_expression(expression) -> PackageReference: try: return package_constraint_to_reference(PackageConstraint.parse(expression)) except ValueError: @@ -247,7 +247,7 @@ def validate_package_tree(packages: Dict[str, Package]): continue component_version = conflicting_package.components[component] - log.debug(f'conflicting package {dependency.name}: ' + log.debug(f'conflicting package {conflict.name}: ' f'component {component} version is {component_version}') if constraint.allows(component_version): @@ -397,12 +397,17 @@ def install_from_source(self, if not self.database.has_package(package.name): self.database.add_package(package.name, package.repository) + service_create_opts = { + 'state': feature_state, + 'owner': default_owner, + } + try: with contextlib.ExitStack() as exits: source.install(package) exits.callback(rollback(source.uninstall, package)) - self.service_creator.create(package, state=feature_state, owner=default_owner) + self.service_creator.create(package, **service_create_opts) exits.callback(rollback(self.service_creator.remove, package)) self.service_creator.generate_shutdown_sequence_files( @@ -481,13 +486,7 @@ def uninstall(self, name: str, force=False): self.service_creator.generate_shutdown_sequence_files( self._get_installed_packages_except(package) ) - - # Clean containers based on this image - containers = self.docker.ps(filters={'ancestor': package.image_id}, - all=True) - for container in containers: - self.docker.rm(container.id, force=True) - + self.docker.rm_by_ancestor(package.image_id, force=True) self.docker.rmi(package.image_id, force=True) package.entry.image_id = None except Exception as err: @@ -563,6 +562,13 @@ def upgrade_from_source(self, # After all checks are passed we proceed to actual upgrade + service_create_opts = { + 'register_feature': False, + } + service_remove_opts = { + 'deregister_feature': False, + } + try: with contextlib.ExitStack() as exits: self._uninstall_cli_plugins(old_package) @@ -576,19 +582,15 @@ def upgrade_from_source(self, exits.callback(rollback(self._systemctl_action, old_package, 'start')) - self.service_creator.remove(old_package, deregister_feature=False) + self.service_creator.remove(old_package, **service_remove_opts) exits.callback(rollback(self.service_creator.create, old_package, - register_feature=False)) + **service_create_opts)) - # Clean containers based on the old image - containers = self.docker.ps(filters={'ancestor': old_package.image_id}, - all=True) - for container in containers: - self.docker.rm(container.id, force=True) + self.docker.rm_by_ancestor(old_package.image_id, force=True) - self.service_creator.create(new_package, register_feature=False) + self.service_creator.create(new_package, **service_create_opts) exits.callback(rollback(self.service_creator.remove, new_package, - register_feature=False)) + **service_remove_opts)) self.service_creator.generate_shutdown_sequence_files( self._get_installed_packages_and(new_package) @@ -654,16 +656,16 @@ def migrate_packages(self, old_package_database: PackageDatabase, dockerd_sock: Optional[str] = None): """ - Migrate packages from old database. This function can do a comparison between - current database and the database passed in as argument. If the package is - missing in the current database it will be added. If the package is installed - in the passed database and in the current it is not installed it will be - installed with a passed database package version. If the package is installed - in the passed database and it is installed in the current database but with - older version the package will be upgraded to the never version. If the package - is installed in the passed database and in the current it is installed but with - never version - no actions are taken. If dockerd_sock parameter is passed, the - migration process will use loaded images from docker library of the currently + Migrate packages from old database. This function can do a comparison between + current database and the database passed in as argument. If the package is + missing in the current database it will be added. If the package is installed + in the passed database and in the current it is not installed it will be + installed with a passed database package version. If the package is installed + in the passed database and it is installed in the current database but with + older version the package will be upgraded to the never version. If the package + is installed in the passed database and in the current it is installed but with + never version - no actions are taken. If dockerd_sock parameter is passed, the + migration process will use loaded images from docker library of the currently installed image. Args: @@ -784,7 +786,7 @@ def get_package_source(self, ref = parse_reference_expression(package_expression) return self.get_package_source(package_ref=ref) elif repository_reference: - repo_ref = DockerReference.parse(repository_reference) + repo_ref = utils.DockerReference.parse(repository_reference) repository = repo_ref['name'] reference = repo_ref['tag'] or repo_ref['digest'] reference = reference or 'latest' @@ -815,8 +817,8 @@ def get_package_source(self, if package_entry.default_reference is not None: package_ref.reference = package_entry.default_reference else: - raise PackageManagerError(f'No default reference tag. ' - f'Please specify the version or tag explicitly') + raise PackageManagerError('No default reference tag. ' + 'Please specify the version or tag explicitly') return RegistrySource(package_entry.repository, package_ref.reference, @@ -888,7 +890,7 @@ def get_installed_packages_list(self) -> List[Package]: Installed packages dictionary. """ - return [self.get_installed_package(entry.name) + return [self.get_installed_package(entry.name) for entry in self.database if entry.installed] def _migrate_package_database(self, old_package_database: PackageDatabase): @@ -948,11 +950,11 @@ def _systemctl_action(self, package: Package, action: str): run_command(f'systemctl {action} {name}@{npu}') def _install_cli_plugins(self, package: Package): - for command in ('show', 'config', 'clear'): + for command in SONIC_CLI_COMMANDS: self._install_cli_plugin(package, command) def _uninstall_cli_plugins(self, package: Package): - for command in ('show', 'config', 'clear'): + for command in SONIC_CLI_COMMANDS: self._uninstall_cli_plugin(package, command) def _install_cli_plugin(self, package: Package, command: str): @@ -978,12 +980,17 @@ def get_manager() -> 'PackageManager': PackageManager """ - docker_api = DockerApi(docker.from_env()) + docker_api = DockerApi(docker.from_env(), ProgressManager()) registry_resolver = RegistryResolver() - return PackageManager(DockerApi(docker.from_env(), ProgressManager()), + metadata_resolver = MetadataResolver(docker_api, registry_resolver) + feature_registry = FeatureRegistry(SonicDB) + service_creator = ServiceCreator(feature_registry, + SonicDB) + + return PackageManager(docker_api, registry_resolver, PackageDatabase.from_file(), - MetadataResolver(docker_api, registry_resolver), - ServiceCreator(FeatureRegistry(SonicDB), SonicDB), + metadata_resolver, + service_creator, device_info, filelock.FileLock(PACKAGE_MANAGER_LOCK_FILE, timeout=0)) diff --git a/sonic_package_manager/metadata.py b/sonic_package_manager/metadata.py index 7f7c25ceafbc..62cc8bea532a 100644 --- a/sonic_package_manager/metadata.py +++ b/sonic_package_manager/metadata.py @@ -24,10 +24,10 @@ def deep_update(dst: Dict, src: Dict) -> Dict: for key, value in src.items(): if isinstance(value, dict): - node = dst.setdefault(key, {}) - deep_update(node, value) + node = dst.setdefault(key, {}) + deep_update(node, value) else: - dst[key] = value + dst[key] = value return dst @@ -183,3 +183,4 @@ def from_labels(cls, labels: Dict[str, str]) -> Metadata: raise MetadataError(f'Failed to parse component version: {err}') return Metadata(Manifest.marshal(manifest_dict), components) + diff --git a/sonic_package_manager/registry.py b/sonic_package_manager/registry.py index 8a09d9136e98..8c03b078d275 100644 --- a/sonic_package_manager/registry.py +++ b/sonic_package_manager/registry.py @@ -38,7 +38,7 @@ def get_token(realm, service, scope) -> str: response = requests.get(f'{realm}?scope={scope}&service={service}') if response.status_code != requests.codes.ok: - raise AuthenticationServiceError(f'Failed to retrieve token') + raise AuthenticationServiceError('Failed to retrieve token') content = json.loads(response.content) token = content['token'] diff --git a/sonic_package_manager/service_creator/__init__.py b/sonic_package_manager/service_creator/__init__.py index e2af81ceb59b..b0f4a240865e 100644 --- a/sonic_package_manager/service_creator/__init__.py +++ b/sonic_package_manager/service_creator/__init__.py @@ -1,3 +1,4 @@ #!/usr/bin/env python ETC_SONIC_PATH = '/etc/sonic' +SONIC_CLI_COMMANDS = ('show', 'config', 'clear') diff --git a/sonic_package_manager/service_creator/creator.py b/sonic_package_manager/service_creator/creator.py index 4c618eb7eaf2..4a7b032854c2 100644 --- a/sonic_package_manager/service_creator/creator.py +++ b/sonic_package_manager/service_creator/creator.py @@ -5,7 +5,7 @@ import stat import subprocess from collections import defaultdict -from typing import Dict +from typing import Dict, Type import jinja2 as jinja2 from prettyprinter import pformat @@ -15,6 +15,7 @@ from sonic_package_manager.package import Package from sonic_package_manager.service_creator import ETC_SONIC_PATH from sonic_package_manager.service_creator.feature import FeatureRegistry +from sonic_package_manager.service_creator.sonic_db import SonicDB from sonic_package_manager.service_creator.utils import in_chroot SERVICE_FILE_TEMPLATE = 'sonic.service.j2' @@ -78,12 +79,22 @@ def set_executable_bit(filepath): os.chmod(filepath, st.st_mode | stat.S_IEXEC) +def remove_if_exists(path): + """ Remove filepath if it exists """ + + if not os.path.exists(path): + return + + os.remove(path) + log.info(f'removed {path}') + + def run_command(command: str): """ Run arbitrary bash command. Args: command: String command to execute as bash script Raises: - PackageManagerError: Raised when the command return code + ServiceCreatorError: Raised when the command return code is not 0. """ @@ -104,12 +115,12 @@ class ServiceCreator: def __init__(self, feature_registry: FeatureRegistry, - sonic_db): + sonic_db: Type[SonicDB]): """ Initialize ServiceCreator with: - + Args: feature_registry: FeatureRegistry object. - sonic_db: SonicDb interface. + sonic_db: SonicDB interface. """ self.feature_registry = feature_registry @@ -120,8 +131,8 @@ def create(self, register_feature: bool = True, state: str = 'enabled', owner: str = 'local'): - """ Register package as SONiC service. - + """ Register package as SONiC service. + Args: package: Package object to install. register_feature: Wether to register this package in FEATURE table. @@ -154,7 +165,7 @@ def remove(self, package: Package, deregister_feature: bool = True): """ Uninstall SONiC service provided by the package. - + Args: package: Package object to uninstall. deregister_feature: Wether to deregister this package from FEATURE table. @@ -164,19 +175,12 @@ def remove(self, """ name = package.manifest['service']['name'] - - def remove_file(path): - if os.path.exists(path): - os.remove(path) - log.info(f'removed {path}') - - remove_file(os.path.join(SYSTEMD_LOCATION, f'{name}.service')) - remove_file(os.path.join(SYSTEMD_LOCATION, f'{name}@.service')) - remove_file(os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, f'{name}.sh')) - remove_file(os.path.join(DOCKER_CTL_SCRIPT_LOCATION, f'{name}.sh')) - remove_file(os.path.join(DEBUG_DUMP_SCRIPT_LOCATION, f'{name}')) - remove_file(os.path.join(ETC_SONIC_PATH, f'{name}_reconcile')) - + remove_if_exists(os.path.join(SYSTEMD_LOCATION, f'{name}.service')) + remove_if_exists(os.path.join(SYSTEMD_LOCATION, f'{name}@.service')) + remove_if_exists(os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, f'{name}.sh')) + remove_if_exists(os.path.join(DOCKER_CTL_SCRIPT_LOCATION, f'{name}.sh')) + remove_if_exists(os.path.join(DEBUG_DUMP_SCRIPT_LOCATION, f'{name}')) + remove_if_exists(os.path.join(ETC_SONIC_PATH, f'{name}_reconcile')) self.update_dependent_list_file(package, remove=True) self._post_operation_hook() @@ -185,8 +189,8 @@ def remove_file(path): self.remove_config(package) def generate_container_mgmt(self, package: Package): - """ Generates container management script under /usr/bin/.sh for package. - + """ Generates container management script under /usr/bin/.sh for package. + Args: package: Package object to generate script for. Returns: @@ -228,8 +232,8 @@ def generate_container_mgmt(self, package: Package): log.info(f'generated {script_path}') def generate_service_mgmt(self, package: Package): - """ Generates service management script under /usr/local/bin/.sh for package. - + """ Generates service management script under /usr/local/bin/.sh for package. + Args: package: Package object to generate script for. Returns: @@ -249,8 +253,8 @@ def generate_service_mgmt(self, package: Package): log.info(f'generated {script_path}') def generate_systemd_service(self, package: Package): - """ Generates systemd service(s) file and timer(s) (if needed) for package. - + """ Generates systemd service(s) file and timer(s) (if needed) for package. + Args: package: Package object to generate service for. Returns: @@ -297,13 +301,15 @@ def generate_systemd_service(self, package: Package): def update_dependent_list_file(self, package: Package, remove=False): """ This function updates dependent list file for packages listed in "dependent-of" (path: /etc/sonic/_dependent file). - + Args: package: Package to update packages dependent of it. + remove: True if update for removal process. Returns: None. """ + name = package.manifest['service']['name'] dependent_of = package.manifest['service']['dependent-of'] host_service = package.manifest['service']['host-service'] @@ -337,7 +343,7 @@ def update_dependent(service, name, multi_inst): def generate_dump_script(self, package): """ Generates dump plugin script for package. - + Args: package: Package object to generate dump plugin script for. Returns: @@ -363,7 +369,7 @@ def generate_dump_script(self, package): def get_shutdown_sequence(self, reboot_type: str, packages: Dict[str, Package]): """ Returns shutdown sequence file for particular reboot type. - + Args: reboot_type: Reboot type to generated service shutdown sequence for. packages: Dict of installed packages. @@ -410,7 +416,7 @@ def filter_not_available(services): def generate_shutdown_sequence_file(self, reboot_type: str, packages: Dict[str, Package]): """ Generates shutdown sequence file for particular reboot type (path: /etc/sonic/-reboot_order). - + Args: reboot_type: Reboot type to generated service shutdown sequence for. packages: Dict of installed packages. @@ -421,11 +427,11 @@ def generate_shutdown_sequence_file(self, reboot_type: str, packages: Dict[str, order = self.get_shutdown_sequence(reboot_type, packages) with open(os.path.join(ETC_SONIC_PATH, f'{reboot_type}-reboot_order'), 'w') as file: file.write(' '.join(order)) - + def generate_shutdown_sequence_files(self, packages: Dict[str, Package]): - """ Generates shutdown sequence file for fast and warm reboot. + """ Generates shutdown sequence file for fast and warm reboot. (path: /etc/sonic/-reboot_order). - + Args: packages: Dict of installed packages. Returns: diff --git a/sonic_package_manager/source.py b/sonic_package_manager/source.py index c179e0b3ee19..7a13dccbac59 100644 --- a/sonic_package_manager/source.py +++ b/sonic_package_manager/source.py @@ -26,6 +26,7 @@ def get_metadata(self) -> Metadata: Returns: Metadata """ + raise NotImplementedError def install_image(self, package: Package):