diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index c374301729..528e3d5151 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -544,6 +544,125 @@ def _get_default_value_from_settings(self, parent_path, field_name): return None +class CreateOnlyFilter: + """ + A filtering class for create-only fields. + """ + def __init__(self, path_addressing): + # TODO: create-only fields are hard-coded for now, it should be moved to YANG model + self.path_addressing = path_addressing + self.patterns = [ + ["PORT", "*", "lanes"], + ["LOOPBACK_INTERFACE", "*", "vrf_name"], + ["BGP_NEIGHBOR", "*", "holdtime"], + ["BGP_NEIGHBOR", "*", "keepalive"], + ["BGP_NEIGHBOR", "*", "name"], + ["BGP_NEIGHBOR", "*", "asn"], + ["BGP_NEIGHBOR", "*", "local_addr"], + ["BGP_NEIGHBOR", "*", "nhopself"], + ["BGP_NEIGHBOR", "*", "rrclient"], + ["BGP_PEER_RANGE", "*", "*"], + ["BGP_MONITORS", "*", "holdtime"], + ["BGP_MONITORS", "*", "keepalive"], + ["BGP_MONITORS", "*", "name"], + ["BGP_MONITORS", "*", "asn"], + ["BGP_MONITORS", "*", "local_addr"], + ["BGP_MONITORS", "*", "nhopself"], + ["BGP_MONITORS", "*", "rrclient"], + ["MIRROR_SESSION", "*", "*"], + ] + + def get_filter(self): + return JsonPointerFilter(self.patterns, + self.path_addressing) + +class RemoveCreateOnlyDependencyMoveValidator: + """ + A class to validate all dependencies of create-only fields have been removed before + modifying the create-only fields. + """ + def __init__(self, path_addressing): + self.path_addressing = path_addressing + self.create_only_filter = CreateOnlyFilter(path_addressing).get_filter() + + def validate(self, move, diff): + current_config = diff.current_config + target_config = diff.target_config # Final config after applying whole patch + + processed_tables = set() + for path in self.create_only_filter.get_paths(current_config): + tokens = self.path_addressing.get_path_tokens(path) + table_to_check = tokens[0] + + if table_to_check in processed_tables: + continue + else: + processed_tables.add(table_to_check) + + if table_to_check not in current_config: + continue + + current_members = current_config[table_to_check] + if not current_members: + continue + + if table_to_check not in target_config: + continue + + target_members = target_config[table_to_check] + if not target_members: + continue + + simulated_config = move.apply(current_config) # Config after applying just this move + + for member_name in current_members: + if member_name not in target_members: + continue + + if not self._validate_member(tokens, member_name, + current_config, target_config, simulated_config): + return False + + return True + + def _validate_member(self, tokens, member_name, current_config, target_config, simulated_config): + table_to_check, create_only_field = tokens[0], tokens[-1] + + current_field = self._get_create_only_field( + current_config, table_to_check, member_name, create_only_field) + target_field = self._get_create_only_field( + target_config, table_to_check, member_name, create_only_field) + + if current_field == target_field: + return True + + simulated_member = self.path_addressing.get_from_path( + simulated_config, f"/{table_to_check}/{member_name}") + + if simulated_member is None: + return True + + if table_to_check == "PORT": + current_admin_status = self.path_addressing.get_from_path( + current_config, f"/{table_to_check}/{member_name}/admin_status" + ) + simulated_admin_status = self.path_addressing.get_from_path( + simulated_config, f"/{table_to_check}/{member_name}/admin_status" + ) + if current_admin_status != simulated_admin_status and current_admin_status != "up": + return False + + member_path = f"/{table_to_check}/{member_name}" + for ref_path in self.path_addressing.find_ref_paths(member_path, simulated_config): + if not self.path_addressing.has_path(current_config, ref_path): + return False + + return True + + def _get_create_only_field(self, config, table_to_check, + member_name, create_only_field): + return config[table_to_check][member_name].get(create_only_field, None) + class DeleteWholeConfigMoveValidator: """ A class to validate not deleting whole config as it is not supported by JsonPatch lib. @@ -576,27 +695,7 @@ def __init__(self, path_addressing): self.path_addressing = path_addressing # TODO: create-only fields are hard-coded for now, it should be moved to YANG models - self.create_only_filter = JsonPointerFilter([ - ["PORT", "*", "lanes"], - ["LOOPBACK_INTERFACE", "*", "vrf_name"], - ["BGP_NEIGHBOR", "*", "holdtime"], - ["BGP_NEIGHBOR", "*", "keepalive"], - ["BGP_NEIGHBOR", "*", "name"], - ["BGP_NEIGHBOR", "*", "asn"], - ["BGP_NEIGHBOR", "*", "local_addr"], - ["BGP_NEIGHBOR", "*", "nhopself"], - ["BGP_NEIGHBOR", "*", "rrclient"], - ["BGP_PEER_RANGE", "*", "*"], - ["BGP_MONITORS", "*", "holdtime"], - ["BGP_MONITORS", "*", "keepalive"], - ["BGP_MONITORS", "*", "name"], - ["BGP_MONITORS", "*", "asn"], - ["BGP_MONITORS", "*", "local_addr"], - ["BGP_MONITORS", "*", "nhopself"], - ["BGP_MONITORS", "*", "rrclient"], - ["MIRROR_SESSION", "*", "*"], - ], - path_addressing) + self.create_only_filter = CreateOnlyFilter(path_addressing).get_filter() def validate(self, move, diff): simulated_config = move.apply(diff.current_config) @@ -921,6 +1020,8 @@ def validate(self, move, diff): for required_path, required_value in data[path]: current_value = self.identifier.get_value_or_default(current_config, required_path) simulated_value = self.identifier.get_value_or_default(simulated_config, required_path) + if simulated_value is None: # Simulated config does not have this value at all. + continue if current_value != simulated_value and simulated_value != required_value: return False @@ -1000,6 +1101,65 @@ def generate(self, diff): for move in single_run_generator.generate(): yield move +class RemoveCreateOnlyDependencyMoveGenerator: + """ + A class to generate the create-only fields' dependency removing moves + """ + def __init__(self, path_addressing): + self.path_addressing = path_addressing + self.create_only_filter = CreateOnlyFilter(path_addressing).get_filter() + + def generate(self, diff): + current_config = diff.current_config + target_config = diff.target_config # Final config after applying whole patch + + processed_tables = set() + for path in self.create_only_filter.get_paths(current_config): + tokens = self.path_addressing.get_path_tokens(path) + table_to_check, create_only_field = tokens[0], tokens[-1] + + if table_to_check in processed_tables: + continue + else: + processed_tables.add(table_to_check) + + if table_to_check not in current_config: + continue + + current_members = current_config[table_to_check] + if not current_members: + continue + + if table_to_check not in target_config: + continue + + target_members = target_config[table_to_check] + if not target_members: + continue + + for member_name in current_members: + if member_name not in target_members: + continue + + current_field = self._get_create_only_field( + current_config, table_to_check, member_name, create_only_field) + target_field = self._get_create_only_field( + target_config, table_to_check, member_name, create_only_field) + + if current_field == target_field: + continue + + member_path = f"/{table_to_check}/{member_name}" + + for ref_path in self.path_addressing.find_ref_paths(member_path, current_config): + yield JsonMove(diff, OperationType.REMOVE, + self.path_addressing.get_path_tokens(ref_path)) + + def _get_create_only_field(self, config, table_to_check, + member_name, create_only_field): + return config[table_to_check][member_name].get(create_only_field, None) + + class SingleRunLowLevelMoveGenerator: """ A class that can only run once to assist LowLevelMoveGenerator with generating the moves. @@ -1271,6 +1431,8 @@ def extend(self, move, diff): for required_path, required_value in data[path]: current_value = self.identifier.get_value_or_default(current_config, required_path) simulated_value = self.identifier.get_value_or_default(simulated_config, required_path) + if simulated_value is None: # Simulated config does not have this value at all. + continue if current_value != simulated_value and simulated_value != required_value: flip_path_value_tuples.add((required_path, required_value)) @@ -1473,7 +1635,8 @@ def __init__(self, operation_wrapper, config_wrapper, path_addressing): self.path_addressing = path_addressing def create(self, algorithm=Algorithm.DFS): - move_generators = [LowLevelMoveGenerator(self.path_addressing)] + move_generators = [RemoveCreateOnlyDependencyMoveGenerator(self.path_addressing), + LowLevelMoveGenerator(self.path_addressing)] # TODO: Enable TableLevelMoveGenerator once it is confirmed whole table can be updated at the same time move_non_extendable_generators = [KeyLevelMoveGenerator()] move_extenders = [RequiredValueMoveExtender(self.path_addressing, self.operation_wrapper), @@ -1485,6 +1648,7 @@ def create(self, algorithm=Algorithm.DFS): NoDependencyMoveValidator(self.path_addressing, self.config_wrapper), CreateOnlyMoveValidator(self.path_addressing), RequiredValueMoveValidator(self.path_addressing), + RemoveCreateOnlyDependencyMoveValidator(self.path_addressing), NoEmptyTableMoveValidator(self.path_addressing)] move_wrapper = MoveWrapper(move_generators, move_non_extendable_generators, move_extenders, move_validators) diff --git a/tests/generic_config_updater/files/config_db_with_port_critical.json b/tests/generic_config_updater/files/config_db_with_port_critical.json index 5853bfe5ea..2b2007d068 100644 --- a/tests/generic_config_updater/files/config_db_with_port_critical.json +++ b/tests/generic_config_updater/files/config_db_with_port_critical.json @@ -36,6 +36,23 @@ "lanes": "41,42,43,44", "pfc_asym": "off", "speed": "40000" + }, + "Ethernet28": { + "alias": "fortyGigE0/28", + "description": "Servers5:eth0", + "index": "6", + "lanes": "53,54,55,56", + "pfc_asym": "off", + "speed": "40000" + }, + "Ethernet32": { + "alias": "fortyGigE0/32", + "description": "Servers6:eth0", + "index": "7", + "lanes": "57,58,59,60", + "mtu": "9100", + "pfc_asym": "off", + "speed": "40000" } }, "BUFFER_PG": { @@ -44,6 +61,9 @@ }, "Ethernet12|0": { "profile": "ingress_lossy_profile" + }, + "Ethernet28|0": { + "profile": "ingress_lossy_profile" } } } diff --git a/tests/generic_config_updater/files/patch_sorter_test_success.json b/tests/generic_config_updater/files/patch_sorter_test_success.json index b4f1f141c3..217737e412 100644 --- a/tests/generic_config_updater/files/patch_sorter_test_success.json +++ b/tests/generic_config_updater/files/patch_sorter_test_success.json @@ -574,38 +574,6 @@ } ], "expected_changes": [ - [ - { - "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132" - } - ], - [ - { - "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128" - } - ], - [ - { - "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", - "value": {} - } - ], - [ - { - "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" - } - ], - [ - { - "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128", - "value": {} - } - ], [ { "op": "remove", @@ -646,13 +614,6 @@ "path": "/LOOPBACK_INTERFACE/Loopback0|1100:1::32~1128", "value": {} } - ], - [ - { - "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132", - "value": {} - } ] ] }, @@ -809,51 +770,12 @@ "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE", - "value": { - "NO-NSW-PACL-V4": { - "type": "L3" - } - } - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], [ { "op": "remove", "path": "/VLAN_MEMBER" } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", - "value": "NO-NSW-PACL-V4" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" - } - ], [ { "op": "remove", @@ -1124,6 +1046,12 @@ "path": "/VLAN_MEMBER/Vlan100|Ethernet3" } ], + [ + { + "op": "remove", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/0" + } + ], [ { "op": "replace", @@ -1184,7 +1112,7 @@ [ { "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/0" } ], [ @@ -1196,7 +1124,7 @@ [ { "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/0" } ], [ @@ -1207,72 +1135,27 @@ ], [ { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" - } - ], - [ - { - "op": "remove", - "path": "/PORT/Ethernet3" + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/0", + "value": "Ethernet0" } ], [ { "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" } ], [ { "op": "remove", - "path": "/VLAN_MEMBER" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] + "path": "/PORT/Ethernet3" } ], [ { "op": "remove", - "path": "/ACL_TABLE" - } - ], - [ - { - "op": "add", - "path": "/VLAN_MEMBER", - "value": { - "Vlan100|Ethernet0": { - "tagging_mode": "untagged" - } - } - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE", - "value": { - "NO-NSW-PACL-V4": { - "type": "L3" - } - } - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" } ], [ @@ -1281,12 +1164,6 @@ "path": "/VLAN_MEMBER" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" - } - ], [ { "op": "remove", @@ -1318,13 +1195,6 @@ } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", - "value": "NO-NSW-PACL-V4" - } - ], [ { "op": "add", @@ -2323,38 +2193,6 @@ } ], "expected_changes": [ - [ - { - "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" - } - ], - [ - { - "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128" - } - ], - [ - { - "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132", - "value": {} - } - ], - [ - { - "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132" - } - ], - [ - { - "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128", - "value": {} - } - ], [ { "op": "remove", @@ -2373,13 +2211,6 @@ "path": "/LOOPBACK_INTERFACE/Loopback1" } ], - [ - { - "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132", - "value": {} - } - ], [ { "op": "add", @@ -2565,64 +2396,6 @@ "path": "/VLAN_MEMBER" } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE" - } - ], - [ - { - "op": "add", - "path": "/VLAN_MEMBER", - "value": { - "Vlan100|Ethernet0": { - "tagging_mode": "untagged" - } - } - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE", - "value": { - "NO-NSW-PACL-V4": { - "type": "L3" - } - } - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], - [ - { - "op": "remove", - "path": "/VLAN_MEMBER" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" - } - ], [ { "op": "remove", @@ -2654,13 +2427,6 @@ } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", - "value": "NO-NSW-PACL-V4" - } - ], [ { "op": "add", diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 2ef18e1fc4..900538dd6e 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -1884,6 +1884,22 @@ def _get_critical_port_change_test_cases(self): } }) }, + # Additional cases where the full port is getting deleted + # If port is getting deleted, there is no point in checking if there are critical changes depending on it + "NOT_PORT_UP__STATUS_CHANGING__UNDER_PORT__PORT_EXIST__PORT_DELETION": { + "expected": True, + "config": Files.CONFIG_DB_WITH_PORT_CRITICAL, + "move": ps.JsonMove.from_operation({ "op": "remove", "path": "/PORT/Ethernet32" }) + }, + "NOT_PORT_UP__STATUS_CHANGING__NOT_UNDER_PORT__PORT_EXIST__PORT_DELETION": { + "expected": True, + "config": Files.CONFIG_DB_WITH_PORT_CRITICAL, + "target_config": self._apply_operations(Files.CONFIG_DB_WITH_PORT_CRITICAL, [ + { "op": "remove", "path": "/PORT/Ethernet28" }, + { "op": "remove", "path": "/BUFFER_PG/Ethernet28|0" }, + ]), + "move": ps.JsonMove.from_operation({ "op": "remove", "path": "/PORT/Ethernet28" }) + }, } def test_validate__no_critical_port_changes(self): @@ -1937,6 +1953,106 @@ def _get_no_critical_port_change_test_cases(self): def _apply_operations(self, config, operations): return jsonpatch.JsonPatch(operations).apply(config) +class RemoveCreateOnlyDependencyMoveValidator(unittest.TestCase): + def setUp(self): + path_addressing = PathAddressing(ConfigWrapper()) + self.validator = ps.RemoveCreateOnlyDependencyMoveValidator(path_addressing) + + def test_validate__lane_replacement_change(self): + test_cases = self._get_lane_replacement_change_test_cases() + for test_case_name in test_cases: + with self.subTest(name=test_case_name): + self._run_single_test(test_cases[test_case_name]) + + def _run_single_test(self, test_case): + # Arrange + expected = test_case['expected'] + current_config = test_case['config'] + move = test_case['move'] + target_config = test_case.get('target_config', move.apply(current_config)) + diff = ps.Diff(current_config, target_config) + + # Act and Assert + self.assertEqual(expected, self.validator.validate(move, diff)) + + def _apply_operations(self, config, operations): + return jsonpatch.JsonPatch(operations).apply(config) + + def _get_lane_replacement_change_test_cases(self): + return { + "PORT_NOT_IN_CURRENT_CONFIG": { + "expected": True, + "config": {}, + "move": Mock(), + "target_config": Files.DPB_1_SPLIT_FULL_CONFIG + }, + "PORT_NOT_IN_TARGET_CONFIG": { + "expected": True, + "config": Files.DPB_1_SPLIT_FULL_CONFIG, + "move": Mock(), + "target_config": {} + }, + "PORT_EMPTY_IN_CURRENT_CONFIG": { + "expected": True, + "config": {"PORT": {}}, + "move": Mock(), + "target_config": Files.DPB_1_SPLIT_FULL_CONFIG + }, + "PORT_EMPTY_IN_TARGET_CONFIG": { + "expected": True, + "config": Files.DPB_1_SPLIT_FULL_CONFIG, + "move": Mock(), + "target_config": {"PORT": {}} + }, + "SAME_LANE_IN_BOTH_CONFIG": { + "expected": True, + "config": Files.DPB_1_SPLIT_FULL_CONFIG, + "move": ps.JsonMove.from_operation({ + "op": "remove", + "path": "/ACL_TABLE" + }) + }, + "LANE_DIFF__CURRENT_DOWN__SIMULATED_UP": { + "expected": False, + "config": Files.DPB_1_SPLIT_FULL_CONFIG, + "move": ps.JsonMove.from_operation({ + "op": "add", + "path": "/PORT/Ethernet0/admin_status", + "value": "up" + }), + "target_config":Files.DPB_4_SPLITS_FULL_CONFIG, + }, + "LANE_DIFF__STATUS_SAME__SIMULATED_EXTRA_DEPDENDENCY": { + "expected": False, + "config": self._apply_operations(Files.DPB_1_SPLIT_FULL_CONFIG, [ + {"op": "remove", "path": "/ACL_TABLE"}, + ]), + "move": ps.JsonMove.from_operation({ + "op": "add", + "path": "/ACL_TABLE", + "value": { + "NO-NSW-PACL-V4": { + "type": "L3", + "policy_desc": "NO-NSW-PACL-V4", + "ports": [ + "Ethernet0" + ] + } + } + }), + "target_config": Files.DPB_4_SPLITS_FULL_CONFIG + }, + "LANE_DIFF__STATUS_SAME__SIMULATED_LESS_DEPDENDENCY": { + "expected": True, + "config": Files.DPB_1_SPLIT_FULL_CONFIG, + "move": ps.JsonMove.from_operation({ + "op": "remove", + "path": "/ACL_TABLE" + }), + "target_config": Files.DPB_4_SPLITS_FULL_CONFIG + } + } + class TestTableLevelMoveGenerator(unittest.TestCase): def setUp(self): self.generator = ps.TableLevelMoveGenerator() @@ -2313,6 +2429,85 @@ def get_diff(self, target_config_ops = None, current_config_ops = None): return ps.Diff(current_config, target_config) +class RemoveCreateOnlyDependencyMoveGenerator(unittest.TestCase): + def setUp(self): + config_wrapper = ConfigWrapper() + path_addressing = PathAddressing(config_wrapper) + self.generator = ps.RemoveCreateOnlyDependencyMoveGenerator(path_addressing) + + def test_generate__no_port_table__no_moves(self): + current_config = {} + target_config = {"PORT": {"Ethernet0": {}}} + + # No PORT table in current_config + diff = ps.Diff(current_config, target_config) + moves = list(self.generator.generate(diff)) + self.verify_moves([], moves) + + # No PORT table in target_config + diff = ps.Diff(target_config, current_config) + moves = list(self.generator.generate(diff)) + self.verify_moves([], moves) + + def test_generate__empty_port_content__no_moves(self): + current_config = {"PORT": {}} + target_config = {"PORT": {"Ethernet0": {}}} + + # Empty PORT content in current_config + diff = ps.Diff(current_config, target_config) + moves = list(self.generator.generate(diff)) + self.verify_moves([], moves) + + # Empty PORT content in target_config + diff = ps.Diff(target_config, current_config) + moves = list(self.generator.generate(diff)) + self.verify_moves([], moves) + + def test_generate__same_lanes__no_moves(self): + current_config = Files.CROPPED_CONFIG_DB_AS_JSON + patch = jsonpatch.JsonPatch([ + {'op': 'remove', 'path': '/VLAN_MEMBER'} + ]) + target_config = patch.apply(Files.CROPPED_CONFIG_DB_AS_JSON) + + # Remove VLAN_MEMBER in target_config, lanes are same + diff = ps.Diff(current_config, target_config) + moves = list(self.generator.generate(diff)) + self.verify_moves([], moves) + + # Add VLAN_MEMBER in current_config, lanes are same + diff = ps.Diff(target_config, current_config) + moves = list(self.generator.generate(diff)) + self.verify_moves([], moves) + + def test_generate__dpb_4_to_1_example(self): + # Arrange + diff = ps.Diff(Files.DPB_4_SPLITs_FULL_CONFIG, Files.DPB_1_SPLIT_FULL_CONFIG) + + # Act + moves = list(self.generator.generate(diff)) + + # Assert + self.verify_moves([{'op': 'remove', 'path': '/ACL_TABLE/NO-NSW-PACL-V4/ports/0'}, + {'op': 'remove', 'path': '/VLAN_MEMBER/Vlan100|Ethernet0'}], + moves) + + def test_generate__dpb_1_to_4_example(self): + # Arrange + diff = ps.Diff(Files.DPB_1_SPLIT_FULL_CONFIG, Files.DPB_4_SPLITS_FULL_CONFIG) + + # Act + moves = list(self.generator.generate(diff)) + + # Assert + self.verify_moves([{'op': 'remove', 'path': '/ACL_TABLE/NO-NSW-PACL-V4/ports/0'}, + {'op': 'remove', 'path': '/VLAN_MEMBER/Vlan100|Ethernet0'}], + moves) + + def verify_moves(self, ops, moves): + moves_ops = [list(move.patch)[0] for move in moves] + self.assertCountEqual(ops, moves_ops) + class TestRequiredValueMoveExtender(unittest.TestCase): def setUp(self): path_addressing = PathAddressing() @@ -2543,6 +2738,26 @@ def test_extend__multiple_changes__multiple_extend_moves(self): # Assert self._verify_moves(expected, actual) + def test_extend__port_deletion__no_extension(self): + # Arrange + move = ps.JsonMove.from_operation({ + "op":"remove", + "path":"/PORT/Ethernet28" + }) + current_config = Files.CONFIG_DB_WITH_PORT_CRITICAL + target_config = self._apply_operations(Files.CONFIG_DB_WITH_PORT_CRITICAL, [ + { "op": "remove", "path": "/PORT/Ethernet28" }, + { "op": "remove", "path": "/BUFFER_PG/Ethernet28|0" } + ]) + diff = ps.Diff(current_config, target_config) + expected = [] + + # Act + actual = self.extender.extend(move, diff) + + # Assert + self._verify_moves(expected, actual) + def _verify_moves(self, ex_ops, moves): moves_ops = [list(move.patch)[0] for move in moves] self.assertCountEqual(ex_ops, moves_ops) @@ -3021,7 +3236,8 @@ def verify(self, algo, algo_class): # Arrange config_wrapper = ConfigWrapper() factory = ps.SortAlgorithmFactory(OperationWrapper(), config_wrapper, PathAddressing(config_wrapper)) - expected_generators = [ps.LowLevelMoveGenerator] + expected_generators = [ps.RemoveCreateOnlyDependencyMoveGenerator, + ps.LowLevelMoveGenerator] expected_non_extendable_generators = [ps.KeyLevelMoveGenerator] expected_extenders = [ps.RequiredValueMoveExtender, ps.UpperLevelMoveExtender, @@ -3032,6 +3248,7 @@ def verify(self, algo, algo_class): ps.NoDependencyMoveValidator, ps.CreateOnlyMoveValidator, ps.RequiredValueMoveValidator, + ps.RemoveCreateOnlyDependencyMoveValidator, ps.NoEmptyTableMoveValidator] # Act