-
Notifications
You must be signed in to change notification settings - Fork 30
Add distro specific network interface class #132
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 distro specific network interface class #132
Conversation
lib/linux_admin/network_interface.rb
Outdated
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.
You'll never hit this raise, right?
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.
Right! That was from before I put the generic class in.
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.
@bdunne Should we also clear the DNS server and search order here and assume that will be set by DHCP?
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.
yes
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.
Done
99dcd80
to
f77d6b8
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.
What is the RH
in NetworkInterfaceRH
? Does that stand for Red Hat (the company) or RHEL (the OS)? If the latter, then I would rename to NetworkInterfaceRHEL
.
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.
My intention was "Red Hat" as the class is able to provide network configuration for CentOS, Fedora, and RHEL.
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.
What if it's IPv6?
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.
Typo, this will work for both, I used to have two different parts to handle the different types and forgot to change the message/docs
Awesome stuff here! |
676147f
to
e483a86
Compare
lib/linux_admin/network_interface.rb
Outdated
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.
This should be message, result
The NetworkInterface switches which object gets instantiated based on the distro that we are running on. https://trello.com/c/rWKh4KQs
This prevents us from shelling out every time a user asks for some network setting. Also created a reload method to re-parse the settings. https://trello.com/c/rWKh4KQs
…h NetworkInterface#reload https://trello.com/c/rWKh4KQs
Also defined an exception type for the NetworkInterface classes and raise that when `run!` raises a CommandResultError https://trello.com/c/rWKh4KQs
7c2c432
to
74ba406
Compare
Checked commits carbonin/linux_admin@964e81e~...74ba406 with ruby 1.9.3, rubocop 0.34.2, and haml-lint 0.13.0 lib/linux_admin/network_interface.rb
spec/network_interface/network_interface_rh_spec.rb
|
👍 LGTM, will merge when green. |
Add distro specific network interface class
Implements the current functions of the LinuxAdmin::IpAddress class as well as distro specific functions for setting IP addresses and DNS servers.
Uses the distro detection in LinuxAdmin::Distros to determine which sub-class to instantiate at run-time.
Currently supports Red Hat based distros (CentOS, Fedora, and RHEL).
@bdunne @Fryguy please review.