From d0f16c0d79b64ce9adc8535e370ed257a6dc3940 Mon Sep 17 00:00:00 2001 From: Lawrence Lee Date: Tue, 10 Nov 2020 15:06:35 -0800 Subject: [PATCH] Make backend device checking more robust (#5730) Treat devices that are ToRRouters (ToRRouters and BackEndToRRouters) the same when rendering templates Except for BackEndToRRouters belonging to a storage cluster, since these devices have extra sub-interfaces created Treat devices that are LeafRouters (LeafRouters and BackEndLeafRouters) the same when rendering templates Signed-off-by: Lawrence Lee --- .../Arista-7060CX-32S-Q32/sai.profile.j2 | 2 +- .../Celestica-DX010-C32/sai.profile.j2 | 2 +- .../Force10-S6100/sai.profile.j2 | 2 +- .../DellEMC-Z9264f-Q64/sai.profile.j2 | 2 +- .../db98cx8580_16cd/buffers_config.j2 | 6 +++--- .../db98cx8580_32cd/buffers_config.j2 | 6 +++--- .../db98cx8580_16cd/buffers_config.j2 | 6 +++--- .../db98cx8580_32cd/buffers_config.j2 | 6 +++--- dockers/docker-orchagent/switch.json.j2 | 6 +++--- ...docker-router-advertiser.supervisord.conf.j2 | 2 +- files/build_templates/buffers_config.j2 | 6 +++--- files/build_templates/qos_config.j2 | 4 ++-- src/sonic-config-engine/minigraph.py | 17 ++++++++++++++--- .../tests/simple-sample-graph-case.xml | 1 + .../tests/simple-sample-graph.xml | 1 + .../tests/test_minigraph_case.py | 5 +++++ 16 files changed, 46 insertions(+), 28 deletions(-) diff --git a/device/arista/x86_64-arista_7060_cx32s/Arista-7060CX-32S-Q32/sai.profile.j2 b/device/arista/x86_64-arista_7060_cx32s/Arista-7060CX-32S-Q32/sai.profile.j2 index e2ad59262c..5a50185247 100644 --- a/device/arista/x86_64-arista_7060_cx32s/Arista-7060CX-32S-Q32/sai.profile.j2 +++ b/device/arista/x86_64-arista_7060_cx32s/Arista-7060CX-32S-Q32/sai.profile.j2 @@ -1,7 +1,7 @@ {# Get sai.profile based on switch_role #} {%- if DEVICE_METADATA is defined -%} {%- set switch_role = DEVICE_METADATA['localhost']['type'] -%} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {% set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th-a7060-cx32s-32x40G-t0.config.bcm' -%} {%- else %} {%- set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th-a7060-cx32s-32x40G-t1.config.bcm' -%} diff --git a/device/celestica/x86_64-cel_seastone-r0/Celestica-DX010-C32/sai.profile.j2 b/device/celestica/x86_64-cel_seastone-r0/Celestica-DX010-C32/sai.profile.j2 index 21bf6cd6e7..5feba49569 100644 --- a/device/celestica/x86_64-cel_seastone-r0/Celestica-DX010-C32/sai.profile.j2 +++ b/device/celestica/x86_64-cel_seastone-r0/Celestica-DX010-C32/sai.profile.j2 @@ -1,7 +1,7 @@ {# Get sai.profile based on switch_role #} {%- if DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined -%} {%- set switch_role = DEVICE_METADATA['localhost']['type'] -%} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {% set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th-seastone-dx010-32x100G-t0.config.bcm' -%} {%- else %} {% set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th-seastone-dx010-32x100G-t1.config.bcm' -%} diff --git a/device/dell/x86_64-dell_s6100_c2538-r0/Force10-S6100/sai.profile.j2 b/device/dell/x86_64-dell_s6100_c2538-r0/Force10-S6100/sai.profile.j2 index d2c7942770..44992f92f4 100644 --- a/device/dell/x86_64-dell_s6100_c2538-r0/Force10-S6100/sai.profile.j2 +++ b/device/dell/x86_64-dell_s6100_c2538-r0/Force10-S6100/sai.profile.j2 @@ -1,7 +1,7 @@ {# Get sai.profile based on switch_role #} {%- if DEVICE_METADATA is defined -%} {%- set switch_role = DEVICE_METADATA['localhost']['type'] -%} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {% set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th-s6100-64x40G-t0.config.bcm' -%} {%- else %} {%- set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th-s6100-64x40G-t1.config.bcm' -%} diff --git a/device/dell/x86_64-dellemc_z9264f_c3538-r0/DellEMC-Z9264f-Q64/sai.profile.j2 b/device/dell/x86_64-dellemc_z9264f_c3538-r0/DellEMC-Z9264f-Q64/sai.profile.j2 index bfb71fa71e..7c6d12b4b1 100644 --- a/device/dell/x86_64-dellemc_z9264f_c3538-r0/DellEMC-Z9264f-Q64/sai.profile.j2 +++ b/device/dell/x86_64-dellemc_z9264f_c3538-r0/DellEMC-Z9264f-Q64/sai.profile.j2 @@ -1,7 +1,7 @@ {# Get sai.profile based on switch_role #} {%- if DEVICE_METADATA is defined -%} {%- set switch_role = DEVICE_METADATA['localhost']['type'] -%} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {% set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th2-z9264f-64x40G-t0.config.bcm' -%} {%- else %} {%- set sai_profile_contents = 'SAI_INIT_CONFIG_FILE=/usr/share/sonic/hwsku/th2-z9264f-64x40G-t1.config.bcm' -%} diff --git a/device/marvell/arm64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 b/device/marvell/arm64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 index a5212d979f..5431fbe72c 100644 --- a/device/marvell/arm64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 +++ b/device/marvell/arm64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 @@ -9,9 +9,9 @@ def {# Determine device topology and filename postfix #} {%- if DEVICE_METADATA is defined %} {%- set switch_role = DEVICE_METADATA['localhost']['type'] %} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {%- set filename_postfix = 't0' %} -{%- elif switch_role.lower() == 'leafrouter' %} +{%- elif 'leafrouter' in switch_role.lower() %} {%- set filename_postfix = 't1' %} {%- else %} {%- set filename_postfix = set_default_topology() %} @@ -62,7 +62,7 @@ def {%- if cable_len -%} {{ cable_len.0 }} {%- else %} - {%- if switch_role.lower() == 'torrouter' %} + {%- if 'torrouter' in switch_role.lower() %} {%- for local_port in VLAN_MEMBER %} {%- if local_port[1] == port_name %} {%- set roles3 = switch_role + '_' + 'server' %} diff --git a/device/marvell/arm64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 b/device/marvell/arm64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 index a5212d979f..5431fbe72c 100644 --- a/device/marvell/arm64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 +++ b/device/marvell/arm64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 @@ -9,9 +9,9 @@ def {# Determine device topology and filename postfix #} {%- if DEVICE_METADATA is defined %} {%- set switch_role = DEVICE_METADATA['localhost']['type'] %} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {%- set filename_postfix = 't0' %} -{%- elif switch_role.lower() == 'leafrouter' %} +{%- elif 'leafrouter' in switch_role.lower() %} {%- set filename_postfix = 't1' %} {%- else %} {%- set filename_postfix = set_default_topology() %} @@ -62,7 +62,7 @@ def {%- if cable_len -%} {{ cable_len.0 }} {%- else %} - {%- if switch_role.lower() == 'torrouter' %} + {%- if 'torrouter' in switch_role.lower() %} {%- for local_port in VLAN_MEMBER %} {%- if local_port[1] == port_name %} {%- set roles3 = switch_role + '_' + 'server' %} diff --git a/device/marvell/x86_64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 b/device/marvell/x86_64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 index a5212d979f..5431fbe72c 100644 --- a/device/marvell/x86_64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 +++ b/device/marvell/x86_64-marvell_db98cx8580_16cd-r0/db98cx8580_16cd/buffers_config.j2 @@ -9,9 +9,9 @@ def {# Determine device topology and filename postfix #} {%- if DEVICE_METADATA is defined %} {%- set switch_role = DEVICE_METADATA['localhost']['type'] %} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {%- set filename_postfix = 't0' %} -{%- elif switch_role.lower() == 'leafrouter' %} +{%- elif 'leafrouter' in switch_role.lower() %} {%- set filename_postfix = 't1' %} {%- else %} {%- set filename_postfix = set_default_topology() %} @@ -62,7 +62,7 @@ def {%- if cable_len -%} {{ cable_len.0 }} {%- else %} - {%- if switch_role.lower() == 'torrouter' %} + {%- if 'torrouter' in switch_role.lower() %} {%- for local_port in VLAN_MEMBER %} {%- if local_port[1] == port_name %} {%- set roles3 = switch_role + '_' + 'server' %} diff --git a/device/marvell/x86_64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 b/device/marvell/x86_64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 index a5212d979f..5431fbe72c 100644 --- a/device/marvell/x86_64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 +++ b/device/marvell/x86_64-marvell_db98cx8580_32cd-r0/db98cx8580_32cd/buffers_config.j2 @@ -9,9 +9,9 @@ def {# Determine device topology and filename postfix #} {%- if DEVICE_METADATA is defined %} {%- set switch_role = DEVICE_METADATA['localhost']['type'] %} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() %} {%- set filename_postfix = 't0' %} -{%- elif switch_role.lower() == 'leafrouter' %} +{%- elif 'leafrouter' in switch_role.lower() %} {%- set filename_postfix = 't1' %} {%- else %} {%- set filename_postfix = set_default_topology() %} @@ -62,7 +62,7 @@ def {%- if cable_len -%} {{ cable_len.0 }} {%- else %} - {%- if switch_role.lower() == 'torrouter' %} + {%- if 'torrouter' in switch_role.lower() %} {%- for local_port in VLAN_MEMBER %} {%- if local_port[1] == port_name %} {%- set roles3 = switch_role + '_' + 'server' %} diff --git a/dockers/docker-orchagent/switch.json.j2 b/dockers/docker-orchagent/switch.json.j2 index c359dd8052..4d2a0be1b0 100644 --- a/dockers/docker-orchagent/switch.json.j2 +++ b/dockers/docker-orchagent/switch.json.j2 @@ -3,11 +3,11 @@ {% set hash_seed = 0 %} {% set hash_seed_offset = 0 %} {% if DEVICE_METADATA.localhost.type %} -{% if DEVICE_METADATA.localhost.type == "ToRRouter" %} +{% if "ToRRouter" in DEVICE_METADATA.localhost.type %} {% set hash_seed = 0 %} -{% elif DEVICE_METADATA.localhost.type == "LeafRouter" %} +{% elif "LeafRouter" in DEVICE_METADATA.localhost.type %} {% set hash_seed = 10 %} -{% elif DEVICE_METADATA.localhost.type == "SpineRouter" %} +{% elif "SpineRouter" in DEVICE_METADATA.localhost.type %} {% set hash_seed = 25 %} {% endif %} {% endif %} diff --git a/dockers/docker-router-advertiser/docker-router-advertiser.supervisord.conf.j2 b/dockers/docker-router-advertiser/docker-router-advertiser.supervisord.conf.j2 index 9e75941170..5a6416b0ff 100644 --- a/dockers/docker-router-advertiser/docker-router-advertiser.supervisord.conf.j2 +++ b/dockers/docker-router-advertiser/docker-router-advertiser.supervisord.conf.j2 @@ -30,7 +30,7 @@ dependent_startup=true {# Router advertiser should only run on ToR (T0) devices which have #} {# at least one VLAN interface which has an IPv6 address asigned #} {%- set vlan_v6 = namespace(count=0) -%} -{%- if DEVICE_METADATA.localhost.type == "ToRRouter" -%} +{%- if "ToRRouter" in DEVICE_METADATA.localhost.type and DEVICE_METADATA.localhost.type != "MgmtToRRouter" -%} {%- if VLAN_INTERFACE -%} {%- for (name, prefix) in VLAN_INTERFACE|pfx_filter -%} {# If this VLAN has an IPv6 address... #} diff --git a/files/build_templates/buffers_config.j2 b/files/build_templates/buffers_config.j2 index 0eb976599a..f5dbb9784f 100644 --- a/files/build_templates/buffers_config.j2 +++ b/files/build_templates/buffers_config.j2 @@ -9,9 +9,9 @@ def {# Determine device topology and filename postfix #} {%- if DEVICE_METADATA is defined and DEVICE_METADATA['localhost']['type'] is defined %} {%- set switch_role = DEVICE_METADATA['localhost']['type'] %} -{%- if switch_role.lower() == 'torrouter' %} +{%- if 'torrouter' in switch_role.lower() and 'mgmt' not in switch_role.lower()%} {%- set filename_postfix = 't0' %} -{%- elif switch_role.lower() == 'leafrouter' %} +{%- elif 'leafrouter' in switch_role.lower() and 'mgmt' not in switch_role.lower()%} {%- set filename_postfix = 't1' %} {%- else %} {%- set filename_postfix = set_default_topology() %} @@ -69,7 +69,7 @@ def {%- if cable_len -%} {{ cable_len.0 }} {%- else %} - {%- if switch_role.lower() == 'torrouter' %} + {%- if 'torrouter' in switch_role.lower() and 'mgmt' not in switch_role.lower()%} {%- for local_port in VLAN_MEMBER %} {%- if local_port[1] == port_name %} {%- set roles3 = switch_role + '_' + 'server' %} diff --git a/files/build_templates/qos_config.j2 b/files/build_templates/qos_config.j2 index 9420be7a06..a7c361d69f 100644 --- a/files/build_templates/qos_config.j2 +++ b/files/build_templates/qos_config.j2 @@ -73,7 +73,7 @@ "7": "7" } }, -{% if 'type' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['type'] in backend_device_types %} +{% if 'type' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['type'] in backend_device_types and 'storage_device' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['storage_device'] == 'true' %} "DOT1P_TO_TC_MAP": { "AZURE": { "0": "0", @@ -177,7 +177,7 @@ "PORT_QOS_MAP": { {% for port in PORT_ACTIVE %} "{{ port }}": { -{% if 'type' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['type'] in backend_device_types %} +{% if 'type' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['type'] in backend_device_types and 'storage_device' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['storage_device'] == 'true' %} "dot1p_to_tc_map" : "[DOT1P_TO_TC_MAP|AZURE]", {% else %} "dscp_to_tc_map" : "[DSCP_TO_TC_MAP|AZURE]", diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 96b68d7d22..c564d60519 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -98,6 +98,7 @@ def parse_png(png, hname): port_speeds = {} console_ports = {} mux_cable_ports = {} + is_storage_device = False for child in png: if child.tag == str(QName(ns, "DeviceInterfaceLinks")): for link in child.findall(str(QName(ns, "DeviceLinkBase"))): @@ -153,6 +154,12 @@ def parse_png(png, hname): device_data['deployment_id'] = deployment_id devices[name] = device_data + if name == hname: + cluster = device.find(str(QName(ns, "ClusterName"))) + + if cluster != None and "str" in cluster.text.lower(): + is_storage_device = True + if child.tag == str(QName(ns, "DeviceInterfaceLinks")): for if_link in child.findall(str(QName(ns, 'DeviceLinkBase'))): if str(QName(ns3, "type")) in if_link.attrib: @@ -179,7 +186,7 @@ def parse_png(png, hname): mux_cable_ports[intf_name] = "true" - return (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds, console_ports, mux_cable_ports) + return (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speeds, console_ports, mux_cable_ports, is_storage_device) def parse_asic_external_link(link, asic_name, hostname): neighbors = {} @@ -915,6 +922,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw hostname = None linkmetas = {} host_lo_intfs = None + is_storage_device = False local_devices = [] # hostname is the asic_name, get the asic_id from the asic_name @@ -948,7 +956,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw elif child.tag == str(QName(ns, "CpgDec")): (bgp_sessions, bgp_internal_sessions, bgp_asn, bgp_peers_with_range, bgp_monitors) = parse_cpg(child, hostname) elif child.tag == str(QName(ns, "PngDec")): - (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speed_png, console_ports, mux_cable_ports) = parse_png(child, hostname) + (neighbors, devices, console_dev, console_port, mgmt_dev, mgmt_port, port_speed_png, console_ports, mux_cable_ports, is_storage_device) = parse_png(child, hostname) elif child.tag == str(QName(ns, "UngDec")): (u_neighbors, u_devices, _, _, _, _, _, _) = parse_png(child, hostname) elif child.tag == str(QName(ns, "MetadataDeclaration")): @@ -992,6 +1000,9 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw 'synchronous_mode': 'enable' } } + + if is_storage_device: + results['DEVICE_METADATA']['localhost']['storage_device'] = "true" # for this hostname, if sub_role is defined, add sub_role in # device_metadata if sub_role is not None: @@ -1159,7 +1170,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw results['PORTCHANNEL_INTERFACE'] = pc_intfs - if current_device['type'] in backend_device_types: + if current_device['type'] in backend_device_types and is_storage_device: del results['INTERFACE'] del results['PORTCHANNEL_INTERFACE'] diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case.xml b/src/sonic-config-engine/tests/simple-sample-graph-case.xml index a3383a0946..c499dc1576 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case.xml @@ -254,6 +254,7 @@ switch-t0 Force10-S6000 + AAA00PrdStr00
diff --git a/src/sonic-config-engine/tests/simple-sample-graph.xml b/src/sonic-config-engine/tests/simple-sample-graph.xml index b99da804ea..7215209da5 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph.xml @@ -268,6 +268,7 @@ switch-t0 Force10-S6000 + AAA00PrdStr00 ARISTA01T1 diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py index de4f6b77ce..4ab1915864 100644 --- a/src/sonic-config-engine/tests/test_minigraph_case.py +++ b/src/sonic-config-engine/tests/test_minigraph_case.py @@ -185,6 +185,11 @@ class TestCfgGenCaseInsensitive(TestCase): else: self.assertTrue("mux_cable" not in port) + def test_minigraph_storage_device(self): + argument = '-m "' + self.sample_graph + '" -p "' + self.port_config + '" -v "DEVICE_METADATA[\'localhost\'][\'storage_device\']"' + output = self.run_script(argument) + self.assertEqual(output.strip(), "true") + def test_minigraph_tunnel_table(self): argument = '-m "' + self.sample_graph + '" -p "' + self.port_config + '" -v "TUNNEL"' expected_tunnel = {