Redesign of the 'debops.ifupdown' Ansible role

[drybjed]

Requesting a review from @ypid before merging.

[drybjed]

There's one issue I would like to solve before a new release: role merges the YAML dictionaries only in alphabetical order, it means that any changes to network interfaces in separate labeled dictionaries need to have names that sort in the correct order. Any suggestions how to solve that without breaking functionality are welcome.

[drybjed]

Thanks for the review, I'll try to address the comments tommorow, since it's a bit late.

[ypid]

@drybjed Would it make sense to have a rollback feature when applying the changes?

[ypid]

@drybjed I am done with my review. Final remarks, fixes and a feature migration from debops.subnetwork to this role can be found in https://github.com/drybjed/ansible-ifupdown/pull/4 and here in this PR. Really awesome work :+1: Beautiful design.

https://keyhole.pubsrv.ypid.de/ which hosts https://me.ypid.de/ is up-to-date and survived :wink: Here is an example use case of the role I use on keyhole.pubsrv.ypid.de:

# network__ipv6_prefix: '{{ ansible_default_ipv6.address + "/" + ansible_default_ipv6.prefix }}'
network__ipv6_prefix: '2a02:c207:3001:3999::/64'
network__ipv6_internal0_prefix: '{{ network__ipv6_prefix | ipsubnet(80, 1) }}'

network__networks:
  lxc:
    ipv4: '10.246.0.1/24'
    ipv6: '{{ network__ipv6_internal0_prefix | ipaddr("1") }}'

network__addresses:
  keyhole:
    # Explicitly mentioned here so that other containers in the domain can also
    # easily use it.
    ipv4: '5.189.164.41/24'
    ipv6: '2a02:c207:3001:3999:0000:0000:0000:2342/64'
  lxc_keyhole_host:
    ipv6: '{{ network__ipv6_internal0_prefix | ipaddr("1") }}'
  cache:
    ipv6: '{{ network__ipv6_internal0_prefix | ipaddr("2") }}'
  www:
    ipv6: '{{ network__ipv6_internal0_prefix | ipaddr("3") }}'

ifupdown__interface_layout: 'manual'
ifupdown__host_interfaces:

  # Was already statically configured by contabo.de during VPS deployment. I
  # just redefine it here for documentation purposes and to have more
  # control over the network configuration.
  'eth0':
    inet: 'static'
    inet6: 'static'
    addresses:
      - '{{ network__addresses.keyhole.ipv4 }}'
      - '{{ network__addresses.keyhole.ipv6 }}'
    gateways:
      - '{{ ansible_default_ipv4.gateway|d("") }}'
      - 'fe80::1'
    dns_nameservers:
      # Needs to be static as `ansible_dns` only contains localhost after dnsmasq
      # has been setup.
      - '213.136.95.10'
      - '213.136.95.11'
    options4: |
      post-up route add -net 5.189.164.0/24 gw 5.189.164.1 dev eth0
    # Preconfigured by contabo.de:
    # contabo.de seems to use /24 networks for VPS but the VPSes can not
    # connect to each other directly although they are on the same network.
    # Seems iproute2 has a check for already existing routes which prevents
    # to add the route. The metric could be used. The old and legacy `route`
    # command does not check this and just adds the route.
    options6: |
      accept_ra 0
      autoconf 0
      privext 0
    # Was set to 0 by contabo.de as privext is based on SLAAC which is
    # probably not supported.

  'internal0':
    type: 'bridge'
    inet: 'static'
    inet6: 'static'
    # TODO: Disable and explicitly configure the Firewall to only allow
    # required connections between hosts.
    forward_interface_ferm_rule: 'outerface (eth0 internal0) ACCEPT'
    forward_outerface_ferm_rule_enabled: False
    addresses: '{{ network__networks.lxc.values() + ["fe80::1/64"] }}'
    add_options6: |
      pre-up echo 0 > /proc/sys/net/ipv6/conf/internal0/accept_dad
      {% for ipv6_address in network__addresses.values() | map(attribute="ipv6") | ipaddr("address") | sort %}
      pre-up ip ne add proxy {{ ipv6_address }} dev eth0
      post-down ip ne del proxy {{ ipv6_address }} dev eth0
      {% endfor %}

I guess I should include that into https://github.com/debops/examples. Lets see when I come around to add it there. Edit: Damn, my IPv6 setup broke (ref: https://www.ssllabs.com/ssltest/analyze.html?d=ypid.de), but I am sure this needs to be fixed in my inventory. -> Fixed, see below.

[drybjed]

About the rollback, do you mean automatic or just the ability to get back old configuration by hand?

Automatic might be unreliable. It makes sense when you configure all interfaces for the first time, check connectivity to some IP address and in case of any issues roll back. When you set up additional interface, you would need to add IP address behind that interface to the check list, otherwise host still has good connectivity even if internal interface configuration is wrong.

I suppose it's a food for thought. I would leave that for the next role iteration though.

[ypid]

@drybjed Thanks for the feedback. I opened an issue to describe and track this outside of the PR: https://github.com/debops/ansible-ifupdown/issues/59

[ypid]

Fixed my v6 setup :wink: I should have checked /run/network/ifstate or ifup which also suggested that the interface could not be brought up correctly. The problem was that I was a bit to optimistic and wanted to switch from route add which my hoster used to ip route which is smarter and does not add a duplicate route (with the same metric) for the network setup that my hoster has chosen. Switched back to route add and now everything works nicely again.

TODOs after merging:

• [x] Remove https://github.com/debops/ansible-subnetwork from docs
• [ ] Merge: https://github.com/debops/ansible-tinc/pull/35

[ypid]

One thing I noticed during testing. In my example above, I configured a bridge "internal0" with additional options. That caused the required options:

bridge_ports none
bridge_fd 0

to not be added by default. I had to specific them manually for the bridge to be brought up. Maybe the role could check if required options are set even when custom options are given. That would mean to either split the user given options or also use a dict for them so that the template can set default options per type.

[drybjed]

Hmmm... Currently you can specify add_options for the role to not drop the autogenerated options.

I don't think that going with a YAML dict format for this is a good idea, how would you handle pre-up, post-down, etc.? I suppose that these could be recognized as dictionary keys with lists for multiple toptions, but that gets complicated... I wanted to stick to known format from interfaces(5) so that moving existing configuration to debops.ifupdown would be easier.

[ypid]

@drybjed I was foolish enough think that you did not already consider and solve this :wink: Sorry for that. The add_options* options should address this and I will test this later. I must have overlooked them in the docs before. Thanks :+1:

[drybjed]

@ypid I think that role is ready to be released now, nothing more to add I can think of at the moment. Any last thoughts?

[ypid]

@drybjed Can you give me time until the evening so that I can test the add_options and check all the review comments? Other than that, sounds good. Nice job :+1:

[drybjed]

@ypid Sure.