Skip to content

Commit b3a5052

Browse files
authored
[GCU] Using simulated config instead of target config when validating replace operation in NoDependencyMoveValidator (sonic-net#1987)
#### What I did Using `SimulatedConfig` instead of `TargetConfig` for validating a move using `NoDependencyMoveValidator` SimulatedConfig: config after applying the given move TargetConfig: the final config state that's required by the patch The problem is if the moves is to update a list item, the list item location in the `TargetConfig` might have different location in the `CurrentConfig`. The reason for that is the `TargetConfig` has the final outcome after applying the `patch`, but the move might be just making a small change towards this goal. Example: Assume current_config ``` { "VLAN": { "Vlan100": { "vlanid": "100", "dhcp_servers": [ "192.0.0.1", "192.0.0.2" ] } } } ``` TargetConfig: ``` { "VLAN": { "Vlan100": { "vlanid": "100", "dhcp_servers": [ "192.0.0.3" ] } } } ``` Move: ```python ps.JsonMove(diff, OperationType.REPLACE, ["VLAN", "Vlan100", "dhcp_servers", 1], ["VLAN", "Vlan100", "dhcp_servers", 0]) ``` The move means: ``` Replace the value in CurrentConfig that has path `/VLAN/Vlan100/dhcp_servers/1` with the value from the TargetConfig that has the path `/VLAN/Vlan100/dhcp_servers/0` ``` Notice how the array index in CurrentConfig does not exist in TargetConfig Instead of using TargetConfig to validate, use SimulatedConfig which is the config after applying the move. In this case it would be: ``` { "VLAN": { "Vlan100": { "vlanid": "100", "dhcp_servers": [ "192.0.0.1", "192.0.0.3" ] } } } ``` #### How I did it Replace `diff.target_config` with `simulated_config` #### How to verify it added a unit-test
1 parent 35cb524 commit b3a5052

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

generic_config_updater/patch_sorter.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -551,10 +551,12 @@ def _validate_replace(self, move, diff):
551551
simulated_config = move.apply(diff.current_config)
552552
deleted_paths, added_paths = self._get_paths(diff.current_config, simulated_config, [])
553553

554+
# For deleted paths, we check the current config has no dependencies between nodes under the removed path
554555
if not self._validate_paths_config(deleted_paths, diff.current_config):
555556
return False
556557

557-
if not self._validate_paths_config(added_paths, diff.target_config):
558+
# For added paths, we check the simulated config has no dependencies between nodes under the added path
559+
if not self._validate_paths_config(added_paths, simulated_config):
558560
return False
559561

560562
return True
@@ -1020,7 +1022,7 @@ def __init__(self, move_wrapper):
10201022
self.move_wrapper = move_wrapper
10211023
self.mem = {}
10221024

1023-
def rec(self, diff):
1025+
def sort(self, diff):
10241026
if diff.has_no_diff():
10251027
return []
10261028

tests/generic_config_updater/patch_sorter_test.py

+31
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,37 @@ def test_validate__replace_whole_config_item_same_ref_same__true(self):
10541054
# Act and assert
10551055
self.assertTrue(self.validator.validate(move, diff))
10561056

1057+
def test_validate__replace_list_item_different_location_than_target_and_no_deps__true(self):
1058+
# Arrange
1059+
current_config = {
1060+
"VLAN": {
1061+
"Vlan100": {
1062+
"vlanid": "100",
1063+
"dhcp_servers": [
1064+
"192.0.0.1",
1065+
"192.0.0.2"
1066+
]
1067+
}
1068+
}
1069+
}
1070+
target_config = {
1071+
"VLAN": {
1072+
"Vlan100": {
1073+
"vlanid": "100",
1074+
"dhcp_servers": [
1075+
"192.0.0.3"
1076+
]
1077+
}
1078+
}
1079+
}
1080+
diff = ps.Diff(current_config, target_config)
1081+
# the target tokens point to location 0 which exist in target_config
1082+
# but the replace operation is operating on location 1 in current_config
1083+
move = ps.JsonMove(diff, OperationType.REPLACE, ["VLAN", "Vlan100", "dhcp_servers", 1], ["VLAN", "Vlan100", "dhcp_servers", 0])
1084+
1085+
# Act and assert
1086+
self.assertTrue(self.validator.validate(move, diff))
1087+
10571088
def prepare_config(self, config, patch):
10581089
return patch.apply(config)
10591090

0 commit comments

Comments
 (0)