Don't create the members@ array in config_db for PC when reading from…

… minigraph (#13660) Fixes #11873. #### Why I did it When loading from minigraph, for port channels, don't create the members@ array in config_db in the PORTCHANNEL table. This is no longer needed or used. In addition, when adding a port channel member from the CLI, that member doesn't get added into the members@ array, resulting in a bit of inconsistency. This gets rid of that inconsistency.
sonic-net · Mar 20, 2023 · 60067e7 · 60067e7
1 parent 7b61e89
commit 60067e7
Show file tree

Hide file tree

Showing 10 changed files with 30 additions and 96 deletions.
diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py
@@ -532,9 +532,9 @@ def parse_dpg(dpg, hname):
                 intfs_inpc.append(pcmbr_list[i])
                 pc_members[(pcintfname, pcmbr_list[i])] = {}
             if pcintf.find(str(QName(ns, "Fallback"))) != None:
-                pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text, 'min_links': str(int(math.ceil(len() * 0.75))), 'lacp_key': 'auto'}
+                pcs[pcintfname] = {'fallback': pcintf.find(str(QName(ns, "Fallback"))).text, 'min_links': str(int(math.ceil(len() * 0.75))), 'lacp_key': 'auto'}
             else:
-                pcs[pcintfname] = {'members': pcmbr_list, 'min_links': str(int(math.ceil(len(pcmbr_list) * 0.75))), 'lacp_key': 'auto' }
+                pcs[pcintfname] = {'min_links': str(int(math.ceil(len(pcmbr_list) * 0.75))), 'lacp_key': 'auto' }
         port_nhipv4_map = {}
         port_nhipv6_map = {}
         nhg_int = ""
@@ -1244,7 +1244,7 @@ def filter_acl_table_for_backend(acls, vlan_members):
                              }
     return filter_acls
 
-def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role, device_type, is_storage_device, vlan_members):
+def filter_acl_table_bindings(acls, neighbors, port_channels, pc_members, sub_role, device_type, is_storage_device, vlan_members):
     if device_type == 'BackEndToRRouter' and is_storage_device:
         return filter_acl_table_for_backend(acls, vlan_members)
 
@@ -1263,8 +1263,8 @@ def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role, device_t
 
     # Get the front panel port channel.
     for port_channel_intf in port_channels:
-        backend_port_channel = any(lag_member in backplane_port_list \
-                                   for lag_member in port_channels[port_channel_intf]['members'])
+        backend_port_channel = any(lag_member[1] in backplane_port_list \
+                                   for lag_member in list(pc_members.keys()) if lag_member[0] == port_channel_intf)
         if not backend_port_channel:
             front_port_channel_intf.append(port_channel_intf)
 
@@ -1777,12 +1777,15 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
 
     if port_config_file:
         port_set = set(ports.keys())
-        for (pc_name, mbr_map) in list(pcs.items()):
+        for (pc_name, pc_member) in list(pc_members.keys()):
             # remove portchannels that contain ports not existing in port_config.ini
             # when port_config.ini exists
-            if not set(mbr_map['members']).issubset(port_set):
-                print("Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name, file=sys.stderr)
+            if (pc_name, pc_member) in pc_members and pc_member not in port_set:
+                print("Warning: ignore '%s' as at least one of its member interfaces ('%s') is not in the port_config.ini" % (pc_name, pc_member), file=sys.stderr)
                 del pcs[pc_name]
+                pc_mbr_del_keys = [f for f in list(pc_members.keys()) if f[0] == pc_name]
+                for pc_mbr_del_key in pc_mbr_del_keys:
+                    del pc_members[pc_mbr_del_key]
 
     # set default port channel MTU as 9100 and admin status up and default TPID 0x8100
     for pc in pcs.values():
@@ -1886,7 +1889,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
     results['DHCP_RELAY'] = dhcp_relay_table
     results['NTP_SERVER'] = dict((item, {}) for item in ntp_servers)
     results['TACPLUS_SERVER'] = dict((item, {'priority': '1', 'tcp_port': '49'}) for item in tacacs_servers)
-    results['ACL_TABLE'] = filter_acl_table_bindings(acls, neighbors, pcs, sub_role, current_device['type'], is_storage_device, vlan_members)
+    results['ACL_TABLE'] = filter_acl_table_bindings(acls, neighbors, pcs, pc_members, sub_role, current_device['type'], is_storage_device, vlan_members)
     results['FEATURE'] = {
         'telemetry': {
             'state': 'enabled'

diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py
@@ -330,15 +330,15 @@ def test_minigraph_portchannels(self, **kwargs):
         output = self.run_script(argument)
         self.assertEqual(
             utils.to_dict(output.strip()),
-            utils.to_dict("{'PortChannel1': {'admin_status': 'up', 'min_links': '1', 'members': ['Ethernet4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
+            utils.to_dict("{'PortChannel1': {'admin_status': 'up', 'min_links': '1', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
         )
 
     def test_minigraph_portchannel_with_more_member(self):
         argument = ['-m', self.sample_graph_pc_test, '-p', self.port_config, '-v', 'PORTCHANNEL']
         output = self.run_script(argument)
         self.assertEqual(
             utils.to_dict(output.strip()),
-            utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '3', 'members': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}"))
+            utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '3', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}"))
 
     def test_minigraph_portchannel_members(self):
         argument = ['-m', self.sample_graph_pc_test, '-p', self.port_config, '-v', "PORTCHANNEL_MEMBER.keys()|list"]

diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py
@@ -158,7 +158,7 @@ def test_minigraph_portchannels(self):
         output = self.run_script(argument)
         self.assertEqual(
             utils.to_dict(output.strip()),
-            utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '1', 'members': ['Ethernet4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
+            utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '1', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
         )
 
     def test_minigraph_console_mgmt_feature(self):

diff --git a/src/sonic-config-engine/tests/test_multinpu_cfggen.py b/src/sonic-config-engine/tests/test_multinpu_cfggen.py
@@ -161,16 +161,16 @@ def test_frontend_asic_portchannels(self):
         argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "--var-json", "PORTCHANNEL"]
         output = json.loads(self.run_script(argument))
         self.assertDictEqual(output, \
-                {'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet0', 'Ethernet4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
-                 'PortChannel4001': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP0', 'Ethernet-BP4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
-                 'PortChannel4002': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP8', 'Ethernet-BP12'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
+                {'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
+                 'PortChannel4001': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
+                 'PortChannel4002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
 
     def test_backend_asic_portchannels(self):
         argument = ["-m", self.sample_graph, "-p", self.port_config[3], "-n", "asic3", "--var-json", "PORTCHANNEL"]
         output = json.loads(self.run_script(argument))
         self.assertDictEqual(output, \
-                {'PortChannel4013': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP384', 'Ethernet-BP388'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
-                 'PortChannel4014': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP392', 'Ethernet-BP396'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
+                {'PortChannel4013': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
+                 'PortChannel4014': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
 
     def test_frontend_asic_portchannel_mem(self):
         argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "-v", "PORTCHANNEL_MEMBER.keys()|list"]

diff --git a/src/sonic-yang-mgmt/tests/test_cfghelp.py b/src/sonic-yang-mgmt/tests/test_cfghelp.py
@@ -71,17 +71,17 @@
 
 """
 
-portchannel_table_field_output="""\
+vlan_table_field_output="""\
 
-PORTCHANNEL
-Description: PORTCHANNEL part of config_db.json
+VLAN
+Description: VLAN part of config_db.json
 
 key - name
-+---------+-------------------------------------------+-------------+-----------+-------------+
-| Field   | Description                               | Mandatory   | Default   | Reference   |
-+=========+===========================================+=============+===========+=============+
-| members | The field contains list of unique members |             |           | PORT:name   |
-+---------+-------------------------------------------+-------------+-----------+-------------+
++--------------+------------------------------------------------------------------------+-------------+-----------+-------------+
+| Field        | Description                                                            | Mandatory   | Default   | Reference   |
++==============+========================================================================+=============+===========+=============+
+| dhcp_servers | The field contains list of unique membersConfigure the dhcp v4 servers |             |           |             |
++--------------+------------------------------------------------------------------------+-------------+-----------+-------------+
 
 """
 
@@ -153,9 +153,9 @@ def test_single_field(self):
         self.assertEqual(output, techsupport_table_field_output)
 
     def test_leaf_list(self):
-        argument = ['-t', 'PORTCHANNEL', '-f', 'members']
+        argument = ['-t', 'VLAN', '-f', 'dhcp_servers']
         output = self.run_script(argument)
-        self.assertEqual(output, portchannel_table_field_output)
+        self.assertEqual(output, vlan_table_field_output)
 
     def test_leaf_list_map(self):
         argument = ['-t', 'DSCP_TO_TC_MAP']

diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json
@@ -82,39 +82,26 @@
             "PortChannel0003": {
                 "admin_status": "up",
                 "min_links": "1",
-                "members": [
-                    "Ethernet1"
-                ],
                 "tpid": "0x8100",
                 "mtu": "9100",
                 "lacp_key": "auto"
             },
             "PortChannel0004": {
                 "admin_status": "up",
                 "min_links": "1",
-                "members": [
-                    "Ethernet2"
-                ],
                 "tpid": "0x9200",
                 "mtu": "9100",
                 "lacp_key": "auto"
             },
             "PortChannel2": {
                 "admin_status": "up",
                 "min_links": "1",
-                "members": [
-                    "Ethernet12"
-                ],
                 "tpid": "0x9200",
                 "mtu": "9100",
                 "lacp_key": "auto"
             },
             "PortChannel42": {
                 "admin_status": "up",
-                "members": [
-                    "Ethernet-BP0",
-                    "Ethernet-BP4"
-                ],
                 "min_links": "2",
                 "mtu": "9100",
                 "tpid": "0x8100",

diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/mclag.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/mclag.json
@@ -29,19 +29,13 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "lacp_key": "auto",
                         "name": "PortChannel2"
                     },
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet10"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "lacp_key": "auto",
@@ -151,9 +145,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "lacp_key": "auto",

diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/portchannel.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/portchannel.json
@@ -21,9 +21,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "tpid": "0x8100",
@@ -57,9 +54,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1024",
                         "mtu": "9100",
                         "name": "PortChannel0001"
@@ -89,9 +83,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1025",
                         "mtu": "9100",
                         "name": "PortChannel0001"
@@ -335,9 +326,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "name": "PortChannel0001"
@@ -382,9 +370,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "name": "PortChannel0001"
@@ -428,9 +413,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "name": "PortChannel0001"

diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan_sub_interface.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan_sub_interface.json
@@ -196,9 +196,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "tpid": "0x8100",
@@ -246,9 +243,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "tpid": "0x8100",
@@ -296,9 +290,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "tpid": "0x8100",
@@ -346,9 +337,6 @@
                 "PORTCHANNEL_LIST": [
                     {
                         "admin_status": "up",
-                        "members": [
-                            "Ethernet0"
-                        ],
                         "min_links": "1",
                         "mtu": "9100",
                         "tpid": "0x8100",

diff --git a/src/sonic-yang-models/yang-models/sonic-portchannel.yang b/src/sonic-yang-models/yang-models/sonic-portchannel.yang
@@ -58,23 +58,6 @@ module sonic-portchannel {
                     }
                 }
 
-                leaf-list members {
-                    /* leaf-list members are unique by default */
-                    type union {
-                        type leafref {
-                            path /port:sonic-port/port:PORT/port:PORT_LIST/port:name;
-                        }
-                        type string {
-                            pattern "";
-                        }
-                    }
-                    /* Today in SONiC, we do not delete the list once
-                     * created, instead we set to empty list. Due to that
-                     * below default values are needed.
-                     */
-                    default "";
-                }
-
                 leaf min_links {
                     type uint16 {
                         range 1..1024;