Skip to content

[db_migrator] Fix migration of Loopback data: handle all Loopback interfaces #2560

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

Merged
merged 3 commits into from
Dec 16, 2022
Merged

[db_migrator] Fix migration of Loopback data: handle all Loopback interfaces #2560

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b295a9
[db_migrator] Fix migration of Loopback data: handle all Loopback int…
vaibhavhd Dec 15, 2022
6e70f2d
Address review comments: only replace Lo with Loopback
vaibhavhd Dec 15, 2022
8f3f4be
Address review comments: only replace Lo with Loopback
vaibhavhd Dec 15, 2022
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
39 changes: 24 additions & 15 deletions scripts/db_migrator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,24 +178,33 @@ def migrate_intf_table(self):
if self.appDB is None:
return

data = self.appDB.keys(self.appDB.APPL_DB, "INTF_TABLE:*")

if data is None:
# Get Lo interface corresponding to IP(v4/v6) address from CONFIG_DB.
configdb_data = self.configDB.get_keys('LOOPBACK_INTERFACE')
lo_addr_to_int = dict()
for int_data in configdb_data:
if type(int_data) == tuple and len(int_data) > 1:
intf_name = int_data[0]
intf_addr = int_data[1]
lo_addr_to_int.update({intf_addr: intf_name})

lo_data = self.appDB.keys(self.appDB.APPL_DB, "INTF_TABLE:lo*")
if lo_data is None:
return
for lo_row in lo_data:
# Example of lo_row: 'INTF_TABLE:lo:10.1.0.32/32'
# Example of lo_dict: {'family': 'IPv4', 'scope': 'global'}
lo_dict = self.appDB.get_all(self.appDB.APPL_DB, lo_row)
lo_addr = lo_row.split('INTF_TABLE:lo:')[1]
lo_name = lo_addr_to_int.get(lo_addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

A logical difference from previous implementation is that, if config_db data is empty, previously we hardcode to 'Loopback0' whereas here it will come as empty which can cause swss crash. For safe side, I suggest if 'lo_name' is empty use the previous logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, addressed this in latest.

lo_table = "INTF_TABLE:" + lo_name + ":" + lo_addr
for key, value in lo_dict.items():
log.log_notice("Set table {} with field {} and value {}".format(
lo_table, key, value))
self.appDB.set(self.appDB.APPL_DB, lo_table, key, value)
# delete the old row with name as 'lo' as new row has been added
self.appDB.delete(self.appDB.APPL_DB, lo_row)
Copy link
Contributor

Choose a reason for hiding this comment

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

You are deleting the iterating element in the loop. Not sure if it can cause issues. Why don't we use the same logic as before and just replace the if_name from your new dict - lo_addr_to_int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this in latest - removed the inner loop (handling of field, value). In the updated logic, only name lo is replaced with name from config db.


if_db = []
for key in data:
if_name = key.split(":")[1]
if if_name == "lo":
self.appDB.delete(self.appDB.APPL_DB, key)
key = key.replace(if_name, "Loopback0")
log.log_info('Migrating lo entry to ' + key)
self.appDB.set(self.appDB.APPL_DB, key, 'NULL', 'NULL')

if '/' not in key:
if_db.append(key.split(":")[1])
continue

data = self.appDB.keys(self.appDB.APPL_DB, "INTF_TABLE:*")
for key in data:
if_name = key.split(":")[1]
Expand Down
20 changes: 20 additions & 0 deletions tests/db_migrator_input/appl_db/loopback_interface_migrate_from_1_0_1_expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"INTF_TABLE:Loopback0" : {"NULL": "NULL"},
"INTF_TABLE:Loopback0:10.1.0.32/32" : {
"scope": "global",
"family": "IPv4"
},
"INTF_TABLE:Loopback0:FC00:1::32/128" : {
"scope": "global",
"family": "IPv6"
},
"INTF_TABLE:Loopback1" : {"NULL": "NULL"},
"INTF_TABLE:Loopback1:10.20.8.199/32" : {
"scope": "global",
"family": "IPv4"
},
"INTF_TABLE:Loopback1:2001:506:28:500::1/128" : {
"scope": "global",
"family": "IPv6"
}
}
18 changes: 18 additions & 0 deletions tests/db_migrator_input/appl_db/loopback_interface_migrate_from_1_0_1_input.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"INTF_TABLE:lo:10.1.0.32/32" : {
"scope": "global",
"family": "IPv4"
},
"INTF_TABLE:lo:FC00:1::32/128" : {
"scope": "global",
"family": "IPv6"
},
"INTF_TABLE:lo:10.20.8.199/32" : {
"scope": "global",
"family": "IPv4"
},
"INTF_TABLE:lo:2001:506:28:500::1/128" : {
"scope": "global",
"family": "IPv6"
}
}
8 changes: 8 additions & 0 deletions tests/db_migrator_input/config_db/loopback_interface_migrate_from_1_0_1_expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"LOOPBACK_INTERFACE|Loopback0" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback0|10.1.0.32/32" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback0|FC00:1::32/128" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback1" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback1|10.20.8.199/32" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback1|2001:506:28:500::1/128" : {"NULL": "NULL"}
}
7 changes: 7 additions & 0 deletions tests/db_migrator_input/config_db/loopback_interface_migrate_from_1_0_1_input.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"LOOPBACK_INTERFACE|Loopback0|10.1.0.32/32" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback0|FC00:1::32/128" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback1|10.20.8.199/32" : {"NULL": "NULL"},
"LOOPBACK_INTERFACE|Loopback1|2001:506:28:500::1/128" : {"NULL": "NULL"},
"VERSIONS|DATABASE": {"VERSION": "version_1_0_1"}
}
42 changes: 42 additions & 0 deletions tests/db_migrator_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,3 +476,45 @@ def test_warm_upgrade_to_2_0_2(self):
expected_table = expected_db.cfgdb.get_table(table)
diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
assert not diff

class Test_Migrate_Loopback(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
dbconnector.dedicated_dbs['APPL_DB'] = None

def test_migrate_loopback_int(self):
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'loopback_interface_migrate_from_1_0_1_input')
dbconnector.dedicated_dbs['APPL_DB'] = os.path.join(mock_db_path, 'appl_db', 'loopback_interface_migrate_from_1_0_1_input')

import db_migrator
dbmgtr = db_migrator.DBMigrator(None)
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'loopback_interface_migrate_from_1_0_1_expected')
dbconnector.dedicated_dbs['APPL_DB'] = os.path.join(mock_db_path, 'appl_db', 'loopback_interface_migrate_from_1_0_1_expected')
expected_db = Db()

# verify migrated configDB
resulting_table = dbmgtr.configDB.get_table("LOOPBACK_INTERFACE")
expected_table = expected_db.cfgdb.get_table("LOOPBACK_INTERFACE")
diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
assert not diff

# verify migrated appDB
expected_appl_db = SonicV2Connector(host='127.0.0.1')
expected_appl_db.connect(expected_appl_db.APPL_DB)
expected_keys = expected_appl_db.keys(expected_appl_db.APPL_DB, "INTF_TABLE:*")
expected_keys.sort()
resulting_keys = dbmgtr.appDB.keys(dbmgtr.appDB.APPL_DB, "INTF_TABLE:*")
resulting_keys.sort()
assert expected_keys == resulting_keys
for key in expected_keys:
resulting_keys = dbmgtr.appDB.get_all(dbmgtr.appDB.APPL_DB, key)
expected_keys = expected_appl_db.get_all(expected_appl_db.APPL_DB, key)
diff = DeepDiff(resulting_keys, expected_keys, ignore_order=True)
assert not diff