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.

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

[test][db migrator] Enhancement: handle the case that no buffer change in the latest database version #1566

Merged
merged 1 commit into from
May 3, 2021

Conversation

stephenxs
Copy link
Collaborator

What I did

Enhancement: handle the case that no buffer change in the latest database version
Current, the following two versions are the same:

  • The latest version changed by mellanox_buffer_migrator
  • The latest version in CONFIG_DB

That won't be true if another part in CONFIG_DB is updated. In that case, the latest version in CONFIG_DB will be greater than the latest version in mellanox_buffer_migrator.
However, this can break the buffer migrator unit test:

  • The db_migrator will always migrate the database to the latest version
  • The config database version check will fail in case the latest version in the config database doesn't match that defined in the buffer migrator.

This is to support this case.

Signed-off-by: Stephen Sun stephens@nvidia.com

How I did it

Advance the database version in the expected config database before version checking if the following conditions satisfied:

  • The version in the expected database is the latest version in the Mellanox buffer migrator
  • The version in the result database is greater than the latest version in the Mellanox buffer migrator

How to verify it

Run unit test.

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)

…sted

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@liat-grozovik
Copy link
Collaborator

@stephenxs is this fix needed for 202012? if so, please check if it can be cherry picked or need to have new PR against 202012.

@stephenxs
Copy link
Collaborator Author

@stephenxs is this fix needed for 202012? if so, please check if it can be cherry picked or need to have new PR against 202012.

Hi @liat-grozovik
I see a PR depending on this one (1473) was tagged as "Request for 202012". So, yes, we need it for 202012.
Just checked, it can be cherry-picked smoothly.

@liat-grozovik
Copy link
Collaborator

@neethajohn could you please review?

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jleveque jleveque merged commit 2e09b22 into sonic-net:master May 3, 2021
@stephenxs stephenxs deleted the new-db-version branch May 3, 2021 23:20
daall pushed a commit that referenced this pull request May 5, 2021
…sted (#1566)

Enhancement: handle the case that no buffer change in the latest database version
Current, the following two versions are the same:
- The latest version changed by mellanox_buffer_migrator
- The latest version in CONFIG_DB

That won't be true if another part in CONFIG_DB is updated. In that case, the latest version in CONFIG_DB will be greater than the latest version in mellanox_buffer_migrator.
However, this can break the buffer migrator unit test:
- The db_migrator will always migrate the database to the latest version
- The config database version check will fail in case the latest version in the config database doesn't match that defined in the buffer migrator.

This is to support this case.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
liat-grozovik pushed a commit that referenced this pull request May 23, 2021
…rator test cases as well (#1614)

- What I did
Originally, the method advance_version_for_expected_database was introduced (in #1566) to handle the case the latest version in CONFIG_DB is greater than the latest version in mellanox_buffer_migrator.
Now there are other database migrators whose test cases can also encounter this situation, like port auto-negotiation (#1568) and port-channel for LACP key (#1473).
So I would like to make the method public, available for all database migrators.
Related database migrator test cases have been updated accordingly.

- How to verify it
Run the unit test.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
gitsabari pushed a commit to gitsabari/sonic-utilities that referenced this pull request Jun 15, 2021
…sted (sonic-net#1566)

Enhancement: handle the case that no buffer change in the latest database version
Current, the following two versions are the same:
- The latest version changed by mellanox_buffer_migrator
- The latest version in CONFIG_DB

That won't be true if another part in CONFIG_DB is updated. In that case, the latest version in CONFIG_DB will be greater than the latest version in mellanox_buffer_migrator.
However, this can break the buffer migrator unit test:
- The db_migrator will always migrate the database to the latest version
- The config database version check will fail in case the latest version in the config database doesn't match that defined in the buffer migrator.

This is to support this case.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
gitsabari pushed a commit to gitsabari/sonic-utilities that referenced this pull request Jun 15, 2021
…rator test cases as well (sonic-net#1614)

- What I did
Originally, the method advance_version_for_expected_database was introduced (in sonic-net#1566) to handle the case the latest version in CONFIG_DB is greater than the latest version in mellanox_buffer_migrator.
Now there are other database migrators whose test cases can also encounter this situation, like port auto-negotiation (sonic-net#1568) and port-channel for LACP key (sonic-net#1473).
So I would like to make the method public, available for all database migrators.
Related database migrator test cases have been updated accordingly.

- How to verify it
Run the unit test.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-utilities that referenced this pull request Aug 10, 2021
…rator test cases as well (sonic-net#1614)

- What I did
Originally, the method advance_version_for_expected_database was introduced (in sonic-net#1566) to handle the case the latest version in CONFIG_DB is greater than the latest version in mellanox_buffer_migrator.
Now there are other database migrators whose test cases can also encounter this situation, like port auto-negotiation (sonic-net#1568) and port-channel for LACP key (sonic-net#1473).
So I would like to make the method public, available for all database migrators.
Related database migrator test cases have been updated accordingly.

- How to verify it
Run the unit test.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
yxieca pushed a commit that referenced this pull request Oct 18, 2021
What I did
The port-channel key migrator was introduced in version 2_0_2 so the expected database version of the test case should be 2_0_2.
It was modified to 2_0_3 when the new version was introduced by mistake. This won't fail the test but disable the require its database version to be updated every time a new version is introduced. (Refer #1566 and #1614 for details)
This is to correct it by changing it back to 2_0_2.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants