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

Configure all interfaces where DHCP answers #50

Merged
merged 2 commits into from
May 21, 2019

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented May 15, 2019

  • Configure all interfaces where DHCP answer

  • Fix grep stderr output in is_raspberry that happens in non-rpi devices

@aplanas
Copy link
Contributor Author

aplanas commented May 15, 2019

Note that is on top of the other branch. I will rebase accordingly if the other branch gets merged.

@Vogtinator
Copy link
Member

Vogtinator commented May 15, 2019

Question is whether the pre-MULTINET behaviour was actually useful.

IMO we can just use this by default, I don't see any downside in enabling DHCP for all interfaces. Which interface is first is also not really guaranteed anyway.

@gmoro: Any reason this was done this IMO overcomplicated way? I'd rather just enable DHCP for all interfaces which get an IP.

@aplanas
Copy link
Contributor Author

aplanas commented May 15, 2019

@Vogtinator I agree with your comment. I would drop this logic of IPADDRESS vs GATEWAYS, but maybe there were a reason for that.

@Vogtinator
Copy link
Member

@aplanas: I'd say: Go for it. Then we also don't need a config option.

@aplanas
Copy link
Contributor Author

aplanas commented May 15, 2019

@Vogtinator done.

files/usr/share/defaults/jeos-firstboot.conf Outdated Show resolved Hide resolved
files/usr/lib/jeos-firstboot Outdated Show resolved Hide resolved
@Vogtinator
Copy link
Member

IMO GATEWAYS should also be ignored. Getting an IP address is good enough for DHCP activation.

@aplanas
Copy link
Contributor Author

aplanas commented May 15, 2019

IMO GATEWAYS should also be ignored. Getting an IP address is good enough for DHCP activation.

I agree. I will change that according to @gmoro comments.

@gmoro
Copy link
Collaborator

gmoro commented May 15, 2019

Question is whether the pre-MULTINET behaviour was actually useful.

IMO we can just use this by default, I don't see any downside in enabling DHCP for all interfaces. Which interface is first is also not really guaranteed anyway.

@gmoro: Any reason this was done this IMO overcomplicated way? I'd rather just enable DHCP for all interfaces which get an IP.

This part was actually coded by the maintainer of wicked, and the idea was to use the first configurable network, of course we can rethink it, if we have multiple nets which one are we going to use for the default route? What exactly would be the use case for multiple networks to be configured in the firstboot?

@Vogtinator
Copy link
Member

This part was actually coded by the maintainer of wicked, and the idea was to use the first configurable network, of course we can rethink it, if we have multiple nets which one are we going to use for the default route?

Neither of those options are perfect, but potentially having conflicting options for a default route (which shouldn't happen in a well defined environment) might be better than having to set up DHCP for other interfaces manually in case the selection was wrong.

That depends on wicked's behaviour for the conflict situation though, do you have any more info about that?

@aplanas
Copy link
Contributor Author

aplanas commented May 15, 2019

@gmoro: Any reason this was done this IMO overcomplicated way? I'd rather just enable DHCP for all interfaces which get an IP.

This part was actually coded by the maintainer of wicked, and the idea was to use the first configurable network, of course we can rethink it, if we have multiple nets which one are we going to use for the default route?

Uhm, I wonder if this is not task for the DHCP administrator to decide, and not for firstboot.

What exactly would be the use case for multiple networks to be configured in the firstboot?

The use case is simple: derivatives from JeOS can be used in scenarios were multiple interfaces are available. In cloud and kubernetes cloud-init or ignition will configure the interfaces, but in other environments firstboot will need to assign some same defaults.

@Vogtinator
Copy link
Member

LGTM, just needs to be rebased on master after the other PR gets merged

@aplanas
Copy link
Contributor Author

aplanas commented May 20, 2019

Rebased on top of the other PR (that is now also rebased from the new master)

@aplanas aplanas force-pushed the fix_multiple_interfaces branch 3 times, most recently from 1c750d3 to 5dc3ec4 Compare May 21, 2019 07:30
@gmoro gmoro self-requested a review May 21, 2019 09:23
@gmoro
Copy link
Collaborator

gmoro commented May 21, 2019

That depends on wicked's behaviour for the conflict situation though, do you have any more info about that?

I don't, but let's go with it as it is.

Copy link
Collaborator

@gmoro gmoro left a comment

Choose a reason for hiding this comment

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

LGTM

@Vogtinator
Copy link
Member

Don't merge before #49 got merged and this PR rebased on top.

@aplanas aplanas force-pushed the fix_multiple_interfaces branch 3 times, most recently from 05bed79 to ff584c4 Compare May 21, 2019 11:51
If we have multiple interfaces that are configured via DHCP, only
the first one that provides an IP and a gateway is configured.

This patch configures all interfaces where a DHCP resolver is
answering the request.
@Vogtinator Vogtinator merged commit 23bc1d3 into openSUSE:master May 21, 2019
@Vogtinator Vogtinator changed the title Add JEOS_MULTINET configuration option Configure all interfaces where DHCP answers May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants