Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GCU] Disallowing DeleteInsteadOfReplaceMoveExtender from generating delete whole config move #2006

Merged
merged 1 commit into from
Jan 13, 2022
Merged

[GCU] Disallowing DeleteInsteadOfReplaceMoveExtender from generating delete whole config move #2006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions generic_config_updater/patch_sorter.py
View file
Open in desktop
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
View file
Open in desktop
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": {
Copy link
Contributor Author

@ghooo ghooo Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case related to issue #2000 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve the issue

"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
View file
Open in desktop
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