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

Recent commit breaks nhrp_redundancy topotest #16690

Closed
2 tasks done
donaldsharp opened this issue Aug 29, 2024 · 2 comments · Fixed by #16698
Closed
2 tasks done

Recent commit breaks nhrp_redundancy topotest #16690

donaldsharp opened this issue Aug 29, 2024 · 2 comments · Fixed by #16698
Assignees
@louis-6wind
Labels
triage Needs further investigation

Comments

@donaldsharp
Copy link
Member

Description

This commit:

commit af54901405474b0623bda1899424ec18a3240c71 (origin/pr/16640)
Author: Louis Scalbert <[email protected]>
Date:   Fri Aug 23 16:05:45 2024 +0200

    nhrpd: fix sending /32 shortcut

Has broken nhrp_redundancy topotest. This is because the test was looking for iptables but the coder and the CI system does not have it installed so the test is being skipped.

This commit needs to either be fixed ( so the test passes again ) or the test needs to be fixed, if it is wrong.

Additionally the ci system will add the iptables package and this test will start failing so this must be fixed.

Version

latest master

How to reproduce

run nrhp_redundancy with iptables installed

Expected behavior

test to pass

Actual behavior

test does not pass

Additional context

No response

Checklist

  • I have searched the open issues for this bug.
  • I have not included sensitive information in this report.
@donaldsharp donaldsharp added the triage Needs further investigation label Aug 29, 2024
@donaldsharp
Copy link
Member Author

@louis-6wind this is your commit that exhibits this problem. This must be fixed or we will back out the above commit.

louis-6wind added a commit to louis-6wind/frr that referenced this issue Aug 30, 2024
@louis-6wind
tests: fix prefix length in nhrp_redundancy
43cc850 
The expected prefix should be 5.5.5.0/24 otherwise the hosts behind NHRP
client 1 nhc1 (aka. r5) are not reachable via NHRP.

The issue was not seen in the FRR official CI because the tests were
skipped because iptables were missing in CI machines.

It solves the 16690 issue.

Fixes: FRRouting#16690
Signed-off-by: Louis Scalbert <[email protected]>
@louis-6wind louis-6wind mentioned this issue Aug 30, 2024
Merged
@louis-6wind
Copy link
Contributor

Thank you @donaldsharp for noticing. The test was incorrect

louis-6wind added a commit to louis-6wind/frr that referenced this issue Aug 30, 2024
@louis-6wind
tests: fix prefix length in nhrp_redundancy
cfb057a 
The expected prefix should be 5.5.5.0/24 otherwise the hosts behind NHRP
client 1 nhc1 (aka. r5) are not reachable via NHRP.

The issue was not seen in the FRR official CI because the tests were
skipped because iptables were missing in CI machines.

It solves the 16690 issue.

Fixes: FRRouting#16690
Signed-off-by: Louis Scalbert <[email protected]>
mergify bot pushed a commit that referenced this issue Sep 2, 2024
@louis-6wind @mergify
tests: fix prefix length in nhrp_redundancy
33b216e 
The expected prefix should be 5.5.5.0/24 otherwise the hosts behind NHRP
client 1 nhc1 (aka. r5) are not reachable via NHRP.

The issue was not seen in the FRR official CI because the tests were
skipped because iptables were missing in CI machines.

It solves the 16690 issue.

Fixes: #16690
Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit cfb057a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@louis-6wind louis-6wind

Labels
triage Needs further investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

nhrpd, test: fix nhrp_redundancy topotest louis-6wind/frr
2 participants
@donaldsharp @louis-6wind