Skip to content

Commit

Permalink
[GCU] Disallowing DeleteInsteadOfReplaceMoveExtender from generating …
Browse files Browse the repository at this point in the history
…delete whole config move (#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)
  • Loading branch information
malletvapid23 committed Jan 13, 2022
1 parent 72e2b90 commit 410aa36
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 2 deletions.
4 changes: 4 additions & 0 deletions generic_config_updater/patch_sorter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
83 changes: 83 additions & 0 deletions tests/generic_config_updater/files/patch_sorter_test_success.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {}
}
]
]
}
}
12 changes: 10 additions & 2 deletions tests/generic_config_updater/patch_sorter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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=[]):
"""
Expand Down

0 comments on commit 410aa36

Please sign in to comment.