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

[db_migrator] Set correct CURRENT_VERSION, extend UT #2895

Merged
merged 1 commit into from
Jul 6, 2023
Merged

[db_migrator] Set correct CURRENT_VERSION, extend UT #2895

merged 1 commit into from
Jul 6, 2023

Conversation

vadymhlushko-mlnx
Copy link
Contributor

What I did

Set the correct value for the db_migrator class variable CURRENT_VERSION, because if the new DB version was introduced the CURRENT_VERSION variable should have the newest version value.

How I did it

Edit the db_migrator.py.

How to verify it

Extend existing UT.

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)

@vadymhlushko-mlnx
[db_migrator] Set correct CURRENT_VERSION, extend UT
1d93499 
Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>
vaibhavhd
vaibhavhd reviewed Jul 6, 2023
View reviewed changes
tests/db_migrator_test.py
assert not self.check_config_db(dbmgtr.configDB, expected_db.cfgdb)
assert dbmgtr.CURRENT_VERSION == expected_db.cfgdb.get_entry('VERSIONS', 'DATABASE')['VERSION']
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need a dedicated test just for checking this assertion.

My goal is to use UT to always catch the issue that this PR is addressing -- missing update to CURRENT_VERSION

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see how to achieve that, because not every UT mocks the config_db with a VERSIONS key

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. May be we should always verify this assertion in the new UTs that we add. Specifically, in the changes where new version is introduced.

We can have that conversation later, let's take this bug fix for now.

@vaibhavhd vaibhavhd mentioned this pull request Jul 6, 2023
Merged
@vaibhavhd vaibhavhd merged commit 61bad06 into sonic-net:master Jul 6, 2023
@vaibhavhd
Copy link
Contributor

MSFT ADO : 24471596

dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Jul 11, 2023
@dgsudharsan
[submodule] Advance sonic-utilities pointer
03d3573 
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [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))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
@dgsudharsan dgsudharsan mentioned this pull request Jul 11, 2023
Merged
11 tasks
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 11, 2023
@dgsudharsan
[submodule] Advance sonic-utilities pointer (#15779)
5cbae46 
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
@bingwang-ms
Copy link
Contributor

@StormLiangMS Please help cherry-pick this PR to 202305. Thanks!

StormLiangMS pushed a commit that referenced this pull request Aug 14, 2023
@vadymhlushko-mlnx @StormLiangMS
[db_migrator] Set correct CURRENT_VERSION, extend UT (#2895)
dc83719 
Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>

What I did
Set the correct value for the db_migrator class variable CURRENT_VERSION, because if the new DB version was introduced the CURRENT_VERSION variable should have the newest version value.

How I did it
Edit the db_migrator.py.

How to verify it
Extend existing UT.
pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
@vadymhlushko-mlnx @pdhruv-marvell
[db_migrator] Set correct CURRENT_VERSION, extend UT (sonic-net#2895)
79eec39 
Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>

What I did
Set the correct value for the db_migrator class variable CURRENT_VERSION, because if the new DB version was introduced the CURRENT_VERSION variable should have the newest version value.

How I did it
Edit the db_migrator.py.

How to verify it
Extend existing UT.
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
@dgsudharsan @sonic-otn
[submodule] Advance sonic-utilities pointer (sonic-net#15779)
b777108 
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [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))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vaibhavhd vaibhavhd vaibhavhd approved these changes

Assignees
No one assigned
Labels
Included in 202305 Branch Request for 202305 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vadymhlushko-mlnx @vaibhavhd @bingwang-ms @StormLiangMS