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

tests: cleanup ospf6 ecmp inter area #17707

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gromit1811
Copy link
Contributor

Replaces/supersedes #16811. Addresses all review comments.

Compare JSON routes to check for success instead of counting nexthops. Also fix bad interface shutdown config.

Note that like #16811, this does not fix intermittent test case failures. These are actual ospf6d errors, not test case bugs. See #16197 for details, in particular comments starting at #16197 (comment)

@frrbot frrbot bot added the tests Topotests, make check, etc label Dec 22, 2024
@ton31337
Copy link
Member

Why open this instead of amending #16811 and continue the work there?

@gromit1811
Copy link
Contributor Author

Because @louis-6wind said that he doesn't have time to complete #16811 and I don't have write access there (at least if I understand correctly how github handles PRs internally)

@ton31337
Copy link
Member

Oh, I see, thanks, I missed Lous was the author.

@ton31337
Copy link
Member

Can you fix the syling (frrbot)?

Use router_json_cmp when checking for proper nexthops to get better
diagnostics in case of mismatches. Add step() messages to see which check
failed.

Based on original PR#16811 commit:

    expect_num_nexthops() errors are not understandable.

    Use router_json_cmp.

    Signed-off-by: Louis Scalbert <[email protected]>

Signed-off-by: Martin Buck <[email protected]>
Drop eth3 shutdown from ospf6d.conf - it doesn't do anything there. And it
actually shouldn't do anything: eth3 on r7/r8 are used as loopback-like
interfaces to inject the address on eth2 into OSPFv3. So they need to be up
for eth2 to work as expected.

Based on original PR#16811 commit:

    eth3 shutdown is not applied because it is ospf6d.conf.

    Signed-off-by: Louis Scalbert <[email protected]>

Signed-off-by: Martin Buck <[email protected]>
@gromit1811
Copy link
Contributor Author

Ah, didn't notice that. Fixed. CI failed the first time, which is not impossible but pretty unlikely given the rareness of the bug this test case checks. Let's see whether it succeeds this time. If it fails again, there might be a bug in the routes the test case compares against.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master size/XL tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants