From 97f2f3b5c7159c05f33268f01a9e10c10a68c82a Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Tue, 23 Feb 2021 16:52:13 +0000 Subject: [PATCH] Refactor veth configuration role variable generation Move veth logic to the new networks filter plugin module. This is in preparation for addition of filters for systemd-networkd. Story: 2004960 Task: 41920 Change-Id: I10d80713c4bac21b6aed6f4b6b15919d490b753c --- ansible/network.yml | 43 +--- kayobe/plugins/filter/networks.py | 146 +++++++++++++ kayobe/tests/unit/plugins/filter/__init__.py | 0 .../unit/plugins/filter/test_networks.py | 199 ++++++++++++++++++ 4 files changed, 349 insertions(+), 39 deletions(-) create mode 100644 kayobe/tests/unit/plugins/filter/__init__.py create mode 100644 kayobe/tests/unit/plugins/filter/test_networks.py diff --git a/ansible/network.yml b/ansible/network.yml index 4a6465712..a9c15791b 100644 --- a/ansible/network.yml +++ b/ansible/network.yml @@ -75,43 +75,8 @@ tags: - config - network - vars: - veth_mtu_map: {} - veth_interfaces: [] - pre_tasks: - # When these networks are VLANs, we need to use the underlying tagged - # interface rather than the untagged interface. We therefore strip - # the . suffix of the interface name. We use a union here as a single - # tagged interface may be shared between these networks. - - name: Update a fact containing bridges to be patched to the Neutron OVS bridge - set_fact: - veth_mtu_map: > - {{ veth_mtu_map | combine({interface: mtu}) }} - with_items: "{{ [provision_wl_net_name, cleaning_net_name] + external_net_names | unique | list }}" - when: - - item in network_interfaces - - item | net_is_bridge + tasks: + - import_role: + name: veth vars: - interface: "{{ item | net_interface | replace('.' ~ item | net_vlan | default('!nomatch!'), '') }}" - # Determine the MTU as the maximum of all subinterface MTUs. Only - # interfaces with an explicit MTU set will be taken account of. If no - # interface has an explicit MTU set, then the corresponding veth will - # not either. - mtu_list: "{{ [veth_mtu_map.get(interface), item | net_mtu] | select | map('int') | list }}" - mtu: "{{ mtu_list | max if mtu_list | length > 0 else None }}" - - - name: Update a fact containing veth interfaces - set_fact: - veth_interfaces: > - {{ veth_interfaces + - [{'device': network_patch_prefix ~ item.key ~ network_patch_suffix_phy, - 'bootproto': 'static', - 'bridge': item.key, - 'mtu': item.value, - 'peer_device': network_patch_prefix ~ item.key ~ network_patch_suffix_ovs, - 'peer_bootproto': 'static', - 'peer_mtu': item.value, - 'onboot': 'yes'}] }} - with_dict: "{{ veth_mtu_map }}" - roles: - - role: veth + veth_interfaces: "{{ network_interfaces | net_ovs_veths }}" diff --git a/kayobe/plugins/filter/networks.py b/kayobe/plugins/filter/networks.py index 3722a6840..058fe2224 100644 --- a/kayobe/plugins/filter/networks.py +++ b/kayobe/plugins/filter/networks.py @@ -15,10 +15,132 @@ from ansible import errors import jinja2 import netaddr +import re from kayobe.plugins.filter import utils +def get_and_validate_interface(context, name, inventory_hostname): + """Return a validated interface for a network. + + :param context: a Jinja2 Context object. + :param name: name of the network. + :param inventory_hostname: Ansible inventory hostname. + :returns: a validated interface for a network. + :raises: ansible.errors.AnsibleFilterError + """ + device = net_interface(context, name, inventory_hostname) + if not device: + raise errors.AnsibleFilterError( + "Network interface for network '%s' on host '%s' not found" % + (name, inventory_hostname)) + return device + + +def _get_veth_interface(context, bridge, inventory_hostname): + """Return a veth link name for a bridge. + + :param context: a Jinja2 Context object. + :param bridge: name of the bridge interface into which the veth is plugged. + :param inventory_hostname: Ansible inventory hostname. + :returns: a veth link name for a bridge. + """ + prefix = utils.get_hostvar(context, 'network_patch_prefix', + inventory_hostname) + suffix = utils.get_hostvar(context, 'network_patch_suffix_phy', + inventory_hostname) + return prefix + bridge + suffix + + +def _get_veth_peer(context, bridge, inventory_hostname): + """Return a veth peer name for a bridge. + + :param context: a Jinja2 Context object. + :param bridge: name of the bridge interface into which the veth is plugged. + :param inventory_hostname: Ansible inventory hostname. + :returns: a veth peer name for a bridge. + """ + prefix = utils.get_hostvar(context, 'network_patch_prefix', + inventory_hostname) + suffix = utils.get_hostvar(context, 'network_patch_suffix_ovs', + inventory_hostname) + return prefix + bridge + suffix + + +def get_ovs_veths(context, names, inventory_hostname): + """Return a list of dicts describing veth pairs to plug into Open vSwitch. + + :param context: a Jinja2 Context object. + :param names: list of names of networks. + :param inventory_hostname: Ansible inventory hostname. + :returns: a list of dicts describing veth pairs. Each dict has keys 'name', + 'peer', 'bridge', and 'mtu'. + """ + # The following networks need to be plugged into Open vSwitch: + # * workload provisioning network + # * workload cleaning network + # * neutron external networks + ironic_networks = [ + utils.get_hostvar(context, 'provision_wl_net_name', + inventory_hostname), + utils.get_hostvar(context, 'cleaning_net_name', inventory_hostname), + ] + external_networks = utils.get_hostvar(context, 'external_net_names', + inventory_hostname) + veth_networks = ironic_networks + (external_networks or []) + + # Make a list of all bridge interfaces. + bridges = net_select_bridges(context, names, inventory_hostname) + bridge_interfaces = [net_interface(context, bridge, inventory_hostname) + for bridge in bridges] + + # Dict mapping bridge interfaces to the MTU of a connected veth pair. + veth_mtu_map = {} + for name in veth_networks: + if name not in names: + continue + device = get_and_validate_interface(context, name, inventory_hostname) + # When these networks are VLANs, we need to use the underlying tagged + # interface rather than the untagged interface. We therefore strip the + # . suffix of the interface name. We use a union here as a single + # tagged interface may be shared between these networks. + vlan = net_vlan(context, name, inventory_hostname) + if vlan: + parent_or_device = get_vlan_parent(device, vlan) + else: + parent_or_device = device + if parent_or_device in bridge_interfaces: + # Determine the MTU as the maximum of all subinterface MTUs. Only + # interfaces with an explicit MTU set will be taken account of. If + # no interface has an explicit MTU set, then the corresponding veth + # will not either. + # Allow for the case where an MTU is not specified. + mtu = net_mtu(context, name, inventory_hostname) + veth_mtu_map.setdefault(parent_or_device, mtu) + if (veth_mtu_map.get(parent_or_device) or 0) < (mtu or 0): + veth_mtu_map[parent_or_device] = mtu + + return [ + { + 'name': _get_veth_interface(context, bridge, inventory_hostname), + 'peer': _get_veth_peer(context, bridge, inventory_hostname), + 'bridge': bridge, + 'mtu': mtu + } + for bridge, mtu in veth_mtu_map.items() + ] + + +def get_vlan_parent(device, vlan): + """Return the parent interface of a VLAN subinterface. + + :param device: VLAN interface name. + :param vlan: VLAN ID. + :returns: parent interface name. + """ + return re.sub(r'\.{}$'.format(vlan), '', device) + + @jinja2.contextfilter def net_attr(context, name, attr, inventory_hostname=None): var_name = "%s_%s" % (name, attr) @@ -480,6 +602,29 @@ def net_libvirt_vm_network(context, name, inventory_hostname=None): } +@jinja2.contextfilter +def net_ovs_veths(context, names, inventory_hostname=None): + """Return a list of virtual Ethernet pairs for OVS. + + The format is as expected by the veth_interfaces variable of the Kayobe + veth role. + """ + veths = get_ovs_veths(context, names, inventory_hostname) + return [ + { + 'device': veth['name'], + 'bootproto': 'static', + 'bridge': veth['bridge'], + 'mtu': veth['mtu'], + 'peer_device': veth['peer'], + 'peer_bootproto': 'static', + 'peer_mtu': veth['mtu'], + 'onboot': 'yes', + } + for veth in veths + ] + + def get_filters(): return { 'net_attr': net_attr, @@ -526,4 +671,5 @@ def get_filters(): 'net_libvirt_network_name': net_libvirt_network_name, 'net_libvirt_network': net_libvirt_network, 'net_libvirt_vm_network': net_libvirt_vm_network, + 'net_ovs_veths': net_ovs_veths, } diff --git a/kayobe/tests/unit/plugins/filter/__init__.py b/kayobe/tests/unit/plugins/filter/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/kayobe/tests/unit/plugins/filter/test_networks.py b/kayobe/tests/unit/plugins/filter/test_networks.py new file mode 100644 index 000000000..3e72b7786 --- /dev/null +++ b/kayobe/tests/unit/plugins/filter/test_networks.py @@ -0,0 +1,199 @@ +# Copyright (c) 2021 StackHPC Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import copy +import unittest + +import jinja2 + +from kayobe.plugins.filter import networks + + +class BaseNetworksTest(unittest.TestCase): + + maxDiff = 2000 + + variables = { + # Inventory hostname, used to index IP list. + "inventory_hostname": "test-host", + # net1: Ethernet on eth0 with IP 1.2.3.4/24. + "net1_interface": "eth0", + # net2: VLAN on eth0.2 with VLAN 2 on interface eth0. + "net2_interface": "eth0.2", + "net2_vlan": 2, + # net3: bridge on br0 with ports eth0 and eth1. + "net3_interface": "br0", + "net3_bridge_ports": [], + # net4: VLAN on br0.4 with VLAN 4 on bridge br0. + "net4_interface": "br0.4", + "net4_vlan": 4, + # net5: VLAN on br0.5 with VLAN 5 on bridge br0. + "net5_interface": "br0.5", + "net5_vlan": 5, + # Veth pair patch link prefix and suffix. + "network_patch_prefix": "p-", + "network_patch_suffix_ovs": "-ovs", + "network_patch_suffix_phy": "-phy", + } + + def setUp(self): + # Bandit complains about Jinja2 autoescaping without nosec. + self.env = jinja2.Environment() # nosec + self.context = self._make_context(self.variables) + + def _make_context(self, parent): + return self.env.context_class( + self.env, parent=parent, name='dummy', blocks={}) + + def _update_context(self, variables): + updated_vars = copy.deepcopy(self.variables) + updated_vars.update(variables) + self.context = self._make_context(updated_vars) + + +class TestNetworks(BaseNetworksTest): + + def test_get_ovs_veths_empty(self): + veths = networks.get_ovs_veths(self.context, [], None) + self.assertEqual([], veths) + + def test_get_ovs_veths_eth(self): + # Ethernet does not need a veth pair. + self._update_context({"external_net_names": ["net1"]}) + veths = networks.get_ovs_veths(self.context, ["net1"], None) + self.assertEqual([], veths) + + def test_get_ovs_veths_eth_vlan(self): + # VLAN on Ethernet does not need a veth pair. + self._update_context({"external_net_names": ["net2"]}) + veths = networks.get_ovs_veths(self.context, ["net2"], None) + self.assertEqual([], veths) + + def test_get_ovs_veths_bridge(self): + # Bridge needs a veth pair. + self._update_context({"external_net_names": ["net3"]}) + veths = networks.get_ovs_veths(self.context, ["net3"], None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": None, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_provision_wl(self): + # Bridge needs a veth pair. + self._update_context({"provision_wl_net_name": "net3"}) + veths = networks.get_ovs_veths(self.context, ["net3"], None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": None, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_cleaning(self): + # Bridge needs a veth pair. + self._update_context({"cleaning_net_name": "net3"}) + veths = networks.get_ovs_veths(self.context, ["net3"], None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": None, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_mtu(self): + # Use the MTU of bridge. + self._update_context({"external_net_names": ["net3"], + "net3_mtu": 1400}) + veths = networks.get_ovs_veths(self.context, ["net3"], None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": 1400, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_vlan(self): + # VLAN on a bridge needs a veth pair. + self._update_context({"external_net_names": ["net4"]}) + veths = networks.get_ovs_veths(self.context, ["net3", "net4"], None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": None, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_vlan_mtu(self): + # Use the MTU of VLAN on a bridge. + self._update_context({"external_net_names": ["net4"], + "net4_mtu": 1400}) + veths = networks.get_ovs_veths(self.context, ["net3", "net4"], None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": 1400, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_vlan_multiple(self): + # Multiple VLANs on a bridge need a single veth pair. + self._update_context({"external_net_names": ["net4", "net5"]}) + veths = networks.get_ovs_veths(self.context, ["net3", "net4", "net5"], + None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": None, + } + ] + self.assertEqual(expected, veths) + + def test_get_ovs_veths_bridge_vlan_multiple_mtu(self): + # Use the highest MTU of multiple VLANs on a bridge. + self._update_context({"external_net_names": ["net4", "net5"], + "net4_mtu": 1400, + "net5_mtu": 1500}) + veths = networks.get_ovs_veths(self.context, ["net3", "net4", "net5"], + None) + expected = [ + { + "name": "p-br0-phy", + "peer": "p-br0-ovs", + "bridge": "br0", + "mtu": 1500, + } + ] + self.assertEqual(expected, veths)