From 1ae12dc6f8e5375ca7ee7023af68d8004b2b0410 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Mon, 26 Apr 2021 11:43:41 +0100
Subject: [PATCH] Ubuntu: define implied VLAN parent interfaces in networkd
On CentOS, when using the MichaelRigart.interfaces role, defining a VLAN
interface implies the existence of its parent interface. This is handled
via the CentOS network interface scripts. With systemd-networkd, the
parent interface is not implicit, and must be configured. The parent
interface network configuration file needs to contain this:
[Network]
VLAN = eth0.42
This change adds support for these implied parent interfaces. It also
removes the Broadcast = true option, which was added in an attempt to
fix an issue but is not supported in the [Network] section, and
generates a warning:
Unknown key name 'Broadcast' in section 'Network', ignoring.
Story: 2004960
Task: 42368
Task: 42367
Change-Id: I5d105006fad6e7e80473b9d9fa693de644c35a6d
---
kayobe/plugins/filter/networkd.py | 96 ++++++--
.../unit/plugins/filter/test_networkd.py | 217 +++++++++++++++++-
tox.ini | 3 +-
3 files changed, 294 insertions(+), 22 deletions(-)
diff --git a/kayobe/plugins/filter/networkd.py b/kayobe/plugins/filter/networkd.py
index 55272247..dac05b30 100644
--- a/kayobe/plugins/filter/networkd.py
+++ b/kayobe/plugins/filter/networkd.py
@@ -284,7 +284,6 @@ def _network(context, name, inventory_hostname, bridge, bond, vlan_interfaces):
{
'Network': [
{'Address': ip},
- {'Broadcast': 'true' if ip else None},
{'Gateway': gateway},
{'DHCP': ('yes' if bootproto and bootproto.lower() == 'dhcp'
else None)},
@@ -317,6 +316,34 @@ def _network(context, name, inventory_hostname, bridge, bond, vlan_interfaces):
return _filter_options(config)
+def _vlan_parent_network(device, mtu, vlan_interfaces):
+ """Return a networkd network configuration for a VLAN parent interface.
+
+ :param device: name of the interface.
+ :param mtu: Interface MTU.
+ :param vlan_interfaces: List of VLAN subinterfaces of the interface.
+ """
+ config = [
+ {
+ 'Match': [
+ {'Name': device},
+ ]
+ },
+ {
+ 'Network': [
+ {'VLAN': vlan_interface}
+ for vlan_interface in vlan_interfaces
+ ]
+ },
+ {
+ 'Link': [
+ {'MTUBytes': mtu},
+ ]
+ }
+ ]
+ return _filter_options(config)
+
+
def _bridge_port_network(context, name, port, inventory_hostname,
vlan_interfaces):
"""Return a networkd network configuration for a bridge port.
@@ -437,6 +464,25 @@ def _veth_peer_network(context, veth, inventory_hostname):
return _filter_options(config)
+def _add_to_result(result, prefix, device, config):
+ """Add configuration for an interface to a filter result.
+
+ :param result: the result dict.
+ :param prefix: the systemd-networkd configuration file prefix.
+ :param device: the interface being configured.
+ :param config: the configuration to add to the result.
+ :raises: AnsibleFilterError if the interface already exists in the result.
+ """
+ key = "%s%s" % (prefix, device)
+ # Catch the case where interface configuration is added multiple times.
+ # This should not happen.
+ if key in result:
+ raise errors.AnsibleFilterError(
+ "Programming error: duplicate interface configuration for %s"
+ % device)
+ result[key] = config
+
+
@jinja2.contextfilter
def networkd_netdevs(context, names, inventory_hostname=None):
"""Return a dict representation of networkd NetDev configuration.
@@ -459,7 +505,7 @@ def networkd_netdevs(context, names, inventory_hostname=None):
device = networks.get_and_validate_interface(context, name,
inventory_hostname)
netdev = _vlan_netdev(context, name, inventory_hostname)
- result["%s%s" % (prefix, device)] = netdev
+ _add_to_result(result, prefix, device, netdev)
# Bridges.
for name in networks.net_select_bridges(context, names,
@@ -467,21 +513,21 @@ def networkd_netdevs(context, names, inventory_hostname=None):
device = networks.get_and_validate_interface(context, name,
inventory_hostname)
netdev = _bridge_netdev(context, name, inventory_hostname)
- result["%s%s" % (prefix, device)] = netdev
+ _add_to_result(result, prefix, device, netdev)
# Bonds.
for name in networks.net_select_bonds(context, names, inventory_hostname):
device = networks.get_and_validate_interface(context, name,
inventory_hostname)
netdev = _bond_netdev(context, name, inventory_hostname)
- result["%s%s" % (prefix, device)] = netdev
+ _add_to_result(result, prefix, device, netdev)
# Virtual Ethernet pairs.
veths = networks.get_ovs_veths(context, names, inventory_hostname)
for veth in veths:
netdev = _veth_netdev(context, veth, inventory_hostname)
device = veth['name']
- result["%s%s" % (prefix, device)] = netdev
+ _add_to_result(result, prefix, device, netdev)
return result
@@ -551,9 +597,10 @@ def networkd_networks(context, names, inventory_hostname=None):
device = networks.get_and_validate_interface(context, name,
inventory_hostname)
vlan = networks.net_vlan(context, name, inventory_hostname)
+ mtu = networks.net_mtu(context, name, inventory_hostname)
parent = networks.get_vlan_parent(device, vlan)
vlan_interfaces = interface_to_vlans.setdefault(parent, [])
- vlan_interfaces.append(device)
+ vlan_interfaces.append({"device": device, "mtu": mtu})
# Prefix for configuration file names.
prefix = utils.get_hostvar(context, "networkd_prefix", inventory_hostname)
@@ -568,8 +615,22 @@ def networkd_networks(context, names, inventory_hostname=None):
bond = bond_member_to_bond.get(device)
vlan_interfaces = interface_to_vlans.get(device, [])
net = _network(context, name, inventory_hostname, bridge, bond,
- vlan_interfaces)
- result["%s%s" % (prefix, device)] = net
+ [vlan["device"] for vlan in vlan_interfaces])
+ _add_to_result(result, prefix, device, net)
+
+ # VLAN parent interfaces that are not in configured networks, bridge ports
+ # or bond members.
+ implied_vlan_parents = (set(interface_to_vlans) -
+ set(interfaces) -
+ set(bridge_port_to_bridge) -
+ set(bond_member_to_bond))
+ for device in implied_vlan_parents:
+ vlan_interfaces = interface_to_vlans[device]
+ mtu = max([vlan["mtu"] for vlan in vlan_interfaces])
+ net = _vlan_parent_network(device, mtu,
+ [vlan["device"]
+ for vlan in vlan_interfaces])
+ _add_to_result(result, prefix, device, net)
# Bridge ports that are not in configured networks.
for name in networks.net_select_bridges(context, names,
@@ -580,9 +641,10 @@ def networkd_networks(context, names, inventory_hostname=None):
inventory_hostname)
for port in set(bridge_ports) - set(interfaces):
vlan_interfaces = interface_to_vlans.get(port, [])
- netdev = _bridge_port_network(context, name, port,
- inventory_hostname, vlan_interfaces)
- result["%s%s" % (prefix, port)] = netdev
+ net = _bridge_port_network(context, name, port, inventory_hostname,
+ [vlan["device"]
+ for vlan in vlan_interfaces])
+ _add_to_result(result, prefix, port, net)
# Bond members that are not in configured networks.
for name in networks.net_select_bonds(context, names, inventory_hostname):
@@ -592,20 +654,22 @@ def networkd_networks(context, names, inventory_hostname=None):
inventory_hostname)
for member in set(bond_members) - set(interfaces):
vlan_interfaces = interface_to_vlans.get(member, [])
- netdev = _bond_member_network(context, name, member,
- inventory_hostname, vlan_interfaces)
- result["%s%s" % (prefix, member)] = netdev
+ net = _bond_member_network(context, name, member,
+ inventory_hostname,
+ [vlan["device"]
+ for vlan in vlan_interfaces])
+ _add_to_result(result, prefix, member, net)
# Virtual Ethernet pairs for Open vSwitch.
veths = networks.get_ovs_veths(context, names, inventory_hostname)
for veth in veths:
net = _veth_network(context, veth, inventory_hostname)
device = veth['name']
- result["%s%s" % (prefix, device)] = net
+ _add_to_result(result, prefix, device, net)
net = _veth_peer_network(context, veth, inventory_hostname)
device = veth['peer']
- result["%s%s" % (prefix, device)] = net
+ _add_to_result(result, prefix, device, net)
return result
diff --git a/kayobe/tests/unit/plugins/filter/test_networkd.py b/kayobe/tests/unit/plugins/filter/test_networkd.py
index 5d17f1cb..12d2c3d6 100644
--- a/kayobe/tests/unit/plugins/filter/test_networkd.py
+++ b/kayobe/tests/unit/plugins/filter/test_networkd.py
@@ -298,7 +298,6 @@ class TestNetworkdNetworks(BaseNetworkdTest):
{
"Network": [
{"Address": "1.2.3.4/24"},
- {"Broadcast": "true"},
]
},
]
@@ -347,7 +346,6 @@ class TestNetworkdNetworks(BaseNetworkdTest):
{
"Network": [
{"Address": "1.2.3.4/24"},
- {"Broadcast": "true"},
{"Gateway": "1.2.3.1"},
{"DHCP": "yes"},
{'UseGateway': "false"},
@@ -400,6 +398,18 @@ class TestNetworkdNetworks(BaseNetworkdTest):
def test_vlan(self):
nets = networkd.networkd_networks(self.context, ["net2"])
expected = {
+ "50-kayobe-eth0": [
+ {
+ "Match": [
+ {"Name": "eth0"}
+ ],
+ },
+ {
+ "Network": [
+ {"VLAN": "eth0.2"},
+ ]
+ },
+ ],
"50-kayobe-eth0.2": [
{
"Match": [
@@ -422,7 +432,6 @@ class TestNetworkdNetworks(BaseNetworkdTest):
{
"Network": [
{"Address": "1.2.3.4/24"},
- {"Broadcast": "true"},
{"VLAN": "eth0.2"},
]
},
@@ -527,6 +536,106 @@ class TestNetworkdNetworks(BaseNetworkdTest):
}
self.assertEqual(expected, nets)
+ def test_bridge_with_bridge_port_vlan(self):
+ # Test the case where one of the bridge ports has a VLAN subinterface.
+ self._update_context({
+ "net2_interface": "eth1.2",
+ })
+ nets = networkd.networkd_networks(self.context, ["net2", "net3"])
+ expected = {
+ "50-kayobe-br0": [
+ {
+ "Match": [
+ {"Name": "br0"}
+ ]
+ },
+ ],
+ "50-kayobe-eth0": [
+ {
+ "Match": [
+ {"Name": "eth0"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bridge": "br0"},
+ ]
+ },
+ ],
+ "50-kayobe-eth1": [
+ {
+ "Match": [
+ {"Name": "eth1"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bridge": "br0"},
+ {"VLAN": "eth1.2"}
+ ]
+ },
+ ],
+ "50-kayobe-eth1.2": [
+ {
+ "Match": [
+ {"Name": "eth1.2"}
+ ]
+ },
+ ],
+ }
+ self.assertEqual(expected, nets)
+
+ def test_bridge_with_bridge_port_vlan_net(self):
+ # Test the case where one of the bridge ports has a VLAN subinterface,
+ # and is also a Kayobe network (here eth0 is net1).
+ self._update_context({
+ "net1_ips": None,
+ })
+ nets = networkd.networkd_networks(self.context,
+ ["net1", "net2", "net3"])
+ expected = {
+ "50-kayobe-br0": [
+ {
+ "Match": [
+ {"Name": "br0"}
+ ]
+ },
+ ],
+ "50-kayobe-eth0": [
+ {
+ "Match": [
+ {"Name": "eth0"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bridge": "br0"},
+ {"VLAN": "eth0.2"}
+ ]
+ },
+ ],
+ "50-kayobe-eth0.2": [
+ {
+ "Match": [
+ {"Name": "eth0.2"}
+ ]
+ },
+ ],
+ "50-kayobe-eth1": [
+ {
+ "Match": [
+ {"Name": "eth1"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bridge": "br0"},
+ ]
+ },
+ ]
+ }
+ self.assertEqual(expected, nets)
+
def test_bridge_no_interface(self):
self._update_context({"net3_interface": None})
self.assertRaises(errors.AnsibleFilterError,
@@ -617,6 +726,106 @@ class TestNetworkdNetworks(BaseNetworkdTest):
}
self.assertEqual(expected, nets)
+ def test_bond_with_bond_member_vlan(self):
+ # Test the case where one of the bond members has a VLAN subinterface.
+ self._update_context({
+ "net2_interface": "eth1.2",
+ })
+ nets = networkd.networkd_networks(self.context, ["net2", "net4"])
+ expected = {
+ "50-kayobe-bond0": [
+ {
+ "Match": [
+ {"Name": "bond0"}
+ ]
+ },
+ ],
+ "50-kayobe-eth0": [
+ {
+ "Match": [
+ {"Name": "eth0"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bond": "bond0"},
+ ]
+ },
+ ],
+ "50-kayobe-eth1": [
+ {
+ "Match": [
+ {"Name": "eth1"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bond": "bond0"},
+ {"VLAN": "eth1.2"},
+ ]
+ },
+ ],
+ "50-kayobe-eth1.2": [
+ {
+ "Match": [
+ {"Name": "eth1.2"}
+ ]
+ },
+ ],
+ }
+ self.assertEqual(expected, nets)
+
+ def test_bond_with_bond_member_vlan_net(self):
+ # Test the case where one of the bond members has a VLAN subinterface,
+ # and is also a Kayobe network (here eth0 is net1).
+ self._update_context({
+ "net1_ips": None,
+ })
+ nets = networkd.networkd_networks(self.context,
+ ["net1", "net2", "net4"])
+ expected = {
+ "50-kayobe-bond0": [
+ {
+ "Match": [
+ {"Name": "bond0"}
+ ]
+ },
+ ],
+ "50-kayobe-eth0": [
+ {
+ "Match": [
+ {"Name": "eth0"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bond": "bond0"},
+ {"VLAN": "eth0.2"},
+ ]
+ },
+ ],
+ "50-kayobe-eth0.2": [
+ {
+ "Match": [
+ {"Name": "eth0.2"}
+ ]
+ },
+ ],
+ "50-kayobe-eth1": [
+ {
+ "Match": [
+ {"Name": "eth1"}
+ ]
+ },
+ {
+ "Network": [
+ {"Bond": "bond0"},
+ ]
+ },
+ ]
+ }
+ self.assertEqual(expected, nets)
+
def test_bond_no_interface(self):
self._update_context({"net4_interface": None})
self.assertRaises(errors.AnsibleFilterError,
@@ -737,7 +946,6 @@ class TestNetworkdNetworks(BaseNetworkdTest):
{
"Network": [
{"Address": "1.2.3.4/24"},
- {"Broadcast": "true"},
]
},
]
@@ -760,7 +968,6 @@ class TestNetworkdNetworks(BaseNetworkdTest):
{
"Network": [
{"Address": "1.2.3.4/24"},
- {"Broadcast": "true"},
{"VLAN": "eth0.2"},
]
},
diff --git a/tox.ini b/tox.ini
index 43e829a2..bb75ac49 100644
--- a/tox.ini
+++ b/tox.ini
@@ -135,8 +135,9 @@ commands =
[flake8]
# E123, E125 skipped as they are invalid PEP-8.
+# W504 line break after binary operator
show-source = True
-ignore = E123,E125
+ignore = E123,E125,W504
builtins = _
exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build
--
GitLab