[test_fib] Improve test_fib (sonic-net#4193)

Approach
What is the motivation for this PR?
Improve test_fib stability over dualtor testbed

How did you do it?
Add a few improvements:

For test_basic_fib:
1.1 Add back the logic to toggle the ports to random side.
1.2 Use fixture toggle_all_simulator_ports_to_random_side to do the
toggle instead and add extra check _check_mux_status_consistency to
ensure the mux status from the DUTs and mux simulator is consistent
after the toggle.

For test_hash:
2.1 Use toggle_all_simulator_ports_to_rand_selected_tor to do the
toggle because it has check to verify the toggle.

Signed-off-by: Longxiang Lyu [email protected]

How did you verify/test it?
Run it over dualtor, t0 and t1 testbed.

tests/common/dualtor/mux_simulator_control.py

@@ -1,5 +1,7 @@
 import logging
 import pytest
+import time
+import json
 
 import requests
 
@@ -331,6 +333,7 @@ def _check_all_active(duthost):
 
     pytest_assert(RETRY > 0, "Failed to toggle all ports to {}".format(rand_one_dut_hostname))
 
+
 @pytest.fixture
 def toggle_all_simulator_ports_to_another_side(mux_server_url):
     """
@@ -340,12 +343,72 @@ def toggle_all_simulator_ports_to_another_side(mux_server_url):
     """
     _toggle_all_simulator_ports(mux_server_url, TOGGLE)
 
+
 @pytest.fixture
-def toggle_all_simulator_ports_to_random_side(mux_server_url):
+def toggle_all_simulator_ports_to_random_side(duthosts, mux_server_url, tbinfo):
     """
     A module level fixture to toggle all ports to a random side.
     """
+    def _get_mux_status(duthost):
+        cmd = 'show mux status --json'
+        return json.loads(duthost.shell(cmd)['stdout'])
+
+    def _check_mux_status_consistency():
+        """Ensure mux status is consistent between the ToRs and mux simulator."""
+        upper_tor_mux_status = _get_mux_status(upper_tor_host)
+        lower_tor_mux_status = _get_mux_status(lower_tor_host)
+        simulator_mux_status = _get(mux_server_url)
+
+        if not upper_tor_mux_status:
+            logging.warn("Failed to retrieve mux status from the upper tor")
+            return False
+        if not lower_tor_mux_status:
+            logging.warn("Failed to retrieve mux status from the lower tor")
+            return False
+        if not simulator_mux_status:
+            logging.warn("Failed to retrieve mux status from the mux simulator")
+            return False
+
+        if not set(upper_tor_mux_status.keys()) == set(lower_tor_mux_status.keys()):
+            logging.warn("Ports mismatch between the upper tor and lower tor")
+            return False
+
+        # get mapping from port indices to mux status
+        simulator_port_mux_status = {int(k.split('-')[-1]):v for k,v in simulator_mux_status.items()}
+        for intf in upper_tor_mux_status['MUX_CABLE']:
+            intf_index = port_indices[intf]
+            if intf_index not in simulator_port_mux_status:
+                logging.warn("No mux status for interface %s from mux simulator", intf)
+                return False
+
+            simulator_status = simulator_port_mux_status[intf_index]
+            upper_tor_status = upper_tor_mux_status['MUX_CABLE'][intf]['STATUS']
+            lower_tor_status = lower_tor_mux_status['MUX_CABLE'][intf]['STATUS']
+
+            if upper_tor_status == 'active' and lower_tor_status == 'standby' and simulator_status['active_side'] == 'upper_tor':
+                continue
+            if upper_tor_status == 'standby' and lower_tor_status == 'active' and simulator_status['active_side'] == 'lower_tor':
+                continue
+            logging.warn(
+                "For interface %s, upper tor mux status: %s, lower tor mux status: %s, simulator status: %s",
+                intf, upper_tor_status, lower_tor_status, simulator_status
+            )
+            logging.warn("Inconsistent mux status for interface %s", intf)
+            return False
+        return True
+
+    if 'dualtor' not in tbinfo['topo']['name']:
+        return
+
     _toggle_all_simulator_ports(mux_server_url, RANDOM)
+    upper_tor_host, lower_tor_host = duthosts[0], duthosts[1]
+    mg_facts = upper_tor_host.get_extended_minigraph_facts(tbinfo)
+    port_indices = mg_facts['minigraph_port_indices']
+    pytest_assert(
+        utilities.wait_until(30, 5, _check_mux_status_consistency),
+        "Mux status is inconsistent between the DUTs and mux simulator after toggle"
+    )
+
 
 @pytest.fixture
 def simulator_server_down(set_drop, set_output):

tests/fib/test_fib.py

@@ -1,20 +1,19 @@
 import time
 import logging
-import random
 
 from datetime import datetime
 
 import pytest
-import requests
 
 from tests.common.fixtures.ptfhost_utils import change_mac_addresses        # lgtm[py/unused-import]
 from tests.common.fixtures.ptfhost_utils import remove_ip_addresses         # lgtm[py/unused-import]
 from tests.common.fixtures.ptfhost_utils import copy_ptftests_directory     # lgtm[py/unused-import]
 from tests.common.fixtures.ptfhost_utils import set_ptf_port_mapping_mode   # lgtm[py/unused-import]
 from tests.common.fixtures.ptfhost_utils import ptf_test_port_map
 from tests.ptf_runner import ptf_runner
-from tests.common.helpers.assertions import pytest_assert
 from tests.common.dualtor.mux_simulator_control import mux_server_url
+from tests.common.dualtor.mux_simulator_control import toggle_all_simulator_ports_to_rand_selected_tor
+from tests.common.dualtor.mux_simulator_control import toggle_all_simulator_ports_to_random_side
 from tests.common.utilities import is_ipv4_address
 
 from tests.common.fixtures.fib_utils import fib_info_files_per_function
@@ -66,27 +65,10 @@ def single_fib_for_duts(tbinfo):
         return True
     return False
 
-def set_mux_side(tbinfo, mux_server_url, side):
-    if 'dualtor' in tbinfo['topo']['name']:
-        res = requests.post(mux_server_url, json={"active_side": side})
-        pytest_assert(res.status_code==200, 'Failed to set active side: {}'.format(res.text))
-        return res.json()   # Response is new mux_status of all mux Y-cables.
-    return {}
-
-
-@pytest.fixture
-def set_mux_random(tbinfo, mux_server_url):
-    return set_mux_side(tbinfo, mux_server_url, 'random')
-
-
-@pytest.fixture
-def set_mux_same_side(tbinfo, mux_server_url):
-    return set_mux_side(tbinfo, mux_server_url, random.choice(['upper_tor', 'lower_tor']))
-
 
 @pytest.mark.parametrize("ipv4, ipv6, mtu", [pytest.param(True, True, 1514)])
 def test_basic_fib(duthosts, ptfhost, ipv4, ipv6, mtu,
-                #    set_mux_random,
+                   toggle_all_simulator_ports_to_random_side,
                    fib_info_files_per_function,
                    tbinfo, mux_server_url, router_macs,
                    ignore_ttl, single_fib_for_duts):
@@ -273,7 +255,7 @@ def add_default_route_to_dut(duts_running_config_facts, duthosts, tbinfo):
 
 
 def test_hash(add_default_route_to_dut, duthosts, fib_info_files_per_function, setup_vlan, hash_keys, ptfhost, ipver,
-              set_mux_same_side,
+              toggle_all_simulator_ports_to_rand_selected_tor,
               tbinfo, mux_server_url, router_macs,
               ignore_ttl, single_fib_for_duts):