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

networkd: force bringing up devices with no IP addresses #34

Closed
wants to merge 2 commits into from

Conversation

cyphermox
Copy link
Contributor

Signed-off-by: Mathieu Trudel-Lapierre mathieu.trudel-lapierre@canonical.com

@cyphermox
Copy link
Contributor Author

This is failing tests on purpose for now, as it's up for review but might need more work to make sure things are working correctly.

The intent is to fix bringing up anonymous bridges, it seems to work reasonably well for that. Non-virtual interfaces will also be affected though, and we should make sure they are fine to be UP, NO-CARRIER in this case. Where people absolutely want the devices to be unconfigured, then we should expect the devices to not be listed at all in netplan YAML.

@cyphermox cyphermox added the RFC Request for comment (don't merge yet) label May 29, 2018
src/networkd.c Outdated
especially useful for bringing up anonymous bridges
*/
if (!def->dhcp4 && !def->dhcp6 && !def->ip4_addresses && !def->ip6_addresses)
g_string_append(network, "ConfigureWithoutCarrier=true\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

In some cases folks want something like lxdbr0 or virbr0 where the bridge itself has an address, but no members. This check looks like it won't set ConfigureWithoutCarrier=true if you assign any DHCP or IP values. I understand DHCP, as it needs an interface to emit the DHCP request, but static IP assignment should be OK, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a different issue; a bridge with no members will be handled separately in a rework of the validation code.

Here, it's exactly what we want to "configure" the interface is there's no carrier and no address; and afterwards we'll add "configuring with no carrier" for the interface-less case.

Copy link

Choose a reason for hiding this comment

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

A have different case where I need the ConfigureWithoutCarrier=true with an address configured, but maybe no carrier.
I have an interface which should serve as DHCP/DNS server via dnsmasq. The interface may have no carrier, but it should be started anyway because dnsmasq fails to bind the IP if the interface is not up on start of dnsmasq.

So I would propose to bring the interface up if there is an address configured, or to add a new option to force the start of the interface.

Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
@codecov-io
Copy link

Codecov Report

Merging #34 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #34   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          32     32           
  Lines        3974   3977    +3     
=====================================
+ Hits         3974   3977    +3
Impacted Files Coverage Δ
tests/generator/test_common.py 100% <ø> (ø) ⬆️
tests/generator/test_ethernets.py 100% <100%> (ø) ⬆️
src/networkd.c 100% <100%> (ø) ⬆️
tests/generator/test_tunnels.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb71b87...d96a6a0. Read the comment docs.

Copy link

@vorlonofportland vorlonofportland left a comment

Choose a reason for hiding this comment

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

I would expect there to be some documentation changes with this, as this is a behavior change.

@cyphermox cyphermox removed the RFC Request for comment (don't merge yet) label Mar 20, 2019
@cyphermox
Copy link
Contributor Author

Merged as of a5389e5

@cyphermox cyphermox closed this Apr 23, 2019
@KalenAnson
Copy link

@cyphermox Looking this closed PR over, I see that e681f1e did not make the cut (or was unintentionally omitted). I am referring specifically to configuring a physical device without a carrier. Master Ref: https://github.com/CanonicalLtd/netplan/blob/master/src/networkd.c#L576

The update to the documentation seems to indicate that the change in behavior in this PR would include the condition that the linked commit addresses, i.e. an interface specified in the netplan config will be configured by the renderer (regardless of carrier link state presumably).

If an interface is defined with an ID in a configuration file; it will be brought up by the applicable renderer. To not have netplan touch an interface at all, it should be completely omitted from the netplan configuration files.

Can you shed some light on why this additional behavior change was not introduced to how netplan functions?

It would be very convenient for an interface that has no carrier to still be able to receive a static address so that services that depend on it would not have to specify interface requirements in both their own config files as well as systemd service files in the form of 'after's, 'bindsto's and 'wants' (dhcp servers, dns servers, et.al.).

@sabado
Copy link

sabado commented Nov 12, 2020

Switch to Netplan and mantain a bug from '17, put automatic updates by default in server edition. Did you guys have worked on Microsoft or what?

@vorlonofportland
Copy link

vorlonofportland commented Nov 12, 2020 via email

@n-cc
Copy link
Contributor

n-cc commented Sep 14, 2021

d4884cf adds support for networkd's ConfigureWithoutCarrier option via the ignore-carrier key.

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.

8 participants