Add an option to do a safe config reload (#5374)

* Add an option to do a safe config reload

When doing a config reload, test cases cannot assume that the DUT is
available immediately after `config_reload` returns. Instead, they need
to wait on `duthost.critical_services_fully_started` to be true.

Therefore, in `config_reload` add an option `safe_reload` (defaults to
False to preserve current behavior) that will block on this so that the
function returns when the DUT is ready. In addition, have two test
cases (for now; other test cases will be checked later) use safe reload
so that the DUT is guaranteed to be ready when the function returns.

Otherwise, if the setup for the next test case is running while the
DUT is still bringing up the containers, then some check there could
fail.

Signed-off-by: Saikrishna Arcot <[email protected]>

Author: saiarcot895

tests/arp/test_neighbor_mac.py

@@ -3,7 +3,6 @@
 import time
 
 from tests.common.helpers.assertions import pytest_assert
-from tests.common.config_reload import config_reload
 
 logger = logging.getLogger(__name__)
 

tests/arp/test_neighbor_mac_noptf.py

@@ -77,7 +77,7 @@ def setupDutConfig(self, duthosts, enum_rand_one_per_hwsku_frontend_hostname):
         yield
 
         logger.info("Reload Config DB")
-        config_reload(duthost, config_source='config_db', wait=120)
+        config_reload(duthost, config_source='config_db', safe_reload=True)
 
     @pytest.fixture(params=[4, 6])
     def ipVersion(self, request):

tests/cacl/test_cacl_application.py

@@ -106,7 +106,7 @@ def clean_scale_rules(duthosts, rand_one_dut_hostname, collect_ignored_rules):
     # delete the tmp file
     duthost.file(path=SCALE_ACL_FILE, state='absent')
     logger.info("Reload config to recover configuration.")
-    config_reload(duthost)
+    config_reload(duthost, safe_reload=True)
 
 def is_acl_rule_empty(duthost):
     """

tests/common/config_reload.py

@@ -1,6 +1,9 @@
 import time
 import logging
 
+from tests.common.helpers.assertions import pytest_assert
+from tests.common.utilities import wait_until
+
 logger = logging.getLogger(__name__)
 
 config_sources = ['config_db', 'minigraph']
@@ -36,7 +39,7 @@ def config_force_option_supported(duthost):
         return True
     return False
 
-def config_reload(duthost, config_source='config_db', wait=120, start_bgp=True, start_dynamic_buffer=True):
+def config_reload(duthost, config_source='config_db', wait=120, start_bgp=True, start_dynamic_buffer=True, safe_reload=False):
     """
     reload SONiC configuration
     :param duthost: DUT host object
@@ -77,4 +80,15 @@ def config_reload(duthost, config_source='config_db', wait=120, start_bgp=True,
     modular_chassis = duthost.get_facts().get("modular_chassis")
     wait = max(wait, 240) if modular_chassis else wait
 
-    time.sleep(wait)
+    if safe_reload:
+        # The wait time passed in might not be guaranteed to cover the actual
+        # time it takes for containers to come back up. Therefore, add 5
+        # minutes to the maximum wait time. If it's ready sooner, then the
+        # function will return sooner.
+        pytest_assert(wait_until(wait + 300, 20, 0, duthost.critical_services_fully_started),
+                "All critical services should be fully started!")
+        if config_source == 'minigraph':
+            pytest_assert(wait_until(300, 20, 0, chk_for_pfc_wd, duthost),
+                    "PFC_WD is missing in CONFIG-DB")
+    else:
+        time.sleep(wait)