Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't create the members@ array in config_db for PC when reading from minigraph #13660

Merged
merged 5 commits into from
Feb 23, 2023
Merged

Don't create the members@ array in config_db for PC when reading from minigraph #13660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
13a5b91
Don't create the members@ array in config_db for PC when reading from…
saiarcot895 Feb 3, 2023
00ab627
Update tests and remove other references to member array
saiarcot895 Feb 4, 2023
5c31781
Remove reference to members@ array from yang file
saiarcot895 Feb 11, 2023
fbb14c8
Update yang test files to reflect members array removal
saiarcot895 Feb 13, 2023
53e3d60
Update test script to check for a different leaf-list entry
saiarcot895 Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'}
Copy link
Collaborator

@qiluo-msft qiluo-msft Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

members

Do you want to update src/sonic-yang-models/yang-models/sonic-portchannel.yang at the same time?

@ganglyu for vis. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, removed from the yang file.

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 = ""
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -1760,12 +1760,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():
Expand Down Expand Up @@ -1869,7 +1872,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'
Expand Down
4 changes: 2 additions & 2 deletions src/sonic-config-engine/tests/test_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,15 +328,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"]
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/test_minigraph_case.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
10 changes: 5 additions & 5 deletions src/sonic-config-engine/tests/test_multinpu_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
20 changes: 10 additions & 10 deletions src/sonic-yang-mgmt/tests/test_cfghelp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 | | | |
+--------------+------------------------------------------------------------------------+-------------+-----------+-------------+

"""

Expand Down Expand Up @@ -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']
Expand Down
13 changes: 0 additions & 13 deletions src/sonic-yang-models/tests/files/sample_config_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,39 +81,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"
Expand Down
9 changes: 0 additions & 9 deletions src/sonic-yang-models/tests/yang_model_tests/tests_config/mclag.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -151,9 +145,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"lacp_key": "auto",
Expand Down
18 changes: 0 additions & 18 deletions src/sonic-yang-models/tests/yang_model_tests/tests_config/portchannel.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"tpid": "0x8100",
Expand Down Expand Up @@ -55,9 +52,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1024",
"mtu": "9100",
"name": "PortChannel0001"
Expand Down Expand Up @@ -87,9 +81,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1025",
"mtu": "9100",
"name": "PortChannel0001"
Expand Down Expand Up @@ -303,9 +294,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"name": "PortChannel0001"
Expand Down Expand Up @@ -350,9 +338,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"name": "PortChannel0001"
Expand Down Expand Up @@ -396,9 +381,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"name": "PortChannel0001"
Expand Down
12 changes: 0 additions & 12 deletions src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan_sub_interface.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"tpid": "0x8100",
Expand Down Expand Up @@ -246,9 +243,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"tpid": "0x8100",
Expand Down Expand Up @@ -296,9 +290,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"tpid": "0x8100",
Expand Down Expand Up @@ -346,9 +337,6 @@
"PORTCHANNEL_LIST": [
{
"admin_status": "up",
"members": [
"Ethernet0"
],
"min_links": "1",
"mtu": "9100",
"tpid": "0x8100",
Expand Down
17 changes: 0 additions & 17 deletions src/sonic-yang-models/yang-models/sonic-portchannel.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down