Skip to content

Commit bce9106

Browse files
authored
config: allow golden config to override mac, platform, asic_id (#291)
Currently generate_sysinfo() unconditionally overwrites mac, platform, and asic_id in the golden config with values from the running config or hardware detection. This prevents users from intentionally overriding these fields via 'config load_minigraph -o' or 'config override-config-table'. Change generate_sysinfo() to only backfill these fields when they are not explicitly present in the golden config input. If the golden config provides mac/platform/asic_id, those values are preserved. Add unit test to verify explicit sysinfo values in golden config are not overwritten by hardware-detected values. Signed-off-by: securely1g <securely1g@users.noreply.github.com> Co-authored-by: securely1g <securely1g@users.noreply.github.com>
1 parent 9f669da commit bce9106

3 files changed

Lines changed: 102 additions & 2 deletions

File tree

config/main.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2244,8 +2244,11 @@ def generate_sysinfo(cur_config, config_input, ns=None):
22442244
if not platform:
22452245
platform = device_info.get_platform()
22462246

2247-
device_metadata['localhost']['mac'] = mac
2248-
device_metadata['localhost']['platform'] = platform
2247+
# Only backfill sysinfo fields if not explicitly provided in golden config
2248+
if 'mac' not in device_metadata['localhost']:
2249+
device_metadata['localhost']['mac'] = mac
2250+
if 'platform' not in device_metadata['localhost']:
2251+
device_metadata['localhost']['platform'] = platform
22492252

22502253
return
22512254

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
{
2+
"localhost": {
3+
"DEVICE_METADATA": {
4+
"localhost": {
5+
"default_bgp_status": "down",
6+
"default_pfcwd_status": "enable",
7+
"deployment_id": "1",
8+
"docker_routing_config_mode": "separated",
9+
"hostname": "sonic-switch",
10+
"hwsku": "Mellanox-SN3800-D112C8",
11+
"peer_switch": "sonic-switch",
12+
"type": "ToRRouter",
13+
"suppress-fib-pending": "enabled",
14+
"mac": "aa:bb:cc:dd:ee:ff",
15+
"platform": "custom-platform"
16+
}
17+
}
18+
},
19+
"asic0": {
20+
"DEVICE_METADATA": {
21+
"localhost": {
22+
"asic_name": "asic0",
23+
"bgp_asn": "65100",
24+
"cloudtype": "None",
25+
"default_bgp_status": "down",
26+
"default_pfcwd_status": "enable",
27+
"deployment_id": "None",
28+
"docker_routing_config_mode": "separated",
29+
"hostname": "sonic",
30+
"hwsku": "multi_asic",
31+
"region": "None",
32+
"sub_role": "FrontEnd",
33+
"type": "LeafRouter",
34+
"mac": "aa:bb:cc:dd:ee:ff",
35+
"platform": "custom-platform",
36+
"asic_id": "ff:00:00"
37+
}
38+
}
39+
},
40+
"asic1": {
41+
"DEVICE_METADATA": {
42+
"localhost": {
43+
"asic_name": "asic1",
44+
"bgp_asn": "65100",
45+
"cloudtype": "None",
46+
"default_bgp_status": "down",
47+
"default_pfcwd_status": "enable",
48+
"deployment_id": "None",
49+
"docker_routing_config_mode": "separated",
50+
"hostname": "sonic",
51+
"hwsku": "multi_asic",
52+
"region": "None",
53+
"sub_role": "BackEnd",
54+
"type": "LeafRouter",
55+
"mac": "aa:bb:cc:dd:ee:ff",
56+
"platform": "custom-platform",
57+
"asic_id": "ff:00:00"
58+
}
59+
}
60+
}
61+
}

tests/config_override_test.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
MULTI_ASIC_MACSEC_OV = os.path.join(DATA_DIR, "multi_asic_macsec_ov.json")
2525
MULTI_ASIC_FEATURE_RM = os.path.join(DATA_DIR, "multi_asic_feature_rm.json")
2626
MULTI_ASIC_DEVICE_METADATA_GEN_SYSINFO = os.path.join(DATA_DIR, "multi_asic_dm_gen_sysinfo.json")
27+
MULTI_ASIC_DEVICE_METADATA_EXPLICIT_SYSINFO = os.path.join(DATA_DIR, "multi_asic_dm_explicit_sysinfo.json")
2728
MULTI_ASIC_MISSING_LOCALHOST_OV = os.path.join(DATA_DIR, "multi_asic_missing_localhost.json")
2829
MULTI_ASIC_MISSING_ASIC_OV = os.path.join(DATA_DIR, "multi_asic_missing_asic.json")
2930

@@ -401,6 +402,41 @@ def read_json_file_side_effect(filename):
401402
assert platform == "multi_asic"
402403
assert mac == "11:22:33:44:55:66"
403404

405+
def test_device_metadata_explicit_sysinfo_override(self):
406+
"""Test that explicit mac/platform/asic_id in golden config are preserved."""
407+
def read_json_file_side_effect(filename):
408+
with open(MULTI_ASIC_DEVICE_METADATA_EXPLICIT_SYSINFO, "r") as f:
409+
device_metadata = json.load(f)
410+
return device_metadata
411+
db = Db()
412+
cfgdb_clients = db.cfgdb_clients
413+
414+
with mock.patch('config.main.read_json_file',
415+
mock.MagicMock(side_effect=read_json_file_side_effect)),\
416+
mock.patch('sonic_py_common.device_info.get_platform',
417+
return_value="multi_asic"),\
418+
mock.patch('sonic_py_common.device_info.get_system_mac',
419+
return_value="11:22:33:44:55:66\n"),\
420+
mock.patch('sonic_py_common.multi_asic.get_asic_device_id',
421+
return_value="06:00:00\n"):
422+
runner = CliRunner()
423+
result = runner.invoke(config.config.commands["override-config-table"],
424+
['golden_config_db.json'], obj=db)
425+
assert result.exit_code == 0
426+
427+
for ns, config_db in cfgdb_clients.items():
428+
mac = config_db.get_config()['DEVICE_METADATA']['localhost'].get('mac')
429+
platform = config_db.get_config()['DEVICE_METADATA']['localhost'].get('platform')
430+
# Golden config explicit values should be preserved, not overwritten
431+
assert mac == "aa:bb:cc:dd:ee:ff", \
432+
f"Expected golden config mac 'aa:bb:cc:dd:ee:ff' but got '{mac}' for namespace '{ns}'"
433+
assert platform == "custom-platform", \
434+
f"Expected golden config platform 'custom-platform' but got '{platform}' for namespace '{ns}'"
435+
if ns != config.DEFAULT_NAMESPACE and ns != HOST_NAMESPACE:
436+
asic_id = config_db.get_config()['DEVICE_METADATA']['localhost'].get('asic_id')
437+
assert asic_id == "ff:00:00", \
438+
f"Expected golden config asic_id 'ff:00:00' but got '{asic_id}' for namespace '{ns}'"
439+
404440
def test_masic_missig_localhost_override(self):
405441
def read_json_file_side_effect(filename):
406442
with open(MULTI_ASIC_MISSING_LOCALHOST_OV, "r") as f:

0 commit comments

Comments
 (0)