Migrate GNMI table #3053
Migrate GNMI table #3053
Conversation
scripts/db_migrator.py
Outdated
| def migrate_gnmi(self): | ||
| # GNMI - add missing key | ||
| if not self.minigraph_data or 'GNMI' not in self.minigraph_data: | ||
| # If there's no GNMI in minigraph, copy configuration from CONFIG_DB TELEMETRY table |
There was a problem hiding this comment.
If there's no GNMI in minigraph, copy configuration from CONFIG_DB TELEMETRY table
Could you double check if this works with L2 switch mode? https://github.com/sonic-net/SONiC/wiki/L2-Switch-mode #Closed
There was a problem hiding this comment.
We previously has a bug that mistakenly generating config for telemetry in L2 mode.
There was a problem hiding this comment.
sonic-net/sonic-mgmt#8777
L2 switch mode is a test gap, and this issue is still open.
When there's no GNMI table in minigraph, db migrator will copy from CONFIG_DB TELEMETRY table to CONFIG_DB GNMI table.
When there's GNMI table in minigraph, db migrator will copy from minigraph GNMI table to CONFIG_DB GNMI table.
For L2 switch mode, minigraph is empty and CONFIG_DB does not have TELEMETRY table, and then db migrator will not generate config for GNMI.
| @@ -1090,7 +1109,8 @@ def version_4_0_4(self): | |||
| Version 4_0_4. | |||
| """ | |||
| log.log_info('Handling version_4_0_4') | |||
|
|
|||
| # Updating GNMI table | |||
| self.migrate_gnmi() | |||
There was a problem hiding this comment.
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?
| # GNMI - add missing key | ||
| if not self.minigraph_data or 'GNMI' not in self.minigraph_data: | ||
| # If there's no GNMI in minigraph, copy configuration from CONFIG_DB TELEMETRY table | ||
| gnmi = self.configDB.get_entry('TELEMETRY', 'gnmi') |
There was a problem hiding this comment.
yes, if TELEMETRY table is missing, db_migrator will not generate GNMI table.
| 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')) |
There was a problem hiding this comment.
Fixed, now I check GNMI table at first
| @@ -6,6 +6,6 @@ | |||
| "state": "enabled" | |||
| }, | |||
| "VERSIONS|DATABASE": { | |||
| "VERSION": "version_4_0_4" | |||
| "VERSION": "version_202405_01" | |||
There was a problem hiding this comment.
You should not need to make this change. Is there a reason for this change?
There was a problem hiding this comment.
There's no version_4_0_4 in db_migrator.py, does it make sense to use it in test?
What I did
We will use GNMI container to replace TELEMETRY container in public repo.
GNMI is using GNMI table for configuration, so we need to migrate configuration in CONFIG_DB.
For L2 mode, there's no GNMI configuration in minigraph, and there's no TELEMETRY table in CONFIG_DB, so we will not generate GNMI table.
How I did it
If there's no minigraph and golden config, copy GNMI configuration from TELEMETRY table.
If there're minigraph and golden config, copy GNMI configuration from minigraph or golden config.
How to verify it
Run db migrator unit test, test with minigraph and test without minigraph.
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)