Skip to content

Commit

Permalink
[config]: Create portchannel with LACP key (sonic-net#1473)
Browse files Browse the repository at this point in the history
What I did
Fix issue - sonic-net/sonic-buildimage#4009
Change the LACP key to be generated from the Port Channel name instead of always being 0.
When upgrading without warm-reboot update old port channels to use the new default.

How I did it
When adding a new port-channel add by default the key lacp_key with the value 'auto' to the port-channel table, this is done to change the port-channel LACP key to be generated from the Port Channel name instead of always being 0.

When upgrading without warm-reboot, also update old port channels to use the new default.
This is not done on warm-reboot to avoid the link from going down.
  • Loading branch information
DavidZagury authored Jun 21, 2021
1 parent 6f74ba5 commit 2cdadb5
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 3 deletions.
3 changes: 2 additions & 1 deletion config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,8 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback):
ctx.fail("{} already exists!".format(portchannel_name))

fvs = {'admin_status': 'up',
'mtu': '9100'}
'mtu': '9100',
'lacp_key': 'auto'}
if min_links != 0:
fvs['min_links'] = str(min_links)
if fallback != 'false':
Expand Down
19 changes: 17 additions & 2 deletions scripts/db_migrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def __init__(self, namespace, socket=None):
none-zero values.
build: sequentially increase within a minor version domain.
"""
self.CURRENT_VERSION = 'version_2_0_0'
self.CURRENT_VERSION = 'version_2_0_2'

self.TABLE_NAME = 'VERSIONS'
self.TABLE_KEY = 'DATABASE'
Expand Down Expand Up @@ -527,9 +527,24 @@ def version_2_0_0(self):

def version_2_0_1(self):
"""
Current latest version. Nothing to do here.
Version 2_0_1.
"""
log.log_info('Handling version_2_0_1')
warmreboot_state = self.stateDB.get(self.stateDB.STATE_DB, 'WARM_RESTART_ENABLE_TABLE|system', 'enable')

if warmreboot_state != 'true':
portchannel_table = self.configDB.get_table('PORTCHANNEL')
for name, data in portchannel_table.items():
data['lacp_key'] = 'auto'
self.configDB.set_entry('PORTCHANNEL', name, data)
self.set_version('version_2_0_2')
return 'version_2_0_2'

def version_2_0_2(self):
"""
Current latest version. Nothing to do here.
"""
log.log_info('Handling version_2_0_2')
return None

def get_version(self):
Expand Down
39 changes: 39 additions & 0 deletions tests/db_migrator_input/config_db/portchannel-expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"PORTCHANNEL|PortChannel0": {
"admin_status": "up",
"members@": "Ethernet0,Ethernet4",
"min_links": "2",
"mtu": "9100",
"lacp_key": "auto"
},
"PORTCHANNEL|PortChannel1": {
"admin_status": "up",
"members@": "Ethernet8,Ethernet12",
"min_links": "2",
"mtu": "9100",
"lacp_key": "auto"
},
"PORTCHANNEL|PortChannel0123": {
"admin_status": "up",
"members@": "Ethernet16",
"min_links": "1",
"mtu": "9100",
"lacp_key": "auto"
},
"PORTCHANNEL|PortChannel0011": {
"admin_status": "up",
"members@": "Ethernet20,Ethernet24",
"min_links": "2",
"mtu": "9100",
"lacp_key": "auto"
},
"PORTCHANNEL|PortChannel9999": {
"admin_status": "up",
"mtu": "9100",
"lacp_key": "auto"
},
"VERSIONS|DATABASE": {
"VERSION": "version_2_0_2"
}
}

33 changes: 33 additions & 0 deletions tests/db_migrator_input/config_db/portchannel-input.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"PORTCHANNEL|PortChannel0": {
"admin_status": "up",
"members@": "Ethernet0,Ethernet4",
"min_links": "2",
"mtu": "9100"
},
"PORTCHANNEL|PortChannel1": {
"admin_status": "up",
"members@": "Ethernet8,Ethernet12",
"min_links": "2",
"mtu": "9100"
},
"PORTCHANNEL|PortChannel0123": {
"admin_status": "up",
"members@": "Ethernet16",
"min_links": "1",
"mtu": "9100"
},
"PORTCHANNEL|PortChannel0011": {
"admin_status": "up",
"members@": "Ethernet20,Ethernet24",
"min_links": "2",
"mtu": "9100"
},
"PORTCHANNEL|PortChannel9999": {
"admin_status": "up",
"mtu": "9100"
},
"VERSIONS|DATABASE": {
"VERSION": "version_2_0_1"
}
}
23 changes: 23 additions & 0 deletions tests/db_migrator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,26 @@ def test_init_config_feature_migration(self):
assert not diff

assert not expected_db.cfgdb.get_table('CONTAINER_FEATURE')


class TestLacpKeyMigrator(object):
@classmethod
def setup_class(cls):
os.environ['UTILITIES_UNIT_TESTING'] = "2"

@classmethod
def teardown_class(cls):
os.environ['UTILITIES_UNIT_TESTING'] = "0"
dbconnector.dedicated_dbs['CONFIG_DB'] = None

def test_lacp_key_migrator(self):
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'portchannel-input')
import db_migrator
dbmgtr = db_migrator.DBMigrator(None)
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'portchannel-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_2_0_2')

assert dbmgtr.configDB.get_table('PORTCHANNEL') == expected_db.cfgdb.get_table('PORTCHANNEL')
assert dbmgtr.configDB.get_table('VERSIONS') == expected_db.cfgdb.get_table('VERSIONS')

0 comments on commit 2cdadb5

Please sign in to comment.