From 1bef20f82858667ba39a6d5c126d370a58eb7b4e Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 20 Mar 2023 17:31:03 +0200 Subject: [PATCH 1/4] [sonic-package-manager] support extension with multiple YANG modules Signed-off-by: Stepan Blyschak --- sonic_package_manager/manifest.py | 1 + sonic_package_manager/metadata.py | 19 +++- .../service_creator/creator.py | 64 ++++++++----- .../test_service_creator.py | 93 ++++++++++++++++++- 4 files changed, 148 insertions(+), 29 deletions(-) diff --git a/sonic_package_manager/manifest.py b/sonic_package_manager/manifest.py index dc5b16b1d8..57cf038a60 100644 --- a/sonic_package_manager/manifest.py +++ b/sonic_package_manager/manifest.py @@ -212,6 +212,7 @@ def unmarshal(self, value): ManifestField('clear', DefaultMarshaller(str), ''), ManifestField('auto-generate-show', DefaultMarshaller(bool), False), ManifestField('auto-generate-config', DefaultMarshaller(bool), False), + ManifestArray('source-yang-modules', DefaultMarshaller(str)), ]) ]) diff --git a/sonic_package_manager/metadata.py b/sonic_package_manager/metadata.py index 9cfa25e94e..792f2a97fb 100644 --- a/sonic_package_manager/metadata.py +++ b/sonic_package_manager/metadata.py @@ -4,10 +4,11 @@ import json import tarfile -from typing import Dict, Optional +from typing import Dict, List from sonic_package_manager import utils from sonic_package_manager.errors import MetadataError +from sonic_package_manager.logger import log from sonic_package_manager.manifest import Manifest from sonic_package_manager.version import Version @@ -54,7 +55,7 @@ class Metadata: manifest: Manifest components: Dict[str, Version] = field(default_factory=dict) - yang_module_str: Optional[str] = None + yang_modules: List[str] = field(default_factory=list) class MetadataResolver: @@ -164,6 +165,16 @@ def from_labels(cls, labels: Dict[str, str]) -> Metadata: except ValueError as err: raise MetadataError(f'Failed to parse component version: {err}') - yang_module_str = sonic_metadata.get('yang-module') + labels_yang_modules = sonic_metadata.get('yang-module') + yang_modules = [] - return Metadata(Manifest.marshal(manifest_dict), components, yang_module_str) + if isinstance(labels_yang_modules, str): + yang_modules.append(labels_yang_modules) + log.debug("Found one YANG module") + elif isinstance(labels_yang_modules, dict): + yang_modules.extend(labels_yang_modules.values()) + log.debug("Found YANG modules: {}", labels_yang_modules.keys()) + else: + log.debug("No YANG modules found") + + return Metadata(Manifest.marshal(manifest_dict), components, yang_modules) diff --git a/sonic_package_manager/service_creator/creator.py b/sonic_package_manager/service_creator/creator.py index 17993be83c..62fc783321 100644 --- a/sonic_package_manager/service_creator/creator.py +++ b/sonic_package_manager/service_creator/creator.py @@ -517,18 +517,19 @@ def remove_config(self, package): None """ - if not package.metadata.yang_module_str: + if not package.metadata.yang_modules: return - module_name = self.cfg_mgmt.get_module_name(package.metadata.yang_module_str) - for tablename, module in self.cfg_mgmt.sy.confDbYangMap.items(): - if module.get('module') != module_name: - continue + for module in package.metadata.yang_modules: + module_name = self.cfg_mgmt.get_module_name(module) + for tablename, module in self.cfg_mgmt.sy.confDbYangMap.items(): + if module.get('module') != module_name: + continue - for conn in self.sonic_db.get_connectors(): - keys = conn.get_table(tablename).keys() - for key in keys: - conn.set_entry(tablename, key, None) + for conn in self.sonic_db.get_connectors(): + keys = conn.get_table(tablename).keys() + for key in keys: + conn.set_entry(tablename, key, None) def validate_config(self, config): """ Validate configuration through YANG. @@ -559,10 +560,11 @@ def install_yang_module(self, package: Package): None """ - if not package.metadata.yang_module_str: + if not package.metadata.yang_modules: return - self.cfg_mgmt.add_module(package.metadata.yang_module_str) + for module in package.metadata.yang_modules: + self.cfg_mgmt.add_module(module) def uninstall_yang_module(self, package: Package): """ Uninstall package's yang module in the system. @@ -573,11 +575,12 @@ def uninstall_yang_module(self, package: Package): None """ - if not package.metadata.yang_module_str: + if not package.metadata.yang_modules: return - module_name = self.cfg_mgmt.get_module_name(package.metadata.yang_module_str) - self.cfg_mgmt.remove_module(module_name) + for module in package.metadata.yang_modules: + module_name = self.cfg_mgmt.get_module_name(module) + self.cfg_mgmt.remove_module(module_name) def install_autogen_cli_all(self, package: Package): """ Install autogenerated CLI plugins for package. @@ -613,15 +616,16 @@ def install_autogen_cli(self, package: Package, command: str): None """ - if package.metadata.yang_module_str is None: + if not package.metadata.yang_modules: return if f'auto-generate-{command}' not in package.manifest['cli']: return if not package.manifest['cli'][f'auto-generate-{command}']: return - module_name = self.cfg_mgmt.get_module_name(package.metadata.yang_module_str) - self.cli_gen.generate_cli_plugin(command, module_name) - log.debug(f'{command} command line interface autogenerated for {module_name}') + + for module_name in self._get_yang_modules_for_auto_gen(package): + self.cli_gen.generate_cli_plugin(command, module_name) + log.debug(f'{command} command line interface autogenerated for {module_name}') def uninstall_autogen_cli(self, package: Package, command: str): """ Uninstall autogenerated CLI plugins for package for particular command. @@ -633,18 +637,34 @@ def uninstall_autogen_cli(self, package: Package, command: str): None """ - if package.metadata.yang_module_str is None: + if not package.metadata.yang_modules: return if f'auto-generate-{command}' not in package.manifest['cli']: return if not package.manifest['cli'][f'auto-generate-{command}']: return - module_name = self.cfg_mgmt.get_module_name(package.metadata.yang_module_str) - self.cli_gen.remove_cli_plugin(command, module_name) - log.debug(f'{command} command line interface removed for {module_name}') + + for module_name in self._get_yang_modules_for_auto_gen(package): + self.cli_gen.remove_cli_plugin(command, module_name) + log.debug(f'{command} command line interface removed for {module_name}') def _post_operation_hook(self): """ Common operations executed after service is created/removed. """ if not in_chroot(): run_command('systemctl daemon-reload') + + def _get_yang_modules_for_auto_gen(self, package: Package): + source_yang_modules = package.manifest['cli']['source-yang-modules'] + + def filter_yang_modules_for_auto_gen(module_name): + if not source_yang_modules: + return True + if module_name in source_yang_modules: + return True + return False + + filtered_yang_modules = filter(filter_yang_modules_for_auto_gen, + map(self.cfg_mgmt.get_module_name, package.metadata.yang_modules)) + return list(filtered_yang_modules) + diff --git a/tests/sonic_package_manager/test_service_creator.py b/tests/sonic_package_manager/test_service_creator.py index c97d362626..928acc477c 100644 --- a/tests/sonic_package_manager/test_service_creator.py +++ b/tests/sonic_package_manager/test_service_creator.py @@ -140,7 +140,7 @@ def test_service_creator_yang(sonic_fs, manifest, mock_sonic_db, }) entry = PackageEntry('test', 'azure/sonic-test') - package = Package(entry, Metadata(manifest, yang_module_str=test_yang)) + package = Package(entry, Metadata(manifest, yang_modules=[test_yang])) service_creator.create(package) mock_config_mgmt.add_module.assert_called_with(test_yang) @@ -154,7 +154,7 @@ def test_service_creator_yang(sonic_fs, manifest, mock_sonic_db, }, }, } - package = Package(entry, Metadata(manifest, yang_module_str=test_yang)) + package = Package(entry, Metadata(manifest, yang_modules=[test_yang])) service_creator.create(package) @@ -173,6 +173,42 @@ def test_service_creator_yang(sonic_fs, manifest, mock_sonic_db, mock_config_mgmt.remove_module.assert_called_with(test_yang_module) +def test_service_creator_multi_yang(sonic_fs, manifest, mock_config_mgmt, service_creator): + test_yang = 'TEST YANG' + test_yang_2 = 'TEST YANG 2' + + def get_module_name(module_src): + if module_src == test_yang: + return 'sonic-test' + elif module_src == test_yang_2: + return 'sonic-test-2' + else: + raise ValueError(f'Unknown module {module_src}') + + entry = PackageEntry('test', 'azure/sonic-test') + package = Package(entry, Metadata(manifest, yang_modules=[test_yang, test_yang_2])) + service_creator.create(package) + + mock_config_mgmt.add_module.assert_has_calls( + [ + call(test_yang), + call(test_yang_2) + ], + any_order=True, + ) + + mock_config_mgmt.get_module_name = Mock(side_effect=get_module_name) + + service_creator.remove(package) + mock_config_mgmt.remove_module.assert_has_calls( + [ + call(get_module_name(test_yang)), + call(get_module_name(test_yang_2)) + ], + any_order=True, + ) + + def test_service_creator_autocli(sonic_fs, manifest, mock_cli_gen, mock_config_mgmt, service_creator): test_yang = 'TEST YANG' @@ -182,7 +218,7 @@ def test_service_creator_autocli(sonic_fs, manifest, mock_cli_gen, manifest['cli']['auto-generate-config'] = True entry = PackageEntry('test', 'azure/sonic-test') - package = Package(entry, Metadata(manifest, yang_module_str=test_yang)) + package = Package(entry, Metadata(manifest, yang_modules=[test_yang])) mock_config_mgmt.get_module_name = Mock(return_value=test_yang_module) service_creator.create(package) @@ -204,6 +240,57 @@ def test_service_creator_autocli(sonic_fs, manifest, mock_cli_gen, ) +def test_service_creator_multi_yang_filter_auto_cli_modules(sonic_fs, manifest, mock_cli_gen, + mock_config_mgmt, service_creator): + test_yang = 'TEST YANG' + test_yang_2 = 'TEST YANG 2' + test_yang_3 = 'TEST YANG 3' + test_yang_4 = 'TEST YANG 4' + + def get_module_name(module_src): + if module_src == test_yang: + return 'sonic-test' + elif module_src == test_yang_2: + return 'sonic-test-2' + elif module_src == test_yang_3: + return 'sonic-test-3' + elif module_src == test_yang_4: + return 'sonic-test-4' + else: + raise ValueError(f'Unknown module {module_src}') + + manifest['cli']['auto-generate-show'] = True + manifest['cli']['auto-generate-config'] = True + manifest['cli']['source-yang-modules'] = ['sonic-test-2', 'sonic-test-4'] + + entry = PackageEntry('test', 'azure/sonic-test') + package = Package(entry, Metadata(manifest, yang_modules=[test_yang, test_yang_2, test_yang_3, test_yang_4])) + mock_config_mgmt.get_module_name = Mock(side_effect=get_module_name) + service_creator.create(package) + + assert mock_cli_gen.generate_cli_plugin.call_count == 4 + mock_cli_gen.generate_cli_plugin.assert_has_calls( + [ + call('show', get_module_name(test_yang_2)), + call('show', get_module_name(test_yang_4)), + call('config', get_module_name(test_yang_2)), + call('config', get_module_name(test_yang_4)), + ], + any_order=True + ) + + service_creator.remove(package) + assert mock_cli_gen.remove_cli_plugin.call_count == 4 + mock_cli_gen.remove_cli_plugin.assert_has_calls( + [ + call('show', get_module_name(test_yang_2)), + call('show', get_module_name(test_yang_4)), + call('config', get_module_name(test_yang_2)), + call('config', get_module_name(test_yang_4)), + ], + any_order=True + ) + def test_feature_registration(mock_sonic_db, manifest): mock_connector = Mock() mock_connector.get_entry = Mock(return_value={}) From 92489c222492d8bd1faedfffce5ee25528981b1e Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 22 Mar 2023 17:19:19 +0200 Subject: [PATCH 2/4] add metadata UT Signed-off-by: Stepan Blyschak --- tests/sonic_package_manager/test_metadata.py | 52 ++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/sonic_package_manager/test_metadata.py b/tests/sonic_package_manager/test_metadata.py index aee2f49428..96f9bbc38d 100644 --- a/tests/sonic_package_manager/test_metadata.py +++ b/tests/sonic_package_manager/test_metadata.py @@ -1,14 +1,36 @@ #!/usr/bin/env python +import json import contextlib from unittest.mock import Mock, MagicMock +import pytest + from sonic_package_manager.database import PackageEntry from sonic_package_manager.errors import MetadataError +from sonic_package_manager.manifest import Manifest from sonic_package_manager.metadata import MetadataResolver from sonic_package_manager.version import Version +@pytest.fixture +def manifest_str(): + return json.dumps({ + 'package': { + 'name': 'test', + 'version': '1.0.0', + }, + 'service': { + 'name': 'test', + 'asic-service': False, + 'host-service': True, + }, + 'container': { + 'privileged': True, + }, + }) + + def test_metadata_resolver_local(mock_registry_resolver, mock_docker_api): metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) # it raises exception because mock manifest is not a valid manifest @@ -35,3 +57,33 @@ def return_mock_registry(repository): mock_registry.manifest.assert_called_once_with('test-repository', '1.2.0') mock_registry.blobs.assert_called_once_with('test-repository', 'some-digest') mock_docker_api.labels.assert_not_called() + + +def test_metadata_construction(manifest_str): + metadata = MetadataResolver.from_labels({ + 'com': { + 'azure': { + 'sonic': { + 'manifest': manifest_str, + 'yang-module': 'TEST' + } + } + } + }) + assert metadata.yang_modules == ['TEST'] + + metadata = MetadataResolver.from_labels({ + 'com': { + 'azure': { + 'sonic': { + 'manifest': manifest_str, + 'yang-module': { + 'sonic-test': 'TEST', + 'sonic-test-2': 'TEST 2', + }, + }, + }, + }, + }) + assert metadata.yang_modules == ['TEST', 'TEST 2'] + From fe14c51f61033832e5113a89d781987311508595 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 22 Mar 2023 18:22:34 +0200 Subject: [PATCH 3/4] fix log.debug issue Signed-off-by: Stepan Blyschak --- sonic_package_manager/metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sonic_package_manager/metadata.py b/sonic_package_manager/metadata.py index 792f2a97fb..b44b658a74 100644 --- a/sonic_package_manager/metadata.py +++ b/sonic_package_manager/metadata.py @@ -173,7 +173,7 @@ def from_labels(cls, labels: Dict[str, str]) -> Metadata: log.debug("Found one YANG module") elif isinstance(labels_yang_modules, dict): yang_modules.extend(labels_yang_modules.values()) - log.debug("Found YANG modules: {}", labels_yang_modules.keys()) + log.debug(f"Found YANG modules: {labels_yang_modules.keys()}") else: log.debug("No YANG modules found") From b94dac09137da88dcd271500d558720259d4af38 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 24 Mar 2023 13:04:55 +0200 Subject: [PATCH 4/4] seperate config and show Signed-off-by: Stepan Blyschak --- sonic_package_manager/manifest.py | 3 ++- sonic_package_manager/service_creator/creator.py | 8 ++++---- tests/sonic_package_manager/test_service_creator.py | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sonic_package_manager/manifest.py b/sonic_package_manager/manifest.py index 57cf038a60..8534c2bff9 100644 --- a/sonic_package_manager/manifest.py +++ b/sonic_package_manager/manifest.py @@ -212,7 +212,8 @@ def unmarshal(self, value): ManifestField('clear', DefaultMarshaller(str), ''), ManifestField('auto-generate-show', DefaultMarshaller(bool), False), ManifestField('auto-generate-config', DefaultMarshaller(bool), False), - ManifestArray('source-yang-modules', DefaultMarshaller(str)), + ManifestArray('auto-generate-show-source-yang-modules', DefaultMarshaller(str)), + ManifestArray('auto-generate-config-source-yang-modules', DefaultMarshaller(str)), ]) ]) diff --git a/sonic_package_manager/service_creator/creator.py b/sonic_package_manager/service_creator/creator.py index 62fc783321..c49d939208 100644 --- a/sonic_package_manager/service_creator/creator.py +++ b/sonic_package_manager/service_creator/creator.py @@ -623,7 +623,7 @@ def install_autogen_cli(self, package: Package, command: str): if not package.manifest['cli'][f'auto-generate-{command}']: return - for module_name in self._get_yang_modules_for_auto_gen(package): + for module_name in self._get_yang_modules_for_auto_gen(command, package): self.cli_gen.generate_cli_plugin(command, module_name) log.debug(f'{command} command line interface autogenerated for {module_name}') @@ -644,7 +644,7 @@ def uninstall_autogen_cli(self, package: Package, command: str): if not package.manifest['cli'][f'auto-generate-{command}']: return - for module_name in self._get_yang_modules_for_auto_gen(package): + for module_name in self._get_yang_modules_for_auto_gen(command, package): self.cli_gen.remove_cli_plugin(command, module_name) log.debug(f'{command} command line interface removed for {module_name}') @@ -654,8 +654,8 @@ def _post_operation_hook(self): if not in_chroot(): run_command('systemctl daemon-reload') - def _get_yang_modules_for_auto_gen(self, package: Package): - source_yang_modules = package.manifest['cli']['source-yang-modules'] + def _get_yang_modules_for_auto_gen(self, command: str, package: Package): + source_yang_modules = package.manifest['cli'][f'auto-generate-{command}-source-yang-modules'] def filter_yang_modules_for_auto_gen(module_name): if not source_yang_modules: diff --git a/tests/sonic_package_manager/test_service_creator.py b/tests/sonic_package_manager/test_service_creator.py index 928acc477c..86844f1279 100644 --- a/tests/sonic_package_manager/test_service_creator.py +++ b/tests/sonic_package_manager/test_service_creator.py @@ -261,7 +261,8 @@ def get_module_name(module_src): manifest['cli']['auto-generate-show'] = True manifest['cli']['auto-generate-config'] = True - manifest['cli']['source-yang-modules'] = ['sonic-test-2', 'sonic-test-4'] + manifest['cli']['auto-generate-show-source-yang-modules'] = ['sonic-test-2', 'sonic-test-4'] + manifest['cli']['auto-generate-config-source-yang-modules'] = ['sonic-test-2', 'sonic-test-4'] entry = PackageEntry('test', 'azure/sonic-test') package = Package(entry, Metadata(manifest, yang_modules=[test_yang, test_yang_2, test_yang_3, test_yang_4]))