From a933b20d6fdbf9cb913c450c7bff37b2718a1796 Mon Sep 17 00:00:00 2001 From: Neetha John Date: Wed, 27 Oct 2021 09:27:18 -0700 Subject: [PATCH] [minigraph] Add tagged vlan member support for storage backend (#9045) Signed-off-by: Neetha John Why I did it Storage T0's have all vlan members as tagged How I did it Since currently minigraph does not have a unique way to identify if a vlan member is tagged/untagged and to ensure other scenarios are not broken, the logic used is to just update the vlan member type as 'tagged' when we determine that it is a storage backend device. This change will apply only to storage backend T0's since storage backend T1's will not have vlan member information How to verify it Updated the storage backend T0 testcases to check for tagged vlan members Added testcase to check if a T1 and backend T1 device generates an empty vlan member table Existing vlan member testcases are good enough for checking if any regression has been caused for regular T0's Build sonic_config_engine-1.0-py3-none-any.whl successfully --- src/sonic-config-engine/minigraph.py | 6 ++ src/sonic-config-engine/tests/test_cfggen.py | 91 ++++++++++++++++---- 2 files changed, 79 insertions(+), 18 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 3f0e437b61..6b55ab6687 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -1459,6 +1459,9 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw del results['PORTCHANNEL_INTERFACE'] is_storage_device = True results['VLAN_SUB_INTERFACE'] = vlan_sub_intfs + # storage backend T0 have all vlan members tagged + for vlan in vlan_members: + vlan_members[vlan]["tagging_mode"] = "tagged" elif current_device['type'] in backend_device_types and (resource_type is None or 'Storage' in resource_type): del results['INTERFACE'] del results['PORTCHANNEL_INTERFACE'] @@ -1484,6 +1487,9 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw sub_intf = pc_intf + VLAN_SUB_INTERFACE_SEPARATOR + VLAN_SUB_INTERFACE_VLAN_ID vlan_sub_intfs[sub_intf] = {"admin_status" : "up"} results['VLAN_SUB_INTERFACE'] = vlan_sub_intfs + # storage backend T0 have all vlan members tagged + for vlan in vlan_members: + vlan_members[vlan]["tagging_mode"] = "tagged" elif resource_type is not None and 'Storage' in resource_type: is_storage_device = True diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 0a0dd54a76..b2f8532b76 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -8,6 +8,8 @@ from unittest import TestCase TOR_ROUTER = 'ToRRouter' BACKEND_TOR_ROUTER = 'BackEndToRRouter' +LEAF_ROUTER = 'LeafRouter' +BACKEND_LEAF_ROUTER = 'BackEndLeafRouter' class TestCfgGen(TestCase): @@ -124,17 +126,29 @@ class TestCfgGen(TestCase): def test_var_json_data(self, **kwargs): graph_file = kwargs.get('graph_file', self.sample_graph_simple) + tag_mode = kwargs.get('tag_mode', 'untagged') argument = '-m "' + graph_file + '" -p "' + self.port_config + '" --var-json VLAN_MEMBER' output = self.run_script(argument) - self.assertEqual( - utils.to_dict(output.strip()), - utils.to_dict( - '{\n "Vlan1000|Ethernet8": {\n "tagging_mode": "untagged"\n },' - ' \n "Vlan2000|Ethernet12": {\n "tagging_mode": "tagged"\n },' - ' \n "Vlan2001|Ethernet12": {\n "tagging_mode": "tagged"\n },' - ' \n "Vlan2020|Ethernet12": {\n "tagging_mode": "tagged"\n }\n}' + if tag_mode == "tagged": + self.assertEqual( + utils.to_dict(output.strip()), + utils.to_dict( + '{\n "Vlan1000|Ethernet8": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan2000|Ethernet12": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan2001|Ethernet12": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan2020|Ethernet12": {\n "tagging_mode": "tagged"\n }\n}' + ) + ) + else: + self.assertEqual( + utils.to_dict(output.strip()), + utils.to_dict( + '{\n "Vlan1000|Ethernet8": {\n "tagging_mode": "untagged"\n },' + ' \n "Vlan2000|Ethernet12": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan2001|Ethernet12": {\n "tagging_mode": "tagged"\n },' + ' \n "Vlan2020|Ethernet12": {\n "tagging_mode": "tagged"\n }\n}' + ) ) - ) def test_read_yaml(self): argument = '-v yml_item -y ' + os.path.join(self.test_dir, 'test.yml') @@ -226,17 +240,29 @@ class TestCfgGen(TestCase): def test_minigraph_vlan_members(self, **kwargs): graph_file = kwargs.get('graph_file', self.sample_graph_simple) + tag_mode = kwargs.get('tag_mode', 'untagged') argument = '-m "' + graph_file + '" -p "' + self.port_config + '" -v VLAN_MEMBER' output = self.run_script(argument) - self.assertEqual( - utils.to_dict(output.strip()), - utils.to_dict( - "{('Vlan2000', 'Ethernet12'): {'tagging_mode': 'tagged'}, " - "('Vlan1000', 'Ethernet8'): {'tagging_mode': 'untagged'}, " - "('Vlan2020', 'Ethernet12'): {'tagging_mode': 'tagged'}, " - "('Vlan2001', 'Ethernet12'): {'tagging_mode': 'tagged'}}" + if tag_mode == "tagged": + self.assertEqual( + utils.to_dict(output.strip()), + utils.to_dict( + "{('Vlan2000', 'Ethernet12'): {'tagging_mode': 'tagged'}, " + "('Vlan1000', 'Ethernet8'): {'tagging_mode': 'tagged'}, " + "('Vlan2020', 'Ethernet12'): {'tagging_mode': 'tagged'}, " + "('Vlan2001', 'Ethernet12'): {'tagging_mode': 'tagged'}}" + ) + ) + else: + self.assertEqual( + utils.to_dict(output.strip()), + utils.to_dict( + "{('Vlan2000', 'Ethernet12'): {'tagging_mode': 'tagged'}, " + "('Vlan1000', 'Ethernet8'): {'tagging_mode': 'untagged'}, " + "('Vlan2020', 'Ethernet12'): {'tagging_mode': 'tagged'}, " + "('Vlan2001', 'Ethernet12'): {'tagging_mode': 'tagged'}}" + ) ) - ) def test_minigraph_vlan_interfaces(self, **kwargs): graph_file = kwargs.get('graph_file', self.sample_graph_simple) @@ -606,6 +632,33 @@ class TestCfgGen(TestCase): def test_minigraph_sub_port_intf_sub(self, check_stderr=True): self.verify_sub_intf(graph_file=self.sample_subintf_graph, check_stderr=check_stderr) + def test_minigraph_no_vlan_member(self, check_stderr=True): + self.verify_no_vlan_member() + + def test_minigraph_sub_port_no_vlan_member(self, check_stderr=True): + try: + print('\n Change device type to %s' % (BACKEND_LEAF_ROUTER)) + if check_stderr: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (LEAF_ROUTER, BACKEND_LEAF_ROUTER, self.sample_graph), stderr=subprocess.STDOUT, shell=True) + else: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (LEAF_ROUTER, BACKEND_LEAF_ROUTER, self.sample_graph), shell=True) + + self.test_jinja_expression(self.sample_graph, BACKEND_LEAF_ROUTER) + self.verify_no_vlan_member() + finally: + print('\n Change device type back to %s' % (LEAF_ROUTER)) + if check_stderr: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (BACKEND_LEAF_ROUTER, LEAF_ROUTER, self.sample_graph), stderr=subprocess.STDOUT, shell=True) + else: + output = subprocess.check_output("sed -i \'s/%s/%s/g\' %s" % (BACKEND_LEAF_ROUTER, LEAF_ROUTER, self.sample_graph), shell=True) + + self.test_jinja_expression(self.sample_graph, LEAF_ROUTER) + + def verify_no_vlan_member(self): + argument = '-m "' + self.sample_graph + '" -p "' + self.port_config + '" -v "VLAN_MEMBER"' + output = self.run_script(argument) + self.assertEqual(output.strip(), "{}") + def verify_sub_intf(self, **kwargs): graph_file = kwargs.get('graph_file', self.sample_graph_simple) check_stderr = kwargs.get('check_stderr', True) @@ -629,9 +682,7 @@ class TestCfgGen(TestCase): self.assertEqual(output.strip(), "") # All the other tables stay unchanged - self.test_var_json_data(graph_file=graph_file) self.test_minigraph_vlans(graph_file=graph_file) - self.test_minigraph_vlan_members(graph_file=graph_file) self.test_minigraph_vlan_interfaces(graph_file=graph_file) self.test_minigraph_portchannels(graph_file=graph_file) self.test_minigraph_ethernet_interfaces(graph_file=graph_file) @@ -666,6 +717,10 @@ class TestCfgGen(TestCase): ) ) + # VLAN_MEMBER table should have all tagged members + self.test_var_json_data(graph_file=graph_file, tag_mode='tagged') + self.test_minigraph_vlan_members(graph_file=graph_file, tag_mode='tagged') + finally: print('\n Change device type back to %s' % (TOR_ROUTER)) if check_stderr: