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

Add LinuxAdmin::NetworkInterface.list #249

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

agrare
Copy link
Member

@agrare agrare commented Aug 1, 2024

Previously there wasn't a method for listing interfaces on the system, which was okay because we assumed an eth0 was available but as we move away from that and use the standard predictable network interface names it would be helpful to get a list.

This can be followed up with attributes indicating if an interface is a LOOPBACK type and if the interface is the dev registered for the default route.

Related ManageIQ/manageiq-appliance-build#573

private_class_method def self.ip_link
require "json"

ip_link = Common.run!(Common.cmd("ip"), :params => ["--json", "link"])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE I think the --json output format would be helpful for e.g. ip addr show also which currently does some regex parsing of the output but I wanted to leave that for a standalone PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agrare agrare requested a review from bdunne August 1, 2024 16:32
@agrare agrare force-pushed the add_network_interface_list branch 3 times, most recently from dc04eb2 to 861f7f8 Compare August 1, 2024 16:37
@Fryguy
Copy link
Member

Fryguy commented Aug 1, 2024

ip doesn't exist on a Mac - is that ok for this PR?

@agrare
Copy link
Member Author

agrare commented Aug 1, 2024

ip doesn't exist on a Mac - is that ok for this PR?

I think you're looking for the OsxAdmin repo 😆

In all seriousness though, LinuxAdmin::NetworkInterface#initialize calls ip addr show @interface, and ip route already so I don't think this would work on a mac as is.

@Fryguy
Copy link
Member

Fryguy commented Aug 2, 2024

code-wise LGTM - There are a lot of rubocop issues, and I wasn't sure if the new code was just following an existing pattern, or if this should be cleaned up.

@agrare
Copy link
Member Author

agrare commented Aug 2, 2024

There are a lot of rubocop issues, and I wasn't sure if the new code was just following an existing pattern

Yeah that is just how the specs are currently written so I wanted to keep it looking consistent.

@agrare
Copy link
Member Author

agrare commented Aug 5, 2024

@Fryguy I tackled the rubocop warnings here in #252

@Fryguy
Copy link
Member

Fryguy commented Aug 5, 2024

Merged #252 252, but now this is conflict

@Fryguy Fryguy self-assigned this Aug 5, 2024
@agrare
Copy link
Member Author

agrare commented Aug 5, 2024

Yes this added some ip command mocks so conflict was expected

Previously there wasn't a method for listing interfaces on the system,
which was okay because we assumed an eth0 was available but as we move
away from that and use the standard predictable network interface names
it would be helpful to get a list.
@agrare agrare force-pushed the add_network_interface_list branch from 19fb4df to 5204709 Compare August 5, 2024 18:09
@miq-bot
Copy link
Member

miq-bot commented Aug 5, 2024

Checked commit agrare@5204709 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@agrare
Copy link
Member Author

agrare commented Aug 5, 2024

Rebased

@agrare agrare removed the unmergeable label Aug 5, 2024
@Fryguy Fryguy merged commit ae97093 into ManageIQ:master Aug 5, 2024
5 checks passed
@agrare agrare deleted the add_network_interface_list branch August 5, 2024 20:07
bdunne added a commit that referenced this pull request Aug 13, 2024
Breaking Changes:
- Use --json for ip command output #251

Added:
- Add network interface loopback #254
- Add LinuxAdmin::NetworkInterface.list #249

Changed:
- Parse config file on NetworkInterface#reload #253

Test:
- Use ruby 3.1 and rails 7 for code coverage #250
- Fix rubocop warnings in NetworkInterface Spec #252
- Update paambaati/codeclimate-action action to v8 #248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants