From 91fc36860c34d75c779f14ac93e8173a34b3de68 Mon Sep 17 00:00:00 2001 From: ghooo Date: Mon, 28 Mar 2022 10:48:41 -0700 Subject: [PATCH 1/4] [GCU] Adding non-extendable generators --- generic_config_updater/patch_sorter.py | 72 +- .../files/patch_sorter_test_success.json | 868 +++++++----------- .../patch_sorter_test.py | 30 +- 3 files changed, 382 insertions(+), 588 deletions(-) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index e89c542e73..ca75c4baa4 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -290,29 +290,40 @@ 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): 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 +339,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 +937,37 @@ def validate(self, move, diff): return True +class TableLevelMoveGenerator: + 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: + 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): + 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 +1454,7 @@ def __init__(self, operation_wrapper, config_wrapper, path_addressing): def create(self, algorithm=Algorithm.DFS): move_generators = [LowLevelMoveGenerator(self.path_addressing)] + move_non_extendable_generators = [TableLevelMoveGenerator(), KeyLevelMoveGenerator()] move_extenders = [RequiredValueMoveExtender(self.path_addressing, self.operation_wrapper), UpperLevelMoveExtender(), DeleteInsteadOfReplaceMoveExtender(), @@ -1419,7 +1467,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 894f68896c..cfeacaeab5 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" - } ] ] }, @@ -418,130 +388,40 @@ "op": "add", "path": "/ACL_TABLE", "value": { + "DATAACL": { + "policy_desc": "DATAACL", + "ports": [ + "Ethernet4" + ], + "stage": "ingress", + "type": "L3" + }, + "EVERFLOW": { + "policy_desc": "EVERFLOW", + "ports": [ + "Ethernet8" + ], + "stage": "ingress", + "type": "MIRROR" + }, + "EVERFLOWV6": { + "policy_desc": "EVERFLOWV6", + "ports": [ + "Ethernet4", + "Ethernet8" + ], + "stage": "ingress", + "type": "MIRRORV6" + }, "NO-NSW-PACL-V4": { + "policy_desc": "NO-NSW-PACL-V4", + "ports": [ + "Ethernet0" + ], "type": "L3" } } } - ], - [ - { - "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": { - "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": { - "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": { - "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" - } ] ] }, @@ -969,22 +849,31 @@ "value": { "Ethernet0": { "alias": "Eth1/1", + "description": "", "lanes": "65", + "speed": "10000" + }, + "Ethernet1": { + "alias": "Eth1/2", + "description": "", + "lanes": "66", + "speed": "10000" + }, + "Ethernet2": { + "alias": "Eth1/3", + "description": "", + "lanes": "67", + "speed": "10000" + }, + "Ethernet3": { + "alias": "Eth1/4", "description": "", + "lanes": "68", "speed": "10000" } } } ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0" - ] - } - ], [ { "op": "add", @@ -992,6 +881,15 @@ "value": { "Vlan100|Ethernet0": { "tagging_mode": "untagged" + }, + "Vlan100|Ethernet1": { + "tagging_mode": "untagged" + }, + "Vlan100|Ethernet2": { + "tagging_mode": "untagged" + }, + "Vlan100|Ethernet3": { + "tagging_mode": "untagged" } } } @@ -999,41 +897,17 @@ [ { "op": "add", - "path": "/PORT/Ethernet3", - "value": { - "alias": "Eth1/4", - "lanes": "68", - "description": "", - "speed": "10000" - } + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] } ], [ { "op": "add", "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1", - "value": "Ethernet3" - } - ], - [ - { - "op": "add", - "path": "/VLAN_MEMBER/Vlan100|Ethernet3", - "value": { - "tagging_mode": "untagged" - } - } - ], - [ - { - "op": "add", - "path": "/PORT/Ethernet2", - "value": { - "alias": "Eth1/3", - "lanes": "67", - "description": "", - "speed": "10000" - } + "value": "Ethernet1" } ], [ @@ -1046,50 +920,8 @@ [ { "op": "add", - "path": "/VLAN_MEMBER/Vlan100|Ethernet2", - "value": { - "tagging_mode": "untagged" - } - } - ], - [ - { - "op": "add", - "path": "/PORT/Ethernet1", - "value": { - "alias": "Eth1/2", - "lanes": "66", - "description": "", - "speed": "10000" - } - } - ], - [ - { - "op": "add", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1", - "value": "Ethernet1" - } - ], - [ - { - "op": "replace", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", - "value": [ - "Ethernet0", - "Ethernet1", - "Ethernet2", - "Ethernet3" - ] - } - ], - [ - { - "op": "add", - "path": "/VLAN_MEMBER/Vlan100|Ethernet1", - "value": { - "tagging_mode": "untagged" - } + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/3", + "value": "Ethernet3" } ] ] @@ -1220,6 +1052,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 +1133,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 +1142,7 @@ [ { "op": "remove", - "path": "/VLAN_MEMBER/Vlan100|Ethernet2" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" } ], [ @@ -1322,7 +1154,7 @@ [ { "op": "remove", - "path": "/VLAN_MEMBER/Vlan100|Ethernet3" + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports/1" } ], [ @@ -1358,6 +1190,17 @@ "path": "/ACL_TABLE" } ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER", + "value": { + "Vlan100|Ethernet0": { + "tagging_mode": "untagged" + } + } + } + ], [ { "op": "add", @@ -1369,17 +1212,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 +1246,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 +1263,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" + ] + } ] ] }, @@ -1614,23 +1478,11 @@ "op": "add", "path": "/VLAN_INTERFACE", "value": { - "Vlan1000": {} + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {}, + "Vlan1000|fc02:1000::1/64": {} } } - ], - [ - { - "op": "add", - "path": "/VLAN_INTERFACE/Vlan1000|fc02:1000::1~164", - "value": {} - } - ], - [ - { - "op": "add", - "path": "/VLAN_INTERFACE/Vlan1000|192.168.0.1~121", - "value": {} - } ] ] }, @@ -2085,18 +1937,6 @@ } ], "expected_changes": [ - [ - { - "op": "remove", - "path": "/PORT/Ethernet0/alias" - } - ], - [ - { - "op": "remove", - "path": "/PORT/Ethernet0/description" - } - ], [ { "op": "remove", @@ -2154,18 +1994,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 +2206,6 @@ } ], "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 +2496,17 @@ "path": "/ACL_TABLE" } ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER", + "value": { + "Vlan100|Ethernet0": { + "tagging_mode": "untagged" + } + } + } + ], [ { "op": "add", @@ -2763,17 +2518,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 +2552,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 +2569,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 +4043,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 +4500,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 +4507,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 12d76c5283..16751be773 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -502,7 +502,7 @@ def test_ctor__assigns_values_correctly(self): move_validators = Mock() # Act - move_wrapper = ps.MoveWrapper(move_generators, move_extenders, move_validators) + move_wrapper = ps.MoveWrapper(move_generators, [], move_extenders, move_validators) # Assert self.assertIs(move_generators, move_wrapper.move_generators) @@ -512,7 +512,7 @@ def test_ctor__assigns_values_correctly(self): 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 +524,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 +536,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 +548,7 @@ 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 @@ -561,7 +561,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 +574,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 +587,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 +600,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 +613,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, @@ -629,7 +629,7 @@ def test_generate__mixed_extended_moves__unique_moves_returned(self): 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 +637,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 +645,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 +653,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 +662,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) From 7318240fcedad58f179c93ad17f58033ce94e04d Mon Sep 17 00:00:00 2001 From: ghooo Date: Tue, 29 Mar 2022 07:27:57 -0700 Subject: [PATCH 2/4] just saving --- .../patch_sorter_test.py | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 16751be773..2661e264b3 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -498,14 +498,16 @@ 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) @@ -557,6 +559,30 @@ def test_generate__duplicate_generated_moves__unique_moves_returned(self): # Assert self.assertListEqual(expected, actual) + def test_generate__different_move_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_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__single_move_extender__one_extended_move_returned(self): # Arrange move_generators = [self.single_move_generator] From 229a9cc8d22c1da979cb7965a6c58da31bf20e01 Mon Sep 17 00:00:00 2001 From: ghooo Date: Tue, 29 Mar 2022 10:10:41 -0700 Subject: [PATCH 3/4] adding missing unit-tests --- generic_config_updater/patch_sorter.py | 38 ++++ .../patch_sorter_test.py | 183 +++++++++++++++++- 2 files changed, 219 insertions(+), 2 deletions(-) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index ca75c4baa4..3310301e62 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -297,6 +297,18 @@ def __init__(self, move_generators, move_non_extendable_generators, move_extende 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([]) @@ -938,6 +950,18 @@ 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): @@ -953,6 +977,20 @@ def _get_non_existing_tables_tokens(self, config1, 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): diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 2661e264b3..4224d74a08 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -559,7 +559,7 @@ def test_generate__duplicate_generated_moves__unique_moves_returned(self): # Assert self.assertListEqual(expected, actual) - def test_generate__different_move_generators__different_moves_returned(self): + 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, [], []) @@ -571,7 +571,7 @@ def test_generate__different_move_generators__different_moves_returned(self): # Assert self.assertListEqual(expected, actual) - def test_generate__duplicate_generated_moves__unique_moves_returned(self): + 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, [], []) @@ -583,6 +583,19 @@ def test_generate__duplicate_generated_moves__unique_moves_returned(self): # 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 + actual = list(move_wrapper.generate(self.any_diff)) + + # Assert + self.assertListEqual(expected, actual) + def test_generate__single_move_extender__one_extended_move_returned(self): # Arrange move_generators = [self.single_move_generator] @@ -652,6 +665,50 @@ 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] @@ -1888,6 +1945,124 @@ 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__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() @@ -2849,6 +3024,8 @@ 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.TableLevelMoveGenerator, + ps.KeyLevelMoveGenerator] expected_extenders = [ps.RequiredValueMoveExtender, ps.UpperLevelMoveExtender, ps.DeleteInsteadOfReplaceMoveExtender, @@ -2864,12 +3041,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) From 21b6235e927210ad434057fa3a63c6ba000dc840 Mon Sep 17 00:00:00 2001 From: ghooo Date: Tue, 29 Mar 2022 11:58:57 -0700 Subject: [PATCH 4/4] remove TableLevelMoveGenerator --- generic_config_updater/patch_sorter.py | 10 +- .../files/patch_sorter_test_success.json | 198 +++++++++++++----- .../patch_sorter_test.py | 9 +- 3 files changed, 156 insertions(+), 61 deletions(-) diff --git a/generic_config_updater/patch_sorter.py b/generic_config_updater/patch_sorter.py index 3310301e62..97db21b24e 100644 --- a/generic_config_updater/patch_sorter.py +++ b/generic_config_updater/patch_sorter.py @@ -994,7 +994,12 @@ class KeyLevelMoveGenerator: 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): - yield JsonMove(diff, OperationType.REMOVE, tokens) + 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): @@ -1492,7 +1497,8 @@ def __init__(self, operation_wrapper, config_wrapper, path_addressing): def create(self, algorithm=Algorithm.DFS): move_generators = [LowLevelMoveGenerator(self.path_addressing)] - move_non_extendable_generators = [TableLevelMoveGenerator(), KeyLevelMoveGenerator()] + # 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(), 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 cfeacaeab5..c134b0b5e2 100644 --- a/tests/generic_config_updater/files/patch_sorter_test_success.json +++ b/tests/generic_config_updater/files/patch_sorter_test_success.json @@ -388,40 +388,58 @@ "op": "add", "path": "/ACL_TABLE", "value": { - "DATAACL": { - "policy_desc": "DATAACL", - "ports": [ - "Ethernet4" - ], - "stage": "ingress", - "type": "L3" - }, - "EVERFLOW": { - "policy_desc": "EVERFLOW", - "ports": [ - "Ethernet8" - ], - "stage": "ingress", - "type": "MIRROR" - }, - "EVERFLOWV6": { - "policy_desc": "EVERFLOWV6", - "ports": [ - "Ethernet4", - "Ethernet8" - ], - "stage": "ingress", - "type": "MIRRORV6" - }, "NO-NSW-PACL-V4": { + "type": "L3", "policy_desc": "NO-NSW-PACL-V4", "ports": [ "Ethernet0" - ], - "type": "L3" + ] } } } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/DATAACL", + "value": { + "policy_desc": "DATAACL", + "ports": [ + "Ethernet4" + ], + "stage": "ingress", + "type": "L3" + } + } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/EVERFLOW", + "value": { + "policy_desc": "EVERFLOW", + "ports": [ + "Ethernet8" + ], + "stage": "ingress", + "type": "MIRROR" + } + } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE/EVERFLOWV6", + "value": { + "policy_desc": "EVERFLOWV6", + "ports": [ + "Ethernet4", + "Ethernet8" + ], + "stage": "ingress", + "type": "MIRRORV6" + } + } ] ] }, @@ -849,31 +867,49 @@ "value": { "Ethernet0": { "alias": "Eth1/1", - "description": "", "lanes": "65", - "speed": "10000" - }, - "Ethernet1": { - "alias": "Eth1/2", "description": "", - "lanes": "66", - "speed": "10000" - }, - "Ethernet2": { - "alias": "Eth1/3", - "description": "", - "lanes": "67", - "speed": "10000" - }, - "Ethernet3": { - "alias": "Eth1/4", - "description": "", - "lanes": "68", "speed": "10000" } } } ], + [ + { + "op": "add", + "path": "/PORT/Ethernet3", + "value": { + "alias": "Eth1/4", + "lanes": "68", + "description": "", + "speed": "10000" + } + } + ], + [ + { + "op": "add", + "path": "/PORT/Ethernet2", + "value": { + "alias": "Eth1/3", + "lanes": "67", + "description": "", + "speed": "10000" + } + } + ], + [ + { + "op": "add", + "path": "/PORT/Ethernet1", + "value": { + "alias": "Eth1/2", + "lanes": "66", + "description": "", + "speed": "10000" + } + } + ], [ { "op": "add", @@ -881,19 +917,37 @@ "value": { "Vlan100|Ethernet0": { "tagging_mode": "untagged" - }, - "Vlan100|Ethernet1": { - "tagging_mode": "untagged" - }, - "Vlan100|Ethernet2": { - "tagging_mode": "untagged" - }, - "Vlan100|Ethernet3": { - "tagging_mode": "untagged" } } } ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER/Vlan100|Ethernet1", + "value": { + "tagging_mode": "untagged" + } + } + ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER/Vlan100|Ethernet3", + "value": { + "tagging_mode": "untagged" + } + } + ], + [ + { + "op": "add", + "path": "/VLAN_MEMBER/Vlan100|Ethernet2", + "value": { + "tagging_mode": "untagged" + } + } + ], [ { "op": "add", @@ -1478,11 +1532,23 @@ "op": "add", "path": "/VLAN_INTERFACE", "value": { - "Vlan1000": {}, - "Vlan1000|192.168.0.1/21": {}, - "Vlan1000|fc02:1000::1/64": {} + "Vlan1000": {} } } + ], + [ + { + "op": "add", + "path": "/VLAN_INTERFACE/Vlan1000|fc02:1000::1~164", + "value": {} + } + ], + [ + { + "op": "add", + "path": "/VLAN_INTERFACE/Vlan1000|192.168.0.1~121", + "value": {} + } ] ] }, @@ -2206,6 +2272,24 @@ } ], "expected_changes": [ + [ + { + "op": "remove", + "path": "/ACL_TABLE/NO-NSW-PACL-V4" + } + ], + [ + { + "op": "remove", + "path": "/ACL_TABLE/DATAACL" + } + ], + [ + { + "op": "remove", + "path": "/ACL_TABLE/EVERFLOW" + } + ], [ { "op": "remove", diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 4224d74a08..5530de7cef 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -2014,6 +2014,12 @@ def test_generate__keys_in_current_but_not_target__keys_deleted_moves(self): {"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": { @@ -3024,8 +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.TableLevelMoveGenerator, - ps.KeyLevelMoveGenerator] + expected_non_extendable_generators = [ps.KeyLevelMoveGenerator] expected_extenders = [ps.RequiredValueMoveExtender, ps.UpperLevelMoveExtender, ps.DeleteInsteadOfReplaceMoveExtender,