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.
This commit is contained in:
Saikrishna Arcot 2023-02-23 14:49:51 -08:00 committed by GitHub
parent f4550e8b89
commit 3e316cbf24
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 30 additions and 96 deletions

View File

@ -532,9 +532,9 @@ def parse_dpg(dpg, hname):
intfs_inpc.append(pcmbr_list[i]) intfs_inpc.append(pcmbr_list[i])
pc_members[(pcintfname, pcmbr_list[i])] = {} pc_members[(pcintfname, pcmbr_list[i])] = {}
if pcintf.find(str(QName(ns, "Fallback"))) != None: 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: 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_nhipv4_map = {}
port_nhipv6_map = {} port_nhipv6_map = {}
nhg_int = "" nhg_int = ""
@ -1244,7 +1244,7 @@ def filter_acl_table_for_backend(acls, vlan_members):
} }
return filter_acls 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: if device_type == 'BackEndToRRouter' and is_storage_device:
return filter_acl_table_for_backend(acls, vlan_members) 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. # Get the front panel port channel.
for port_channel_intf in port_channels: for port_channel_intf in port_channels:
backend_port_channel = any(lag_member in backplane_port_list \ backend_port_channel = any(lag_member[1] in backplane_port_list \
for lag_member in port_channels[port_channel_intf]['members']) for lag_member in list(pc_members.keys()) if lag_member[0] == port_channel_intf)
if not backend_port_channel: if not backend_port_channel:
front_port_channel_intf.append(port_channel_intf) front_port_channel_intf.append(port_channel_intf)
@ -1763,12 +1763,15 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
if port_config_file: if port_config_file:
port_set = set(ports.keys()) 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 # remove portchannels that contain ports not existing in port_config.ini
# when port_config.ini exists # when port_config.ini exists
if not set(mbr_map['members']).issubset(port_set): if (pc_name, pc_member) in pc_members and pc_member not in port_set:
print("Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name, file=sys.stderr) 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] 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 # set default port channel MTU as 9100 and admin status up and default TPID 0x8100
for pc in pcs.values(): for pc in pcs.values():
@ -1872,7 +1875,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
results['DHCP_RELAY'] = dhcp_relay_table results['DHCP_RELAY'] = dhcp_relay_table
results['NTP_SERVER'] = dict((item, {}) for item in ntp_servers) 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['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'] = { results['FEATURE'] = {
'telemetry': { 'telemetry': {
'state': 'enabled' 'state': 'enabled'

View File

@ -328,7 +328,7 @@ class TestCfgGen(TestCase):
output = self.run_script(argument) output = self.run_script(argument)
self.assertEqual( self.assertEqual(
utils.to_dict(output.strip()), 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): def test_minigraph_portchannel_with_more_member(self):
@ -336,7 +336,7 @@ class TestCfgGen(TestCase):
output = self.run_script(argument) output = self.run_script(argument)
self.assertEqual( self.assertEqual(
utils.to_dict(output.strip()), 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): def test_minigraph_portchannel_members(self):
argument = ['-m', self.sample_graph_pc_test, '-p', self.port_config, '-v', "PORTCHANNEL_MEMBER.keys()|list"] argument = ['-m', self.sample_graph_pc_test, '-p', self.port_config, '-v', "PORTCHANNEL_MEMBER.keys()|list"]

View File

@ -158,7 +158,7 @@ class TestCfgGenCaseInsensitive(TestCase):
output = self.run_script(argument) output = self.run_script(argument)
self.assertEqual( self.assertEqual(
utils.to_dict(output.strip()), 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): def test_minigraph_console_mgmt_feature(self):

View File

@ -161,16 +161,16 @@ class TestMultiNpuCfgGen(TestCase):
argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "--var-json", "PORTCHANNEL"] argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "--var-json", "PORTCHANNEL"]
output = json.loads(self.run_script(argument)) output = json.loads(self.run_script(argument))
self.assertDictEqual(output, \ self.assertDictEqual(output, \
{'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet0', 'Ethernet4'], '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', 'members': ['Ethernet-BP0', 'Ethernet-BP4'], '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', 'members': ['Ethernet-BP8', 'Ethernet-BP12'], '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): def test_backend_asic_portchannels(self):
argument = ["-m", self.sample_graph, "-p", self.port_config[3], "-n", "asic3", "--var-json", "PORTCHANNEL"] argument = ["-m", self.sample_graph, "-p", self.port_config[3], "-n", "asic3", "--var-json", "PORTCHANNEL"]
output = json.loads(self.run_script(argument)) output = json.loads(self.run_script(argument))
self.assertDictEqual(output, \ self.assertDictEqual(output, \
{'PortChannel4013': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP384', 'Ethernet-BP388'], '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', 'members': ['Ethernet-BP392', 'Ethernet-BP396'], '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): def test_frontend_asic_portchannel_mem(self):
argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "-v", "PORTCHANNEL_MEMBER.keys()|list"] argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "-v", "PORTCHANNEL_MEMBER.keys()|list"]

View File

@ -71,17 +71,17 @@ Description: AUTO_TECHSUPPORT part of config_db.json
""" """
portchannel_table_field_output="""\ vlan_table_field_output="""\
PORTCHANNEL VLAN
Description: PORTCHANNEL part of config_db.json Description: VLAN part of config_db.json
key - name key - name
+---------+-------------------------------------------+-------------+-----------+-------------+ +--------------+------------------------------------------------------------------------+-------------+-----------+-------------+
| Field | Description | Mandatory | Default | Reference | | Field | Description | Mandatory | Default | Reference |
+=========+===========================================+=============+===========+=============+ +==============+========================================================================+=============+===========+=============+
| members | The field contains list of unique members | | | PORT:name | | dhcp_servers | The field contains list of unique membersConfigure the dhcp v4 servers | | | |
+---------+-------------------------------------------+-------------+-----------+-------------+ +--------------+------------------------------------------------------------------------+-------------+-----------+-------------+
""" """
@ -153,9 +153,9 @@ class TestCfgHelp(TestCase):
self.assertEqual(output, techsupport_table_field_output) self.assertEqual(output, techsupport_table_field_output)
def test_leaf_list(self): def test_leaf_list(self):
argument = ['-t', 'PORTCHANNEL', '-f', 'members'] argument = ['-t', 'VLAN', '-f', 'dhcp_servers']
output = self.run_script(argument) 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): def test_leaf_list_map(self):
argument = ['-t', 'DSCP_TO_TC_MAP'] argument = ['-t', 'DSCP_TO_TC_MAP']

View File

@ -86,9 +86,6 @@
"PortChannel0003": { "PortChannel0003": {
"admin_status": "up", "admin_status": "up",
"min_links": "1", "min_links": "1",
"members": [
"Ethernet1"
],
"tpid": "0x8100", "tpid": "0x8100",
"mtu": "9100", "mtu": "9100",
"lacp_key": "auto" "lacp_key": "auto"
@ -96,9 +93,6 @@
"PortChannel0004": { "PortChannel0004": {
"admin_status": "up", "admin_status": "up",
"min_links": "1", "min_links": "1",
"members": [
"Ethernet2"
],
"tpid": "0x9200", "tpid": "0x9200",
"mtu": "9100", "mtu": "9100",
"lacp_key": "auto" "lacp_key": "auto"
@ -106,19 +100,12 @@
"PortChannel2": { "PortChannel2": {
"admin_status": "up", "admin_status": "up",
"min_links": "1", "min_links": "1",
"members": [
"Ethernet12"
],
"tpid": "0x9200", "tpid": "0x9200",
"mtu": "9100", "mtu": "9100",
"lacp_key": "auto" "lacp_key": "auto"
}, },
"PortChannel42": { "PortChannel42": {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet-BP0",
"Ethernet-BP4"
],
"min_links": "2", "min_links": "2",
"mtu": "9100", "mtu": "9100",
"tpid": "0x8100" "tpid": "0x8100"

View File

@ -29,9 +29,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"lacp_key": "auto", "lacp_key": "auto",
@ -39,9 +36,6 @@
}, },
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet10"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"lacp_key": "auto", "lacp_key": "auto",
@ -151,9 +145,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"lacp_key": "auto", "lacp_key": "auto",

View File

@ -21,9 +21,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"tpid": "0x8100", "tpid": "0x8100",
@ -55,9 +52,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1024", "min_links": "1024",
"mtu": "9100", "mtu": "9100",
"name": "PortChannel0001" "name": "PortChannel0001"
@ -87,9 +81,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1025", "min_links": "1025",
"mtu": "9100", "mtu": "9100",
"name": "PortChannel0001" "name": "PortChannel0001"
@ -303,9 +294,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"name": "PortChannel0001" "name": "PortChannel0001"
@ -350,9 +338,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"name": "PortChannel0001" "name": "PortChannel0001"
@ -396,9 +381,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"name": "PortChannel0001" "name": "PortChannel0001"

View File

@ -196,9 +196,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"tpid": "0x8100", "tpid": "0x8100",
@ -246,9 +243,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"tpid": "0x8100", "tpid": "0x8100",
@ -296,9 +290,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"tpid": "0x8100", "tpid": "0x8100",
@ -346,9 +337,6 @@
"PORTCHANNEL_LIST": [ "PORTCHANNEL_LIST": [
{ {
"admin_status": "up", "admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1", "min_links": "1",
"mtu": "9100", "mtu": "9100",
"tpid": "0x8100", "tpid": "0x8100",

View File

@ -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 { leaf min_links {
type uint16 { type uint16 {
range 1..1024; range 1..1024;