[GCU] Copying config_db before calling sonic_yang.loadData

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)
 

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)

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"
+        ]
     }
 }

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.assertCountEqual(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:

tests/generic_config_updater/patch_sorter_test.py

@@ -1828,7 +1828,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 = ConfigWrapper()
@@ -1837,6 +1853,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)