[db_migrator] Remove hardcoded config and migrate config from minigraph by vaibhavhd · Pull Request #2887 · sonic-net/sonic-utilities

vaibhavhd · 2023-06-27T21:28:16Z

What I did

Microsoft ADO: 18217044

This PR is follow up to an old PR: #2515

This PR addresses two issues:

Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py.
Not migrating these entries if minigraph.xml file is missing.
Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own.

How I did it

This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator.

How to verify it

Tested on a physical device.

This change works w/ another change to fix config-setup, database dependency sonic-net/sonic-buildimage#14933

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)

…inigraph.py

qiluo-msft · 2023-06-28T22:25:12Z

scripts/db_migrator.py

        # RESTAPI - add missing key
+        if not self.minigraph_data or 'RESTAPI' not in self.minigraph_data:
+            return
+        RESTAPI = self.minigraph_data['RESTAPI']


RESTAPI

RESTAPI -> restapi_data. It's not a constant anymore. Many occurences in this file. #Closed

Updated now

qiluo-msft · 2023-06-28T22:29:09Z

scripts/db_migrator.py

+        # load config data from minigraph to get the default/hardcoded values from minigraph.py
+        # this is to avoid duplicating the hardcoded these values in db_migrator
+        self.minigraph_data = None
+        if os.path.isfile(MINIGRAPH_FILE):


isfile

Could you add a testcase for missing MINIGRAPH_FILE? It would be even better if you can add a testcase for L2 switch mode #Closed

Added a new testcase for missing minigraph. We can discuss what other fields are expected/unexpected in L2 switch's case and add a test for that in future PR. What do you suggest?

Add a test for L2 switch in future is okay.

As per offline discussion: from unit test level the test for missing minigraph and absence of default entries is sufficient. L2 switch config and functionality needs to be tested separately as part of sonic-mgmt test. Raised a test gap for this: sonic-net/sonic-mgmt#8777

yxieca · 2023-06-28T22:51:30Z

scripts/db_migrator.py

+        # this is to avoid duplicating the hardcoded these values in db_migrator
+        self.minigraph_data = None
+        if os.path.isfile(MINIGRAPH_FILE):
+            self.minigraph_data = parse_xml(MINIGRAPH_FILE)


Do you need to protect against parse_xml throwing error?

Sure. Added now.

qiluo-msft · 2023-06-29T18:25:52Z

tests/db_migrator_test.py

+        dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'cross_branch_upgrade_without_mg_2_0_2_expected.json')
+        expected_db = Db()
+
+        expected_db


expected_db

remove this line? #Closed

Removed here, and also in the old testcase.

…ph (#2887) Microsoft ADO: 18217044 This PR is follow up to an old PR: #2515 This PR addresses two issues: Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py. Not migrating these entries if minigraph.xml file is missing. Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own. How I did it This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator. How to verify it Tested on a physical device.

Update sonic-utilities submodule pointer to include the following: * ff380e0 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580)) * 61bad06 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895)) * 6b8ee47 [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885)) * 73d8d63 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750)) * 7bc08c2 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887)) * b1aa942 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676)) * 316b14c Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698)) * dc2945b [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737)) * 8414a70 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525)) * 0b629ba Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882)) * 3ba8241 [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839)) * fceef2e [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789)) Signed-off-by: dgsudharsan <sudharsand@nvidia.com>

Update sonic-utilities submodule pointer to include the following: * ff380e0 [hash]: Implement GH frontend ([#2580](sonic-net/sonic-utilities#2580)) * 61bad06 [db_migrator] Set correct CURRENT_VERSION, extend UT ([#2895](sonic-net/sonic-utilities#2895)) * 6b8ee47 [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([#2885](sonic-net/sonic-utilities#2885)) * 73d8d63 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([#2750](sonic-net/sonic-utilities#2750)) * 7bc08c2 [db_migrator] Remove hardcoded config and migrate config from minigraph ([#2887](sonic-net/sonic-utilities#2887)) * b1aa942 [generate_dump]: Enhance show techsupport for Marvell platform ([#2676](sonic-net/sonic-utilities#2676)) * 316b14c Add support for secure upgrade ([#2698](sonic-net/sonic-utilities#2698)) * dc2945b [dns] Implement config and show commands for static DNS. ([#2737](sonic-net/sonic-utilities#2737)) * 8414a70 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([#2525](sonic-net/sonic-utilities#2525)) * 0b629ba Revert [chassis][voq] Clear fabric counters queue/port (2789) ([#2882](sonic-net/sonic-utilities#2882)) * 3ba8241 [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([#2839](sonic-net/sonic-utilities#2839)) * fceef2e [chassis][voq] Clear fabric counters queue/port ([#2789](sonic-net/sonic-utilities#2789)) Signed-off-by: dgsudharsan <sudharsand@nvidia.com>

…ph (#2887) Microsoft ADO: 18217044 This PR is follow up to an old PR: #2515 This PR addresses two issues: Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py. Not migrating these entries if minigraph.xml file is missing. Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own. How I did it This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator. How to verify it Tested on a physical device.

qiluo-msft · 2023-07-19T23:29:39Z

This commit could not be cleanly cherry-pick to 202012. Please raise another PR on that branch.

…ph (sonic-net#2887) Microsoft ADO: 18217044 This PR is follow up to an old PR: sonic-net#2515 This PR addresses two issues: Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py. Not migrating these entries if minigraph.xml file is missing. Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own. How I did it This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator. How to verify it Tested on a physical device.

StormLiangMS · 2023-07-25T01:45:37Z

caused UT failure in internal branch sync, @vaibhavhd

{
'values_changed': {
"root['certs']['ca_crt']": {
'new_value': '/etc/sonic/credentials/restapica.crt',
'old_value': '/et...name'
]": {'new_value': 'client.restapi.sonic', 'old_value': 'client.restapi.sonic.gbl,vnetds.prod.int.azure-int.net'}}}

liuh-80 · 2023-07-26T06:29:24Z

Cherry-pick to 202012 PR: #2924

…ph (#2887) (#2924) Microsoft ADO: 18217044 This PR is follow up to an old PR: #2515 This PR addresses two issues: Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py. Not migrating these entries if minigraph.xml file is missing. Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own. How I did it This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator. How to verify it Tested on a physical device.

…ph (sonic-net#2887) Microsoft ADO: 18217044 This PR is follow up to an old PR: sonic-net#2515 This PR addresses two issues: Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py. Not migrating these entries if minigraph.xml file is missing. Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own. How I did it This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator. How to verify it Tested on a physical device.

…ph (#2887) Microsoft ADO: 18217044 This PR is follow up to an old PR: #2515 This PR addresses two issues: Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py. Not migrating these entries if minigraph.xml file is missing. Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own. How I did it This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator. How to verify it Tested on a physical device.

Update sonic-utilities submodule pointer to include the following: * ff380e0 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580)) * 61bad06 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895)) * 6b8ee47 [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885)) * 73d8d63 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750)) * 7bc08c2 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887)) * b1aa942 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676)) * 316b14c Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698)) * dc2945b [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737)) * 8414a70 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525)) * 0b629ba Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882)) * 3ba8241 [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839)) * fceef2e [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789)) Signed-off-by: dgsudharsan <sudharsand@nvidia.com>

vaibhavhd added 2 commits June 26, 2023 21:58

[db migrator] Remove hardcoded migration and replace w/ config from m…

3a85220

…inigraph.py

Remove routing config mode from this PR

7ae4c94

vaibhavhd added Request for 202012 Branch Request for 202205 Branch Request for 202211 Branch Request for 202305 Branch labels Jun 27, 2023

vaibhavhd added 3 commits June 28, 2023 19:13

Add sample minigraph.xml for UT

6e28286

Fix test failures

2b66fa3

Fix test input

fcfd1fb

vaibhavhd requested review from qiluo-msft and yxieca June 28, 2023 22:12

qiluo-msft reviewed Jun 28, 2023

View reviewed changes

qiluo-msft requested review from qiluo-msft and zbud-msft June 28, 2023 22:29

yxieca reviewed Jun 28, 2023

View reviewed changes

Review addressed: added a UT for missing minigraph

94bc8bd

yxieca previously approved these changes Jun 29, 2023

View reviewed changes

Fix exception handling

126c187

vaibhavhd dismissed yxieca’s stale review via 126c187 June 29, 2023 16:15

qiluo-msft reviewed Jun 29, 2023

View reviewed changes

Remove unwanted line from unit test

2240d10

vaibhavhd requested a review from qiluo-msft June 29, 2023 20:29

qiluo-msft approved these changes Jun 29, 2023

View reviewed changes

vaibhavhd merged commit 7bc08c2 into sonic-net:master Jun 29, 2023

vaibhavhd deleted the route-mode-separated branch June 29, 2023 21:11

liuh-80 added the Approved for 202012 Branch Approved for 202012 Branch label Jul 6, 2023

vaibhavhd mentioned this pull request Jul 7, 2023

telemetry process can not start if SONiC is installed from ONIE in 202012 sonic-net/sonic-buildimage#12499

Closed

yxieca added the Included in 202205 Branch label Jul 7, 2023

dgsudharsan mentioned this pull request Jul 11, 2023

[submodule] Advance sonic-utilities pointer sonic-net/sonic-buildimage#15779

Merged

11 tasks

StormLiangMS added the Included in 202305 Branch label Jul 19, 2023

qiluo-msft added the Cherry Pick Conflict_202012 label Jul 19, 2023

vaibhavhd mentioned this pull request Jul 21, 2023

[202012][db_migrator] Remove hardcoded config and migrate config from minigraph #2924

Merged

StormLiangMS added the Bug label Jul 25, 2023

StormLiangMS mentioned this pull request Jul 25, 2023

[submodule][202305] Update submodule sonic-utilities to the latest HEAD automatically sonic-net/sonic-buildimage#15951

Merged

liuh-80 added the Cherry Pick PR created_202012 Label for a manually cherry-pick PR created for 202012 cherry-pick conflict PR. label Jul 26, 2023

yxieca added the Included in 202211 Branch label Aug 29, 2023

qiluo-msft mentioned this pull request Sep 27, 2024

Create a test case for testing L2 configuration. sonic-net/sonic-mgmt#14714

Merged

7 tasks

Conversation

vaibhavhd commented Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How I did it

How to verify it

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)

Uh oh!

qiluo-msft Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Jun 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft commented Jul 19, 2023

Uh oh!

StormLiangMS commented Jul 25, 2023

Uh oh!

liuh-80 commented Jul 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vaibhavhd commented Jun 27, 2023 •

edited

Loading

qiluo-msft Jun 28, 2023 •

edited

Loading

qiluo-msft Jun 28, 2023 •

edited

Loading

qiluo-msft Jun 29, 2023 •

edited

Loading