diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 7fd45f618e38..c824407564e6 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -403,7 +403,9 @@ def find_ref_paths(self, path, config): def _find_leafref_paths(self, path, config): sy = self.config_wrapper.create_sonic_yang_with_loaded_models() - sy.loadData(config) + tmp_config = copy.deepcopy(config) + + sy.loadData(tmp_config) xpath = self.convert_path_to_xpath(path, config, sy) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index 3cd9e757fa45..1b1e5e1dd3fb 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -36,6 +36,13 @@ def apply_move(self, move): def has_no_diff(self): return self.current_config == self.target_config + def __str__(self): + return f"""current_config: {self.current_config} +target_config: {self.target_config}""" + + def __repr__(self): + return str(self) + class JsonMove: """ A class similar to JsonPatch operation, but it allows the path to refer to non-existing middle elements. @@ -1333,10 +1340,7 @@ def sort(self, patch, algorithm=Algorithm.DFS, preloaded_current_config=None): current_config = preloaded_current_config if preloaded_current_config else self.config_wrapper.get_config_db_as_json() target_config = self.patch_wrapper.simulate_patch(patch, current_config) - cropped_current_config = self.config_wrapper.crop_tables_without_yang(current_config) - cropped_target_config = self.config_wrapper.crop_tables_without_yang(target_config) - - diff = Diff(cropped_current_config, cropped_target_config) + diff = Diff(current_config, target_config) sort_algorithm = self.sort_algorithm_factory.create(algorithm) moves = sort_algorithm.sort(diff) diff --git a/tests/generic_config_updater/files/patch_sorter_test_failure.json b/tests/generic_config_updater/files/patch_sorter_test_failure.json index 7d5ea0ad9398..90884fb44907 100644 --- a/tests/generic_config_updater/files/patch_sorter_test_failure.json +++ b/tests/generic_config_updater/files/patch_sorter_test_failure.json @@ -67,5 +67,43 @@ "expected_error_substrings": [ "There is no possible sorting" ] + }, + "EMPTYING_A_CONFIGDB_TABLE_AND_CONFIG_HAS_NON_YANG_TABLE__FAILURE": { + "desc": [ + "Emptying a configdb table fails because empty tables are not allowed in configdb.", + "User should remove whole table instead e.g. remove /ACL_TABLE in this case.", + "Also there is a table without YANG in the config, which the sorting logic will loop over.", + "The sorting logic should fail with GenericConfigUpdaterError: 'There is no possible sorting' and not KeyError: 'TABLE_WITHOUT_YANG'" + ], + "current_config": { + "TABLE_WITHOUT_YANG": { + "key1": "value1", + "key2": "value2" + }, + "ACL_TABLE": { + "EVERFLOW": { + "policy_desc": "EVERFLOW", + "ports": [ + "Ethernet0" + ], + "stage": "ingress", + "type": "MIRROR" + } + }, + "PORT": { + "Ethernet0": { + "alias": "Eth1", + "lanes": "65, 66, 67, 68", + "description": "Ethernet0 100G link", + "speed": "100000" + } + } + }, + "patch": [ + {"op": "remove", "path": "/ACL_TABLE/EVERFLOW"} + ], + "expected_error_substrings": [ + "There is no possible sorting" + ] } } \ No newline at end of file diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 083546c98012..dbc93fcf0724 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -1,3 +1,4 @@ +import copy import json import jsonpatch import sonic_yang @@ -674,6 +675,18 @@ def test_find_ref_paths__whole_config_path__returns_all_refs(self): # Assert self.assertEqual(expected, actual) + def test_find_ref_paths__does_not_remove_tables_without_yang(self): + # Arrange + config = Files.CONFIG_DB_AS_JSON # This has a table without yang named 'TABLE_WITHOUT_YANG' + any_path = "" + expected_config = copy.deepcopy(config) + + # Act + self.path_addressing.find_ref_paths(any_path, config) + + # Assert + self.assertEqual(expected_config, config) + def test_convert_path_to_xpath(self): def check(path, xpath, config=None): if not config: diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index e79d18163bd2..72275f2d95fe 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -1829,7 +1829,23 @@ def run_single_failure_case(self, data): if notfound_substrings: self.fail(f"Did not find the substrings {notfound_substrings} in the error: '{error}'") - def create_patch_sorter(self, config=None): + def test_sort__does_not_remove_tables_without_yang_unintentionally_if_generated_change_replaces_whole_config(self): + # Arrange + current_config = Files.CONFIG_DB_AS_JSON # has a table without yang named 'TABLE_WITHOUT_YANG' + any_patch = Files.SINGLE_OPERATION_CONFIG_DB_PATCH + target_config = any_patch.apply(current_config) + sort_algorithm = Mock() + sort_algorithm.sort = lambda diff: [ps.JsonMove(diff, OperationType.REPLACE, [], [])] + patch_sorter = self.create_patch_sorter(current_config, sort_algorithm) + expected = [JsonChange(jsonpatch.JsonPatch([OperationWrapper().create(OperationType.REPLACE, "", target_config)]))] + + # Act + actual = patch_sorter.sort(any_patch) + + # Assert + self.assertEqual(expected, actual) + + def create_patch_sorter(self, config=None, sort_algorithm=None): if config is None: config=Files.CROPPED_CONFIG_DB_AS_JSON config_wrapper = self.config_wrapper @@ -1838,6 +1854,8 @@ def create_patch_sorter(self, config=None): operation_wrapper = OperationWrapper() path_addressing= ps.PathAddressing(config_wrapper) sort_algorithm_factory = ps.SortAlgorithmFactory(operation_wrapper, config_wrapper, path_addressing) + if sort_algorithm: + sort_algorithm_factory.create = MagicMock(return_value=sort_algorithm) return ps.PatchSorter(config_wrapper, patch_wrapper, sort_algorithm_factory)