Skip to content

[GCU] Using simulated config instead of target config when validating replace operation in NoDependencyMoveValidator #1987

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

Merged
ghooo merged 1 commit into from
Dec 27, 2021
Merged

[GCU] Using simulated config instead of target config when validating replace operation in NoDependencyMoveValidator #1987

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
6 changes: 4 additions & 2 deletions generic_config_updater/patch_sorter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,10 +544,12 @@ def _validate_replace(self, move, diff):
simulated_config = move.apply(diff.current_config)
deleted_paths, added_paths = self._get_paths(diff.current_config, simulated_config, [])

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

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

return True
Expand Down Expand Up @@ -1013,7 +1015,7 @@ def __init__(self, move_wrapper):
self.move_wrapper = move_wrapper
self.mem = {}

def rec(self, diff):
def sort(self, diff):
Copy link
Copy Markdown
Contributor Author

@ghooo ghooo Dec 24, 2021

Choose a reason for hiding this comment

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

small change not related to the PR.

The MemoizationSorter is not used so we have not caught this before. This sorter is just added to run experiments on the performance.

if diff.has_no_diff():
return []

Expand Down
31 changes: 31 additions & 0 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 @@ -1054,6 +1054,37 @@ def test_validate__replace_whole_config_item_same_ref_same__true(self):
# Act and assert
self.assertTrue(self.validator.validate(move, diff))

def test_validate__replace_list_item_different_location_than_target_and_no_deps__true(self):
# Arrange
current_config = {
"VLAN": {
"Vlan100": {
"vlanid": "100",
"dhcp_servers": [
"192.0.0.1",
"192.0.0.2"
]
}
}
}
target_config = {
"VLAN": {
"Vlan100": {
"vlanid": "100",
"dhcp_servers": [
"192.0.0.3"
]
}
}
}
diff = ps.Diff(current_config, target_config)
# the target tokens point to location 0 which exist in target_config
# but the replace operation is operating on location 1 in current_config
move = ps.JsonMove(diff, OperationType.REPLACE, ["VLAN", "Vlan100", "dhcp_servers", 1], ["VLAN", "Vlan100", "dhcp_servers", 0])

# Act and assert
self.assertTrue(self.validator.validate(move, diff))

def prepare_config(self, config, patch):
return patch.apply(config)

Expand Down