Skip to content

Commit

Permalink
[minigraph] Add tagged vlan member support for storage backend (#9045)
Browse files Browse the repository at this point in the history 
Signed-off-by: Neetha John <[email protected]>

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
  • Loading branch information
@neethajohn @qiluo-msft
neethajohn authored and qiluo-msft committed Oct 29, 2021
1 parent eb74455 commit a933b20
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 18 deletions.
6 changes: 6 additions & 0 deletions src/sonic-config-engine/minigraph.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand All @@ -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

Expand Down
91 changes: 73 additions & 18 deletions src/sonic-config-engine/tests/test_cfggen.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

TOR_ROUTER = 'ToRRouter'
BACKEND_TOR_ROUTER = 'BackEndToRRouter'
LEAF_ROUTER = 'LeafRouter'
BACKEND_LEAF_ROUTER = 'BackEndLeafRouter'

class TestCfgGen(TestCase):

Expand Down Expand Up @@ -124,17 +126,29 @@ def test_additional_json_data_level2_key(self):

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')
Expand Down Expand Up @@ -226,17 +240,29 @@ def test_minigraph_vlans(self, **kwargs):

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)
Expand Down Expand Up @@ -606,6 +632,33 @@ def test_minigraph_sub_port_intf_resource_type(self, check_stderr=True):
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)
Expand All @@ -629,9 +682,7 @@ def verify_sub_intf(self, **kwargs):
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)
Expand Down Expand Up @@ -666,6 +717,10 @@ def verify_sub_intf(self, **kwargs):
)
)

# 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:
Expand Down

0 comments on commit a933b20

Please sign in to comment.