debops / ansible-ifupdown

Manage network interface configuration in /etc/network/interfaces
GNU General Public License v3.0
25 stars 14 forks source link

Fixes related to hotplug network configuration and interface ordering #67

Closed drybjed closed 7 years ago

drybjed commented 7 years ago

Requesting review from @Timitos since he reported issues with network configuration recently.

ganto commented 7 years ago

Wow... that's fancy. I guess I won't have time for that today. Will try it out tomorrow.

ypid commented 7 years ago

Wow... that's fancy. I guess I won't have time for that today. Will try it out tomorrow.

Same here, I shortly looked over it yesterday. Not sure yet when I can review/test it appropriately.

drybjed commented 7 years ago

Any news from other reviewers? @scibi, @ganto, @Timitos?

ypid commented 7 years ago

I found an issue. I started with something like:

ifupdown__host_interfaces:

  'br_time_net':
    inet: 'manual'
    inet6: 'manual'
    forward: False
    add_options6: 'pre-up echo 0 > "/proc/sys/net/ipv6/conf/${IFACE}/disable_ipv6"'

Then changed the (echo 0 to echo 1) and rerun the role and the reconfigure script failed. I run it manually to see what happend:

> find /run/network/ -type f -print -exec cat {} \;
/run/network/debops-ifupdown-reconfigure,060,br_time_net
changed
/run/network/ifstate
lo=lo
br0=br0
eth0=eth0
br_time_net=br_time_net
/run/network/debops-ifupdown-reconfigure.networking
init/run/network/.ifstate.lock

> /usr/local/lib/ifupdown-reconfigure-interfaces
Detected interfaces to reconfigure: br_time_net
Bringing down 'br_time_net' interface
/usr/local/lib/ifupdown-reconfigure-interfaces: line 162: systemd_ifup_instances[@]: unbound variable

> find /run/network/ -type f -print -exec cat {} \;
/run/network/ifstate
lo=lo
br0=br0
eth0=eth0
br_time_net=br_time_net
/run/network/debops-ifupdown-reconfigure.networking
init/run/network/.ifstate.lock

I checked the script but I am not sure how to fix this. Can you reproduce this?

BTW, is there a better way then the pre-up if I want a bridge without any interfaces attached to and without any configuration? Basically just a bridge or switch I can use to connect vms. If i don’t do the pre-up hack, the bridge still gets a link local IPv6 on the host.

drybjed commented 7 years ago

Haven't checked the issue with the script yet, but to create a simple bridge all you need to do is:

ifupdown__host_interfaces:
  'br_vmbridge': {}

Either make sure that it's name starts with br or set the type to bridge. According to the Wiki, IPv6 requires a link-local address on all interfaces that have this network enabled, so I'm not sure if you even can create an interface without IPv6 link-local - I guess you could but then it wouldn't be usable over IPv6 network.

drybjed commented 7 years ago

@ypid I checked your test case on a Debian Stretch VM. I was able to use pre-up command with both 0 and 1 value, both initially as well as when the interface already existed. This VM had IPv6 enabled globally, does yours have it disabled?

ypid commented 7 years ago

The issue is not really related to IPv6, that was just my usecase. Here is a simplified example. First role run:

ifupdown__host_interfaces:
  'br_test': {}

Second run with ifupdown__reconfigure_auto disabled:

ifupdown__host_interfaces:
  'br_test':
    inet6: 'manual'
> find /run/network/ -type f -print -exec cat {} \;
/run/network/debops-ifupdown-reconfigure,060,br_test
changed
/run/network/ifstate
lo=lo
br0=br0
eth0=eth0
br_test=br_test
/run/network/debops-ifupdown-reconfigure.networking
init/run/network/.ifstate.lock
> ifup br_test
> /usr/local/lib/ifupdown-reconfigure-interfaces
Detected interfaces to reconfigure: br_test
Found active systemd iface@ instances: br_undef_net
Bringing down 'br_test' interface
/usr/local/lib/ifupdown-reconfigure-interfaces: line 162: systemd_ifup_instances[@]: unbound variable
> lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 8.7 (jessie)
Release:        8.7
Codename:       jessie

The issue is that the role can not change an existing interface in my case.

RUNNING HANDLER [debops.ifupdown : Apply ifupdown configuration] ***************
fatal: [vmm01.example.org]: FAILED! => {"changed": true, "failed": true, "rc": 1, "stderr": "Shared connection to 192.0.2.23 closed.\r\n", "stdout": "Detected interfaces to reconfigure: br_test\r\nFound active systemd iface@ instances: br_undef_net\r\nBringing down 'br_test' interface\r\n/var/local/ansibleadmin/.ansible/tmp/ansible-tmp-1491043843.47-135394808497007/ifupdown-reconfigure-interfaces: line 162: systemd_ifup_instances[@]: unbound variable\r\n", "stdout_lines": ["Detected interfaces to reconfigure: br_test", "Found active systemd iface@ instances: br_undef_net", "Bringing down 'br_test' interface", "/var/local/ansibleadmin/.ansible/tmp/ansible-tmp-1491043843.47-135394808497007/ifupdown-reconfigure-interfaces: line 162: systemd_ifup_instances[@]: unbound variable"]}
drybjed commented 7 years ago

That's weird... From your output it looks like script finds br_undef_net interface which doesn't exist. Can you run the reconfigure script with bash -x for the debug output?

ypid commented 7 years ago

The br_undef_net is defined but I removed it from the states for simplicity. I retested it with only br0 and br_test:

https://paste.debian.net/925409/

drybjed commented 7 years ago

@ypid Thanks for the output. Since I cannot reproduce this, can you replace the line 162 of the reconfigure script with:

if [ ${#if_hotplug_interfaces[@]} -gt 0 ] && [ ${#systemd_ifup_instances[@]} -gt 0 ] && containsElement "${1}" "${systemd_ifup_instances[@]}" ; then

and see if that fixes the problem?

ypid commented 7 years ago

Gotcha :wink: That fixes it. systemctl list-units --no-legend --state=active 'ifup@*.service' outputs nothing on my system.

drybjed commented 7 years ago

Great, then I'll add this as well. Thanks for testing.

drybjed commented 7 years ago

@ypid Great, thanks for the approval. Although I would like to merge the debops.ferm changes first (https://github.com/debops/ansible-ferm/pull/103) since the role is a dependency of debops.ifupdown. You could look at that one if you have some time. I'll prepare some changes to debops.sshd as well because the current parameters don't generate proper firewall rules.