From e44c3f60ab016b3d3da4014c7ae684824b19f5c8 Mon Sep 17 00:00:00 2001 From: Mohamed Ghoneim Date: Thu, 16 Dec 2021 12:09:27 -0800 Subject: [PATCH] [generic-config-updater] Improving CreateOnly validator and marking /LOOPBACK_INTERFACE/LOOPBACK#/vrf_name as create-only (#1969) #### What I did Fix #1962 Updated `create-only` flag meaning. From, Field is not replaceable but can be added or deleted. In other words: - Field can be added - Field can be deleted - Field cannot be replaced To, Field is only created, but never modified/updated. In other words: - Field cannot be added, only if the parent is added - Field cannot be deleted, only if the parent is deleted - Field cannot be replaced Also marked `/LOOPBACK_INTERFACE/LOOPBACK#/vrf_name` as `create-only` #### How I did it - Field was already not replaceable -- so no changes - If field is added, but parent already exist -- fail create-only validation - If field is deleted, but parent remain -- fail create-only validation #### How to verify it unit-test #### Examples Check issue to see how `apply-patch` behaved before this fix. Each example below we show configdb, vrf_01, vrf_02, and ip interfaces before and after the update. **Adding vrf_name** ```sh admin@vlab-01:~/lo$ show run all | grep -i 'loopback0\|vrf_01' -a3 } }, "LOOPBACK_INTERFACE": { "Loopback0": {}, "Loopback0|10.1.0.32/32": {}, "Loopback0|FC00:1::32/128": {} }, "MAP_PFC_PRIORITY_TO_QUEUE": { "AZURE": { -- } }, "VRF": { "Vrf_01": {}, "Vrf_02": {} }, "WRED_PROFILE": { admin@vlab-01:~/lo$ show ip route vrf Vrf_01 admin@vlab-01:~/lo$ show ip route vrf Vrf_02 admin@vlab-01:~/lo$ show ip interfaces | grep -i loopback0 Loopback0 10.1.0.32/32 up/up N/A N/A admin@vlab-01:~/lo$ sudo config apply-patch add-lo0-vrf01.json-patch -i /BGP_NEIGHBOR -i /DEVICE_METADATA -i /FEATURE -i /FLEX_COUNTER_TABLE -i /VLAN/Vlan1000/members -i /SCHEDULER -i /QUEUE Patch Applier: Patch application starting. Patch Applier: Patch: [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_01"}] Patch Applier: Getting current config db. Patch Applier: Simulating the target full config after applying the patch. Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: Sorting patch updates. Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, libyang[0]: Must condition "(current() = ../../LOOPBACK_INTERFACE_LIST[name=current()]/name)" not satisfied. (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='10.1.0.32/32']/name) libyang[0]: Must condition not satisfied, Try adding lo<>: {}, Example: 'lo1': {} (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='10.1.0.32/32']/name) sonic_yang(3):Data Loading Failed:Must condition not satisfied, Try adding lo<>: {}, Example: 'lo1': {} Patch Applier: The patch was sorted into 4 changes: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_01"}}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", "value": {}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128", "value": {}}] Patch Applier: Applying 4 changes in order: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_01"}}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", "value": {}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128", "value": {}}] Patch Applier: Verifying patch updates are reflected on ConfigDB. Patch Applier: Patch application completed. Patch applied successfully. admin@vlab-01:~/lo$ show run all | grep -i 'loopback0\|vrf_01' -a3 } }, "LOOPBACK_INTERFACE": { "Loopback0": { "vrf_name": "Vrf_01" }, "Loopback0|10.1.0.32/32": {}, "Loopback0|FC00:1::32/128": {} }, "MAP_PFC_PRIORITY_TO_QUEUE": { "AZURE": { -- } }, "VRF": { "Vrf_01": {}, "Vrf_02": {} }, "WRED_PROFILE": { admin@vlab-01:~/lo$ show ip route vrf Vrf_01 Codes: K - kernel route, C - connected, S - static, R - RIP, O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, f - OpenFabric, > - selected route, * - FIB route, q - queued, r - rejected, b - backup VRF Vrf_01: C>* 10.1.0.32/32 is directly connected, Loopback0, 00:00:19 admin@vlab-01:~/lo$ show ip route vrf Vrf_02 admin@vlab-01:~/lo$ show ip interfaces | grep -i loopback0 Loopback0 Vrf_01 10.1.0.32/32 up/up N/A N/A admin@vlab-01:~/lo$ ``` **Replacing vrf_name** ```sh admin@vlab-01:~/lo$ show run all | grep -i 'loopback0\|vrf_01' -a3 } }, "LOOPBACK_INTERFACE": { "Loopback0": { "vrf_name": "Vrf_01" }, "Loopback0|10.1.0.32/32": {}, "Loopback0|FC00:1::32/128": {} }, "MAP_PFC_PRIORITY_TO_QUEUE": { "AZURE": { -- } }, "VRF": { "Vrf_01": {}, "Vrf_02": {} }, "WRED_PROFILE": { admin@vlab-01:~/lo$ show ip route vrf Vrf_01 Codes: K - kernel route, C - connected, S - static, R - RIP, O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, f - OpenFabric, > - selected route, * - FIB route, q - queued, r - rejected, b - backup VRF Vrf_01: C>* 10.1.0.32/32 is directly connected, Loopback0, 00:00:19 admin@vlab-01:~/lo$ show ip route vrf Vrf_02 admin@vlab-01:~/lo$ show ip interfaces | grep -i loopback0 Loopback0 Vrf_01 10.1.0.32/32 up/up N/A N/A admin@vlab-01:~/lo$ sudo config apply-patch replace-lo0-vrf02.json-patch -i /BGP_NEIGHBOR -i /DEVICE_METADATA -i /FEATURE -i /FLEX_COUNTER_TABLE -i /VLAN/Vlan1000/members -i /SCHEDULER -i /QUEUE Patch Applier: Patch application starting. Patch Applier: Patch: [{"op": "replace", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_02"}] Patch Applier: Getting current config db. Patch Applier: Simulating the target full config after applying the patch. Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: Sorting patch updates. Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, libyang[0]: Must condition "(current() = ../../LOOPBACK_INTERFACE_LIST[name=current()]/name)" not satisfied. (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='10.1.0.32/32']/name) libyang[0]: Must condition not satisfied, Try adding lo<>: {}, Example: 'lo1': {} (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='10.1.0.32/32']/name) sonic_yang(3):Data Loading Failed:Must condition not satisfied, Try adding lo<>: {}, Example: 'lo1': {} Patch Applier: The patch was sorted into 4 changes: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_02"}}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", "value": {}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128", "value": {}}] Patch Applier: Applying 4 changes in order: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_02"}}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", "value": {}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128", "value": {}}] Patch Applier: Verifying patch updates are reflected on ConfigDB. Patch Applier: Patch application completed. Patch applied successfully. admin@vlab-01:~/lo$ show run all | grep -i 'loopback0\|vrf_01' -a3 } }, "LOOPBACK_INTERFACE": { "Loopback0": { "vrf_name": "Vrf_02" }, "Loopback0|10.1.0.32/32": {}, "Loopback0|FC00:1::32/128": {} }, "MAP_PFC_PRIORITY_TO_QUEUE": { "AZURE": { -- } }, "VRF": { "Vrf_01": {}, "Vrf_02": {} }, "WRED_PROFILE": { admin@vlab-01:~/lo$ show ip route vrf Vrf_01 admin@vlab-01:~/lo$ show ip route vrf Vrf_02 Codes: K - kernel route, C - connected, S - static, R - RIP, O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, f - OpenFabric, > - selected route, * - FIB route, q - queued, r - rejected, b - backup VRF Vrf_02: C>* 10.1.0.32/32 is directly connected, Loopback0, 00:00:29 admin@vlab-01:~/lo$ show ip interfaces | grep -i loopback0 Loopback0 Vrf_02 10.1.0.32/32 up/up N/A N/A admin@vlab-01:~/lo$ ``` **Removing vrf_name** ```sh admin@vlab-01:~/lo$ show run all | grep -i 'loopback0\|vrf_01' -a3 } }, "LOOPBACK_INTERFACE": { "Loopback0": { "vrf_name": "Vrf_02" }, "Loopback0|10.1.0.32/32": {}, "Loopback0|FC00:1::32/128": {} }, "MAP_PFC_PRIORITY_TO_QUEUE": { "AZURE": { -- } }, "VRF": { "Vrf_01": {}, "Vrf_02": {} }, "WRED_PROFILE": { admin@vlab-01:~/lo$ show ip route vrf Vrf_01 admin@vlab-01:~/lo$ show ip route vrf Vrf_02 Codes: K - kernel route, C - connected, S - static, R - RIP, O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, f - OpenFabric, > - selected route, * - FIB route, q - queued, r - rejected, b - backup VRF Vrf_02: C>* 10.1.0.32/32 is directly connected, Loopback0, 00:00:29 admin@vlab-01:~/lo$ show ip interfaces | grep -i loopback0 Loopback0 Vrf_02 10.1.0.32/32 up/up N/A N/A admin@vlab-01:~/lo$ sudo config apply-patch remove-lo0-vrf02.json-patch -i /BGP_NEIGHBOR -i /DEVICE_METADATA -i /FEATURE -i /FLEX_COUNTER_TABLE -i /VLAN/Vlan1000/members -i /SCHEDULER -i /QUEUE Patch Applier: Patch application starting. Patch Applier: Patch: [{"op": "remove", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name"}] Patch Applier: Getting current config db. Patch Applier: Simulating the target full config after applying the patch. Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: Sorting patch updates. Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, Note: Below table(s) have no YANG models: BGP_PEER_RANGE, CABLE_LENGTH, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, SYSLOG_SERVER, TELEMETRY, libyang[0]: Must condition "(current() = ../../LOOPBACK_INTERFACE_LIST[name=current()]/name)" not satisfied. (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='10.1.0.32/32']/name) libyang[0]: Must condition not satisfied, Try adding lo<>: {}, Example: 'lo1': {} (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='10.1.0.32/32']/name) sonic_yang(3):Data Loading Failed:Must condition not satisfied, Try adding lo<>: {}, Example: 'lo1': {} Patch Applier: The patch was sorted into 4 changes: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {}}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", "value": {}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128", "value": {}}] Patch Applier: Applying 4 changes in order: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {}}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", "value": {}}] Patch Applier: * [{"op": "add", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128", "value": {}}] Patch Applier: Verifying patch updates are reflected on ConfigDB. Patch Applier: Patch application completed. Patch applied successfully. admin@vlab-01:~/lo$ show run all | grep -i 'loopback0\|vrf_01' -a3 } }, "LOOPBACK_INTERFACE": { "Loopback0": {}, "Loopback0|10.1.0.32/32": {}, "Loopback0|FC00:1::32/128": {} }, "MAP_PFC_PRIORITY_TO_QUEUE": { "AZURE": { -- } }, "VRF": { "Vrf_01": {}, "Vrf_02": {} }, "WRED_PROFILE": { admin@vlab-01:~/lo$ show ip route vrf Vrf_01 admin@vlab-01:~/lo$ show ip route vrf Vrf_02 admin@vlab-01:~/lo$ show ip interfaces | grep -i loopback0 Loopback0 10.1.0.32/32 up/up N/A N/A admin@vlab-01:~/lo$ ``` --- generic_config_updater/patch_sorter.py | 90 +++++++++++++++---- .../config_db_with_loopback_interfaces.json | 16 ++++ .../patch_sorter_test.py | 89 +++++++++++++++--- 3 files changed, 169 insertions(+), 26 deletions(-) create mode 100644 tests/generic_config_updater/files/config_db_with_loopback_interfaces.json diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index b53024e17380..b795421064f1 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -372,40 +372,58 @@ def validate(self, move, diff): class CreateOnlyMoveValidator: """ - A class to validate create-only fields are only added/removed but never replaced. - Parents of create-only fields are also only added/removed but never replaced when they contain - a modified create-only field. + A class to validate create-only fields are only created, but never modified/updated. In other words: + - Field cannot be replaced. + - Field cannot be added, only if the parent is added. + - Field cannot be deleted, only if the parent is deleted. """ def __init__(self, path_addressing): self.path_addressing = path_addressing - def validate(self, move, diff): - if move.op_type != OperationType.REPLACE: - return True + # TODO: create-only fields are hard-coded for now, it should be moved to YANG models + # Each pattern consist of a list of tokens. Token matching starts from the root level of the config. + # Each token is either a specific key or '*' to match all keys. + self.create_only_patterns = [ + ["PORT", "*", "lanes"], + ["LOOPBACK_INTERFACE", "*", "vrf_name"], + ] - # The 'create-only' field needs to be common between current and simulated anyway but different. - # This means it is enough to just get the paths from current_config, paths that are not common can be ignored. - paths = self._get_create_only_paths(diff.current_config) + def validate(self, move, diff): simulated_config = move.apply(diff.current_config) + paths = set(list(self._get_create_only_paths(diff.current_config)) + list(self._get_create_only_paths(simulated_config))) for path in paths: tokens = self.path_addressing.get_path_tokens(path) if self._value_exist_but_different(tokens, diff.current_config, simulated_config): return False + if self._value_added_but_parent_exist(tokens, diff.current_config, simulated_config): + return False + if self._value_removed_but_parent_remain(tokens, diff.current_config, simulated_config): + return False return True - # TODO: create-only fields are hard-coded for now, it should be moved to YANG models def _get_create_only_paths(self, config): - if "PORT" not in config: + for pattern in self.create_only_patterns: + for create_only_path in self._get_create_only_path_recursive(config, pattern, [], 0): + yield create_only_path + + def _get_create_only_path_recursive(self, config, pattern_tokens, matching_tokens, idx): + if idx == len(pattern_tokens): + yield '/' + '/'.join(matching_tokens) return - ports = config["PORT"] + matching_keys = [] + if pattern_tokens[idx] == "*": + matching_keys = config.keys() + elif pattern_tokens[idx] in config: + matching_keys = [pattern_tokens[idx]] - for port in ports: - attrs = ports[port] - if "lanes" in attrs: - yield f"/PORT/{port}/lanes" + for key in matching_keys: + matching_tokens.append(key) + for create_only_path in self._get_create_only_path_recursive(config[key], pattern_tokens, matching_tokens, idx+1): + yield create_only_path + matching_tokens.pop() def _value_exist_but_different(self, tokens, current_config_ptr, simulated_config_ptr): for token in tokens: @@ -422,6 +440,46 @@ def _value_exist_but_different(self, tokens, current_config_ptr, simulated_confi return current_config_ptr != simulated_config_ptr + def _value_added_but_parent_exist(self, tokens, current_config_ptr, simulated_config_ptr): + # if value is not added, return false + if not self._exist_only_in_first(tokens, simulated_config_ptr, current_config_ptr): + return False + + # if parent is added, return false + if self._exist_only_in_first(tokens[:-1], simulated_config_ptr, current_config_ptr): + return False + + # otherwise parent exist and value is added + return True + + def _value_removed_but_parent_remain(self, tokens, current_config_ptr, simulated_config_ptr): + # if value is not removed, return false + if not self._exist_only_in_first(tokens, current_config_ptr, simulated_config_ptr): + return False + + # if parent is removed, return false + if self._exist_only_in_first(tokens[:-1], current_config_ptr, simulated_config_ptr): + return False + + # otherwise parent remained and value is removed + return True + + def _exist_only_in_first(self, tokens, first_config_ptr, second_config_ptr): + for token in tokens: + mod_token = int(token) if isinstance(first_config_ptr, list) else token + + if mod_token not in second_config_ptr: + return True + + if mod_token not in first_config_ptr: + return False + + first_config_ptr = first_config_ptr[mod_token] + second_config_ptr = second_config_ptr[mod_token] + + # tokens exist in both + return False + class NoDependencyMoveValidator: """ A class to validate that the modified configs do not have dependency on each other. This should prevent diff --git a/tests/generic_config_updater/files/config_db_with_loopback_interfaces.json b/tests/generic_config_updater/files/config_db_with_loopback_interfaces.json new file mode 100644 index 000000000000..2c1724a4cdd7 --- /dev/null +++ b/tests/generic_config_updater/files/config_db_with_loopback_interfaces.json @@ -0,0 +1,16 @@ +{ + "LOOPBACK_INTERFACE": { + "Loopback0": {}, + "Loopback0|10.1.0.32/32": {}, + "Loopback0|1100:1::32/128": {}, + "Loopback1": { + "vrf_name": "Vrf_02" + }, + "Loopback1|20.2.0.32/32": {}, + "Loopback1|2200:2::32/128": {} + }, + "VRF": { + "Vrf_01": {}, + "Vrf_02": {} + } +} \ No newline at end of file diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index b1764a8dfc68..ef809ef929c1 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -784,13 +784,6 @@ def setUp(self): self.validator = ps.CreateOnlyMoveValidator(ps.PathAddressing()) self.any_diff = ps.Diff({}, {}) - def test_validate__non_replace_operation__success(self): - # Assert - self.assertTrue(self.validator.validate( \ - ps.JsonMove(self.any_diff, OperationType.ADD, [], []), self.any_diff)) - self.assertTrue(self.validator.validate( \ - ps.JsonMove(self.any_diff, OperationType.REMOVE, [], []), self.any_diff)) - def test_validate__no_create_only_field__success(self): current_config = {"PORT": {}} target_config = {"PORT": {}, "ACL_TABLE": {}} @@ -833,7 +826,7 @@ def test_validate__different_create_only_field_updating_grandparent__failure(sel ["PORT"], False) - def test_validate__same_create_only_field_directly_updated__failure(self): + def test_validate__same_create_only_field_directly_updated__success(self): current_config = {"PORT": {"Ethernet0":{"lanes":"65"}}} target_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} self.verify_diff(current_config, @@ -841,7 +834,7 @@ def test_validate__same_create_only_field_directly_updated__failure(self): ["PORT", "Ethernet0", "lanes"], ["PORT", "Ethernet0", "lanes"]) - def test_validate__same_create_only_field_updating_parent__failure(self): + def test_validate__same_create_only_field_updating_parent__success(self): current_config = {"PORT": {"Ethernet0":{"lanes":"65"}}} target_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} self.verify_diff(current_config, @@ -849,7 +842,7 @@ def test_validate__same_create_only_field_updating_parent__failure(self): ["PORT", "Ethernet0"], ["PORT", "Ethernet0"]) - def test_validate__same_create_only_field_updating_grandparent__failure(self): + def test_validate__same_create_only_field_updating_grandparent__success(self): current_config = {"PORT": {"Ethernet0":{"lanes":"65"}}} target_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} self.verify_diff(current_config, @@ -857,6 +850,62 @@ def test_validate__same_create_only_field_updating_grandparent__failure(self): ["PORT"], ["PORT"]) + def test_validate__added_create_only_field_parent_exist__failure(self): + current_config = {"PORT": {"Ethernet0":{}}} + target_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} + self.verify_diff(current_config, + target_config, + ["PORT"], + ["PORT"], + expected=False) + + def test_validate__added_create_only_field_parent_doesnot_exist__success(self): + current_config = {"PORT": {}} + target_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} + self.verify_diff(current_config, + target_config, + ["PORT"], + ["PORT"]) + + def test_validate__removed_create_only_field_parent_remain__failure(self): + current_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} + target_config = {"PORT": {"Ethernet0":{}}} + self.verify_diff(current_config, + target_config, + ["PORT"], + ["PORT"], + expected=False) + + def test_validate__removed_create_only_field_parent_doesnot_remain__success(self): + current_config = {"PORT": {"Ethernet0":{"lanes":"65"}}, "ACL_TABLE": {}} + target_config = {"PORT": {}} + self.verify_diff(current_config, + target_config, + ["PORT"], + ["PORT"]) + + def test_hard_coded_create_only_paths(self): + config = { + "PORT": { + "Ethernet0":{"lanes":"65"}, + "Ethernet1":{}, + "Ethernet2":{"lanes":"66,67"} + }, + "LOOPBACK_INTERFACE": { + "Loopback0":{"vrf_name":"vrf0"}, + "Loopback1":{}, + "Loopback2":{"vrf_name":"vrf1"}, + }} + expected = [ + "/PORT/Ethernet0/lanes", + "/PORT/Ethernet2/lanes", + "/LOOPBACK_INTERFACE/Loopback0/vrf_name", + "/LOOPBACK_INTERFACE/Loopback2/vrf_name" + ] + actual = self.validator._get_create_only_paths(config) + + self.assertCountEqual(expected, actual) + def verify_diff(self, current_config, target_config, current_config_tokens=None, target_config_tokens=None, expected=True): # Arrange current_config_tokens = current_config_tokens if current_config_tokens else [] @@ -1741,6 +1790,26 @@ def test_sort__replacing_create_only_field__success(self): # Assert self.assertNotEqual(None, actual) + def test_sort__adding_create_only_field__success(self): + # Arrange + patch = jsonpatch.JsonPatch([{"op":"add", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value":"Vrf_01"}]) + + # Act + actual = self.create_patch_sorter(Files.CONFIG_DB_WITH_LOOPBACK_INTERFACES).sort(patch) + + # Assert + self.assertNotEqual(None, actual) + + def test_sort__removing_create_only_field__success(self): + # Arrange + patch = jsonpatch.JsonPatch([{"op":"remove", "path": "/LOOPBACK_INTERFACE/Loopback1/vrf_name"}]) + + # Act + actual = self.create_patch_sorter(Files.CONFIG_DB_WITH_LOOPBACK_INTERFACES).sort(patch) + + # Assert + self.assertNotEqual(None, actual) + def test_sort__inter_dependency_within_same_table__success(self): # Arrange patch = jsonpatch.JsonPatch([{"op":"add", "path":"/VLAN_INTERFACE", "value": {