Repository navigation
Factory config test bpi r3 - #1718
Merged
Merged
Conversation
Contributor
Author
|
I feel like the logic about getting the hostname is a bit malplaced in infamy, another solution would be to have all logic in the test, and just have the llscan in infamy. What do you think is best? @wkz |
mattiaswal
marked this pull request as draft
October 21, 2026 06:23
wkz
requested changes
Oct 22, 2026
wkz
left a comment
wkz
left a comment
Contributor
There was a problem hiding this comment.
As discussed AFK, I think a better approach might be:
- Make
ll6ping()return all neighbors - like it should always have done - Refine the attachment logic to pick the management IP from the physical topology when that is defined, otherwise fall back to a ping.
- Require that the ping sees only one neighbor - if multiple peers are on the link, and the user has not specified which one to use, then we should fail with a message saying so. That should help the next person setting up a test system with devices with bridging in their factory configs.
mattiaswal
force-pushed
the
factory-on-bpi
branch
from
October 22, 2026 07:45
ff50e0b to
a3b3aae
Compare
mattiaswal
marked this pull request as ready for review
October 22, 2026 08:15
mattiaswal
force-pushed
the
factory-on-bpi
branch
3 times, most recently
from
October 23, 2026 07:40
4112c85 to
bb452d4
Compare
wkz
approved these changes
Oct 23, 2026
wkz
left a comment
wkz
left a comment
Contributor
There was a problem hiding this comment.
We got there in the end! 😄
Letting the (quite unique) test handle this complexity feels right! 👍
A test may need a DUT outside its logical topology, and the mapping lookup raised KeyError for any name not in it.
A factory config that bridges the mgmt port with links to other DUTs makes all of them answer the host's IPv6LL probe, so the test attaches to the wrong box. Disabling every non-management port on every DUT in the physical topology leaves only the target on that port.
mattiaswal
force-pushed
the
factory-on-bpi
branch
from
October 23, 2026 09:17
bb452d4 to
64fecb4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BPI has a lan bridge causing the factory-config test to fail, since it may connect to some other dut through the bridge.
This adds a possible layer that it need to connect to a unit with the correct hostname, you probably never want to use it outside factory config test.
Description
Checklist
Tick relevant boxes, this PR is-a or has-a: