Skip to content

Commit 9711c6e

Browse files
saiarcot895mssonicbld
saiarcot895
authored and
mssonicbld
committed
Don't create the members@ array in config_db for PC when reading from minigraph (sonic-net#13660)
Fixes sonic-net#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.
1 parent 6644725 commit 9711c6e

File tree

10 files changed

+30
-96
lines changed

10 files changed

+30
-96
lines changed

‎src/sonic-config-engine/minigraph.py

+12-9
Original file line numberDiff line numberDiff line change
@@ -532,9 +532,9 @@ def parse_dpg(dpg, hname):
532532
intfs_inpc.append(pcmbr_list[i])
533533
pc_members[(pcintfname, pcmbr_list[i])] = {}
534534
if pcintf.find(str(QName(ns, "Fallback"))) != None:
535-
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'}
535+
pcs[pcintfname] = {'fallback': pcintf.find(str(QName(ns, "Fallback"))).text, 'min_links': str(int(math.ceil(len() * 0.75))), 'lacp_key': 'auto'}
536536
else:
537-
pcs[pcintfname] = {'members': pcmbr_list, 'min_links': str(int(math.ceil(len(pcmbr_list) * 0.75))), 'lacp_key': 'auto' }
537+
pcs[pcintfname] = {'min_links': str(int(math.ceil(len(pcmbr_list) * 0.75))), 'lacp_key': 'auto' }
538538
port_nhipv4_map = {}
539539
port_nhipv6_map = {}
540540
nhg_int = ""
@@ -1244,7 +1244,7 @@ def filter_acl_table_for_backend(acls, vlan_members):
12441244
}
12451245
return filter_acls
12461246

1247-
def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role, device_type, is_storage_device, vlan_members):
1247+
def filter_acl_table_bindings(acls, neighbors, port_channels, pc_members, sub_role, device_type, is_storage_device, vlan_members):
12481248
if device_type == 'BackEndToRRouter' and is_storage_device:
12491249
return filter_acl_table_for_backend(acls, vlan_members)
12501250

@@ -1263,8 +1263,8 @@ def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role, device_t
12631263

12641264
# Get the front panel port channel.
12651265
for port_channel_intf in port_channels:
1266-
backend_port_channel = any(lag_member in backplane_port_list \
1267-
for lag_member in port_channels[port_channel_intf]['members'])
1266+
backend_port_channel = any(lag_member[1] in backplane_port_list \
1267+
for lag_member in list(pc_members.keys()) if lag_member[0] == port_channel_intf)
12681268
if not backend_port_channel:
12691269
front_port_channel_intf.append(port_channel_intf)
12701270

@@ -1763,12 +1763,15 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
17631763

17641764
if port_config_file:
17651765
port_set = set(ports.keys())
1766-
for (pc_name, mbr_map) in list(pcs.items()):
1766+
for (pc_name, pc_member) in list(pc_members.keys()):
17671767
# remove portchannels that contain ports not existing in port_config.ini
17681768
# when port_config.ini exists
1769-
if not set(mbr_map['members']).issubset(port_set):
1770-
print("Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name, file=sys.stderr)
1769+
if (pc_name, pc_member) in pc_members and pc_member not in port_set:
1770+
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)
17711771
del pcs[pc_name]
1772+
pc_mbr_del_keys = [f for f in list(pc_members.keys()) if f[0] == pc_name]
1773+
for pc_mbr_del_key in pc_mbr_del_keys:
1774+
del pc_members[pc_mbr_del_key]
17721775

17731776
# set default port channel MTU as 9100 and admin status up and default TPID 0x8100
17741777
for pc in pcs.values():
@@ -1872,7 +1875,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
18721875
results['DHCP_RELAY'] = dhcp_relay_table
18731876
results['NTP_SERVER'] = dict((item, {}) for item in ntp_servers)
18741877
results['TACPLUS_SERVER'] = dict((item, {'priority': '1', 'tcp_port': '49'}) for item in tacacs_servers)
1875-
results['ACL_TABLE'] = filter_acl_table_bindings(acls, neighbors, pcs, sub_role, current_device['type'], is_storage_device, vlan_members)
1878+
results['ACL_TABLE'] = filter_acl_table_bindings(acls, neighbors, pcs, pc_members, sub_role, current_device['type'], is_storage_device, vlan_members)
18761879
results['FEATURE'] = {
18771880
'telemetry': {
18781881
'state': 'enabled'

‎src/sonic-config-engine/tests/test_cfggen.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -328,15 +328,15 @@ def test_minigraph_portchannels(self, **kwargs):
328328
output = self.run_script(argument)
329329
self.assertEqual(
330330
utils.to_dict(output.strip()),
331-
utils.to_dict("{'PortChannel1': {'admin_status': 'up', 'min_links': '1', 'members': ['Ethernet4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
331+
utils.to_dict("{'PortChannel1': {'admin_status': 'up', 'min_links': '1', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
332332
)
333333

334334
def test_minigraph_portchannel_with_more_member(self):
335335
argument = ['-m', self.sample_graph_pc_test, '-p', self.port_config, '-v', 'PORTCHANNEL']
336336
output = self.run_script(argument)
337337
self.assertEqual(
338338
utils.to_dict(output.strip()),
339-
utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '3', 'members': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}"))
339+
utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '3', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}"))
340340

341341
def test_minigraph_portchannel_members(self):
342342
argument = ['-m', self.sample_graph_pc_test, '-p', self.port_config, '-v', "PORTCHANNEL_MEMBER.keys()|list"]

‎src/sonic-config-engine/tests/test_minigraph_case.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ def test_minigraph_portchannels(self):
158158
output = self.run_script(argument)
159159
self.assertEqual(
160160
utils.to_dict(output.strip()),
161-
utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '1', 'members': ['Ethernet4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
161+
utils.to_dict("{'PortChannel01': {'admin_status': 'up', 'min_links': '1', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}}")
162162
)
163163

164164
def test_minigraph_console_mgmt_feature(self):

‎src/sonic-config-engine/tests/test_multinpu_cfggen.py

+5-5
Original file line numberDiff line numberDiff line change
@@ -161,16 +161,16 @@ def test_frontend_asic_portchannels(self):
161161
argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "--var-json", "PORTCHANNEL"]
162162
output = json.loads(self.run_script(argument))
163163
self.assertDictEqual(output, \
164-
{'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet0', 'Ethernet4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
165-
'PortChannel4001': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP0', 'Ethernet-BP4'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
166-
'PortChannel4002': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP8', 'Ethernet-BP12'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
164+
{'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
165+
'PortChannel4001': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
166+
'PortChannel4002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
167167

168168
def test_backend_asic_portchannels(self):
169169
argument = ["-m", self.sample_graph, "-p", self.port_config[3], "-n", "asic3", "--var-json", "PORTCHANNEL"]
170170
output = json.loads(self.run_script(argument))
171171
self.assertDictEqual(output, \
172-
{'PortChannel4013': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP384', 'Ethernet-BP388'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
173-
'PortChannel4014': {'admin_status': 'up', 'min_links': '2', 'members': ['Ethernet-BP392', 'Ethernet-BP396'], 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
172+
{'PortChannel4013': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
173+
'PortChannel4014': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
174174

175175
def test_frontend_asic_portchannel_mem(self):
176176
argument = ["-m", self.sample_graph, "-p", self.port_config[0], "-n", "asic0", "-v", "PORTCHANNEL_MEMBER.keys()|list"]

‎src/sonic-yang-mgmt/tests/test_cfghelp.py

+10-10
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,17 @@
7171
7272
"""
7373

74-
portchannel_table_field_output="""\
74+
vlan_table_field_output="""\
7575
76-
PORTCHANNEL
77-
Description: PORTCHANNEL part of config_db.json
76+
VLAN
77+
Description: VLAN part of config_db.json
7878
7979
key - name
80-
+---------+-------------------------------------------+-------------+-----------+-------------+
81-
| Field | Description | Mandatory | Default | Reference |
82-
+=========+===========================================+=============+===========+=============+
83-
| members | The field contains list of unique members | | | PORT:name |
84-
+---------+-------------------------------------------+-------------+-----------+-------------+
80+
+--------------+------------------------------------------------------------------------+-------------+-----------+-------------+
81+
| Field | Description | Mandatory | Default | Reference |
82+
+==============+========================================================================+=============+===========+=============+
83+
| dhcp_servers | The field contains list of unique membersConfigure the dhcp v4 servers | | | |
84+
+--------------+------------------------------------------------------------------------+-------------+-----------+-------------+
8585
8686
"""
8787

@@ -153,9 +153,9 @@ def test_single_field(self):
153153
self.assertEqual(output, techsupport_table_field_output)
154154

155155
def test_leaf_list(self):
156-
argument = ['-t', 'PORTCHANNEL', '-f', 'members']
156+
argument = ['-t', 'VLAN', '-f', 'dhcp_servers']
157157
output = self.run_script(argument)
158-
self.assertEqual(output, portchannel_table_field_output)
158+
self.assertEqual(output, vlan_table_field_output)
159159

160160
def test_leaf_list_map(self):
161161
argument = ['-t', 'DSCP_TO_TC_MAP']

‎src/sonic-yang-models/tests/files/sample_config_db.json

-13
Original file line numberDiff line numberDiff line change
@@ -82,39 +82,26 @@
8282
"PortChannel0003": {
8383
"admin_status": "up",
8484
"min_links": "1",
85-
"members": [
86-
"Ethernet1"
87-
],
8885
"tpid": "0x8100",
8986
"mtu": "9100",
9087
"lacp_key": "auto"
9188
},
9289
"PortChannel0004": {
9390
"admin_status": "up",
9491
"min_links": "1",
95-
"members": [
96-
"Ethernet2"
97-
],
9892
"tpid": "0x9200",
9993
"mtu": "9100",
10094
"lacp_key": "auto"
10195
},
10296
"PortChannel2": {
10397
"admin_status": "up",
10498
"min_links": "1",
105-
"members": [
106-
"Ethernet12"
107-
],
10899
"tpid": "0x9200",
109100
"mtu": "9100",
110101
"lacp_key": "auto"
111102
},
112103
"PortChannel42": {
113104
"admin_status": "up",
114-
"members": [
115-
"Ethernet-BP0",
116-
"Ethernet-BP4"
117-
],
118105
"min_links": "2",
119106
"mtu": "9100",
120107
"tpid": "0x8100",

‎src/sonic-yang-models/tests/yang_model_tests/tests_config/mclag.json

-9
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,13 @@
2929
"PORTCHANNEL_LIST": [
3030
{
3131
"admin_status": "up",
32-
"members": [
33-
"Ethernet0"
34-
],
3532
"min_links": "1",
3633
"mtu": "9100",
3734
"lacp_key": "auto",
3835
"name": "PortChannel2"
3936
},
4037
{
4138
"admin_status": "up",
42-
"members": [
43-
"Ethernet10"
44-
],
4539
"min_links": "1",
4640
"mtu": "9100",
4741
"lacp_key": "auto",
@@ -151,9 +145,6 @@
151145
"PORTCHANNEL_LIST": [
152146
{
153147
"admin_status": "up",
154-
"members": [
155-
"Ethernet0"
156-
],
157148
"min_links": "1",
158149
"mtu": "9100",
159150
"lacp_key": "auto",

‎src/sonic-yang-models/tests/yang_model_tests/tests_config/portchannel.json

-18
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@
2121
"PORTCHANNEL_LIST": [
2222
{
2323
"admin_status": "up",
24-
"members": [
25-
"Ethernet0"
26-
],
2724
"min_links": "1",
2825
"mtu": "9100",
2926
"tpid": "0x8100",
@@ -57,9 +54,6 @@
5754
"PORTCHANNEL_LIST": [
5855
{
5956
"admin_status": "up",
60-
"members": [
61-
"Ethernet0"
62-
],
6357
"min_links": "1024",
6458
"mtu": "9100",
6559
"name": "PortChannel0001"
@@ -89,9 +83,6 @@
8983
"PORTCHANNEL_LIST": [
9084
{
9185
"admin_status": "up",
92-
"members": [
93-
"Ethernet0"
94-
],
9586
"min_links": "1025",
9687
"mtu": "9100",
9788
"name": "PortChannel0001"
@@ -335,9 +326,6 @@
335326
"PORTCHANNEL_LIST": [
336327
{
337328
"admin_status": "up",
338-
"members": [
339-
"Ethernet0"
340-
],
341329
"min_links": "1",
342330
"mtu": "9100",
343331
"name": "PortChannel0001"
@@ -382,9 +370,6 @@
382370
"PORTCHANNEL_LIST": [
383371
{
384372
"admin_status": "up",
385-
"members": [
386-
"Ethernet0"
387-
],
388373
"min_links": "1",
389374
"mtu": "9100",
390375
"name": "PortChannel0001"
@@ -428,9 +413,6 @@
428413
"PORTCHANNEL_LIST": [
429414
{
430415
"admin_status": "up",
431-
"members": [
432-
"Ethernet0"
433-
],
434416
"min_links": "1",
435417
"mtu": "9100",
436418
"name": "PortChannel0001"

‎src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan_sub_interface.json

-12
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,6 @@
196196
"PORTCHANNEL_LIST": [
197197
{
198198
"admin_status": "up",
199-
"members": [
200-
"Ethernet0"
201-
],
202199
"min_links": "1",
203200
"mtu": "9100",
204201
"tpid": "0x8100",
@@ -246,9 +243,6 @@
246243
"PORTCHANNEL_LIST": [
247244
{
248245
"admin_status": "up",
249-
"members": [
250-
"Ethernet0"
251-
],
252246
"min_links": "1",
253247
"mtu": "9100",
254248
"tpid": "0x8100",
@@ -296,9 +290,6 @@
296290
"PORTCHANNEL_LIST": [
297291
{
298292
"admin_status": "up",
299-
"members": [
300-
"Ethernet0"
301-
],
302293
"min_links": "1",
303294
"mtu": "9100",
304295
"tpid": "0x8100",
@@ -346,9 +337,6 @@
346337
"PORTCHANNEL_LIST": [
347338
{
348339
"admin_status": "up",
349-
"members": [
350-
"Ethernet0"
351-
],
352340
"min_links": "1",
353341
"mtu": "9100",
354342
"tpid": "0x8100",

‎src/sonic-yang-models/yang-models/sonic-portchannel.yang

-17
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,6 @@ module sonic-portchannel {
5858
}
5959
}
6060

61-
leaf-list members {
62-
/* leaf-list members are unique by default */
63-
type union {
64-
type leafref {
65-
path /port:sonic-port/port:PORT/port:PORT_LIST/port:name;
66-
}
67-
type string {
68-
pattern "";
69-
}
70-
}
71-
/* Today in SONiC, we do not delete the list once
72-
* created, instead we set to empty list. Due to that
73-
* below default values are needed.
74-
*/
75-
default "";
76-
}
77-
7861
leaf min_links {
7962
type uint16 {
8063
range 1..1024;

0 commit comments

Comments
 (0)