Refactor DPB-VLAN test cases based on DVSlib

Vasant · Vasant · commit 1c656d9440d0 · 2020-05-07T20:22:22.000Z
diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -1585,7 +1585,7 @@ sai_status_t PortsOrch::removePort(sai_object_id_t port_id)
     sai_status_t status = sai_port_api->remove_port(port_id);
     if (status != SAI_STATUS_SUCCESS)
     {
-        return status; 
+        return status;
     }
 
     m_portCount--;
@@ -2274,13 +2274,13 @@ void PortsOrch::doPortTask(Consumer &consumer)
             if (bridge_port_oid != SAI_NULL_OBJECT_ID)
             {
                 // Bridge port OID is set on a port as long as
-                // port is part of at-least one VLAN. 
-                // Ideally this should be tracked by SAI redis. 
+                // port is part of at-least one VLAN.
+                // Ideally this should be tracked by SAI redis.
                 // Until then, let this snippet be here.
                 SWSS_LOG_WARN("Cannot remove port as bridge port OID is present %lx", bridge_port_oid);
                 it++;
                 continue;
-            } 
+            }
 
             if (m_portList[alias].m_init)
             {
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -15,6 +15,7 @@
 from swsscommon import swsscommon
 from dvslib import dvs_database as dvs_db
 from dvslib import dvs_acl
+from dvslib import dvs_vlan
 
 def ensure_system(cmd):
     (rc, output) = commands.getstatusoutput(cmd)
@@ -1009,6 +1010,14 @@ def testlog(request, dvs):
     yield testlog
     dvs.runcmd("logger === finish test %s ===" % request.node.name)
 
+@pytest.yield_fixture(scope="class")
+def dvs_vlan_manager(request, dvs):
+    request.cls.dvs_vlan = dvs_vlan.DVSVlan(dvs.get_asic_db(),
+                                            dvs.get_config_db(),
+                                            dvs.get_state_db(),
+                                            dvs.get_counters_db(),
+                                            dvs.get_app_db())
+
 @pytest.yield_fixture(scope="class")
 def dvs_acl_manager(request, dvs):
     request.cls.dvs_acl = dvs_acl.DVSAcl(dvs.get_asic_db(),
diff --git a/tests/dvslib/dvs_vlan.py b/tests/dvslib/dvs_vlan.py
@@ -0,0 +1,94 @@
+from dvs_database import DVSDatabase
+
+class DVSVlan(object):
+    def __init__(self, adb, cdb, sdb, cntrdb, appdb):
+        self.asic_db = adb
+        self.config_db = cdb
+        self.state_db = sdb
+        self.counters_db = cntrdb
+        self.app_db = appdb
+
+    def create_vlan(self, vlan):
+        vlan = "Vlan{}".format(vlan)
+        vlan_entry = {"vlanid": vlan}
+        self.config_db.create_entry("VLAN", vlan, vlan_entry)
+
+    def remove_vlan(self, vlan):
+        vlan = "Vlan{}".format(vlan)
+        self.config_db.delete_entry("VLAN", vlan)
+
+    def create_vlan_member(self, vlan, interface, tagging_mode="untagged"):
+        member = "Vlan{}|{}".format(vlan, interface)
+        if tagging_mode:
+            member_entry = {"tagging_mode": tagging_mode}
+        else:
+            member_entry = {"no_tag_mode": ""}
+
+        self.config_db.create_entry("VLAN_MEMBER", member, member_entry)
+
+    def remove_vlan_member(self, vlan, interface):
+        member = "Vlan{}|{}".format(vlan, interface)
+        self.config_db.delete_entry("VLAN_MEMBER", member)
+
+    def create_port_channel(self, lag_id, admin_status="up", mtu="1500"):
+        lag = "PortChannel{}".format(lag_id)
+        lag_entry = {"admin_status": admin_status, "mtu": mtu}
+        self.config_db.create_entry("PORTCHANNEL", lag, lag_entry)
+
+    def remove_port_channel(self, lag_id):
+        lag = "PortChannel{}".format(lag_id)
+        self.config_db.delete_entry("PORTCHANNEL", lag)
+
+    def create_port_channel_member(self, lag_id, interface):
+        member = "PortChannel{}|{}".format(lag_id, interface)
+        member_entry = {"NULL": "NULL"}
+        self.config_db.create_entry("PORTCHANNEL_MEMBER", member, member_entry)
+
+    def remove_port_channel_member(self, lag_id, interface):
+        member = "PortChannel{}|{}".format(lag_id, interface)
+        self.config_db.delete_entry("PORTCHANNEL_MEMBER", member)
+
+    def check_app_db_vlan_fields(self, fvs, admin_status="up", mtu="9100"):
+        assert fvs.get("admin_status") == admin_status
+        assert fvs.get("mtu") == mtu
+
+    def check_app_db_vlan_member_fields(self, fvs, tagging_mode="untagged"):
+        assert fvs.get("tagging_mode") == tagging_mode
+
+    def check_state_db_vlan_fields(self, fvs, state="ok"):
+        assert fvs.get("state") == state
+
+    def check_state_db_vlan_member_fields(self, fvs, state="ok"):
+        assert fvs.get("state") == state
+
+    def verify_vlan(self, vlan_oid, vlan_id):
+        vlan = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_VLAN", vlan_oid)
+        assert vlan.get("SAI_VLAN_ATTR_VLAN_ID") == vlan_id
+
+    def get_and_verify_vlan_ids(self,
+                                expected_num,
+                                polling_config=DVSDatabase.DEFAULT_POLLING_CONFIG):
+        vlan_entries = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_VLAN",
+                                                    expected_num + 1,
+                                                    polling_config)
+        return [v for v in vlan_entries if v != self.asic_db.default_vlan_id]
+
+    def verify_vlan_member(self, vlan_oid, iface, tagging_mode="SAI_VLAN_TAGGING_MODE_UNTAGGED"):
+        member_ids = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER", 1)
+        member = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER", member_ids[0])
+        assert member == {"SAI_VLAN_MEMBER_ATTR_VLAN_TAGGING_MODE": tagging_mode,
+                          "SAI_VLAN_MEMBER_ATTR_VLAN_ID": vlan_oid,
+                          "SAI_VLAN_MEMBER_ATTR_BRIDGE_PORT_ID": self.get_bridge_port_id(iface)}
+
+    def get_and_verify_vlan_member_ids(self, expected_num):
+        return self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER", expected_num)
+
+    def get_bridge_port_id(self, expected_iface):
+        bridge_port_id = self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT", 1)[0]
+        bridge_port = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT", bridge_port_id)
+        #TBD: port_to_id_map may NOT be updated one in case port is deleted and re-created.
+        #     Hence the map needs refreshed. Need to think trough and decide when and where
+        #     to do it.
+        assert self.asic_db.port_to_id_map[bridge_port["SAI_BRIDGE_PORT_ATTR_PORT_ID"]] == expected_iface
+        return bridge_port_id
+
diff --git a/tests/test_fdb_update.py b/tests/test_fdb_update.py
@@ -265,7 +265,7 @@ def test_FDBLearnedAndUpdated(self, dvs, testlog):
         # clear fdb
         dvs.runcmd("sonic-clear fdb all")
 
-    def test_FDBLearnedAndFlushed(dvs, testlog):
+    def test_FDBLearnedAndFlushed(self, dvs, testlog):
         dvs.setup_db()
 
         dvs.runcmd("sonic-clear fdb all")
diff --git a/tests/test_port_dpb_vlan.py b/tests/test_port_dpb_vlan.py
@@ -1,15 +1,10 @@
-from swsscommon import swsscommon
-import redis
 import time
-import os
 import pytest
-from pytest import *
-import json
-import re
 from port_dpb import Port
 from port_dpb import DPB
 
 @pytest.mark.usefixtures('dpb_setup_fixture')
+@pytest.mark.usefixtures('dvs_vlan_manager')
 class TestPortDPBVlan(object):
     def check_syslog(self, dvs, marker, log, expected_cnt):
         (exitcode, num) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \"%s\" | wc -l" % (marker, log)])
@@ -19,13 +14,13 @@ def check_syslog(self, dvs, marker, log, expected_cnt):
     @pytest.mark.skip()
     '''
     def test_dependency(self, dvs):
-        dpb = DPB()
-        dvs.setup_db()
+
+        vlan = "100"
         p = Port(dvs, "Ethernet0")
         p.sync_from_config_db()
-        dvs.create_vlan("100")
+        self.dvs_vlan.create_vlan(vlan)
         #print "Created VLAN100"
-        dvs.create_vlan_member("100", p.get_name())
+        self.dvs_vlan.create_vlan_member(vlan, p.get_name())
         #print "Added Ethernet0 to VLAN100"
         marker = dvs.add_log_marker()
         p.delete_from_config_db()
@@ -34,7 +29,7 @@ def test_dependency(self, dvs):
         self.check_syslog(dvs, marker, "Cannot remove port as bridge port OID is present", 1)
         assert(p.exists_in_asic_db() == True)
 
-        dvs.remove_vlan_member("100", p.get_name())
+        self.dvs_vlan.remove_vlan_member(vlan, p.get_name())
         time.sleep(1)
         # Verify that port is deleted
         assert(p.exists_in_asic_db() == False)
@@ -49,19 +44,20 @@ def test_dependency(self, dvs):
         p.verify_asic_db()
         #print "ASIC DB verification passed!"
 
-        dvs.remove_vlan("100")
+        self.dvs_vlan.remove_vlan(vlan)
 
     '''
     @pytest.mark.skip()
     '''
     def test_one_port_one_vlan(self, dvs):
         dpb = DPB()
-        dvs.setup_db()
+        vlan = "100"
 
         # Breakout testing with VLAN dependency
-        dvs.create_vlan("100")
+        self.dvs_vlan.create_vlan(vlan)
         #print "Created VLAN100"
-        dvs.create_vlan_member("100", "Ethernet0")
+        self.dvs_vlan.create_vlan_member(vlan, "Ethernet0")
+        self.dvs_vlan.get_and_verify_vlan_member_ids(1)
         #print "Added Ethernet0 to VLAN100"
 
         p = Port(dvs, "Ethernet0")
@@ -72,17 +68,20 @@ def test_one_port_one_vlan(self, dvs):
         assert(p.exists_in_asic_db() == True)
         #print "Ethernet0 deleted from config DB and APP DB, waiting to be removed from VLAN"
 
-        dvs.remove_vlan_member("100", "Ethernet0")
-        time.sleep(1)
+        self.dvs_vlan.remove_vlan_member(vlan, "Ethernet0")
+        self.dvs_vlan.get_and_verify_vlan_member_ids(0)
         assert(p.exists_in_asic_db() == False)
         #print "Ethernet0 removed from VLAN and also from ASIC DB"
 
         dpb.create_child_ports(dvs, p, 4)
 
         # Breakin testing with VLAN dependency
         port_names = ["Ethernet0", "Ethernet1", "Ethernet2", "Ethernet3"]
+        vlan_member_count = 0
         for pname in port_names:
-            dvs.create_vlan_member("100", pname)
+            self.dvs_vlan.create_vlan_member(vlan, pname)
+            vlan_member_count = vlan_member_count + 1
+            self.dvs_vlan.get_and_verify_vlan_member_ids(vlan_member_count)
         #print "Add %s to VLAN"%port_names
 
         child_ports = []
@@ -97,8 +96,9 @@ def test_one_port_one_vlan(self, dvs):
         #print "Deleted %s from config DB and APP DB"%port_names
 
         for cp in child_ports:
-            dvs.remove_vlan_member("100", cp.get_name())
-            time.sleep(1)
+            self.dvs_vlan.remove_vlan_member(vlan, cp.get_name())
+            vlan_member_count = vlan_member_count - 1
+            self.dvs_vlan.get_and_verify_vlan_member_ids(vlan_member_count)
             assert(cp.exists_in_asic_db() == False)
         #print "Deleted %s from VLAN"%port_names
 
@@ -111,22 +111,22 @@ def test_one_port_one_vlan(self, dvs):
         p.verify_asic_db()
         #print "ASIC DB verification passed!"
 
-        dvs.remove_vlan("100")
+        self.dvs_vlan.remove_vlan(vlan)
 
     '''
     @pytest.mark.skip()
     '''
     def test_one_port_multiple_vlan(self, dvs):
-        dpb = DPB()
-        dvs.setup_db()
 
-        dvs.create_vlan("100")
-        dvs.create_vlan("101")
-        dvs.create_vlan("102")
+        dpb = DPB()
+        vlans = ["100", "101", "102"]
+        for vlan in vlans:
+            self.dvs_vlan.create_vlan(vlan)
         #print "Created VLAN100, VLAN101, and VLAN102"
-        dvs.create_vlan_member("100", "Ethernet0")
-        dvs.create_vlan_member("101", "Ethernet0")
-        dvs.create_vlan_member("102", "Ethernet0")
+
+        for vlan in vlans:
+            self.dvs_vlan.create_vlan_member(vlan, "Ethernet0")
+        self.dvs_vlan.get_and_verify_vlan_member_ids(len(vlans))
         #print "Added Ethernet0 to all three VLANs"
 
         p = Port(dvs, "Ethernet0")
@@ -137,20 +137,20 @@ def test_one_port_multiple_vlan(self, dvs):
         assert(p.exists_in_asic_db() == True)
         #print "Ethernet0 deleted from config DB and APP DB, waiting to be removed from VLANs"
 
-        dvs.remove_vlan_member("100", "Ethernet0")
-        time.sleep(1)
+        self.dvs_vlan.remove_vlan_member(vlans[0], "Ethernet0")
+        self.dvs_vlan.get_and_verify_vlan_member_ids(len(vlans)-1)
         assert(p.exists_in_asic_db() == True)
         #print "Ethernet0 removed from VLAN100 and its still present in ASIC DB"
 
-        dvs.remove_vlan_member("101", "Ethernet0")
-        time.sleep(1)
+        self.dvs_vlan.remove_vlan_member(vlans[1], "Ethernet0")
+        self.dvs_vlan.get_and_verify_vlan_member_ids(len(vlans)-2)
         assert(p.exists_in_asic_db() == True)
         #print "Ethernet0 removed from VLAN101 and its still present in ASIC DB"
 
-        dvs.remove_vlan_member("102", "Ethernet0")
-        time.sleep(1)
+        self.dvs_vlan.remove_vlan_member(vlans[2], "Ethernet0")
+        self.dvs_vlan.get_and_verify_vlan_member_ids(0)
         assert(p.exists_in_asic_db() == False)
-        #print "Ethernet0 removed from VLAN101 and also from ASIC DB"
+        #print "Ethernet0 removed from VLAN102 and also from ASIC DB"
 
         dpb.create_child_ports(dvs, p, 4)
         #print "1X40G ---> 4x10G verified"
@@ -174,9 +174,9 @@ def test_one_port_multiple_vlan(self, dvs):
         #print "Added port:%s and verified all DBs"%p.get_name()
 
         # Remove all three VLANs
-        dvs.remove_vlan("100")
-        dvs.remove_vlan("101")
-        dvs.remove_vlan("102")
+        self.dvs_vlan.remove_vlan("100")
+        self.dvs_vlan.remove_vlan("101")
+        self.dvs_vlan.remove_vlan("102")
         #print "All three VLANs removed"
 
     '''
@@ -197,13 +197,14 @@ def test_all_port_10_vlans(self, dvs):
             vlan_names.append(str(start_vlan + i))
 
         for vlan_name in vlan_names:
-            dvs.create_vlan(vlan_name)
+            self.dvs_vlan.create_vlan(vlan_name)
         #print "%d VLANs created"%num_vlans
 
         for port_name in port_names:
             for vlan_name in vlan_names:
-                dvs.create_vlan_member_tagged(vlan_name, port_name)
+                self.dvs_vlan.create_vlan_member(vlan_name, port_name, tagging_mode = "tagged")
         #print "All %d ports are added to all %d VLANs"%(num_ports,num_vlans)
+        self.dvs_vlan.get_and_verify_vlan_member_ids(num_ports*num_vlans)
 
         ports = []
         for port_name in port_names:
@@ -216,8 +217,9 @@ def test_all_port_10_vlans(self, dvs):
             assert(p.exists_in_app_db() == False)
             assert(p.exists_in_asic_db() == True)
             for vlan_name in vlan_names:
-                dvs.remove_vlan_member(vlan_name, port_name)
-            time.sleep(1)
+                self.dvs_vlan.remove_vlan_member(vlan_name, port_name)
+
+            self.dvs_vlan.get_and_verify_vlan_member_ids((num_ports*num_vlans)-(len(ports)*num_vlans))
             assert(p.exists_in_asic_db() == False)
         #print "All %d ports are removed from all %d VLANs and deleted"%(num_ports,num_vlans)
 
@@ -229,5 +231,5 @@ def test_all_port_10_vlans(self, dvs):
         #print "Re-created all %d ports"%num_ports
 
         for vlan_name in vlan_names:
-            dvs.remove_vlan(vlan_name)
+            self.dvs_vlan.remove_vlan(vlan_name)
         #print "All %d VLANs removed"%num_vlans
diff --git a/tests/test_vlan.py b/tests/test_vlan.py

Original file line number	Diff line number	Diff line change
`@@ -1585,7 +1585,7 @@ sai_status_t PortsOrch::removePort(sai_object_id_t port_id)`
`1585`	`1585`	`sai_status_t status = sai_port_api->remove_port(port_id);`
`1586`	`1586`	`if (status != SAI_STATUS_SUCCESS)`
`1587`	`1587`	`{`
`1588`		`- return status;`
	`1588`	`+ return status;`
`1589`	`1589`	`}`
`1590`	`1590`
`1591`	`1591`	`m_portCount--;`
`@@ -2274,13 +2274,13 @@ void PortsOrch::doPortTask(Consumer &consumer)`
`2274`	`2274`	`if (bridge_port_oid != SAI_NULL_OBJECT_ID)`
`2275`	`2275`	`{`
`2276`	`2276`	`// Bridge port OID is set on a port as long as`
`2277`		`- // port is part of at-least one VLAN.`
`2278`		`- // Ideally this should be tracked by SAI redis.`
	`2277`	`+ // port is part of at-least one VLAN.`
	`2278`	`+ // Ideally this should be tracked by SAI redis.`
`2279`	`2279`	`// Until then, let this snippet be here.`
`2280`	`2280`	`SWSS_LOG_WARN("Cannot remove port as bridge port OID is present %lx", bridge_port_oid);`
`2281`	`2281`	`it++;`
`2282`	`2282`	`continue;`
`2283`		`- }`
	`2283`	`+ }`
`2284`	`2284`
`2285`	`2285`	`if (m_portList[alias].m_init)`
`2286`	`2286`	`{`