-
Notifications
You must be signed in to change notification settings - Fork 112
test: improve method for finding secondary interface #792
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
Conversation
Reviewer's GuideTest playbook now dynamically determines the actual network interface name (not just aliases) by parsing “ip addr show” output and expands its fallback search to include common secondary interfaces (eth1, ens4). File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @richm - I've reviewed your changes - here's some feedback:
- Avoid calling
ip addr showtwice per interface by capturing its output once for parsing, which will simplify the logic and reduce overhead. - Consider using
ip -o link showorip -j linkfor easier and more robust extraction of the real interface name instead of relying onawkover indented lines.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid calling `ip addr show` twice per interface by capturing its output once for parsing, which will simplify the logic and reduce overhead.
- Consider using `ip -o link show` or `ip -j link` for easier and more robust extraction of the real interface name instead of relying on `awk` over indented lines.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #792 +/- ##
==========================================
+ Coverage 43.11% 43.25% +0.13%
==========================================
Files 12 12
Lines 3124 3121 -3
==========================================
+ Hits 1347 1350 +3
+ Misses 1777 1771 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Ignore ansible-lint failures for now: They added a new check - task names must be unique in a given file - this is not urgent to fix. |
|
[citest] |
|
The centos-10 failures are known - 802.1x tests are not working The fedora 42 failures are known - ansible 2.19 issues |
In some cases, the interface given in MAC_ADDR_MATCH_INTERFACE can be an
alias or altname. The test cannot use the altname, it must use the "real"
interface name.
For example, on some systems, if `MAC_ADDR_MATCH_INTERFACE=enX1`, the test
will fail because it is an altname for `ens4`:
```
+ ip addr show enX1
3: ens4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000
link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff
altname enp0s4
altname enx525400123457
altname enX1
```
The test will now parse the output of `ip addr show $name` to get the real interface name.
Also, improve the fallback method to look for common secondary interface names
such as eth1 and ens4 in case MAC_ADDR_MATCH_INTERFACE is not one of these.
Signed-off-by: Rich Megginson <[email protected]>
478502d to
8c45acf
Compare
|
[citest] |
In some cases, the interface given in MAC_ADDR_MATCH_INTERFACE can be an
alias or altname. The test cannot use the altname, it must use the "real"
interface name.
For example, on some systems, if
MAC_ADDR_MATCH_INTERFACE=enX1, the testwill fail because it is an altname for
ens4:The test will now parse the output of
ip addr show $nameto get the real interface name.Also, improve the fallback method to look for common secondary interface names
such as eth1 and ens4 in case MAC_ADDR_MATCH_INTERFACE is not one of these.
Signed-off-by: Rich Megginson [email protected]
Summary by Sourcery
Improve detection of secondary network interfaces in MAC address match tests to handle alias names and provide common fallbacks.
Tests:
interfaces_to_checklist combining the default interface and common names for lookupinterfaces_to_checkip addr showoutput and default to UNKNOWN_DEVICE if none is found