-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: Remove some inconsistent local labels in check in test_ldp_vpls_topo1 #3779
tests: Remove some inconsistent local labels in check in test_ldp_vpls_topo1 #3779
Conversation
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-6687/ This is a comment from an automated CI system. Warnings Generated during build:Debian 8 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 8 amd64 build:
Ubuntu 14.04 amd64 build: Successful with additional warningsDebian Package lintian failed for Ubuntu 14.04 amd64 build:
Ubuntu 16.04 amd64 build: Successful with additional warningsDebian Package lintian failed for Ubuntu 16.04 amd64 build:
Ubuntu 18.04 amd64 build: Successful with additional warningsDebian Package lintian failed for Ubuntu 18.04 amd64 build:
Debian 9 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 9 amd64 build:
Ubuntu 16.04 i386 build: Successful with additional warningsDebian Package lintian failed for Ubuntu 16.04 i386 build:
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base3 Static Analyzer issues remaining.See details at |
"remoteLabel":"imp-null", | ||
"inUse":1 | ||
}, | ||
{ | ||
"addressFamily":"ipv4", | ||
"prefix":"2.2.2.2/32", | ||
"neighborId":"3.3.3.3", | ||
"localLabel":"17", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this topotest can still fail with these changes since the remoteLabel
of one router is the localLabel
of another one (so remoteLabel
is also unpredictable). I think we need to either a) stop checking the remoteLabel
as well or b) only check if the local/remote labels are equal to imp-null
or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mwinter-osr can you please respond to this or close the PR? This review comment is getting close to 2 months old 🙂
Signed-off-by: Martin Winter <mwinter@opensourcerouting.org>
f097981
to
97ec268
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the PR to remove the remoteLabel checks as well (since they are also unpredictable). Checking if the "inUse" fields are correct or not should be enough for the purposes of this test.
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7144/ This is a comment from an automated CI system. clang_check |
Summary
Fixes #3718.
Remove inconsistent labels from check in test_ldp_vpls_topo1
Signed-off-by: Martin Winter mwinter@opensourcerouting.org