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

Migrate GNMI table #3053

Merged
merged 7 commits into from
Jan 29, 2024
Merged

Migrate GNMI table #3053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
78d8d0b
Migrate GNMI table
ganglyu Nov 27, 2023
4e7d3f7
Fix test input
ganglyu Nov 27, 2023
74e7d6a
Fix merge conflict
ganglyu Nov 29, 2023
845d19e
Fix merge error
ganglyu Nov 29, 2023
32bdd5f
Fix conflict
ganglyu Dec 26, 2023
a75bb57
Fix ut
ganglyu Dec 26, 2023
25e80ba
Check GNMI table at first
ganglyu Dec 28, 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
37 changes: 36 additions & 1 deletion scripts/db_migrator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,30 @@ def migrate_telemetry(self):
if not certs:
self.configDB.set_entry("TELEMETRY", "certs", telemetry_data.get("certs"))

def migrate_gnmi(self):
# If there's GNMI table in CONFIG_DB, no need to migrate
gnmi = self.configDB.get_entry('GNMI', 'gnmi')
certs = self.configDB.get_entry('GNMI', 'certs')
if gnmi and certs:
return
if self.config_src_data:
if 'GNMI' in self.config_src_data:
# If there's GNMI in minigraph or golden config, copy configuration from config_src_data
gnmi_data = self.config_src_data['GNMI']
log.log_notice('Migrate GNMI configuration')
if 'gnmi' in gnmi_data:
self.configDB.set_entry("GNMI", "gnmi", gnmi_data.get('gnmi'))
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

self.configDB

There are upgrade/downgrade use cases. If self.configDB['GNMI'] already exists, you should not modify, right? #Closed

Copy link
Contributor Author

@ganglyu ganglyu Dec 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed, now I check GNMI table at first

if 'certs' in gnmi_data:
self.configDB.set_entry("GNMI", "certs", gnmi_data.get('certs'))
else:
# If there's no minigraph or golden config, copy configuration from CONFIG_DB TELEMETRY table
gnmi = self.configDB.get_entry('TELEMETRY', 'gnmi')
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

get_entry

Will this line still work if TELEMETRY table is completely missing in ConfigDB? #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, if TELEMETRY table is missing, db_migrator will not generate GNMI table.

if gnmi:
self.configDB.set_entry("GNMI", "gnmi", gnmi)
certs = self.configDB.get_entry('TELEMETRY', 'certs')
if certs:
self.configDB.set_entry("GNMI", "certs", certs)

def migrate_console_switch(self):
# CONSOLE_SWITCH - add missing key
if not self.config_src_data or 'CONSOLE_SWITCH' not in self.config_src_data:
Expand Down Expand Up @@ -1137,9 +1161,20 @@ def version_202311_01(self):
def version_202311_02(self):
"""
Version 202311_02.
This is current last erversion for 202311 branch
"""
log.log_info('Handling version_202311_02')
# Update GNMI table
self.migrate_gnmi()
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding to an existing version is not safe. What if this operation was executed on a device running an older sonic image which already has database version reached 4.0.4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


self.set_version('version_202311_03')
return 'version_202311_03'

def version_202311_03(self):
"""
Version 202311_03.
This is current last erversion for 202311 branch
"""
log.log_info('Handling version_202311_03')
self.set_version('version_202405_01')
return 'version_202405_01'

Expand Down
2 changes: 1 addition & 1 deletion tests/db_migrator_input/config_db/dns-nameserver-expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
"state": "enabled"
},
"VERSIONS|DATABASE": {
"VERSION": "version_4_0_4"
"VERSION": "version_202405_01"
Copy link
Contributor

@yxieca yxieca Jan 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You should not need to make this change. Is there a reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no version_4_0_4 in db_migrator.py, does it make sense to use it in test?

}
}
15 changes: 15 additions & 0 deletions tests/db_migrator_input/config_db/gnmi-configdb-expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"GNMI|gnmi": {
"client_auth": "true",
"log_level": "2",
"port": "50051"
},
"GNMI|certs": {
"server_key": "/etc/sonic/telemetry/streamingtelemetryserver.key",
"ca_crt": "/etc/sonic/telemetry/dsmsroot.cer",
"server_crt": "/etc/sonic/telemetry/streamingtelemetryserver.cer"
},
"VERSIONS|DATABASE": {
"VERSION": "version_202405_01"
}
}
15 changes: 15 additions & 0 deletions tests/db_migrator_input/config_db/gnmi-input.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"TELEMETRY|gnmi": {
"client_auth": "true",
"log_level": "2",
"port": "50051"
},
"TELEMETRY|certs": {
"server_key": "/etc/sonic/telemetry/streamingtelemetryserver.key",
"ca_crt": "/etc/sonic/telemetry/dsmsroot.cer",
"server_crt": "/etc/sonic/telemetry/streamingtelemetryserver.cer"
},
"VERSIONS|DATABASE": {
"VERSION": "version_202311_01"
}
}
15 changes: 15 additions & 0 deletions tests/db_migrator_input/config_db/gnmi-minigraph-expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"GNMI|gnmi": {
"client_auth": "true",
"log_level": "2",
"port": "50052"
},
"GNMI|certs": {
"server_key": "/etc/sonic/telemetry/streamingtelemetryserver.key",
"ca_crt": "/etc/sonic/telemetry/dsmsroot.cer",
"server_crt": "/etc/sonic/telemetry/streamingtelemetryserver.cer"
},
"VERSIONS|DATABASE": {
"VERSION": "version_202405_01"
}
}
59 changes: 58 additions & 1 deletion tests/db_migrator_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ def test_dns_nameserver_migrator(self):
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'dns-nameserver-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_4_0_4')
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202405_01')
resulting_keys = dbmgtr.configDB.keys(dbmgtr.configDB.CONFIG_DB, 'DNS_NAMESERVER*')
expected_keys = expected_db.cfgdb.keys(expected_db.cfgdb.CONFIG_DB, 'DNS_NAMESERVER*')

Expand Down Expand Up @@ -767,6 +767,7 @@ def test_fast_reboot_upgrade_to_4_0_3(self):
assert not self.check_config_db(dbmgtr.configDB, expected_db.cfgdb)
assert dbmgtr.CURRENT_VERSION == expected_db.cfgdb.get_entry('VERSIONS', 'DATABASE')['VERSION'], '{} {}'.format(dbmgtr.CURRENT_VERSION, dbmgtr.get_version())


class TestSflowSampleDirectionMigrator(object):
@classmethod
def setup_class(cls):
Expand Down Expand Up @@ -885,3 +886,59 @@ def test_init(self, mock_args):
mock_args.return_value=argparse.Namespace(namespace=None, operation='get_version', socket=None)
import db_migrator
db_migrator.main()


class TestGNMIMigrator(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_dns_nameserver_migrator_minigraph(self):
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'gnmi-input')
import db_migrator
dbmgtr = db_migrator.DBMigrator(None)
# Set config_src_data
dbmgtr.config_src_data = {
'GNMI': {
'gnmi': {
"client_auth": "true",
"log_level": "2",
"port": "50052"
},
'certs': {
"server_key": "/etc/sonic/telemetry/streamingtelemetryserver.key",
"ca_crt": "/etc/sonic/telemetry/dsmsroot.cer",
"server_crt": "/etc/sonic/telemetry/streamingtelemetryserver.cer"
}
}
}
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'gnmi-minigraph-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202405_01')
resulting_table = dbmgtr.configDB.get_table("GNMI")
expected_table = expected_db.cfgdb.get_table("GNMI")

diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
assert not diff

def test_dns_nameserver_migrator_configdb(self):
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'gnmi-input')
import db_migrator
dbmgtr = db_migrator.DBMigrator(None)
# Set config_src_data
dbmgtr.config_src_data = {}
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'gnmi-configdb-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202405_01')
resulting_table = dbmgtr.configDB.get_table("GNMI")
expected_table = expected_db.cfgdb.get_table("GNMI")

diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
assert not diff
Loading