[dvs] Validate that SWSS is ready to receive input before starting te…

…sts (sonic-net#1385)

- Enhance AsicDbValidator to be stricter
- Add a new SWSS ready-check to validate that orchagent is done processing ports

Signed-off-by: Danny Allen <daall@microsoft.com>

tests/conftest.py

@@ -15,13 +15,20 @@
 
 from datetime import datetime
 from swsscommon import swsscommon
-from dvslib import dvs_database as dvs_db
+from dvslib.dvs_database import DVSDatabase
+from dvslib.dvs_common import PollingConfig, wait_for_result
 from dvslib.dvs_acl import DVSAcl
 from dvslib import dvs_vlan
 from dvslib import dvs_lag
 from dvslib import dvs_mirror
 from dvslib import dvs_policer
 
+# FIXME: For the sake of stabilizing the PR pipeline we currently assume there are 32 front-panel
+# ports in the system (much like the rest of the test suite). This should be adjusted to accomodate
+# a dynamic number of ports. GitHub Issue: Azure/sonic-swss#1384.
+NUM_PORTS = 32
+
+
 def ensure_system(cmd):
     if sys.version_info < (3, 0):
         (rc, output) = commands.getstatusoutput(cmd)
@@ -30,6 +37,7 @@ def ensure_system(cmd):
     if rc:
         raise RuntimeError('Failed to run command: %s. rc=%d. output: %s' % (cmd, rc, output))
 
+
 def pytest_addoption(parser):
     parser.addoption("--dvsname", action="store", default=None,
                       help="dvs name")
@@ -38,54 +46,37 @@ def pytest_addoption(parser):
     parser.addoption("--imgname", action="store", default="docker-sonic-vs",
                       help="image name")
 
-class AsicDbValidator(object):
-    def __init__(self, dvs):
-        self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
 
-        # get default dot1q vlan id
-        atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN")
+class AsicDbValidator(DVSDatabase):
+    def __init__(self, db_id: str, connector: str):
+        DVSDatabase.__init__(self, db_id, connector)
 
-        keys = atbl.getKeys()
-        assert len(keys) == 1
-        self.default_vlan_id = keys[0]
+        # Get default .1Q Vlan ID
+        self.default_vlan_id = self.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_VLAN", 1)[0]
 
-        # build port oid to front port name mapping
+        # Build port OID to front port name mapping
         self.portoidmap = {}
         self.portnamemap = {}
         self.hostifoidmap = {}
         self.hostifnamemap = {}
-        atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF")
-        keys = atbl.getKeys()
 
-        assert len(keys) == 32
+        keys = self.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF", NUM_PORTS)
         for k in keys:
-            (status, fvs) = atbl.get(k)
-
-            assert status == True
-
-            for fv in fvs:
-                if fv[0] == "SAI_HOSTIF_ATTR_OBJ_ID":
-                    port_oid = fv[1]
-                elif fv[0] == "SAI_HOSTIF_ATTR_NAME":
-                    port_name = fv[1]
+            fvs = self.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_HOSTIF", k)
+            port_oid = fvs.get("SAI_HOSTIF_ATTR_OBJ_ID")
+            port_name = fvs.get("SAI_HOSTIF_ATTR_NAME")
 
             self.portoidmap[port_oid] = port_name
             self.portnamemap[port_name] = port_oid
             self.hostifoidmap[k] = port_name
             self.hostifnamemap[port_name] = k
 
-        # get default acl table and acl rules
-        atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE")
-        keys = atbl.getKeys()
-
-        assert len(keys) >= 0
-        self.default_acl_tables = keys
+        # Get default ACL table and ACL rules
+        self.default_acl_tables = self.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE")
+        assert len(self.default_acl_tables) >= 0
 
-        atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY")
-        keys = atbl.getKeys()
+        self.default_acl_entries = self.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", 0)
 
-        assert len(keys) == 0
-        self.default_acl_entries = keys
 
 class ApplDbValidator(object):
     def __init__(self, dvs):
@@ -220,7 +211,7 @@ def __init__(self, name=None, imgname=None, keeptb=False, fakeplatform=None):
 
             # create virtual servers
             self.servers = []
-            for i in range(32):
+            for i in range(NUM_PORTS):
                 server = VirtualServer(ctn_sw_name, self.ctn_sw_pid, i)
                 self.servers.append(server)
 
@@ -239,7 +230,7 @@ def __init__(self, name=None, imgname=None, keeptb=False, fakeplatform=None):
 
             # create virtual server
             self.servers = []
-            for i in range(32):
+            for i in range(NUM_PORTS):
                 server = VirtualServer(self.ctn_sw.name, self.ctn_sw_pid, i)
                 self.servers.append(server)
 
@@ -256,7 +247,6 @@ def __init__(self, name=None, imgname=None, keeptb=False, fakeplatform=None):
                     volumes={ self.mount: { 'bind': '/var/run/redis', 'mode': 'rw' } })
 
         self.redis_sock = self.mount + '/' + "redis.sock"
-        self.check_ctn_status_and_db_connect()
 
         # DB wrappers are declared here, lazy-loaded in the tests
         self.app_db = None
@@ -266,6 +256,9 @@ def __init__(self, name=None, imgname=None, keeptb=False, fakeplatform=None):
         self.flex_db = None
         self.state_db = None
 
+        # Make sure everything is up and running before turning over control to the caller
+        self.check_ready_status_and_init_db()
+
     def destroy(self):
         if self.appldb:
             del self.appldb
@@ -276,24 +269,30 @@ def destroy(self):
             for s in self.servers:
                 s.destroy()
 
-    def check_ctn_status_and_db_connect(self):
+    def check_ready_status_and_init_db(self):
         try:
             # temp fix: remove them once they are moved to vs start.sh
             self.ctn.exec_run("sysctl -w net.ipv6.conf.default.disable_ipv6=0")
             for i in range(0, 128, 4):
                 self.ctn.exec_run("sysctl -w net.ipv6.conf.eth%d.disable_ipv6=1" % (i + 1))
-            self.check_ready()
+
+            # Verify that all of the device services have started.
+            self.check_services_ready()
+
+            # Initialize the databases.
             self.init_asicdb_validator()
             self.appldb = ApplDbValidator(self)
-        except:
+
+            # Verify that SWSS has finished initializing.
+            self.check_swss_ready()
+
+        except Exception:
             self.get_logs()
             self.destroy()
             raise
 
-
-    def check_ready(self, timeout=30):
-        '''check if all processes in the dvs is ready'''
-
+    def check_services_ready(self, timeout=30):
+        """"Check if all processes in the DVS are ready."""
         re_space = re.compile('\s+')
         process_status = {}
         ready = False
@@ -333,6 +332,30 @@ def check_ready(self, timeout=30):
 
             time.sleep(1)
 
+    def check_swss_ready(self, timeout=300):
+        """Verify that SWSS is ready to receive inputs.
+
+        Almost every part of orchagent depends on ports being created and initialized
+        before they can proceed with their processing. If we start the tests after orchagent
+        has started running but before it has had time to initialize all the ports, then the
+        first several tests will fail.
+        """
+        num_ports = NUM_PORTS
+
+        # Verify that all ports have been initialized and configured
+        app_db = self.get_app_db()
+        startup_polling_config = PollingConfig(5, timeout, strict=True)
+
+        def _polling_function():
+            port_table_keys = app_db.get_keys("PORT_TABLE")
+            return ("PortInitDone" in port_table_keys and "PortConfigDone" in port_table_keys, None)
+
+        wait_for_result(_polling_function, startup_polling_config)
+
+        # Verify that all ports have been created
+        asic_db = self.get_asic_db()
+        asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_ports + 1)  # +1 CPU Port
+
     def net_cleanup(self):
         """clean up network, remove extra links"""
 
@@ -363,7 +386,7 @@ def restart(self):
         if self.appldb:
             del self.appldb
         self.ctn_restart()
-        self.check_ctn_status_and_db_connect()
+        self.check_ready_status_and_init_db()
 
     # start processes in SWSS
     def start_swss(self):
@@ -401,7 +424,7 @@ def stop_fpmsyncd(dvs):
         time.sleep(1)
 
     def init_asicdb_validator(self):
-        self.asicdb = AsicDbValidator(self)
+        self.asicdb = AsicDbValidator(self.ASIC_DB_ID, self.redis_sock)
 
     def runcmd(self, cmd):
         res = self.ctn.exec_run(cmd)
@@ -898,13 +921,15 @@ def setReadOnlyAttr(self, obj, attr, val):
 
     def get_app_db(self):
         if not self.app_db:
-            self.app_db = dvs_db.DVSDatabase(self.APP_DB_ID, self.redis_sock)
+            self.app_db = DVSDatabase(self.APP_DB_ID, self.redis_sock)
 
         return self.app_db
 
+    # FIXME: Now that AsicDbValidator is using DVSDatabase we should converge this with
+    # that implementation. Save it for a follow-up PR.
     def get_asic_db(self):
         if not self.asic_db:
-            db = dvs_db.DVSDatabase(self.ASIC_DB_ID, self.redis_sock)
+            db = DVSDatabase(self.ASIC_DB_ID, self.redis_sock)
             db.default_acl_tables = self.asicdb.default_acl_tables
             db.default_acl_entries = self.asicdb.default_acl_entries
             db.port_name_map = self.asicdb.portnamemap
@@ -917,25 +942,25 @@ def get_asic_db(self):
 
     def get_counters_db(self):
         if not self.counters_db:
-            self.counters_db = dvs_db.DVSDatabase(self.COUNTERS_DB_ID, self.redis_sock)
+            self.counters_db = DVSDatabase(self.COUNTERS_DB_ID, self.redis_sock)
 
         return self.counters_db
 
     def get_config_db(self):
         if not self.config_db:
-            self.config_db = dvs_db.DVSDatabase(self.CONFIG_DB_ID, self.redis_sock)
+            self.config_db = DVSDatabase(self.CONFIG_DB_ID, self.redis_sock)
 
         return self.config_db
 
     def get_flex_db(self):
         if not self.flex_db:
-            self.flex_db = dvs_db.DVSDatabase(self.FLEX_COUNTER_DB_ID, self.redis_sock)
+            self.flex_db = DVSDatabase(self.FLEX_COUNTER_DB_ID, self.redis_sock)
 
         return self.flex_db
 
     def get_state_db(self):
         if not self.state_db:
-            self.state_db = dvs_db.DVSDatabase(self.STATE_DB_ID, self.redis_sock)
+            self.state_db = DVSDatabase(self.STATE_DB_ID, self.redis_sock)
 
         return self.state_db