[GCU] Disallowing DeleteInsteadOfReplaceMoveExtender from generating …

…delete whole config move (sonic-net#2006) #### What I did Not generating delete whole config move because it is not allowed by JsonPatch library and it is not possible for ConfigDb to be equal to NULL. This solves the first problem in issue sonic-net/sonic-utilities#2000 (comment) #### How I did it - Verified UpperLevelMoveExtender does not generate delete whole config move - Modified DeleteInsteadOfReplaceMoveExtender to not generate delete whole config move - DeleteRefsMoveExtender cannot generate delete whole config move, because it deletes the referrer leafs #### How to verify it unit-test #### Previous command output (if the output of a command-line utility has changed) #### New command output (if the output of a command-line utility has changed)
vivekrnv · Jan 13, 2022 · fc0ecb6 · fc0ecb6
1 parent dd8c8fe
commit fc0ecb6
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 2 deletions.
diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py
@@ -936,6 +936,10 @@ def extend(self, move, diff):
         if operation_type != OperationType.REPLACE:
             return
 
+        # Cannot delete the whole config, JsonPatch lib does not support it
+        if not move.current_config_tokens:
+            return
+
         new_move = JsonMove(diff, OperationType.REMOVE, move.current_config_tokens)
 
         yield new_move

diff --git a/tests/generic_config_updater/files/patch_sorter_test_success.json b/tests/generic_config_updater/files/patch_sorter_test_success.json
@@ -2811,5 +2811,88 @@
                 }
             ]
         ]
+    },
+    "ADDING_LOOPBACK0_VRF_NAME__DELETES_LOOPBACK0_AND_IPS_DOES_NOT_AFFECT_OTHER_TABLES": {
+        "desc": ["Adding loopback vrf name, deletes loopback0 and the associated ips. ",
+                 "It does not affect other tables."],
+        "current_config": {
+            "CABLE_LENGTH": {
+                "AZURE": {
+                    "Ethernet0": "0m",
+                    "Ethernet100": "0m"
+                }
+            },
+            "LOOPBACK_INTERFACE": {
+                "Loopback0": {},
+                "Loopback0|10.1.0.32/32": {},
+                "Loopback0|1100:1::32/128": {}
+            },
+            "VRF": {
+                "Vrf_01": {},
+                "Vrf_02": {}
+            },
+            "PORT": {
+                "Ethernet0": {
+                    "lanes": "25,26,27,28",
+                    "speed": "10000"
+                },
+                "Ethernet100": {
+                    "lanes": "121,122,123,124",
+                    "speed": "10000"
+                }
+            }
+        },
+        "patch": [
+            {
+                "op": "add",
+                "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name",
+                "value": "Vrf_01"
+            }
+        ],
+        "expected_changes": [
+            [
+                {
+                    "op": "remove",
+                    "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132"
+                }
+            ],
+            [
+                {
+                    "op": "remove",
+                    "path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128"
+                }
+            ],
+            [
+                {
+                    "op": "remove",
+                    "path": "/LOOPBACK_INTERFACE"
+                }
+            ],
+            [
+                {
+                    "op": "add",
+                    "path": "/LOOPBACK_INTERFACE",
+                    "value": {
+                        "Loopback0":{
+                            "vrf_name": "Vrf_01"
+                        }
+                    }
+                }
+            ],
+            [
+                {
+                    "op": "add",
+                    "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132",
+                    "value": {}
+                }
+            ],
+            [
+                {
+                    "op": "add",
+                    "path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128",
+                    "value": {}
+                }
+            ]
+        ]
     }
 }
diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py
@@ -1591,6 +1591,14 @@ def test_extend__replace_table__replace_whole_config(self):
                     cc_ops=[{'op':'replace', 'path':'/VLAN/Vlan1000/dhcp_servers/1', 'value':'192.0.0.7'}],
                     ex_ops=[{'op':'replace', 'path':'', 'value':Files.CROPPED_CONFIG_DB_AS_JSON}])
 
+    def test_extend__remove_table_while_config_has_only_that_table__replace_whole_config_with_empty_config(self):
+        self.verify(OperationType.REMOVE,
+                    ["VLAN"],
+                    ["VLAN"],
+                    cc_ops=[{'op':'replace', 'path':'', 'value':{'VLAN':{}}}],
+                    tc_ops=[{'op':'replace', 'path':'', 'value':{}}],
+                    ex_ops=[{'op':'replace', 'path':'', 'value':{}}])
+
     def verify(self, op_type, ctokens, ttokens=None, cc_ops=[], tc_ops=[], ex_ops=[]):
         """
         cc_ops, tc_ops are used to build the diff object.
@@ -1649,12 +1657,12 @@ def test_extend__replace_table__delete_table(self):
                     cc_ops=[{'op':'replace', 'path':'/ACL_TABLE/EVERFLOW/policy_desc', 'value':'old_desc'}],
                     ex_ops=[{'op':'remove', 'path':'/ACL_TABLE'}])
 
-    def test_extend__replace_whole_config__delete_whole_config(self):
+    def test_extend__replace_whole_config__no_moves(self):
         self.verify(OperationType.REPLACE,
                     [],
                     [],
                     cc_ops=[{'op':'replace', 'path':'/ACL_TABLE/EVERFLOW/policy_desc', 'value':'old_desc'}],
-                    ex_ops=[{'op':'remove', 'path':''}])
+                    ex_ops=[])
 
     def verify(self, op_type, ctokens, ttokens=None, cc_ops=[], tc_ops=[], ex_ops=[]):
         """