Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions orchagent/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,28 @@ int main(int argc, char **argv)
attr.value.u32 = SAI_SWITCH_TYPE_FABRIC;
attrs.push_back(attr);

//Read switch_id from config_db.
Table cfgDeviceMetaDataTable(&config_db, CFG_DEVICE_METADATA_TABLE_NAME);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if VS tests are adopted for this change for Voq?

string value;
if (cfgDeviceMetaDataTable.hget("localhost", "switch_id", value))
{
if (value.size())
{
gVoqMySwitchId = stoi(value);
}

if (gVoqMySwitchId < 0)
{
SWSS_LOG_ERROR("Invalid fabric switch id %d configured", gVoqMySwitchId);
exit(EXIT_FAILURE);
}
}
else
{
SWSS_LOG_ERROR("Fabric switch id is not configured");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like its a breaking change. You are exiting orchagent if switch_id is not present in db. Is this intenational. @judyjoseph , @arlakshm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prsunny We have the same condition for linecard asics. In latest SAI, switch_id was made mandatory on switch create of fabric asic opencomputeproject/SAI#1793. So, we need this change to pass the correct switch-id. The configuration is expected to have switch_id for both linecard asic and fabric asic.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, please approve the PR

exit(EXIT_FAILURE);
}

attr.id = SAI_SWITCH_ATTR_SWITCH_ID;
attr.value.u32 = gVoqMySwitchId;
attrs.push_back(attr);
Expand Down
14 changes: 13 additions & 1 deletion tests/dvslib/dvs_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,19 @@ def delete_field(self, table_name: str, key: str, field: str) -> None:
"""
table = swsscommon.Table(self.db_connection, table_name)
table.hdel(key, field)


def set_field(self, table_name: str, key: str, field: str, value: str) -> None:
"""Add/Update a field in an entry stored at `key` in the specified table.

Args:
table_name: The name of the table where the entry is being removed.
key: The key that maps to the entry being added/updated.
field: The field that needs to be added/updated.
value: The value that is set for the field.
"""
table = swsscommon.Table(self.db_connection, table_name)
table.hset(key, field, value)

def get_keys(self, table_name: str) -> List[str]:
"""Get all of the keys stored in the specified table.

Expand Down
48 changes: 48 additions & 0 deletions tests/test_fabric_switch_id.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from dvslib.dvs_common import wait_for_result, PollingConfig
import pytest

class TestFabricSwitchId(object):
def check_syslog(self, dvs, marker, log):
def do_check_syslog():
(ec, out) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \'%s\' | wc -l" %(marker, log)])
return (int(out.strip()) >= 1, None)
max_poll = PollingConfig(polling_interval=5, timeout=600, strict=True)
wait_for_result(do_check_syslog, polling_config=max_poll)

def test_invalid_fabric_switch_id(self, vst):
# Find supervisor dvs.
dvs = None
config_db = None
for name in vst.dvss.keys():
dvs = vst.dvss[name]
config_db = dvs.get_config_db()
metatbl = config_db.get_entry("DEVICE_METADATA", "localhost")
cfg_switch_type = metatbl.get("switch_type")
if cfg_switch_type == "fabric":
break
assert dvs and config_db

# Verify orchagent's handling of invalid fabric switch_id in following cases:
# - Invalid fabric switch_id, e.g, -1, is set.
# - fabric switch_id is missing in ConfigDb.
for invalid_switch_id in (-1, None):
print(f"Test invalid switch id {invalid_switch_id}")
if invalid_switch_id is None:
config_db.delete_field("DEVICE_METADATA", "localhost", "switch_id")
expected_log = "Fabric switch id is not configured"
else:
config_db.set_field("DEVICE_METADATA", "localhost", "switch_id", str(invalid_switch_id))
expected_log = f"Invalid fabric switch id {invalid_switch_id} configured"

# Restart orchagent and verify orchagent behavior by checking syslog.
dvs.stop_swss()
marker = dvs.add_log_marker()
dvs.start_swss()
self.check_syslog(dvs, marker, expected_log)


# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down before retrying
def test_nonflaky_dummy():
pass

1 change: 1 addition & 0 deletions tests/virtual_chassis/8/default_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"chassis_db_address" : "10.8.1.200",
"inband_address" : "10.8.1.200/24",
"switch_type": "fabric",
"switch_id": "0",
"sub_role" : "BackEnd",
"start_chassis_db" : "1",
"comment" : "default_config for a vs that runs chassis_db"
Expand Down