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

allow unmanaged NICs #1117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

allow unmanaged NICs #1117

wants to merge 2 commits into from

Conversation

bmwiedemann
Copy link
Member

@bmwiedemann bmwiedemann commented Feb 23, 2017

Why is this change necessary?
We want to allow network interfaces that are not managed by chef

How does it address the issue?
we keep interfaces around when the config file has an "#unmanaged" marker comment

Is there additional information worth sharing like links to a Trello
card, bug references, testing advice or dependencies to other pull
requests?

vuntz said, there is a FATE or bug for this
and also Bernhard needs this for running SOC on multicast-vxlan overlay networks where the underlying NIC (e.g. eth0) still needs to work.

This also includes a cleanup commit that is useful without the 2nd one

nic.destroy

nicfiles.each do |file|
::File.delete(file) if ::File.exists?(file)

Choose a reason for hiding this comment

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

Lint/DeprecatedClassMethods: File.exists? is deprecated in favor of File.exist?.


# Ignore unmanaged interfaces
nicfiles.each do |file|
return if system("fgrep", "-q", "#unmanaged", file)

Choose a reason for hiding this comment

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

Lint/NonLocalExitFromIterator: Non-local exit from iterator, without return value. next, break, Array#find, Array#any?, etc. is preferred.

@vuntz
Copy link
Member

vuntz commented Feb 23, 2017

I'm nearly sure we wanted to do that in network.json...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

5 participants