Merge branch 'master' into bulk_switchover

orchagent/fabricportsorch.cpp

@@ -545,6 +545,7 @@ void FabricPortsOrch::updateFabricDebugCounters()
         int autoIsolated = 0;
         int cfgIsolated = 0;
         int isolated = 0;
+        int origIsolated = 0;
         string lnkStatus = "down";
         string testState = "product";
 
@@ -643,6 +644,12 @@ void FabricPortsOrch::updateFabricDebugCounters()
             if (fvField(val) == "AUTO_ISOLATED")
             {
                 autoIsolated = to_uint<uint8_t>(valuePt);
+                SWSS_LOG_INFO("port %s currently autoisolated: %s", key.c_str(),valuePt.c_str());
+                continue;
+            }
+            if (fvField(val) == "ISOLATED")
+            {
+                origIsolated = to_uint<uint8_t>(valuePt);
                 SWSS_LOG_INFO("port %s currently isolated: %s", key.c_str(),valuePt.c_str());
                 continue;
             }
@@ -816,6 +823,36 @@ void FabricPortsOrch::updateFabricDebugCounters()
             }
             // if "ISOLATED" is true, Call SAI api here to actually isolated the link
             // if "ISOLATED" is false, Call SAP api to actually unisolate the link
+
+            if (origIsolated != isolated)
+            {
+                sai_attribute_t attr;
+                attr.id = SAI_PORT_ATTR_FABRIC_ISOLATE;
+                bool setVal = false;
+                if (isolated == 1)
+                {
+                    setVal = true;
+                }
+                attr.value.booldata = setVal;
+                SWSS_LOG_NOTICE("Set fabric port %d with isolate %d ", lane, isolated);
+                if (m_fabricLanePortMap.find(lane) == m_fabricLanePortMap.end())
+                {
+                    SWSS_LOG_NOTICE("NOT find fabric lane %d ", lane);
+                }
+                else
+                {
+                    sai_status_t status = sai_port_api->set_port_attribute(m_fabricLanePortMap[lane], &attr);
+                    if (status != SAI_STATUS_SUCCESS)
+                    {
+                        SWSS_LOG_ERROR("Failed to set admin status");
+                    }
+                    SWSS_LOG_NOTICE("Set fabric port %d state done %d ", lane, isolated);
+                }
+            }
+            else
+            {
+                SWSS_LOG_INFO( "Same isolation status for %d", lane);
+            }
         }
         else
         {
@@ -1309,9 +1346,6 @@ void FabricPortsOrch::doFabricPortTask(Consumer &consumer)
             }
             SWSS_LOG_NOTICE("key %s alias %s isolateStatus %s lanes %s",
                   key.c_str(), alias.c_str(), isolateStatus.c_str(), lanes.c_str());
-            // Call SAI api to isolate/unisolate the link here.
-            // Isolate the link if isolateStatus is True.
-            // Unisolate the link if isolateStatus is False.
 
             if (isolateStatus == "False")
             {
@@ -1372,6 +1406,26 @@ void FabricPortsOrch::doFabricPortTask(Consumer &consumer)
                     // AUTO_ISOLATED 0
                     m_stateTable->hset(state_key, "AUTO_ISOLATED",
                                        m_defaultAutoIsolated.c_str());
+
+                    sai_attribute_t attr;
+                    attr.id = SAI_PORT_ATTR_FABRIC_ISOLATE;
+                    bool setVal = false;
+                    attr.value.booldata = setVal;
+                    SWSS_LOG_NOTICE("Set port %s to unisolate %s ", alias.c_str(), isolateStatus.c_str());
+                    int idx = stoi(lanes);
+                    if (m_fabricLanePortMap.find(idx) == m_fabricLanePortMap.end())
+                    {
+                        SWSS_LOG_NOTICE("NOT find %s alias. ", alias.c_str());
+                    }
+                    else
+                    {
+                        sai_status_t status = sai_port_api->set_port_attribute(m_fabricLanePortMap[idx], &attr);
+                        if (status != SAI_STATUS_SUCCESS)
+                        {
+                            SWSS_LOG_ERROR("Failed to set admin status");
+                        }
+                        SWSS_LOG_NOTICE( "Set Port %s unisolation state done", alias.c_str());
+                    }
                 }
             }
         }

orchagent/main.cpp

@@ -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);
+        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");
+            exit(EXIT_FAILURE);
+        }
+
         attr.id = SAI_SWITCH_ATTR_SWITCH_ID;
         attr.value.u32 = gVoqMySwitchId;
         attrs.push_back(attr);

tests/dvslib/dvs_database.py

@@ -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.
 

tests/test_fabric_port_isolation.py

@@ -50,6 +50,18 @@ def test_voq_switch_fabric_link(self, vst):
                        # clear the testing errors and wait for link get unisolated.
                        sdb.update_entry("FABRIC_PORT_TABLE", port, {"TEST_CRC_ERRORS": "0"})
                        sdb.wait_for_field_match("FABRIC_PORT_TABLE", port, {"AUTO_ISOLATED": "0"}, polling_config=max_poll)
+
+                       # inject testing errors and wait for link get isolated again.
+                       sdb.update_entry("FABRIC_PORT_TABLE", port, {"TEST_CRC_ERRORS": "2"})
+                       sdb.wait_for_field_match("FABRIC_PORT_TABLE", port, {"AUTO_ISOLATED": "1"}, polling_config=max_poll)
+                       # now test force unisolate this link
+                       configKey = "Fabric"+str(portNum)
+                       curForceStatus = int( config_db.get_entry( "FABRIC_PORT", configKey)['forceUnisolateStatus'] )
+                       curForceStatus += 1
+                       config_db.update_entry("FABRIC_PORT", configKey, {'forceUnisolateStatus': str(curForceStatus)})
+                       config_db.wait_for_field_match("FABRIC_PORT", configKey, {'forceUnisolateStatus': str(curForceStatus)},
+                                                      polling_config=max_poll)
+                       sdb.wait_for_field_match("FABRIC_PORT_TABLE", port, {"AUTO_ISOLATED": "0"}, polling_config=max_poll)
                    finally:
                        # cleanup
                        sdb.update_entry("FABRIC_PORT_TABLE", port, {"TEST_CRC_ERRORS": "0"})

tests/test_fabric_switch_id.py

@@ -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
+

tests/virtual_chassis/8/default_config.json

@@ -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"