diff --git a/config/main.py b/config/main.py index 471a8f920a55..19fcc88494ac 100644 --- a/config/main.py +++ b/config/main.py @@ -2087,20 +2087,28 @@ def warm_restart(ctx, redis_unix_socket_path): ctx.obj = {'db': config_db, 'state_db': state_db, 'prefix': prefix} @warm_restart.command('enable') -@click.argument('module', metavar='', default='system', required=False, type=click.Choice(["system", "swss", "bgp", "teamd"])) +@click.argument('module', metavar='', default='system', required=False) @click.pass_context def warm_restart_enable(ctx, module): state_db = ctx.obj['state_db'] + config_db = ctx.obj['db'] + feature_table = config_db.get_table('FEATURE') + if module != 'system' and module not in feature_table: + exit('Feature {} is unknown'.format(module)) prefix = ctx.obj['prefix'] _hash = '{}{}'.format(prefix, module) state_db.set(state_db.STATE_DB, _hash, 'enable', 'true') state_db.close(state_db.STATE_DB) @warm_restart.command('disable') -@click.argument('module', metavar='', default='system', required=False, type=click.Choice(["system", "swss", "bgp", "teamd"])) +@click.argument('module', metavar='', default='system', required=False) @click.pass_context def warm_restart_enable(ctx, module): state_db = ctx.obj['state_db'] + config_db = ctx.obj['db'] + feature_table = config_db.get_table('FEATURE') + if module != 'system' and module not in feature_table: + exit('Feature {} is unknown'.format(module)) prefix = ctx.obj['prefix'] _hash = '{}{}'.format(prefix, module) state_db.set(state_db.STATE_DB, _hash, 'enable', 'false') diff --git a/scripts/fast-reboot b/scripts/fast-reboot index c782265e6b71..97e8dc6c141f 100755 --- a/scripts/fast-reboot +++ b/scripts/fast-reboot @@ -7,6 +7,7 @@ WARM_DIR=/host/warmboot REDIS_FILE=dump.rdb REBOOT_SCRIPT_NAME=$(basename $0) REBOOT_TYPE="${REBOOT_SCRIPT_NAME}" +SHUTDOWN_ORDER_FILE="/etc/sonic/${REBOOT_TYPE}_order" VERBOSE=no FORCE=no IGNORE_ASIC=no @@ -567,82 +568,72 @@ if [ -x ${LOG_SSD_HEALTH} ]; then fi -# Kill nat docker after saving the conntrack table -debug "Stopping nat ..." -/usr/local/bin/dump_nat_entries.py -docker kill nat > /dev/null || true -systemctl stop nat -debug "Stopped nat ..." - -# Kill radv before stopping BGP service to prevent announcing our departure. -debug "Stopping radv service..." -systemctl stop radv -debug "Stopped radv service..." - -# Kill bgpd to start the bgp graceful restart procedure -debug "Stopping bgp ..." -systemctl stop bgp -debug "Stopped bgp ..." - -# Kill sflow docker -debug "Stopping sflow ..." -container kill sflow &> /dev/null || debug "Docker sflow is not running ($?) ..." -systemctl stop sflow -debug "Stopped sflow ..." - -# Kill lldp, otherwise it sends informotion about reboot. -# We call `docker kill lldp` to ensure the container stops as quickly as possible, -# then immediately call `systemctl stop lldp` to prevent the service from -# restarting the container automatically. -container kill lldp &> /dev/null || debug "Docker lldp is not running ($?) ..." -systemctl stop lldp - -if [[ "$REBOOT_TYPE" = "fast-reboot" ]]; then - debug "Stopping teamd ..." - systemctl stop teamd - debug "Stopped teamd ..." +if [[ -f ${SHUTDOWN_ORDER_FILE} ]]; then + SERVICES_TO_STOP="$(cat ${SHUTDOWN_ORDER_FILE})" +else + # TODO: to be removed once sonic-buildimage change is in + if [[ "${REBOOT_TYPE}" == "fast-reboot" ]]; then + SERVICES_TO_STOP="nat radv bgp sflow lldp swss teamd syncd" + elif [[ "${REBOOT_TYPE}" == "fastfast-reboot" || "${REBOOT_TYPE}" == "warm-reboot" ]]; then + SERVICES_TO_STOP="nat radv bgp sflow lldp teamd swss syncd" + else + error "Unexpected reboot type ${REBOOT_TYPE}" + exit $EXIT_FAILURE + fi fi -debug "Stopping swss service ..." -systemctl stop swss -debug "Stopped swss service ..." +for service in ${SERVICES_TO_STOP}; do + debug "Stopping ${service} ..." -if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" ]]; then - # Pre-shutdown syncd - initialize_pre_shutdown + # TODO: These exceptions for nat, sflow, lldp + # have to be coded in corresponding service scripts - if [[ "x$sonic_asic_type" == x"mellanox" ]]; then - check_issu_bank_file + if [[ "${service}" = "nat" ]]; then + /usr/local/bin/dump_nat_entries.py fi - request_pre_shutdown - - wait_for_pre_shutdown_complete_or_fail - - if [[ "x$sonic_asic_type" == x"mellanox" ]]; then - check_issu_bank_file + if [[ "${service}" = "nat" || "${service}" = "sflow" || "${service}" = "lldp" ]]; then + container kill "${service}" &> /dev/null || debug "Docker ${service} is not running ($?) ..." fi - # Warm reboot: dump state to host disk - if [[ "$REBOOT_TYPE" = "fastfast-reboot" ]]; then - sonic-db-cli ASIC_DB FLUSHDB > /dev/null - sonic-db-cli COUNTERS_DB FLUSHDB > /dev/null - sonic-db-cli FLEX_COUNTER_DB FLUSHDB > /dev/null + if [[ "${service}" = "syncd" ]]; then + systemctl stop ${service} || debug "Ignore stopping ${service} service error $?" + else + systemctl stop ${service} fi - # TODO: backup_database preserves FDB_TABLE - # need to cleanup as well for fastfast boot case - backup_database + debug "Stopped ${service}" - # Stop teamd gracefully - debug "Stopping teamd ..." - systemctl stop teamd - debug "Stopped teamd ..." -fi + if [[ "${service}" = "swss" ]]; then + if [[ "$REBOOT_TYPE" = "warm-reboot" || "$REBOOT_TYPE" = "fastfast-reboot" ]]; then + # Pre-shutdown syncd + initialize_pre_shutdown + + if [[ "x$sonic_asic_type" == x"mellanox" ]]; then + check_issu_bank_file + fi -debug "Stopping syncd ..." -systemctl stop syncd || debug "Ignore stopping syncd service error $?" -debug "Stopped syncd ..." + request_pre_shutdown + + wait_for_pre_shutdown_complete_or_fail + + if [[ "x$sonic_asic_type" == x"mellanox" ]]; then + check_issu_bank_file + fi + + # Warm reboot: dump state to host disk + if [[ "$REBOOT_TYPE" = "fastfast-reboot" ]]; then + sonic-db-cli ASIC_DB FLUSHDB > /dev/null + sonic-db-cli COUNTERS_DB FLUSHDB > /dev/null + sonic-db-cli FLEX_COUNTER_DB FLUSHDB > /dev/null + fi + + # TODO: backup_database preserves FDB_TABLE + # need to cleanup as well for fastfast boot case + backup_database + fi + fi +done # Kill other containers to make the reboot faster # We call `docker kill ...` to ensure the container stops as quickly as possible, diff --git a/scripts/generate_shutdown_order.py b/scripts/generate_shutdown_order.py new file mode 100644 index 000000000000..a9a1168a051e --- /dev/null +++ b/scripts/generate_shutdown_order.py @@ -0,0 +1,15 @@ +#!/usr/bin/python3 + +''' This script is used to generate initial warm/fast shutdown order file ''' + +from sonic_package_manager import PackageManager + +def main(): + manager = PackageManager.get_manager() + installed_packages = manager.get_installed_packages() + print('installed packages {}'.format(installed_packages)) + manager.service_creator.generate_shutdown_sequence_files(installed_packages) + print('Done.') + +if __name__ == '__main__': + main() diff --git a/setup.py b/setup.py index de5a3cbdac79..6c8a349c69c8 100644 --- a/setup.py +++ b/setup.py @@ -95,6 +95,7 @@ 'scripts/fdbshow', 'scripts/gearboxutil', 'scripts/generate_dump', + 'scripts/generate_shutdown_order.py', 'scripts/intfutil', 'scripts/intfstat', 'scripts/ipintutil', @@ -187,9 +188,10 @@ 'sonic-py-common', 'sonic-yang-mgmt', 'swsssdk>=2.0.1', - 'tabulate>=0.8.2', - 'www-authenticate>=0.9.2', - 'xmltodict>=0.12.0', + 'tabulate==0.8.2', + 'toposort==1.6', + 'www-authenticate==0.9.2', + 'xmltodict==0.12.0', ], setup_requires= [ 'pytest-runner', diff --git a/sonic_package_manager/manager.py b/sonic_package_manager/manager.py index 6aeb5efd71dc..3caf90d95f15 100644 --- a/sonic_package_manager/manager.py +++ b/sonic_package_manager/manager.py @@ -6,7 +6,7 @@ import pkgutil import tempfile from inspect import signature -from typing import Any, Iterable, Callable, Dict, Optional +from typing import Any, Iterable, List, Callable, Dict, Optional import docker import filelock @@ -375,6 +375,14 @@ def install_from_source(self, self.service_creator.create(package, state=feature_state, owner=default_owner) exits.callback(rollback(self.service_creator.remove, package)) + self.service_creator.generate_shutdown_sequence_files( + self._get_installed_packages_and(package) + ) + exits.callback(rollback( + self.service_creator.generate_shutdown_sequence_files, + self.get_installed_packages()) + ) + if not skip_host_plugins: self._install_cli_plugins(package) exits.callback(rollback(self._uninstall_cli_plugins, package)) @@ -429,6 +437,9 @@ def uninstall(self, name: str, force=False): try: self._uninstall_cli_plugins(package) self.service_creator.remove(package) + 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}, @@ -525,8 +536,8 @@ def upgrade_from_source(self, old_package, 'start')) self.service_creator.remove(old_package, deregister_feature=False) - exits.callback(rollback(self.service_creator.create, - old_package, register_feature=False)) + exits.callback(rollback(self.service_creator.create, old_package, + register_feature=False)) # Clean containers based on the old image containers = self.docker.ps(filters={'ancestor': old_package.image_id}, @@ -538,6 +549,14 @@ def upgrade_from_source(self, exits.callback(rollback(self.service_creator.remove, new_package, register_feature=False)) + self.service_creator.generate_shutdown_sequence_files( + self._get_installed_packages_and(new_package) + ) + exits.callback(rollback( + self.service_creator.generate_shutdown_sequence_files, + self._get_installed_packages_and(old_package)) + ) + if self.feature_registry.is_feature_enabled(new_feature): self._systemctl_action(new_package, 'start') exits.callback(rollback(self._systemctl_action, @@ -818,10 +837,19 @@ def get_installed_packages(self) -> Dict[str, Package]: """ return { - entry.name: self.get_installed_package(entry.name) - for entry in self.database if entry.installed + entry.name: entry for entry in self.get_installed_packages_list() } + def get_installed_packages_list(self) -> List[Package]: + """ Returns a list of installed packages. + + Returns: + Installed packages dictionary. + """ + + return [self.get_installed_package(entry.name) + for entry in self.database if entry.installed] + def _migrate_package_database(self, old_package_database: PackageDatabase): """ Performs part of package migration process. For every package in old_package_database that is not listed in current diff --git a/sonic_package_manager/manifest.py b/sonic_package_manager/manifest.py index b58a0d10f055..c126e2eef129 100644 --- a/sonic_package_manager/manifest.py +++ b/sonic_package_manager/manifest.py @@ -92,8 +92,10 @@ class ManifestRoot(ManifestNode): def marshal(self, value: Optional[dict]): result = {} - if value is None: - value = {} + value = value or {} + + if not isinstance(value, dict): + raise ManifestError(f'"{self.key}" field has to be a dictionary') for item in self.items: next_value = value.get(item.key) @@ -115,7 +117,7 @@ def marshal(self, value): if value is None: if self.default is not None: return self.default - raise ManifestError(f'{self.key} is a required field but it is missing') + raise ManifestError(f'"{self.key}" is a required field but it is missing') try: return_value = self.type.marshal(value) except Exception as err: @@ -130,10 +132,12 @@ class ManifestArray(ManifestNode): type: Any def marshal(self, value): - if value is None: - return [] - return_value = [] + value = value or [] + + if not isinstance(value, list): + raise ManifestError(f'"{self.key}" has to be of type list') + try: for item in value: return_value.append(self.type.marshal(item)) @@ -173,6 +177,14 @@ def unmarshal(self, value): ManifestField('asic-service', DefaultMarshaller(bool), False), ManifestField('host-service', DefaultMarshaller(bool), True), ManifestField('delayed', DefaultMarshaller(bool), False), + ManifestRoot('warm-shutdown', [ + ManifestArray('after', DefaultMarshaller(str)), + ManifestArray('before', DefaultMarshaller(str)), + ]), + ManifestRoot('fast-shutdown', [ + ManifestArray('after', DefaultMarshaller(str)), + ManifestArray('before', DefaultMarshaller(str)), + ]), ]), ManifestRoot('container', [ ManifestField('privileged', DefaultMarshaller(bool), False), @@ -187,6 +199,7 @@ def unmarshal(self, value): ]), ManifestArray('processes', ManifestRoot('processes', [ ManifestField('name', DefaultMarshaller(str)), + ManifestField('reconciles', DefaultMarshaller(bool), False), ])), ManifestRoot('cli', [ ManifestField('mandatory', DefaultMarshaller(bool), False), diff --git a/sonic_package_manager/service_creator/creator.py b/sonic_package_manager/service_creator/creator.py index 54b9315bee0d..4c618eb7eaf2 100644 --- a/sonic_package_manager/service_creator/creator.py +++ b/sonic_package_manager/service_creator/creator.py @@ -4,10 +4,12 @@ import os import stat import subprocess +from collections import defaultdict from typing import Dict import jinja2 as jinja2 from prettyprinter import pformat +from toposort import toposort_flatten, CircularDependencyError from sonic_package_manager.logger import log from sonic_package_manager.package import Package @@ -91,7 +93,7 @@ def run_command(command: str): shell=True, executable='/bin/bash', stdout=subprocess.PIPE) - (out, _) = proc.communicate() + (_, _) = proc.communicate() if proc.returncode != 0: raise ServiceCreatorError(f'Failed to execute "{command}"') @@ -100,25 +102,46 @@ class ServiceCreator: """ Creates and registers services in SONiC based on the package manifest. """ - def __init__(self, feature_registry: FeatureRegistry, sonic_db): + def __init__(self, + feature_registry: FeatureRegistry, + sonic_db): + """ Initialize ServiceCreator with: + + Args: + feature_registry: FeatureRegistry object. + sonic_db: SonicDb interface. + """ + self.feature_registry = feature_registry self.sonic_db = sonic_db def create(self, package: Package, - register_feature=True, - state='enabled', - owner='local'): + register_feature: bool = True, + state: str = 'enabled', + owner: str = 'local'): + """ Register package as SONiC service. + + Args: + package: Package object to install. + register_feature: Wether to register this package in FEATURE table. + state: Default feature state. + owner: Default feature owner. + + Returns: + None + """ + try: self.generate_container_mgmt(package) self.generate_service_mgmt(package) self.update_dependent_list_file(package) self.generate_systemd_service(package) self.generate_dump_script(package) + self.generate_service_reconciliation_file(package) self.set_initial_config(package) - - self.post_operation_hook() + self._post_operation_hook() if register_feature: self.feature_registry.register(package.manifest, @@ -127,7 +150,19 @@ def create(self, self.remove(package, register_feature) raise - def remove(self, package: Package, deregister_feature=True): + 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. + + Returns: + None + """ + name = package.manifest['service']['name'] def remove_file(path): @@ -140,20 +175,24 @@ def remove_file(path): 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')) self.update_dependent_list_file(package, remove=True) - - self.post_operation_hook() + self._post_operation_hook() if deregister_feature: self.feature_registry.deregister(package.manifest['service']['name']) self.remove_config(package) - def post_operation_hook(self): - if not in_chroot(): - run_command('systemctl daemon-reload') - def generate_container_mgmt(self, package: Package): + """ Generates container management script under /usr/bin/.sh for package. + + Args: + package: Package object to generate script for. + Returns: + None + """ + image_id = package.image_id name = package.manifest['service']['name'] container_spec = package.manifest['container'] @@ -189,6 +228,14 @@ 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. + + Args: + package: Package object to generate script for. + Returns: + None + """ + name = package.manifest['service']['name'] multi_instance_services = self.feature_registry.get_multi_instance_features() script_path = os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, f'{name}.sh') @@ -202,6 +249,14 @@ 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. + + Args: + package: Package object to generate service for. + Returns: + None + """ + name = package.manifest['service']['name'] multi_instance_services = self.feature_registry.get_multi_instance_features() @@ -240,6 +295,15 @@ def generate_systemd_service(self, package: Package): log.info(f'generated {output_file}') 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. + Returns: + None. + + """ name = package.manifest['service']['name'] dependent_of = package.manifest['service']['dependent-of'] host_service = package.manifest['service']['host-service'] @@ -272,6 +336,14 @@ def update_dependent(service, name, multi_inst): update_dependent(service, name, multi_inst=True) def generate_dump_script(self, package): + """ Generates dump plugin script for package. + + Args: + package: Package object to generate dump plugin script for. + Returns: + None. + """ + name = package.manifest['service']['name'] if not package.manifest['package']['debug-dump']: @@ -289,31 +361,113 @@ def generate_dump_script(self, package): render_template(scrip_template, script_path, render_ctx, executable=True) log.info(f'generated {script_path}') - def get_tables(self, table_name): - tables = [] + 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. + Returns: + Ordered list of service names. + """ + + shutdown_graph = defaultdict(set) + + def service_exists(service): + for package in packages.values(): + if package.manifest['service']['name'] == service: + return True + log.info(f'Service {service} is not installed, it is skipped...') + return False + + def filter_not_available(services): + return set(filter(service_exists, services)) + + for package in packages.values(): + service_props = package.manifest['service'] + after = filter_not_available(service_props[f'{reboot_type}-shutdown']['after']) + before = filter_not_available(service_props[f'{reboot_type}-shutdown']['before']) + + if not after and not before: + continue - running_table = self.sonic_db.running_table(table_name) - if running_table is not None: - tables.append(running_table) + name = package.manifest['service']['name'] + shutdown_graph[name].update(after) - persistent_table = self.sonic_db.persistent_table(table_name) - if persistent_table is not None: - tables.append(persistent_table) + for service in before: + shutdown_graph[service].update({name}) - initial_table = self.sonic_db.initial_table(table_name) - if initial_table is not None: - tables.append(initial_table) + log.debug(f'shutdown graph {pformat(shutdown_graph)}') - return tables + try: + order = toposort_flatten(shutdown_graph) + except CircularDependencyError as err: + raise ServiceCreatorError(f'Circular dependency found in {reboot_type} error: {err}') + + log.debug(f'shutdown order {pformat(order)}') + return order + + 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. + Returns: + None. + """ + + 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. + (path: /etc/sonic/-reboot_order). + + Args: + packages: Dict of installed packages. + Returns: + None. + """ + + for reboot_type in ('fast', 'warm'): + self.generate_shutdown_sequence_file(reboot_type, packages) + + def generate_service_reconciliation_file(self, package): + """ Generates reconciliation file for package + (path: /etc/sonic/_reconcile). + + Args: + package: Package object to generate service reconciliation file for. + Returns: + None + """ + + name = package.manifest['service']['name'] + all_processes = package.manifest['processes'] + processes = [process['name'] for process in all_processes if process['reconciles']] + with open(os.path.join(ETC_SONIC_PATH, f'{name}_reconcile'), 'w') as file: + file.write(' '.join(processes)) def set_initial_config(self, package): + """ Set initial package configuration from manifest. + This method updates but does not override existing entries in tables. + + Args: + package: Package object to set initial configuration for. + Returns: + None + """ + init_cfg = package.manifest['package']['init-cfg'] for tablename, content in init_cfg.items(): if not isinstance(content, dict): continue - tables = self.get_tables(tablename) + tables = self._get_tables(tablename) for key in content: for table in tables: @@ -325,18 +479,50 @@ def set_initial_config(self, package): table.set(key, fvs) def remove_config(self, package): - # Remove configuration based on init-cfg tables, so having - # init-cfg even with tables without keys might be a good idea. - # TODO: init-cfg should be validated with yang model - # TODO: remove config from tables known to yang model + """ Remove configuration based on init-cfg tables, so having + init-cfg even with tables without keys might be a good idea. + TODO: init-cfg should be validated with yang model + TODO: remove config from tables known to yang model + + Args: + package: Package object remove initial configuration for. + Returns: + None + """ + init_cfg = package.manifest['package']['init-cfg'] for tablename, content in init_cfg.items(): if not isinstance(content, dict): continue - tables = self.get_tables(tablename) + tables = self._get_tables(tablename) for key in content: for table in tables: table._del(key) + + def _get_tables(self, table_name): + """ Return swsscommon Tables for all kinds of configuration DBs """ + + tables = [] + + running_table = self.sonic_db.running_table(table_name) + if running_table is not None: + tables.append(running_table) + + persistent_table = self.sonic_db.persistent_table(table_name) + if persistent_table is not None: + tables.append(persistent_table) + + initial_table = self.sonic_db.initial_table(table_name) + if initial_table is not None: + tables.append(initial_table) + + return tables + + def _post_operation_hook(self): + """ Common operations executed after service is created/removed. """ + + if not in_chroot(): + run_command('systemctl daemon-reload') diff --git a/tests/sonic_package_manager/conftest.py b/tests/sonic_package_manager/conftest.py index cee997596c64..2788a75cd3f2 100644 --- a/tests/sonic_package_manager/conftest.py +++ b/tests/sonic_package_manager/conftest.py @@ -75,6 +75,36 @@ def __init__(self): components={ 'libswsscommon': Version.parse('1.0.0'), 'libsairedis': Version.parse('1.0.0') + }, + warm_shutdown={ + 'before': ['syncd'], + }, + fast_shutdown={ + 'before': ['syncd'], + }, + processes=[ + { + 'name': 'orchagent', + 'reconciles': True, + }, + { + 'name': 'neighsyncd', + 'reconciles': True, + } + ], + ) + self.add('docker-syncd', 'latest', 'syncd', '1.0.0') + self.add('docker-teamd', 'latest', 'teamd', '1.0.0', + components={ + 'libswsscommon': Version.parse('1.0.0'), + 'libsairedis': Version.parse('1.0.0') + }, + warm_shutdown={ + 'before': ['syncd'], + 'after': ['swss'], + }, + fast_shutdown={ + 'before': ['swss'], } ) self.add('Azure/docker-test', '1.6.0', 'test-package', '1.6.0') @@ -108,7 +138,9 @@ def from_tarball(self, filepath: str) -> Manifest: components = self.metadata_store[path][ref]['components'] return Metadata(manifest, components) - def add(self, repo, reference, name, version, components=None): + def add(self, repo, reference, name, version, components=None, + warm_shutdown=None, fast_shutdown=None, + processes=None): repo_dict = self.metadata_store.setdefault(repo, {}) repo_dict[reference] = { 'manifest': { @@ -119,7 +151,10 @@ def add(self, repo, reference, name, version, components=None): }, 'service': { 'name': name, - } + 'warm-shutdown': warm_shutdown or {}, + 'fast-shutdown': fast_shutdown or {}, + }, + 'processes': processes or [], }, 'components': components or {}, } @@ -189,6 +224,26 @@ def fake_db(fake_metadata_resolver): installed=True, built_in=True ) + add_package( + content, + fake_metadata_resolver, + 'docker-syncd', + 'latest', + description='SONiC syncd service', + default_reference='1.0.0', + installed=True, + built_in=True + ) + add_package( + content, + fake_metadata_resolver, + 'docker-teamd', + 'latest', + description='SONiC teamd service', + default_reference='1.0.0', + installed=True, + built_in=True + ) add_package( content, fake_metadata_resolver, diff --git a/tests/sonic_package_manager/test_manager.py b/tests/sonic_package_manager/test_manager.py index c7eb1ca7ac45..48ac6dfda8de 100644 --- a/tests/sonic_package_manager/test_manager.py +++ b/tests/sonic_package_manager/test_manager.py @@ -256,11 +256,17 @@ def test_upgrade_from_file_known_package(package_manager, fake_db, sonic_fs): def test_installation_non_default_owner(package_manager, anything, mock_service_creator): package_manager.install('test-package', default_owner='kube') mock_service_creator.create.assert_called_once_with(anything, state='disabled', owner='kube') + mock_service_creator.generate_shutdown_sequence_files.assert_called_once_with( + package_manager.get_installed_packages() + ) def test_installation_enabled(package_manager, anything, mock_service_creator): package_manager.install('test-package', enable=True) mock_service_creator.create.assert_called_once_with(anything, state='enabled', owner='local') + mock_service_creator.generate_shutdown_sequence_files.assert_called_once_with( + package_manager.get_installed_packages() + ) def test_installation_fault(package_manager, mock_docker_api, mock_service_creator): diff --git a/tests/sonic_package_manager/test_manifest.py b/tests/sonic_package_manager/test_manifest.py index efdcc558ab8b..2f201b81075b 100644 --- a/tests/sonic_package_manager/test_manifest.py +++ b/tests/sonic_package_manager/test_manifest.py @@ -57,6 +57,20 @@ def test_manifest_v1_mounts_invalid(): 'mounts': [{'not-source': 'a', 'target': 'b', 'type': 'bind'}]}}) +def test_manifest_invalid_root_type(): + manifest_json_input = {'package': { 'name': 'test', 'version': '1.0.0'}, + 'service': {'name': 'test'}, 'container': 'abc'} + with pytest.raises(ManifestError): + Manifest.marshal(manifest_json_input) + + +def test_manifest_invalid_array_type(): + manifest_json_input = {'package': { 'name': 'test', 'version': '1.0.0'}, + 'service': {'name': 'test', 'warm-shutdown': {'after': 'bgp'}}} + with pytest.raises(ManifestError): + Manifest.marshal(manifest_json_input) + + def test_manifest_v1_unmarshal(): manifest_json_input = {'package': {'name': 'test', 'version': '1.0.0', 'depends': [ diff --git a/tests/sonic_package_manager/test_service_creator.py b/tests/sonic_package_manager/test_service_creator.py index fec8de600cc7..ffa673753178 100644 --- a/tests/sonic_package_manager/test_service_creator.py +++ b/tests/sonic_package_manager/test_service_creator.py @@ -28,27 +28,58 @@ def manifest(): 'dependent-of': ['swss'], 'asic-service': False, 'host-service': True, + 'warm-shutdown': { + 'before': ['syncd'], + 'after': ['swss'], + }, + 'fast-shutdown': { + 'before': ['swss'], + }, }, 'container': { 'privileged': True, 'volumes': [ '/etc/sonic:/etc/sonic:ro' ] - } + }, + 'processes': [ + { + 'name': 'test-process', + 'reconciles': True, + }, + { + 'name': 'test-process-2', + 'reconciles': False, + }, + { + 'name': 'test-process-3', + 'reconciles': True, + }, + ] }) -def test_service_creator(sonic_fs, manifest, mock_feature_registry, mock_sonic_db): +def test_service_creator(sonic_fs, manifest, package_manager, mock_feature_registry, mock_sonic_db): creator = ServiceCreator(mock_feature_registry, mock_sonic_db) entry = PackageEntry('test', 'azure/sonic-test') package = Package(entry, Metadata(manifest)) + installed_packages = package_manager._get_installed_packages_and(package) creator.create(package) + creator.generate_shutdown_sequence_files(installed_packages) assert sonic_fs.exists(os.path.join(ETC_SONIC_PATH, 'swss_dependent')) assert sonic_fs.exists(os.path.join(DOCKER_CTL_SCRIPT_LOCATION, 'test.sh')) assert sonic_fs.exists(os.path.join(SERVICE_MGMT_SCRIPT_LOCATION, 'test.sh')) assert sonic_fs.exists(os.path.join(SYSTEMD_LOCATION, 'test.service')) + def read_file(name): + with open(os.path.join(ETC_SONIC_PATH, name)) as file: + return file.read() + + assert read_file('warm-reboot_order') == 'swss teamd test syncd' + assert read_file('fast-reboot_order') == 'teamd test swss syncd' + assert read_file('test_reconcile') == 'test-process test-process-3' + def test_service_creator_with_timer_unit(sonic_fs, manifest, mock_feature_registry, mock_sonic_db): creator = ServiceCreator(mock_feature_registry, mock_sonic_db)