diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index e89c542e73b1..97db21b24eba 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -290,29 +290,52 @@ def __hash__(self): return hash((self.op_type, self.path, json.dumps(self.value))) class MoveWrapper: - def __init__(self, move_generators, move_extenders, move_validators): + def __init__(self, move_generators, move_non_extendable_generators, move_extenders, move_validators): self.move_generators = move_generators + self.move_non_extendable_generators = move_non_extendable_generators self.move_extenders = move_extenders self.move_validators = move_validators def generate(self, diff): + """ + Generates all possible moves to help transform diff.current_config to diff.target_config. + + It starts by generating the non-extendable moves i.e. moves that will not extended to e.g. change its parent. + The non-extendable moves are mostly high level moves such as deleting/adding whole tables. + + After that it generates extendable moves i.e. moves that can be extended to e.g. change its parent. + The extendable moves are typically very low level moves that can achieve the minimum disruption guarantee. + + Lastly the moves are extended for example to try to replace the parent config instead, or by deleting + the dependencies of the config. + """ processed_moves = set() + extended_moves = set() moves = deque([]) + for move in self._generate_non_extendable_moves(diff): + if not(move in processed_moves): + processed_moves.add(move) + yield move + for move in self._generate_moves(diff): - if move in processed_moves: - continue - processed_moves.add(move) - yield move - moves.extend(self._extend_moves(move, diff)) + if not(move in processed_moves): + processed_moves.add(move) + yield move + + if not(move in extended_moves): + extended_moves.add(move) + moves.extend(self._extend_moves(move, diff)) while moves: move = moves.popleft() - if move in processed_moves: - continue - processed_moves.add(move) - yield move - moves.extend(self._extend_moves(move, diff)) + if not(move in processed_moves): + processed_moves.add(move) + yield move + + if not(move in extended_moves): + extended_moves.add(move) + moves.extend(self._extend_moves(move, diff)) def validate(self, move, diff): for validator in self.move_validators: @@ -328,6 +351,11 @@ def _generate_moves(self, diff): for move in generator.generate(diff): yield move + def _generate_non_extendable_moves(self, diff): + for generator in self.move_non_extendable_generators: + for move in generator.generate(diff): + yield move + def _extend_moves(self, move, diff): for extender in self.move_extenders: for newmove in extender.extend(move, diff): @@ -921,6 +949,68 @@ def validate(self, move, diff): return True +class TableLevelMoveGenerator: + """ + A class that key level moves. The item name at the root level of ConfigDB is called 'Table'. + + e.g. + { + "Table": ... + } + + This class will generate moves to remove tables if they are in current, but not target. It also add tables + if they are in target but not current configs. + """ + + def generate(self, diff): + # Removing tables in current but not target + for tokens in self._get_non_existing_tables_tokens(diff.current_config, diff.target_config): + yield JsonMove(diff, OperationType.REMOVE, tokens) + + # Adding tables in target but not current + for tokens in self._get_non_existing_tables_tokens(diff.target_config, diff.current_config): + yield JsonMove(diff, OperationType.ADD, tokens, tokens) + + def _get_non_existing_tables_tokens(self, config1, config2): + for table in config1: + if not(table in config2): + yield [table] + +class KeyLevelMoveGenerator: + """ + A class that key level moves. The item name at the root level of ConfigDB is called 'Table', the item + name in the Table level of ConfigDB is called key. + + e.g. + { + "Table": { + "Key": ... + } + } + + This class will generate moves to remove keys if they are in current, but not target. It also add keys + if they are in target but not current configs. + """ + def generate(self, diff): + # Removing keys in current but not target + for tokens in self._get_non_existing_keys_tokens(diff.current_config, diff.target_config): + table = tokens[0] + # if table has a single key, delete the whole table because empty tables are not allowed in ConfigDB + if len(diff.current_config[table]) == 1: + yield JsonMove(diff, OperationType.REMOVE, [table]) + else: + yield JsonMove(diff, OperationType.REMOVE, tokens) + + # Adding keys in target but not current + for tokens in self._get_non_existing_keys_tokens(diff.target_config, diff.current_config): + yield JsonMove(diff, OperationType.ADD, tokens, tokens) + + def _get_non_existing_keys_tokens(self, config1, config2): + for table in config1: + for key in config1[table]: + if not(table in config2) or not (key in config2[table]): + yield [table, key] + class LowLevelMoveGenerator: """ A class to generate the low level moves i.e. moves corresponding to differences between current/target config @@ -1407,6 +1497,8 @@ def __init__(self, operation_wrapper, config_wrapper, path_addressing): def create(self, algorithm=Algorithm.DFS): move_generators = [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), UpperLevelMoveExtender(), DeleteInsteadOfReplaceMoveExtender(), @@ -1419,7 +1511,7 @@ def create(self, algorithm=Algorithm.DFS): RequiredValueMoveValidator(self.path_addressing), NoEmptyTableMoveValidator(self.path_addressing)] - move_wrapper = MoveWrapper(move_generators, move_extenders, move_validators) + move_wrapper = MoveWrapper(move_generators, move_non_extendable_generators, move_extenders, move_validators) if algorithm == Algorithm.DFS: sorter = DfsSorter(move_wrapper) 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 894f68896cba..c134b0b5e243 100644 --- a/tests/generic_config_updater/files/patch_sorter_test_success.json +++ b/tests/generic_config_updater/files/patch_sorter_test_success.json @@ -169,16 +169,10 @@ "op": "add", "path": "/INTERFACE/Ethernet8|10.0.0.1~130", "value": { - "family": "IPv4" + "family": "IPv4", + "scope": "global" } } - ], - [ - { - "op": "add", - "path": "/INTERFACE/Ethernet8|10.0.0.1~130/scope", - "value": "global" - } ] ] }, @@ -282,39 +276,15 @@ "op": "add", "path": "/ACL_TABLE/EVERFLOWV6", "value": { + "policy_desc": "EVERFLOWV6", + "ports": [ + "Ethernet4", + "Ethernet8" + ], + "stage": "ingress", "type": "MIRRORV6" } } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/policy_desc", - "value": "EVERFLOWV6" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports", - "value": [ - "Ethernet4" - ] - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports/1", - "value": "Ethernet8" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/stage", - "value": "ingress" - } ] ] }, @@ -419,129 +389,57 @@ "path": "/ACL_TABLE", "value": { "NO-NSW-PACL-V4": { - "type": "L3" + "type": "L3", + "policy_desc": "NO-NSW-PACL-V4", + "ports": [ + "Ethernet0" + ] } } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", - "value": "NO-NSW-PACL-V4" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], [ { "op": "add", "path": "/ACL_TABLE/DATAACL", "value": { + "policy_desc": "DATAACL", + "ports": [ + "Ethernet4" + ], + "stage": "ingress", "type": "L3" } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/DATAACL/policy_desc", - "value": "DATAACL" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/DATAACL/ports", - "value": [ - "Ethernet4" - ] - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/DATAACL/stage", - "value": "ingress" - } - ], [ { "op": "add", "path": "/ACL_TABLE/EVERFLOW", "value": { + "policy_desc": "EVERFLOW", + "ports": [ + "Ethernet8" + ], + "stage": "ingress", "type": "MIRROR" } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOW/policy_desc", - "value": "EVERFLOW" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOW/ports", - "value": [ - "Ethernet8" - ] - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOW/stage", - "value": "ingress" - } - ], [ { "op": "add", "path": "/ACL_TABLE/EVERFLOWV6", "value": { + "policy_desc": "EVERFLOWV6", + "ports": [ + "Ethernet4", + "Ethernet8" + ], + "stage": "ingress", "type": "MIRRORV6" } } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/policy_desc", - "value": "EVERFLOWV6" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports", - "value": [ - "Ethernet4" - ] - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/ports/1", - "value": "Ethernet8" - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/EVERFLOWV6/stage", - "value": "ingress" - } ] ] }, @@ -979,30 +877,34 @@ [ { "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] + "path": "/PORT/Ethernet3", + "value": { + "alias": "Eth1/4", + "lanes": "68", + "description": "", + "speed": "10000" + } } ], [ { "op": "add", - "path": "/VLAN_MEMBER", + "path": "/PORT/Ethernet2", "value": { - "Vlan100|Ethernet0": { - "tagging_mode": "untagged" - } + "alias": "Eth1/3", + "lanes": "67", + "description": "", + "speed": "10000" } } ], [ { "op": "add", - "path": "/PORT/Ethernet3", + "path": "/PORT/Ethernet1", "value": { - "alias": "Eth1/4", - "lanes": "68", + "alias": "Eth1/2", + "lanes": "66", "description": "", "speed": "10000" } @@ -1011,14 +913,18 @@ [ { "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1", - "value": "Ethernet3" + "path": "/VLAN_MEMBER", + "value": { + "Vlan100|Ethernet0": { + "tagging_mode": "untagged" + } + } } ], [ { "op": "add", - "path": "/VLAN_MEMBER/Vlan100|Ethernet3", + "path": "/VLAN_MEMBER/Vlan100|Ethernet1", "value": { "tagging_mode": "untagged" } @@ -1027,22 +933,12 @@ [ { "op": "add", - "path": "/PORT/Ethernet2", + "path": "/VLAN_MEMBER/Vlan100|Ethernet3", "value": { - "alias": "Eth1/3", - "lanes": "67", - "description": "", - "speed": "10000" + "tagging_mode": "untagged" } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/2", - "value": "Ethernet2" - } - ], [ { "op": "add", @@ -1055,13 +951,10 @@ [ { "op": "add", - "path": "/PORT/Ethernet1", - "value": { - "alias": "Eth1/2", - "lanes": "66", - "description": "", - "speed": "10000" - } + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] } ], [ @@ -1073,23 +966,16 @@ ], [ { - "op": "replace", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0", - "Ethernet1", - "Ethernet2", - "Ethernet3" - ] + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/2", + "value": "Ethernet2" } ], [ { "op": "add", - "path": "/VLAN_MEMBER/Vlan100|Ethernet1", - "value": { - "tagging_mode": "untagged" - } + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/3", + "value": "Ethernet3" } ] ] @@ -1220,6 +1106,24 @@ } ], "expected_changes": [ + [ + { + "op": "remove", + "path": "/VLAN_MEMBER/Vlan100|Ethernet1" + } + ], + [ + { + "op": "remove", + "path": "/VLAN_MEMBER/Vlan100|Ethernet2" + } + ], + [ + { + "op": "remove", + "path": "/VLAN_MEMBER/Vlan100|Ethernet3" + } + ], [ { "op": "replace", @@ -1283,24 +1187,6 @@ "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" - } - ], - [ - { - "op": "remove", - "path": "/VLAN_MEMBER/Vlan100|Ethernet1" - } - ], [ { "op": "remove", @@ -1310,7 +1196,7 @@ [ { "op": "remove", - "path": "/VLAN_MEMBER/Vlan100|Ethernet2" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" } ], [ @@ -1322,7 +1208,7 @@ [ { "op": "remove", - "path": "/VLAN_MEMBER/Vlan100|Ethernet3" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" } ], [ @@ -1358,6 +1244,17 @@ "path": "/ACL_TABLE" } ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER", + "value": { + "Vlan100|Ethernet0": { + "tagging_mode": "untagged" + } + } + } + ], [ { "op": "add", @@ -1369,17 +1266,31 @@ } } ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] + } + ], [ { "op": "remove", - "path": "/PORT" + "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", + "path": "/PORT" } ], [ @@ -1389,22 +1300,13 @@ "value": { "Ethernet0": { "alias": "Eth1", - "lanes": "65, 66, 67, 68", "description": "Ethernet0 100G link", + "lanes": "65, 66, 67, 68", "speed": "100000" } } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], [ { "op": "add", @@ -1415,6 +1317,22 @@ } } } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", + "value": "NO-NSW-PACL-V4" + } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] + } ] ] }, @@ -2085,18 +2003,6 @@ } ], "expected_changes": [ - [ - { - "op": "remove", - "path": "/PORT/Ethernet0/alias" - } - ], - [ - { - "op": "remove", - "path": "/PORT/Ethernet0/description" - } - ], [ { "op": "remove", @@ -2154,18 +2060,6 @@ } ], "expected_changes": [ - [ - { - "op": "remove", - "path": "/INTERFACE/Ethernet8|10.0.0.1~130/family" - } - ], - [ - { - "op": "remove", - "path": "/INTERFACE/Ethernet8|10.0.0.1~130/scope" - } - ], [ { "op": "remove", @@ -2378,90 +2272,24 @@ } ], "expected_changes": [ - [ - { - "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/DATAACL/policy_desc" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/DATAACL/stage" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOW/policy_desc" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOW/stage" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOWV6/policy_desc" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOWV6/ports/0" - } - ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOWV6/stage" - } - ], [ { "op": "remove", "path": "/ACL_TABLE/NO-NSW-PACL-V4" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/DATAACL/ports" - } - ], [ { "op": "remove", "path": "/ACL_TABLE/DATAACL" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOW/ports" - } - ], [ { "op": "remove", "path": "/ACL_TABLE/EVERFLOW" } ], - [ - { - "op": "remove", - "path": "/ACL_TABLE/EVERFLOWV6/ports" - } - ], [ { "op": "remove", @@ -2752,6 +2580,17 @@ "path": "/ACL_TABLE" } ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER", + "value": { + "Vlan100|Ethernet0": { + "tagging_mode": "untagged" + } + } + } + ], [ { "op": "add", @@ -2763,17 +2602,31 @@ } } ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] + } + ], [ { "op": "remove", - "path": "/PORT" + "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", + "path": "/PORT" } ], [ @@ -2783,22 +2636,13 @@ "value": { "Ethernet0": { "alias": "Eth1", - "lanes": "67", "description": "Ethernet0 100G link", + "lanes": "67", "speed": "100000" } } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], [ { "op": "add", @@ -2809,6 +2653,22 @@ } } } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", + "value": "NO-NSW-PACL-V4" + } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] + } ] ] }, @@ -4267,6 +4127,127 @@ } ], "expected_changes": [ + [ + { + "op": "add", + "path": "/BGP_NEIGHBOR/10.0.0.33", + "value": { + "admin_status": "up", + "asn": "64001", + "holdtime": "10", + "keepalive": "3", + "local_addr": "10.0.0.32", + "name": "ARISTA01T0", + "nhopself": "0", + "rrclient": "0" + } + } + ], + [ + { + "op": "add", + "path": "/BGP_NEIGHBOR/fc00::42", + "value": { + "admin_status": "up", + "asn": "64001", + "holdtime": "10", + "keepalive": "3", + "local_addr": "fc00::41", + "name": "ARISTA01T0", + "nhopself": "0", + "rrclient": "0" + } + } + ], + [ + { + "op": "add", + "path": "/BUFFER_PG/Ethernet64|3-4", + "value": { + "profile": "pg_lossless_40000_40m_profile" + } + } + ], + [ + { + "op": "add", + "path": "/BUFFER_PG/Ethernet64|0", + "value": { + "profile": "ingress_lossy_profile" + } + } + ], + [ + { + "op": "add", + "path": "/BUFFER_QUEUE/Ethernet64|0-2", + "value": { + "profile": "egress_lossy_profile" + } + } + ], + [ + { + "op": "add", + "path": "/BUFFER_QUEUE/Ethernet64|3-4", + "value": { + "profile": "egress_lossless_profile" + } + } + ], + [ + { + "op": "add", + "path": "/BUFFER_QUEUE/Ethernet64|5-6", + "value": { + "profile": "egress_lossy_profile" + } + } + ], + [ + { + "op": "add", + "path": "/DEVICE_NEIGHBOR/Ethernet64", + "value": { + "name": "ARISTA01T0", + "port": "Ethernet1" + } + } + ], + [ + { + "op": "add", + "path": "/INTERFACE/Ethernet64", + "value": {} + } + ], + [ + { + "op": "add", + "path": "/INTERFACE/Ethernet64|10.0.0.32~131", + "value": {} + } + ], + [ + { + "op": "add", + "path": "/INTERFACE/Ethernet64|FC00::41~1126", + "value": {} + } + ], + [ + { + "op": "add", + "path": "/PORT_QOS_MAP/Ethernet64", + "value": { + "dscp_to_tc_map": "AZURE", + "pfc_enable": "3,4", + "pfc_to_queue_map": "AZURE", + "tc_to_pg_map": "AZURE", + "tc_to_queue_map": "AZURE" + } + } + ], [ { "op": "add", @@ -4603,88 +4584,6 @@ "value": "up" } ], - [ - { - "op": "add", - "path": "/BUFFER_PG/Ethernet64|3-4", - "value": { - "profile": "pg_lossless_40000_40m_profile" - } - } - ], - [ - { - "op": "add", - "path": "/BUFFER_PG/Ethernet64|0", - "value": { - "profile": "ingress_lossy_profile" - } - } - ], - [ - { - "op": "add", - "path": "/BUFFER_QUEUE/Ethernet64|0-2", - "value": { - "profile": "egress_lossy_profile" - } - } - ], - [ - { - "op": "add", - "path": "/BUFFER_QUEUE/Ethernet64|3-4", - "value": { - "profile": "egress_lossless_profile" - } - } - ], - [ - { - "op": "add", - "path": "/BUFFER_QUEUE/Ethernet64|5-6", - "value": { - "profile": "egress_lossy_profile" - } - } - ], - [ - { - "op": "add", - "path": "/DEVICE_NEIGHBOR/Ethernet64", - "value": { - "name": "ARISTA01T0" - } - } - ], - [ - { - "op": "add", - "path": "/DEVICE_NEIGHBOR/Ethernet64/port", - "value": "Ethernet1" - } - ], - [ - { - "op": "add", - "path": "/INTERFACE/Ethernet64", - "value": {} - } - ], - [ - { - "op": "add", - "path": "/INTERFACE/Ethernet64|10.0.0.32~131", - "value": {} - } - ], - [ - { - "op": "add", - "path": "/INTERFACE/Ethernet64|FC00::41~1126", - "value": {} - } - ], [ { "op": "replace", @@ -4692,81 +4591,12 @@ "value": "ARISTA01T0:Ethernet1" } ], - [ - { - "op": "add", - "path": "/PORT_QOS_MAP/Ethernet64", - "value": { - "dscp_to_tc_map": "AZURE" - } - } - ], - [ - { - "op": "add", - "path": "/PORT_QOS_MAP/Ethernet64/pfc_enable", - "value": "3,4" - } - ], - [ - { - "op": "add", - "path": "/PORT_QOS_MAP/Ethernet64/pfc_to_queue_map", - "value": "AZURE" - } - ], - [ - { - "op": "add", - "path": "/PORT_QOS_MAP/Ethernet64/tc_to_pg_map", - "value": "AZURE" - } - ], - [ - { - "op": "add", - "path": "/PORT_QOS_MAP/Ethernet64/tc_to_queue_map", - "value": "AZURE" - } - ], [ { "op": "add", "path": "/PORT/Ethernet64/admin_status", "value": "up" } - ], - [ - { - "op": "add", - "path": "/BGP_NEIGHBOR/10.0.0.33", - "value": { - "admin_status": "up", - "asn": "64001", - "holdtime": "10", - "keepalive": "3", - "local_addr": "10.0.0.32", - "name": "ARISTA01T0", - "nhopself": "0", - "rrclient": "0" - } - } - ], - [ - { - "op": "add", - "path": "/BGP_NEIGHBOR/fc00::42", - "value": { - "admin_status": "up", - "asn": "64001", - "holdtime": "10", - "keepalive": "3", - "local_addr": "fc00::41", - "name": "ARISTA01T0", - "nhopself": "0", - "rrclient": "0" - } - } ] ] } diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 12d76c528399..5530de7cef9c 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -498,21 +498,23 @@ def setUp(self): def test_ctor__assigns_values_correctly(self): # Arrange move_generators = Mock() + move_non_extendable_generators = Mock() move_extenders = Mock() move_validators = Mock() # Act - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, move_validators) + move_wrapper = ps.MoveWrapper(move_generators, move_non_extendable_generators, move_extenders, move_validators) # Assert self.assertIs(move_generators, move_wrapper.move_generators) + self.assertIs(move_non_extendable_generators, move_wrapper.move_non_extendable_generators) self.assertIs(move_extenders, move_wrapper.move_extenders) self.assertIs(move_validators, move_wrapper.move_validators) def test_generate__single_move_generator__single_move_returned(self): # Arrange move_generators = [self.single_move_generator] - move_wrapper = ps.MoveWrapper(move_generators, [], []) + move_wrapper = ps.MoveWrapper(move_generators, [], [], []) expected = [self.any_move] # Act @@ -524,7 +526,7 @@ def test_generate__single_move_generator__single_move_returned(self): def test_generate__multiple_move_generator__multiple_move_returned(self): # Arrange move_generators = [self.multiple_move_generator] - move_wrapper = ps.MoveWrapper(move_generators, [], []) + move_wrapper = ps.MoveWrapper(move_generators, [], [], []) expected = [self.any_move, self.any_other_move1, self.any_other_move2] # Act @@ -536,7 +538,7 @@ def test_generate__multiple_move_generator__multiple_move_returned(self): def test_generate__different_move_generators__different_moves_returned(self): # Arrange move_generators = [self.single_move_generator, self.another_single_move_generator] - move_wrapper = ps.MoveWrapper(move_generators, [], []) + move_wrapper = ps.MoveWrapper(move_generators, [], [], []) expected = [self.any_move, self.any_other_move1] # Act @@ -548,7 +550,44 @@ def test_generate__different_move_generators__different_moves_returned(self): def test_generate__duplicate_generated_moves__unique_moves_returned(self): # Arrange move_generators = [self.single_move_generator, self.single_move_generator] - move_wrapper = ps.MoveWrapper(move_generators, [], []) + move_wrapper = ps.MoveWrapper(move_generators, [], [], []) + expected = [self.any_move] + + # Act + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + + def test_generate__different_move_non_extendable_generators__different_moves_returned(self): + # Arrange + move_non_extendable_generators = [self.single_move_generator, self.another_single_move_generator] + move_wrapper = ps.MoveWrapper([], move_non_extendable_generators, [], []) + expected = [self.any_move, self.any_other_move1] + + # Act + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + + def test_generate__duplicate_generated_non_extendable_moves__unique_moves_returned(self): + # Arrange + move_non_extendable_generators = [self.single_move_generator, self.single_move_generator] + move_wrapper = ps.MoveWrapper([], move_non_extendable_generators, [], []) + expected = [self.any_move] + + # Act + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + + def test_generate__duplicate_move_between_extendable_and_non_extendable_generators__unique_move_returned(self): + # Arrange + move_generators = [self.single_move_generator] + move_non_extendable_generators = [self.single_move_generator] + move_wrapper = ps.MoveWrapper(move_generators, move_non_extendable_generators, [], []) expected = [self.any_move] # Act @@ -561,7 +600,7 @@ def test_generate__single_move_extender__one_extended_move_returned(self): # Arrange move_generators = [self.single_move_generator] move_extenders = [self.single_move_extender] - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, []) + move_wrapper = ps.MoveWrapper(move_generators, [], move_extenders, []) expected = [self.any_move, self.any_extended_move] # Act @@ -574,7 +613,7 @@ def test_generate__multiple_move_extender__multiple_extended_move_returned(self) # Arrange move_generators = [self.single_move_generator] move_extenders = [self.multiple_move_extender] - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, []) + move_wrapper = ps.MoveWrapper(move_generators, [], move_extenders, []) expected = [self.any_move, self.any_extended_move, self.any_other_extended_move1, self.any_other_extended_move2] # Act @@ -587,7 +626,7 @@ def test_generate__different_move_extenders__different_extended_moves_returned(s # Arrange move_generators = [self.single_move_generator] move_extenders = [self.single_move_extender, self.another_single_move_extender] - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, []) + move_wrapper = ps.MoveWrapper(move_generators, [], move_extenders, []) expected = [self.any_move, self.any_extended_move, self.any_other_extended_move1] # Act @@ -600,7 +639,7 @@ def test_generate__duplicate_extended_moves__unique_moves_returned(self): # Arrange move_generators = [self.single_move_generator] move_extenders = [self.single_move_extender, self.single_move_extender] - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, []) + move_wrapper = ps.MoveWrapper(move_generators, [], move_extenders, []) expected = [self.any_move, self.any_extended_move] # Act @@ -613,7 +652,7 @@ def test_generate__mixed_extended_moves__unique_moves_returned(self): # Arrange move_generators = [self.single_move_generator, self.another_single_move_generator] move_extenders = [self.mixed_move_extender] - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, []) + move_wrapper = ps.MoveWrapper(move_generators, [], move_extenders, []) expected = [self.any_move, self.any_other_move1, self.any_extended_move, @@ -626,10 +665,54 @@ def test_generate__mixed_extended_moves__unique_moves_returned(self): # Assert self.assertListEqual(expected, actual) + def test_generate__multiple_non_extendable_moves__no_moves_extended(self): + # Arrange + move_non_extendable_generators = [self.single_move_generator, self.another_single_move_generator] + move_extenders = [self.mixed_move_extender] + move_wrapper = ps.MoveWrapper([], move_non_extendable_generators, move_extenders, []) + expected = [self.any_move, self.any_other_move1] + + # Act + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + + def test_generate__mixed_extendable_non_extendable_moves__only_extendable_moves_extended(self): + # Arrange + move_generators = [self.another_single_move_generator] # generates: any_other_move1, extends: any_other_extended_move1 + move_non_extendable_generators = [self.single_move_generator] # generates: any_move + move_extenders = [self.mixed_move_extender] + move_wrapper = ps.MoveWrapper(move_generators, move_non_extendable_generators, move_extenders, []) + expected = [self.any_move, + self.any_other_move1, + self.any_other_extended_move1] + + # Act + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + + def test_generate__move_is_extendable_and_non_extendable__move_is_extended(self): + # Arrange + move_generators = [self.single_move_generator] + move_non_extendable_generators = [self.single_move_generator] + move_extenders = [self.single_move_extender] + move_wrapper = ps.MoveWrapper(move_generators, move_non_extendable_generators, move_extenders, []) + expected = [self.any_move, + self.any_extended_move] + + # Act + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + def test_validate__validation_fail__false_returned(self): # Arrange move_validators = [self.fail_move_validator] - move_wrapper = ps.MoveWrapper([], [], move_validators) + move_wrapper = ps.MoveWrapper([], [], [], move_validators) # Act and assert self.assertFalse(move_wrapper.validate(self.any_move, self.any_diff)) @@ -637,7 +720,7 @@ def test_validate__validation_fail__false_returned(self): def test_validate__validation_succeed__true_returned(self): # Arrange move_validators = [self.success_move_validator] - move_wrapper = ps.MoveWrapper([], [], move_validators) + move_wrapper = ps.MoveWrapper([], [], [], move_validators) # Act and assert self.assertTrue(move_wrapper.validate(self.any_move, self.any_diff)) @@ -645,7 +728,7 @@ def test_validate__validation_succeed__true_returned(self): def test_validate__multiple_validators_last_fail___false_returned(self): # Arrange move_validators = [self.success_move_validator, self.success_move_validator, self.fail_move_validator] - move_wrapper = ps.MoveWrapper([], [], move_validators) + move_wrapper = ps.MoveWrapper([], [], [], move_validators) # Act and assert self.assertFalse(move_wrapper.validate(self.any_move, self.any_diff)) @@ -653,7 +736,7 @@ def test_validate__multiple_validators_last_fail___false_returned(self): def test_validate__multiple_validators_succeed___true_returned(self): # Arrange move_validators = [self.success_move_validator, self.success_move_validator, self.success_move_validator] - move_wrapper = ps.MoveWrapper([], [], move_validators) + move_wrapper = ps.MoveWrapper([], [], [], move_validators) # Act and assert self.assertTrue(move_wrapper.validate(self.any_move, self.any_diff)) @@ -662,7 +745,7 @@ def test_simulate__applies_move(self): # Arrange diff = Mock() diff.apply_move.side_effect = create_side_effect_dict({(str(self.any_move), ): self.any_diff}) - move_wrapper = ps.MoveWrapper(None, None, None) + move_wrapper = ps.MoveWrapper(None, None, None, None) # Act actual = move_wrapper.simulate(self.any_move, diff) @@ -1862,6 +1945,130 @@ def _get_no_critical_port_change_test_cases(self): def _apply_operations(self, config, operations): return jsonpatch.JsonPatch(operations).apply(config) +class TestTableLevelMoveGenerator(unittest.TestCase): + def setUp(self): + self.generator = ps.TableLevelMoveGenerator() + + def test_generate__tables_in_current_but_not_target__tables_deleted_moves(self): + self.verify(current = {"ExistingTable": {}, "NonExistingTable1": {}, "NonExistingTable2": {}}, + target = {"ExistingTable": {}}, + ex_ops = [{"op": "remove", 'path': '/NonExistingTable1'}, + {"op": "remove", 'path': '/NonExistingTable2'}]) + + def test_generate__tables_in_target_but_not_current__tables_added_moves(self): + self.verify(current = {"ExistingTable": {}}, + target = {"ExistingTable": {}, "NonExistingTable1": {}, "NonExistingTable2": {}}, + ex_ops = [{"op": "add", 'path': '/NonExistingTable1', 'value': {}}, + {"op": "add", 'path': '/NonExistingTable2', 'value': {}}]) + + def test_generate__all_tables_exist__no_moves(self): + self.verify(current = {"ExistingTable1": { "Key1": "Value1" }, "ExistingTable2": {}}, + target = {"ExistingTable1": {}, "ExistingTable2": { "Key2": "Value2" }}, + ex_ops = []) + + def test_generate__multiple_cases__deletion_precedes_addition(self): + self.verify(current = {"CommonTable": { "Key1": "Value1" }, "CurrentTable": {}}, + target = {"CommonTable": {}, "TargetTable": { "Key2": "Value2" }}, + ex_ops = [{"op": "remove", 'path': '/CurrentTable'}, + {"op": "add", 'path': '/TargetTable', 'value': { "Key2": "Value2" }}]) + + def verify(self, current, target, ex_ops): + # Arrange + diff = ps.Diff(current, target) + + # Act + moves = self.generator.generate(diff) + + # Assert + self.verify_moves(ex_ops, + moves) + + def verify_moves(self, ops, moves): + moves_ops = [list(move.patch)[0] for move in moves] + self.assertCountEqual(ops, moves_ops) + +class TestKeyLevelMoveGenerator(unittest.TestCase): + def setUp(self): + self.generator = ps.KeyLevelMoveGenerator() + + def test_generate__keys_in_current_but_not_target__keys_deleted_moves(self): + self.verify(current = { + "ExistingTable1": { + "ExistingKey11": "Value11", + "NonExistingKey12" : "Value12", + "NonExistingKey13" : "Value13" + }, + "NonExistingTable2": + { + "NonExistingKey21" : "Value21", + "NonExistingKey22" : "Value22" + } + }, + target = { + "ExistingTable1": { + "ExistingKey11": "Value11" + } + }, + ex_ops = [{"op": "remove", 'path': '/ExistingTable1/NonExistingKey12'}, + {"op": "remove", 'path': '/ExistingTable1/NonExistingKey13'}, + {"op": "remove", 'path': '/NonExistingTable2/NonExistingKey21'}, + {"op": "remove", 'path': '/NonExistingTable2/NonExistingKey22'}]) + + + def test_generate__single_key_in_current_but_not_target__whole_table_deleted(self): + self.verify(current = { "ExistingTable1": { "NonExistingKey11" : "Value11" }}, + target = {}, + ex_ops = [{"op": "remove", 'path': '/ExistingTable1'}]) + + def test_generate__keys_in_target_but_not_current__keys_added_moves(self): + self.verify(current = { + "ExistingTable1": { + "ExistingKey11": "Value11" + } + }, + target = { + "ExistingTable1": { + "ExistingKey11": "Value11", + "NonExistingKey12" : "Value12", + "NonExistingKey13" : "Value13" + }, + "NonExistingTable2": + { + "NonExistingKey21" : "Value21", + "NonExistingKey22" : "Value22" + } + }, + ex_ops = [{"op": "add", 'path': '/ExistingTable1/NonExistingKey12', "value": "Value12"}, + {"op": "add", 'path': '/ExistingTable1/NonExistingKey13', "value": "Value13"}, + {"op": "add", 'path': '/NonExistingTable2', "value": { "NonExistingKey21": "Value21" }}, + {"op": "add", 'path': '/NonExistingTable2', "value": { "NonExistingKey22": "Value22" }}]) + + def test_generate__all_keys_exist__no_moves(self): + self.verify(current = {"ExistingTable1": { "Key1": "Value1Current" }, "ExistingTable2": { "Key2": "Value2" }}, + target = {"ExistingTable1": { "Key1": "Value1Target" }, "ExistingTable2": { "Key2": {} } }, + ex_ops = []) + + def test_generate__multiple_cases__deletion_precedes_addition(self): + self.verify(current = {"AnyTable": { "CommonKey": "CurrentValue1", "CurrentKey": "CurrentValue2" }}, + target = {"AnyTable": { "CommonKey": "TargetValue1", "TargetKey": "TargetValue2" }}, + ex_ops = [{"op": "remove", 'path': '/AnyTable/CurrentKey'}, + {"op": "add", 'path': '/AnyTable/TargetKey', 'value': "TargetValue2"}]) + + def verify(self, current, target, ex_ops): + # Arrange + diff = ps.Diff(current, target) + + # Act + moves = self.generator.generate(diff) + + # Assert + self.verify_moves(ex_ops, + moves) + + def verify_moves(self, ops, moves): + moves_ops = [list(move.patch)[0] for move in moves] + self.assertCountEqual(ops, moves_ops) + class TestLowLevelMoveGenerator(unittest.TestCase): def setUp(self): path_addressing = PathAddressing() @@ -2823,6 +3030,7 @@ def verify(self, algo, algo_class): config_wrapper = ConfigWrapper() factory = ps.SortAlgorithmFactory(OperationWrapper(), config_wrapper, PathAddressing(config_wrapper)) expected_generators = [ps.LowLevelMoveGenerator] + expected_non_extendable_generators = [ps.KeyLevelMoveGenerator] expected_extenders = [ps.RequiredValueMoveExtender, ps.UpperLevelMoveExtender, ps.DeleteInsteadOfReplaceMoveExtender, @@ -2838,12 +3046,14 @@ def verify(self, algo, algo_class): # Act sorter = factory.create(algo) actual_generators = [type(item) for item in sorter.move_wrapper.move_generators] + actual_non_extendable_generators = [type(item) for item in sorter.move_wrapper.move_non_extendable_generators] actual_extenders = [type(item) for item in sorter.move_wrapper.move_extenders] actual_validators = [type(item) for item in sorter.move_wrapper.move_validators] # Assert self.assertIsInstance(sorter, algo_class) self.assertCountEqual(expected_generators, actual_generators) + self.assertCountEqual(expected_non_extendable_generators, actual_non_extendable_generators) self.assertCountEqual(expected_extenders, actual_extenders) self.assertCountEqual(expected_validator, actual_validators)