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

Fix vnet_route_check for active and inactive routes, add --all option #3763

Merged
merged 10 commits into from
Feb 20, 2025

Conversation

mramezani95
Copy link
Contributor

@mramezani95 mramezani95 commented Feb 12, 2025

What I did

Added the options -a and --all to scripts/vnet_route_check.py. Both options are equivalent. If none of them is provided, then when finding the VNET routes that are in APP DB but not in ASIC DB, we will ignore routes in APP DB that are not active.
Mock tests in tests/test_vnet_route_check.py are added to test this behavior.

How I did it

If -a and --all are not provided, we first filter routes in APP DB to find active routes and then check which active routes are not in ASIC DB. The status of each route is retrieved from STATE DB. If a route is not found in STATE DB, then it is considered to be active.

How to verify it

You can verify the behavior by running mock tests in tests/test_vnet_route_check.py, or by manually running the vnet_route_check.py script on a DUT.

Previous command output (if the output of a command-line utility has changed)

N/A

New command output (if the output of a command-line utility has changed)

N/A

… we will not check whether inactive APP DB VNET routes are in ASIC DB or not. Added mock tests to test this behavior.

Signed-off-by: Mahdi Ramezani <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Feb 12, 2025

@mramezani95 , can you provide a sample output before and after this fix?

@prsunny
Copy link
Contributor

prsunny commented Feb 12, 2025

@PrasadSamudrala, please review

@mramezani95 mramezani95 changed the title Adding options -a and --all to scripts/vnet_route_check.py Fix vnet_route_check for active and inactive routes, add --all option Feb 12, 2025
Signed-off-by: Mahdi Ramezani <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Mahdi Ramezani <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mramezani95
Copy link
Contributor Author

@mramezani95 , can you provide a sample output before and after this fix?

The format of the output has not changed. This fix only affects which routes are included in the output. Here is an example output:

"results": {
    "missed_in_asic_db_routes": {
        "Vnet_v6": {
            "routes": [
                "fd01:fc00::1/128"
            ]
        }
    }
}

There is no output if there are no discrepancies between VNET routes in ASIC DB and APP DB.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

prsunny
prsunny previously approved these changes Feb 18, 2025
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Mahdi Ramezani <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Mahdi Ramezani <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

lgtm

@qiluo-msft qiluo-msft merged commit 3c50dee into sonic-net:master Feb 20, 2025
7 checks passed
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