[copp] Fix COPP test failures and make clean-up more robust (#1904)

- Add minig_bgp var introduced by 1171
- Disable loganalyzer for LLDP messages
- Updates default swap_syncd behavior to match qos_sai
- Remove docker images after test

Signed-off-by: Danny Allen <[email protected]>

Author: daall

tests/common/system_utils/docker.py

@@ -202,3 +202,8 @@ def restore_default_syncd(dut):
 
     _LOGGER.info("swss has been restarted, waiting 60 seconds to initialize...")
     time.sleep(60)
+
+    # Remove the RPC image from the DUT
+    docker_rpc_image = docker_syncd_name + "-rpc"
+    registry = load_docker_registry_info(dut)
+    dut.command("docker rmi {}/{}:{}".format(registry.host, docker_rpc_image, sonic_version))

tests/copp/conftest.py

@@ -24,8 +24,9 @@ def pytest_addoption(parser):
     )
 
     parser.addoption(
-        "--swap_syncd",
-        action="store_true",
-        default=False,
-        help="Install syncd RPC image for this test case"
+        "--copp_swap_syncd",
+        action="store",
+        type=bool,
+        default=True,
+        help="Swap syncd container with syncd-rpc container",
     )

tests/copp/copp_utils.py

@@ -58,10 +58,11 @@ def configure_ptf(ptf, nn_target_port):
             nn_target_port (int): The port to run NN agent on.
     """
 
+    ptf.script(cmd=_REMOVE_IP_SCRIPT)
     ptf.script(cmd=_ADD_IP_SCRIPT)
 
     facts = {"nn_target_port": nn_target_port}
-    ptf.host.options['variable_manager'].extra_vars.update(facts)
+    ptf.host.options["variable_manager"].extra_vars.update(facts)
     ptf.template(src=_PTF_NN_TEMPLATE, dest=_PTF_NN_DEST)
 
     ptf.supervisorctl(name="ptf_nn_agent", state="restarted")
@@ -77,7 +78,7 @@ def restore_ptf(ptf):
     ptf.script(cmd=_REMOVE_IP_SCRIPT)
 
     facts = {"nn_target_port": DEFAULT_NN_TARGET_PORT}
-    ptf.host.options['variable_manager'].extra_vars.update(facts)
+    ptf.host.options["variable_manager"].extra_vars.update(facts)
 
     ptf.template(src=_PTF_NN_TEMPLATE, dest=_PTF_NN_DEST)
 
@@ -98,7 +99,7 @@ def configure_syncd(dut, nn_target_port):
 
     facts = {"nn_target_port": nn_target_port,
              "nn_target_interface": _map_port_number_to_interface(dut, nn_target_port)}
-    dut.host.options['variable_manager'].extra_vars.update(facts)
+    dut.host.options["variable_manager"].extra_vars.update(facts)
 
     dut.template(src=_SYNCD_NN_TEMPLATE, dest=_SYNCD_NN_DEST)
     dut.command("docker cp {} syncd:/etc/supervisor/conf.d/".format(_SYNCD_NN_DEST))

tests/copp/test_copp.py

@@ -22,30 +22,33 @@
         --pkt_tx_count <n> (int): How many packets to send during each individual test case.
             Default is 100000.
 
-        --swap_syncd: Used to install the RPC syncd image before running the tests. Default
-            is disabled.
+        --copp_swap_syncd: Used to install the RPC syncd image before running the tests. Default
+            is enabled.
 """
 
+import logging
 import time
 import pytest
 from collections import namedtuple
 
-from tests.common.fixtures.ptfhost_utils import copy_ptftests_directory   # lgtm[py/unused-import]
-from tests.common.fixtures.ptfhost_utils import change_mac_addresses      # lgtm[py/unused-import]
 from tests.copp import copp_utils
 from tests.ptf_runner import ptf_runner
 from tests.common.system_utils import docker
-from tests.common.broadcom_data import is_broadcom_device
+
+# Module-level fixtures
+from tests.common.fixtures.ptfhost_utils import copy_ptftests_directory   # lgtm[py/unused-import]
+from tests.common.fixtures.ptfhost_utils import change_mac_addresses      # lgtm[py/unused-import]
 
 pytestmark = [
-    pytest.mark.topology('t1')
+    pytest.mark.topology("t1")
 ]
 
 _COPPTestParameters = namedtuple("_COPPTestParameters",
                                  ["nn_target_port",
                                   "pkt_tx_count",
                                   "swap_syncd",
-                                  "topo"])
+                                  "topo",
+                                  "bgp_graph"])
 _SUPPORTED_TOPOS = ["ptf32", "ptf64", "t1", "t1-lag"]
 _TEST_RATE_LIMIT = 600
 
@@ -58,55 +61,41 @@ class TestCOPP(object):
                                           "IP2ME",
                                           "SNMP",
                                           "SSH"])
-    def test_policer(self, protocol, duthost, ptfhost, _copp_testbed):
+    def test_policer(self, protocol, duthost, ptfhost, copp_testbed):
         """
             Validates that rate-limited COPP groups work as expected.
 
             Checks that the policer enforces the rate limit for protocols
             that have a set rate limit.
         """
-
-        if protocol == "ARP" \
-                and is_broadcom_device(duthost) \
-                and "201811" not in duthost.os_version:
-            pytest.xfail("ARP policy disabled on BRCM devices due to SAI bug")
-
-        if protocol in ["IP2ME", "SNMP", "SSH"] and _copp_testbed.topo == "t1-lag":
-            pytest.xfail("Packets not received due to faulty DIP, see #1171")
-
         _copp_runner(duthost,
                      ptfhost,
                      protocol,
-                     _copp_testbed)
+                     copp_testbed)
 
     @pytest.mark.parametrize("protocol", ["BGP",
                                           "DHCP",
                                           "LACP",
                                           "LLDP",
                                           "UDLD"])
-    def test_no_policer(self, protocol, duthost, ptfhost, _copp_testbed):
+    def test_no_policer(self, protocol, duthost, ptfhost, copp_testbed):
         """
             Validates that non-rate-limited COPP groups work as expected.
 
             Checks that the policer does not enforce a rate limit for protocols
             that do not have any set rate limit.
         """
-
-        if protocol == "BGP" and _copp_testbed.topo == "t1-lag":
-            pytest.xfail("Packets not received due to faulty DIP, see #1171")
-
         _copp_runner(duthost,
                      ptfhost,
                      protocol,
-                     _copp_testbed)
+                     copp_testbed)
 
 @pytest.fixture(scope="class")
-def _copp_testbed(duthost, ptfhost, testbed, request):
+def copp_testbed(duthost, ptfhost, testbed, request):
     """
         Pytest fixture to handle setup and cleanup for the COPP tests.
     """
-
-    test_params = _gather_test_params(testbed, request)
+    test_params = _gather_test_params(testbed, duthost, request)
 
     if test_params.topo not in _SUPPORTED_TOPOS:
         pytest.skip("Topology not supported by COPP tests")
@@ -115,14 +104,35 @@ def _copp_testbed(duthost, ptfhost, testbed, request):
     yield test_params
     _teardown_testbed(duthost, ptfhost, test_params)
 
+@pytest.fixture(autouse=True)
+def ignore_expected_loganalyzer_exceptions(self, duthost, loganalyzer):
+    """
+        Ignore expected failures logs during test execution.
+
+        We disable LLDP during the test, so we expect to see "lldp not running"
+        messages in the logs. All other errors should be treated as errors.
+
+        Args:
+            duthost: DUT fixture
+            loganalyzer: Loganalyzer utility fixture
+    """
+    ignoreRegex = [
+        ".*ERR monit.*'lldpd_monitor' process is not running",
+        ".*ERR monit.*'lldp_syncd' process is not running",
+    ]
+    loganalyzer.ignore_regex.extend(ignoreRegex)
+
+    yield
+
 def _copp_runner(dut, ptf, protocol, test_params):
     """
         Configures and runs the PTF test cases.
     """
 
     params = {"verbose": False,
               "pkt_tx_count": test_params.pkt_tx_count,
-              "target_port": test_params.nn_target_port}
+              "target_port": test_params.nn_target_port,
+              "minig_bgp": test_params.bgp_graph}
 
     dut_ip = dut.setup()["ansible_facts"]["ansible_eth0"]["ipv4"]["address"]
     device_sockets = ["0-{}@tcp://127.0.0.1:10900".format(test_params.nn_target_port),
@@ -141,57 +151,69 @@ def _copp_runner(dut, ptf, protocol, test_params):
                debug_level=None,
                device_sockets=device_sockets)
 
-def _gather_test_params(testbed, request):
+def _gather_test_params(testbed, duthost, request):
     """
         Fetches the test parameters from pytest.
     """
 
     nn_target_port = request.config.getoption("--nn_target_port")
     pkt_tx_count = request.config.getoption("--pkt_tx_count")
-    swap_syncd = request.config.getoption("--swap_syncd")
+    swap_syncd = request.config.getoption("--copp_swap_syncd")
     topo = testbed["topo"]["name"]
+    bgp_graph = duthost.minigraph_facts(host=duthost.hostname)["ansible_facts"]["minigraph_bgp"]
 
     return _COPPTestParameters(nn_target_port=nn_target_port,
                                pkt_tx_count=pkt_tx_count,
                                swap_syncd=swap_syncd,
-                               topo=topo)
+                               topo=topo,
+                               bgp_graph=bgp_graph)
 
 def _setup_testbed(dut, ptf, test_params):
     """
         Sets up the testbed to run the COPP tests.
     """
 
-    # We don't want LLDP to throw off our test results, so we disable it first.
+    logging.info("Disable LLDP for COPP tests")
     dut.command("docker exec lldp supervisorctl stop lldp-syncd")
     dut.command("docker exec lldp supervisorctl stop lldpd")
 
+    logging.info("Set up the PTF for COPP tests")
     copp_utils.configure_ptf(ptf, test_params.nn_target_port)
 
+    logging.info("Update the rate limit for the COPP policer")
     copp_utils.limit_policer(dut, _TEST_RATE_LIMIT)
 
     if test_params.swap_syncd:
+        logging.info("Swap out syncd to use RPC image...")
         docker.swap_syncd(dut)
     else:
         # NOTE: Even if the rpc syncd image is already installed, we need to restart
         # SWSS for the COPP changes to take effect.
+        logging.info("Restart SWSS...")
         _restart_swss(dut)
 
+    logging.info("Configure syncd RPC for testing")
     copp_utils.configure_syncd(dut, test_params.nn_target_port)
 
 def _teardown_testbed(dut, ptf, test_params):
     """
         Tears down the testbed, returning it to its initial state.
     """
 
+    logging.info("Restore PTF post COPP test")
     copp_utils.restore_ptf(ptf)
 
+    logging.info("Restore COPP policer to default settings")
     copp_utils.restore_policer(dut)
 
     if test_params.swap_syncd:
+        logging.info("Restore default syncd docker...")
         docker.restore_default_syncd(dut)
     else:
+        logging.info("Restart SWSS...")
         _restart_swss(dut)
 
+    logging.info("Restore LLDP")
     dut.command("docker exec lldp supervisorctl start lldpd")
     dut.command("docker exec lldp supervisorctl start lldp-syncd")