Skip to content

Commit fc0ecb6

Browse files
authored
[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)
1 parent dd8c8fe commit fc0ecb6

File tree

3 files changed

+97
-2
lines changed

3 files changed

+97
-2
lines changed

generic_config_updater/patch_sorter.py

+4
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,10 @@ def extend(self, move, diff):
936936
if operation_type != OperationType.REPLACE:
937937
return
938938

939+
# Cannot delete the whole config, JsonPatch lib does not support it
940+
if not move.current_config_tokens:
941+
return
942+
939943
new_move = JsonMove(diff, OperationType.REMOVE, move.current_config_tokens)
940944

941945
yield new_move

tests/generic_config_updater/files/patch_sorter_test_success.json

+83
Original file line numberDiff line numberDiff line change
@@ -2811,5 +2811,88 @@
28112811
}
28122812
]
28132813
]
2814+
},
2815+
"ADDING_LOOPBACK0_VRF_NAME__DELETES_LOOPBACK0_AND_IPS_DOES_NOT_AFFECT_OTHER_TABLES": {
2816+
"desc": ["Adding loopback vrf name, deletes loopback0 and the associated ips. ",
2817+
"It does not affect other tables."],
2818+
"current_config": {
2819+
"CABLE_LENGTH": {
2820+
"AZURE": {
2821+
"Ethernet0": "0m",
2822+
"Ethernet100": "0m"
2823+
}
2824+
},
2825+
"LOOPBACK_INTERFACE": {
2826+
"Loopback0": {},
2827+
"Loopback0|10.1.0.32/32": {},
2828+
"Loopback0|1100:1::32/128": {}
2829+
},
2830+
"VRF": {
2831+
"Vrf_01": {},
2832+
"Vrf_02": {}
2833+
},
2834+
"PORT": {
2835+
"Ethernet0": {
2836+
"lanes": "25,26,27,28",
2837+
"speed": "10000"
2838+
},
2839+
"Ethernet100": {
2840+
"lanes": "121,122,123,124",
2841+
"speed": "10000"
2842+
}
2843+
}
2844+
},
2845+
"patch": [
2846+
{
2847+
"op": "add",
2848+
"path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name",
2849+
"value": "Vrf_01"
2850+
}
2851+
],
2852+
"expected_changes": [
2853+
[
2854+
{
2855+
"op": "remove",
2856+
"path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132"
2857+
}
2858+
],
2859+
[
2860+
{
2861+
"op": "remove",
2862+
"path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128"
2863+
}
2864+
],
2865+
[
2866+
{
2867+
"op": "remove",
2868+
"path": "/LOOPBACK_INTERFACE"
2869+
}
2870+
],
2871+
[
2872+
{
2873+
"op": "add",
2874+
"path": "/LOOPBACK_INTERFACE",
2875+
"value": {
2876+
"Loopback0":{
2877+
"vrf_name": "Vrf_01"
2878+
}
2879+
}
2880+
}
2881+
],
2882+
[
2883+
{
2884+
"op": "add",
2885+
"path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132",
2886+
"value": {}
2887+
}
2888+
],
2889+
[
2890+
{
2891+
"op": "add",
2892+
"path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128",
2893+
"value": {}
2894+
}
2895+
]
2896+
]
28142897
}
28152898
}

tests/generic_config_updater/patch_sorter_test.py

+10-2
Original file line numberDiff line numberDiff line change
@@ -1591,6 +1591,14 @@ def test_extend__replace_table__replace_whole_config(self):
15911591
cc_ops=[{'op':'replace', 'path':'/VLAN/Vlan1000/dhcp_servers/1', 'value':'192.0.0.7'}],
15921592
ex_ops=[{'op':'replace', 'path':'', 'value':Files.CROPPED_CONFIG_DB_AS_JSON}])
15931593

1594+
def test_extend__remove_table_while_config_has_only_that_table__replace_whole_config_with_empty_config(self):
1595+
self.verify(OperationType.REMOVE,
1596+
["VLAN"],
1597+
["VLAN"],
1598+
cc_ops=[{'op':'replace', 'path':'', 'value':{'VLAN':{}}}],
1599+
tc_ops=[{'op':'replace', 'path':'', 'value':{}}],
1600+
ex_ops=[{'op':'replace', 'path':'', 'value':{}}])
1601+
15941602
def verify(self, op_type, ctokens, ttokens=None, cc_ops=[], tc_ops=[], ex_ops=[]):
15951603
"""
15961604
cc_ops, tc_ops are used to build the diff object.
@@ -1649,12 +1657,12 @@ def test_extend__replace_table__delete_table(self):
16491657
cc_ops=[{'op':'replace', 'path':'/ACL_TABLE/EVERFLOW/policy_desc', 'value':'old_desc'}],
16501658
ex_ops=[{'op':'remove', 'path':'/ACL_TABLE'}])
16511659

1652-
def test_extend__replace_whole_config__delete_whole_config(self):
1660+
def test_extend__replace_whole_config__no_moves(self):
16531661
self.verify(OperationType.REPLACE,
16541662
[],
16551663
[],
16561664
cc_ops=[{'op':'replace', 'path':'/ACL_TABLE/EVERFLOW/policy_desc', 'value':'old_desc'}],
1657-
ex_ops=[{'op':'remove', 'path':''}])
1665+
ex_ops=[])
16581666

16591667
def verify(self, op_type, ctokens, ttokens=None, cc_ops=[], tc_ops=[], ex_ops=[]):
16601668
"""

0 commit comments

Comments
 (0)