Refactor fixture recover_acl_rule and and use it in module cacl/test_cacl_function.py. (#9312)

Description of PR
In module cacl/test_cacl_function.py, it will use command acl-loader delete to delete all acl rules, no matter if the testbed has DATAACL rules before test or not. If this testbed has DATAACL rules before, this deletion will cause inconsistent between previous running config and current running config, and will cause unnecessary config reload. In PR #9199 , we have a fixture recover_acl_rule to recovery acl rules. So in this PR, I refactor this fixture and use this fixture to recover acl rules in module cacl/test_cacl_function.py.

What is the motivation for this PR?
In module cacl/test_cacl_function.py, it will use command acl-loader delete to delete all acl rules, no matter if the testbed has DATAACL rules before test or not. If this testbed has DATAACL rules before, this deletion will cause inconsistent between previous running config and current running config, and will cause unnecessary config reload. In PR #9199 , we have a fixture recover_acl_rule to recovery acl rules. So in this PR, I refactor this fixture and use this fixture to recover acl rules in module cacl/test_cacl_function.py.

How did you do it?
Move fixture recover_acl_rule to tests/conftest.py and use this fixture to recover acl rules in module cacl/test_cacl_function.py.

How did you verify/test it?
```
07:34:42 conftest.core_dump_and_config_check      L1893 INFO   | Core dump and config check passed for cacl/test_cacl_function.py
```

Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>

Author: yutongzhang-microsoft

tests/cacl/test_cacl_function.py

@@ -19,7 +19,7 @@
 SONIC_SSH_REGEX = 'OpenSSH_[\\w\\.]+ Debian'
 
 
-def test_cacl_function(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds):
+def test_cacl_function(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds, recover_acl_rule):
     """Test control plane ACL functionality on a SONiC device"""
 
     duthost = duthosts[enum_rand_one_per_hwsku_hostname]

tests/common/templates/default_acl_rules.json

@@ -0,0 +1,38 @@
+{
+    "acl": {
+        "acl-sets": {
+            "acl-set": {
+                "dataacl": {
+                    "acl-entries": {
+                        "acl-entry": {
+                            "1": {
+                                "actions": {
+                                    "config": {
+                                        "forwarding-action": "ACCEPT"
+                                    }
+                                },
+                                "config": {
+                                    "sequence-id": 1
+                                },
+                                "l2": {
+                                    "config": {
+                                        "ethertype": "2048",
+                                        "vlan_id": "1000"
+                                    }
+                                },
+                                "input_interface": {
+                                    "interface_ref":
+                                    {
+                                        "config": {
+                                            "interface": "Ethernet12,Ethernet16,Ethernet20,Ethernet24,Ethernet28,Ethernet32,Ethernet36,Ethernet4,Ethernet40,Ethernet44,Ethernet48,Ethernet52,Ethernet56,Ethernet60,Ethernet64,Ethernet68,Ethernet72,Ethernet76,Ethernet8"
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

tests/conftest.py

@@ -5,6 +5,7 @@
 import getpass
 import random
 import re
+import tempfile
 
 import pytest
 import yaml
@@ -2259,3 +2260,43 @@ def format_failure(port, failure):
 # HACK: We are using set_do_not_care_scapy but it will be deprecated.
 if not hasattr(Mask, "set_do_not_care_scapy"):
     Mask.set_do_not_care_scapy = Mask.set_do_not_care_packet
+
+
+@pytest.fixture(scope="module")
+def recover_acl_rule(duthosts, enum_rand_one_per_hwsku_hostname):
+    duthost = duthosts[enum_rand_one_per_hwsku_hostname]
+
+    base_dir = os.path.dirname(os.path.realpath(__file__))
+    template_dir = os.path.join(base_dir, "common/templates")
+    acl_rules_template = "default_acl_rules.json"
+
+    dut_tmp_dir = "/tmp"
+    dut_conf_file_path = os.path.join(dut_tmp_dir, acl_rules_template)
+
+    pre_acl_rules = duthost.acl_facts()["ansible_facts"]["ansible_acl_facts"]["DATAACL"]["rules"]
+
+    yield
+
+    if pre_acl_rules:
+        for key, value in pre_acl_rules.items():
+            if key != "DEFAULT_RULE":
+                seq_id = key.split('_')[1]
+                acl_config = json.loads(open(os.path.join(template_dir, acl_rules_template)).read())
+                acl_entry_template = \
+                    acl_config["acl"]["acl-sets"]["acl-set"]["dataacl"]["acl-entries"]["acl-entry"]["1"]
+                acl_entry_config = acl_config["acl"]["acl-sets"]["acl-set"]["dataacl"]["acl-entries"]["acl-entry"]
+
+                acl_entry_config[seq_id] = copy.deepcopy(acl_entry_template)
+                acl_entry_config[seq_id]["config"]["sequence-id"] = seq_id
+                acl_entry_config[seq_id]["l2"]["config"]["ethertype"] = value["ETHER_TYPE"]
+                acl_entry_config[seq_id]["l2"]["config"]["vlan_id"] = value["VLAN_ID"]
+                acl_entry_config[seq_id]["input_interface"]["interface_ref"]["config"]["interface"] = value["IN_PORTS"]
+
+        with tempfile.NamedTemporaryFile(suffix=".json", prefix="acl_config", mode="w") as fp:
+            json.dump(acl_config, fp)
+            fp.flush()
+            logger.info("Generating config for ACL rule, ACL table - DATAACL")
+            duthost.template(src=fp.name, dest=dut_conf_file_path, force=True)
+
+        logger.info("Applying {}".format(dut_conf_file_path))
+        duthost.command("acl-loader update full {}".format(dut_conf_file_path))

tests/crm/conftest.py

@@ -159,7 +159,7 @@ def set_polling_interval(duthosts, enum_rand_one_per_hwsku_frontend_hostname):
 
 @pytest.fixture(scope="module")
 def collector(duthosts, enum_rand_one_per_hwsku_frontend_hostname):
-    """ Fixture for sharing variables beatween test cases """
+    """ Fixture for sharing variables between test cases """
     duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]
     data = {}
     for asic in duthost.asics:

tests/crm/test_crm.py

@@ -860,47 +860,6 @@ def recreate_acl_table(duthost, ports):
     duthost.shell_cmds(cmds=cmds)
 
 
-@pytest.fixture
-def recover_acl_rule(duthosts, enum_rand_one_per_hwsku_frontend_hostname, enum_frontend_asic_index, collector):
-    duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]
-    asichost = duthost.asic_instance(enum_frontend_asic_index)
-
-    base_dir = os.path.dirname(os.path.realpath(__file__))
-    template_dir = os.path.join(base_dir, "templates")
-    acl_rules_template = "acl.json"
-
-    dut_tmp_dir = "/tmp-{}".format(asichost.asic_index)
-    dut_conf_file_path = os.path.join(dut_tmp_dir, acl_rules_template)
-
-    pre_acl_rules = duthost.acl_facts()["ansible_facts"]["ansible_acl_facts"]["DATAACL"]["rules"]
-
-    yield
-
-    if pre_acl_rules:
-        for key, value in pre_acl_rules.items():
-            if key != "DEFAULT_RULE":
-                seq_id = key.split('_')[1]
-                acl_config = json.loads(open(os.path.join(template_dir, acl_rules_template)).read())
-                acl_entry_template = \
-                    acl_config["acl"]["acl-sets"]["acl-set"]["dataacl"]["acl-entries"]["acl-entry"]["1"]
-                acl_entry_config = acl_config["acl"]["acl-sets"]["acl-set"]["dataacl"]["acl-entries"]["acl-entry"]
-
-                acl_entry_config[seq_id] = copy.deepcopy(acl_entry_template)
-                acl_entry_config[seq_id]["config"]["sequence-id"] = seq_id
-                acl_entry_config[seq_id]["l2"]["config"]["ethertype"] = value["ETHER_TYPE"]
-                acl_entry_config[seq_id]["l2"]["config"]["vlan_id"] = value["VLAN_ID"]
-                acl_entry_config[seq_id]["input_interface"]["interface_ref"]["config"]["interface"] = value["IN_PORTS"]
-
-        with tempfile.NamedTemporaryFile(suffix=".json", prefix="acl_config", mode="w") as fp:
-            json.dump(acl_config, fp)
-            fp.flush()
-            logger.info("Generating config for ACL rule, ACL table - DATAACL")
-            duthost.template(src=fp.name, dest=dut_conf_file_path, force=True)
-
-        logger.info("Applying {}".format(dut_conf_file_path))
-        duthost.command("acl-loader update full {}".format(dut_conf_file_path))
-
-
 def test_acl_entry(duthosts, enum_rand_one_per_hwsku_frontend_hostname, enum_frontend_asic_index,
                    collector, tbinfo, recover_acl_rule):
     duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]