Skip to content

Commit da33cf4

Browse files
abdosiCopilot
andauthored
[minigraph/device_info] Parse device type from device_desc.xml; fix is_voq/packet_chassis to read Config DB directly (Azure#2451)
## Problem After running `config load_mgmt` + `config save`, two failures occur on Supervisor/Linecard devices: ### 1. `bgp_neighbor` script fails on Supervisor **Root cause:** `type` is not populated in `DEVICE_METADATA.localhost` after `config load_mgmt`, so the script fails when checking device type. ### 2. `load_minigraph` fails on Supervisor and Linecard **Root cause:** DB connection to `CHASSIS*DB` fails because `calcmgrd` does not have the iptable rule to allow midplane traffic — this rule is only added when `is_chassis()` returns `True`. ### Why `is_chassis()` returns `False` - **Supervisor:** The current implementation calls a platform API which internally tries to connect to `CHASSIS*DB` — and since that connection is not yet available, it fails. - **Linecard:** `switch_type` is not defined in Config DB at this point, so `is_chassis()` returns `False`. --- ## Fix ### 1. `src/sonic-config-engine/minigraph.py` — `parse_device_desc_xml()` Populate `DEVICE_METADATA.localhost.type` from `<ElementType>` when parsing `device_desc.xml`. Map `Linecard` and `Supervisor` to `SpineRouter`. This fixes the `bgp_neighbor` failure on Supervisor. ```python if d_type: if d_type in ('Linecard', 'Supervisor'): results['DEVICE_METADATA']['localhost']['type'] = 'SpineRouter' else: results['DEVICE_METADATA']['localhost']['type'] = d_type ``` ### 2. `src/sonic-py-common/sonic_py_common/device_info.py` — `is_chassis()` - Remove dependency on `CHASSIS*DB` connection by reading `switch_type` directly from Config DB via `get_localhost_info()` instead of `get_platform_info()` (which uses a cached global and internally may attempt a `CHASSIS*DB` connection). - Add an early check: if `DEVICE_METADATA.localhost.type == 'SpineRouter'`, return `True` immediately — removing the dependency on `switch_type` being defined. ```python def is_chassis(): if get_localhost_info('type') == 'SpineRouter': return True return is_voq_chassis() or is_packet_chassis() ``` --- ## Files Changed | File | Change | |---|---| | `src/sonic-config-engine/minigraph.py` | Populate `type` in `DEVICE_METADATA` from `<ElementType>`; map Linecard/Supervisor → SpineRouter | | `src/sonic-py-common/sonic_py_common/device_info.py` | `is_chassis()` checks `type == SpineRouter` first; `is_voq/packet_chassis()` read Config DB directly | | `src/sonic-py-common/tests/device_info_test.py` | Update mocks and add SpineRouter test case | | `src/sonic-config-engine/tests/test_minigraph_case.py` | Add UTs for device type parsing and Linecard/Supervisor mapping | | `src/sonic-config-engine/tests/simple-sample-device-desc-supervisor.xml` | New test fixture | | `src/sonic-config-engine/tests/simple-sample-device-desc-linecard.xml` | New test fixture | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9504bbf commit da33cf4

6 files changed

Lines changed: 59 additions & 10 deletions

File tree

src/sonic-config-engine/minigraph.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2856,6 +2856,11 @@ def parse_device_desc_xml(filename):
28562856
'hostname': hostname,
28572857
'hwsku': hwsku,
28582858
}}
2859+
if d_type:
2860+
if d_type in ('Linecard', 'Supervisor'):
2861+
results['DEVICE_METADATA']['localhost']['type'] = 'SpineRouter'
2862+
else:
2863+
results['DEVICE_METADATA']['localhost']['type'] = d_type
28592864

28602865
results['LOOPBACK_INTERFACE'] = {'lo': {}, ('lo', lo_prefix): {}}
28612866
if lo_prefix_v6:
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<Device i:type="Linecard" xmlns="Microsoft.Search.Autopilot.Evolution" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
2+
<ElementType>Linecard</ElementType>
3+
<Hostname>switch-linecard</Hostname>
4+
<HwSku>8800-LC</HwSku>
5+
<ClusterName>SEL20</ClusterName>
6+
<ManagementAddress xmlns:a="Microsoft.Search.Autopilot.NetMux">
7+
<a:IPPrefix>10.0.0.101/24</a:IPPrefix>
8+
</ManagementAddress>
9+
<ManagementAddressV6 xmlns:a="Microsoft.Search.Autopilot.NetMux">
10+
<a:IPPrefix>FC00:1::33/64</a:IPPrefix>
11+
</ManagementAddressV6>
12+
</Device>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<Device i:type="Supervisor" xmlns="Microsoft.Search.Autopilot.Evolution" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
2+
<ElementType>Supervisor</ElementType>
3+
<Hostname>switch-supervisor</Hostname>
4+
<HwSku>8800-RP</HwSku>
5+
<ClusterName>SEL20</ClusterName>
6+
<ManagementAddress xmlns:a="Microsoft.Search.Autopilot.NetMux">
7+
<a:IPPrefix>10.0.0.100/24</a:IPPrefix>
8+
</ManagementAddress>
9+
<ManagementAddressV6 xmlns:a="Microsoft.Search.Autopilot.NetMux">
10+
<a:IPPrefix>FC00:1::32/64</a:IPPrefix>
11+
</ManagementAddressV6>
12+
</Device>

src/sonic-config-engine/tests/test_minigraph_case.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ def setUp(self):
2323
self.sample_subintf_graph = os.path.join(self.test_dir, 'sample-graph-subintf.xml')
2424
self.sample_simple_device_desc = os.path.join(self.test_dir, 'simple-sample-device-desc.xml')
2525
self.sample_simple_device_desc_ipv6_only = os.path.join(self.test_dir, 'simple-sample-device-desc-ipv6-only.xml')
26+
self.sample_supervisor_device_desc = os.path.join(self.test_dir, 'simple-sample-device-desc-supervisor.xml')
27+
self.sample_linecard_device_desc = os.path.join(self.test_dir, 'simple-sample-device-desc-linecard.xml')
2628
self.port_config = os.path.join(self.test_dir, 't0-sample-port-config.ini')
2729

2830
def run_script(self, argument, check_stderr=False):
@@ -557,6 +559,19 @@ def test_parse_device_desc_xml_mgmt_interface(self):
557559
self.assertTrue(('eth0', 'FC00:1::32/64') in mgmt_intf.keys())
558560
self.assertTrue(ipaddress.ip_address(u'fc00:1::1') == mgmt_intf[('eth0', 'FC00:1::32/64')]['gwaddr'])
559561

562+
def test_parse_device_desc_xml_device_type(self):
563+
# Regular device type is passed through as-is
564+
result = minigraph.parse_device_desc_xml(self.sample_simple_device_desc)
565+
self.assertEqual(result['DEVICE_METADATA']['localhost']['type'], 'ToRRouter')
566+
567+
# Supervisor ElementType is mapped to SpineRouter
568+
result = minigraph.parse_device_desc_xml(self.sample_supervisor_device_desc)
569+
self.assertEqual(result['DEVICE_METADATA']['localhost']['type'], 'SpineRouter')
570+
571+
# Linecard ElementType is mapped to SpineRouter
572+
result = minigraph.parse_device_desc_xml(self.sample_linecard_device_desc)
573+
self.assertEqual(result['DEVICE_METADATA']['localhost']['type'], 'SpineRouter')
574+
560575
def test_mgmt_device_disable_counters(self):
561576
expected_mgmt_disabled_counters = ["BUFFER_POOL_WATERMARK", "PFCWD", "PG_DROP", "PG_WATERMARK", "PORT_BUFFER_DROP", "QUEUE", "QUEUE_WATERMARK"]
562577
expected_mgmt_enabled_counters = ["ACL", "PORT", "RIF"]

src/sonic-py-common/sonic_py_common/device_info.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -563,16 +563,18 @@ def is_multi_npu():
563563

564564

565565
def is_voq_chassis():
566-
switch_type = get_platform_info().get('switch_type')
567-
return True if switch_type and (switch_type == 'voq' or switch_type == 'fabric') else False
566+
switch_type = get_localhost_info('switch_type')
567+
return switch_type in ('voq', 'fabric') if switch_type else False
568568

569569

570570
def is_packet_chassis():
571-
switch_type = get_platform_info().get('switch_type')
572-
return True if switch_type and switch_type == 'chassis-packet' else False
571+
switch_type = get_localhost_info('switch_type')
572+
return switch_type == 'chassis-packet' if switch_type else False
573573

574574

575575
def is_chassis():
576+
if get_localhost_info('type') == 'SpineRouter':
577+
return True
576578
return is_voq_chassis() or is_packet_chassis()
577579

578580

src/sonic-py-common/tests/device_info_test.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,24 +119,27 @@ def test_get_sonic_version(self, mock_isfile):
119119
# Assert the file was read only once
120120
open_mocked.assert_called_once_with(device_info.SONIC_VERSION_YAML_PATH)
121121

122-
@mock.patch("sonic_py_common.device_info.get_platform_info")
123-
def test_is_chassis(self, mock_platform_info):
124-
mock_platform_info.return_value = {"switch_type": "npu"}
122+
@mock.patch("sonic_py_common.device_info.get_localhost_info")
123+
def test_is_chassis(self, mock_localhost_info):
124+
mock_localhost_info.return_value = "npu"
125125
assert device_info.is_chassis() == False
126126
assert device_info.is_voq_chassis() == False
127127
assert device_info.is_packet_chassis() == False
128128

129-
mock_platform_info.return_value = {"switch_type": "voq"}
129+
mock_localhost_info.return_value = "voq"
130130
assert device_info.is_voq_chassis() == True
131131
assert device_info.is_packet_chassis() == False
132132
assert device_info.is_chassis() == True
133133

134-
mock_platform_info.return_value = {"switch_type": "chassis-packet"}
134+
mock_localhost_info.return_value = "chassis-packet"
135135
assert device_info.is_voq_chassis() == False
136136
assert device_info.is_packet_chassis() == True
137137
assert device_info.is_chassis() == True
138138

139-
mock_platform_info.return_value = {}
139+
mock_localhost_info.return_value = "SpineRouter"
140+
assert device_info.is_chassis() == True
141+
142+
mock_localhost_info.return_value = None
140143
assert device_info.is_voq_chassis() == False
141144
assert device_info.is_packet_chassis() == False
142145
assert device_info.is_chassis() == False

0 commit comments

Comments
 (0)