From ad64ebc921628e6f3967427db38e3e3b6bfffa05 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Wed, 19 Jun 2024 14:40:42 +0100 Subject: [PATCH] Add support for customising Neutron physical network names Previously Kolla Ansible hard-coded Neutron physical networks starting at physnet1 up to physnetN, matching the number of interfaces in neutron_external_interface and bridges in neutron_bridge_name. Sometimes we may want to customise the physical network names used. This may be to allow for not all hosts having access to all physical networks, or to use more descriptive names. For example, in an environment with a separate physical network for Ironic provisioning, controllers might have access to two physical networks, while compute nodes have access to one. This change extends the 'physical_network' network attribute to make it possible to customise the Neutron physical network names used for the OVS, OVN, Linux bridge and OVS DPDK plugins. The default behaviour is unchanged. Depends-On: https://review.opendev.org/c/openstack/kolla-ansible/+/922320 Change-Id: I214444c60653f484fcda6275cc725879d14f9e7a --- ansible/inventory/group_vars/all/kolla | 2 + .../kolla-ansible-host-vars/tests/test.yml | 6 + .../configuration/reference/kolla-ansible.rst | 2 + .../configuration/reference/network.rst | 32 ++++- .../plugins/action/kolla_ansible_host_vars.py | 55 ++++++-- .../action/test_kolla_ansible_host_vars.py | 118 ++++++++++++++++++ ...on-physical-networks-861c2eca701360e8.yaml | 10 ++ 7 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/neutron-physical-networks-861c2eca701360e8.yaml diff --git a/ansible/inventory/group_vars/all/kolla b/ansible/inventory/group_vars/all/kolla index 643bfd081..a6da96cfe 100644 --- a/ansible/inventory/group_vars/all/kolla +++ b/ansible/inventory/group_vars/all/kolla @@ -394,6 +394,7 @@ kolla_overcloud_inventory_pass_through_host_vars_default: - "kolla_external_vip_interface" - "kolla_neutron_external_interfaces" - "kolla_neutron_bridge_names" + - "kolla_neutron_physical_networks" # List of names of additional host variables to pass through from kayobe hosts # to kolla-ansible hosts, if set. See also @@ -424,6 +425,7 @@ kolla_overcloud_inventory_pass_through_host_vars_map_default: kolla_tunnel_interface: "tunnel_interface" kolla_neutron_external_interfaces: "neutron_external_interface" kolla_neutron_bridge_names: "neutron_bridge_name" + kolla_neutron_physical_networks: "neutron_physical_networks" # Dict mapping names of additional variables in # kolla_overcloud_inventory_pass_through_host_vars to the variable to use in diff --git a/ansible/roles/kolla-ansible-host-vars/tests/test.yml b/ansible/roles/kolla-ansible-host-vars/tests/test.yml index 5f1d77274..4ff5d56ef 100644 --- a/ansible/roles/kolla-ansible-host-vars/tests/test.yml +++ b/ansible/roles/kolla-ansible-host-vars/tests/test.yml @@ -16,6 +16,7 @@ kolla_dns_interface: "eth5" kolla_neutron_external_interfaces: "eth6,eth7" kolla_neutron_bridge_names: "br0,br1" + kolla_neutron_physical_networks: "physnet1,physnet2" kolla_provision_interface: "eth8" kolla_inspector_dnsmasq_interface: "eth9" kolla_tunnel_interface: "eth10" @@ -32,6 +33,7 @@ kolla_storage_interface: "eth3" kolla_neutron_external_interfaces: "eth4,eth5" kolla_neutron_bridge_names: "br0,br1" + kolla_neutron_physical_networks: "physnet2,physnet3" kolla_tunnel_interface: "eth6" - name: Test kolla-ansible-host-vars role extras @@ -68,6 +70,7 @@ - "kolla_external_vip_interface" - "kolla_neutron_external_interfaces" - "kolla_neutron_bridge_names" + - "kolla_neutron_physical_networks" kolla_ansible_pass_through_host_vars_map: kolla_network_interface: "network_interface" kolla_api_interface: "api_interface" @@ -81,6 +84,7 @@ kolla_tunnel_interface: "tunnel_interface" kolla_neutron_external_interfaces: "neutron_external_interface" kolla_neutron_bridge_names: "neutron_bridge_name" + kolla_neutron_physical_networks: "neutron_physical_networks" kolla_ansible_inventory_path: "{{ temp_path }}" - name: Check whether inventory host vars files exist @@ -125,6 +129,7 @@ kolla_external_vip_interface: "eth1" neutron_external_interface: "eth6,eth7" neutron_bridge_name: "br0,br1" + neutron_physical_networks: "physnet1,physnet2" test-compute: | --- ansible_host: "1.2.3.6" @@ -134,6 +139,7 @@ tunnel_interface: "eth6" neutron_external_interface: "eth4,eth5" neutron_bridge_name: "br0,br1" + neutron_physical_networks: "physnet2,physnet3" always: - name: Ensure the temporary directory is removed diff --git a/doc/source/configuration/reference/kolla-ansible.rst b/doc/source/configuration/reference/kolla-ansible.rst index d53ecf419..92358647c 100644 --- a/doc/source/configuration/reference/kolla-ansible.rst +++ b/doc/source/configuration/reference/kolla-ansible.rst @@ -479,6 +479,7 @@ defined for a host, it is ignored. - "kolla_external_vip_interface" - "kolla_neutron_external_interfaces" - "kolla_neutron_bridge_names" + - "kolla_neutron_physical_networks" It is possible to extend this list via ``kolla_overcloud_inventory_pass_through_host_vars_extra``. @@ -504,6 +505,7 @@ defined for a host, it is ignored. kolla_tunnel_interface: "tunnel_interface" kolla_neutron_external_interfaces: "neutron_external_interface" kolla_neutron_bridge_names: "neutron_bridge_name" + kolla_neutron_physical_networks: "neutron_physical_networks" It is possible to extend this dict via ``kolla_overcloud_inventory_pass_through_host_vars_map_extra``. diff --git a/doc/source/configuration/reference/network.rst b/doc/source/configuration/reference/network.rst index f091c6f28..bcd7b56a3 100644 --- a/doc/source/configuration/reference/network.rst +++ b/doc/source/configuration/reference/network.rst @@ -79,7 +79,9 @@ supported: ``table`` is the routing table ID. ``physical_network`` Name of the physical network on which this network exists. This aligns with - the physical network concept in neutron. + the physical network concept in neutron. This may be used to customise the + Neutron physical network name used for an external network. This attribute + should be set for all external networks or none. ``libvirt_network_name`` A name to give to a Libvirt network representing this network on the seed hypervisor. @@ -335,6 +337,34 @@ To configure a network called ``example`` with a default route and a - cidr: 10.1.0.0/24 table: exampleroutetable +Configuring Custom Neutron Physical Network Names +------------------------------------------------- + +By default, Kolla Ansible uses Neutron physical network names starting with +``physnet1`` through to ``physnetN`` for each external network interface on a +host. + +Sometimes we may want to customise the physical network names used. This may be +to allow for not all hosts having access to all physical networks, or to use +more descriptive names. + +For example, in an environment with a separate physical network for Ironic +provisioning, controllers might have access to two physical networks, while +compute nodes have access to one. We could have a situation where the +controllers and computes use inconsistent physical network names. To avoid +this, we can add ``physical_network`` attributes to these networks. In the +following example, the Ironic provisioning network is ``provision_wl``, and the +external network is ``external``. + +.. code-block:: yaml + :caption: ``$KAYOBE_CONFIG_PATH/networks.yml`` + + provision_wl_physical_network: physnet1 + external_physical_network: physnet2 + +This ensures that compute nodes treat ``external`` as ``physnet2``, even though +it is the only physical network to which they are attached. + .. _configuration-network-per-host: Per-host Network Configuration diff --git a/kayobe/plugins/action/kolla_ansible_host_vars.py b/kayobe/plugins/action/kolla_ansible_host_vars.py index 2a442b760..ea962b0f6 100644 --- a/kayobe/plugins/action/kolla_ansible_host_vars.py +++ b/kayobe/plugins/action/kolla_ansible_host_vars.py @@ -63,20 +63,27 @@ class ActionModule(ActionBase): except ConfigError as e: errors.append(str(e)) - # Build a list of external network interfaces. - external_interfaces = [] + # Build a dict mapping external network interfaces to a list of Kayobe + # network names that they provide connectivity for. + external_interfaces = {} for network in external_networks: try: iface = self._get_external_interface(network["network"], network["required"]) - if iface and iface not in external_interfaces: - external_interfaces.append(iface) + if iface: + iface_networks = external_interfaces.get(iface, []) + if network["network"] not in iface_networks: + iface_networks.append(network["network"]) + external_interfaces[iface] = iface_networks except ConfigError as e: errors.append(str(e)) if external_interfaces: - facts.update(self._get_external_interface_facts( - external_interfaces)) + try: + facts.update(self._get_external_interface_facts( + external_interfaces)) + except ConfigError as e: + errors.append(str(e)) result['changed'] = False if errors: @@ -141,11 +148,13 @@ class ActionModule(ActionBase): def _get_external_interface_facts(self, external_interfaces): neutron_bridge_names = [] neutron_external_interfaces = [] + neutron_physical_networks = [] + missing_physical_networks = [] bridge_suffix = self._templar.template( "{{ network_bridge_suffix_ovs }}") patch_prefix = self._templar.template("{{ network_patch_prefix }}") patch_suffix = self._templar.template("{{ network_patch_suffix_ovs }}") - for interface in external_interfaces: + for interface, iface_networks in external_interfaces.items(): is_bridge = ("{{ '%s' in (network_interfaces |" "net_select_bridges |" "map('net_interface')) }}" % interface) @@ -161,8 +170,38 @@ class ActionModule(ActionBase): else: external_interface = interface neutron_external_interfaces.append(external_interface) - return { + # One external network interface may be referenced by multiple + # external networks. Check if they have a physical_network + # attribute set, and if so, whether they are consistent. + iface_physical_networks = [] + for iface_network in iface_networks: + physical_network = self._templar.template( + "{{ '%s' | net_physical_network }}" % iface_network) + if (physical_network and + physical_network not in iface_physical_networks): + iface_physical_networks.append(physical_network) + if iface_physical_networks: + if len(iface_physical_networks) > 1: + raise ConfigError( + "Inconsistent 'physical_network' attributes for " + "external networks %s using interface %s: %s" % + (", ".join(iface_networks), interface, + ", ".join(iface_physical_networks))) + neutron_physical_networks += iface_physical_networks + else: + missing_physical_networks += iface_networks + facts = { "kolla_neutron_bridge_names": ",".join(neutron_bridge_names), "kolla_neutron_external_interfaces": ",".join( neutron_external_interfaces), } + if neutron_physical_networks: + if missing_physical_networks: + raise ConfigError( + "Some external networks have a 'physical_network' " + "attribute defined but the following do not: %s" % + ", ".join(missing_physical_networks)) + + facts["kolla_neutron_physical_networks"] = ",".join( + neutron_physical_networks) + return facts diff --git a/kayobe/tests/unit/plugins/action/test_kolla_ansible_host_vars.py b/kayobe/tests/unit/plugins/action/test_kolla_ansible_host_vars.py index ec0a60d0f..edfc152b2 100644 --- a/kayobe/tests/unit/plugins/action/test_kolla_ansible_host_vars.py +++ b/kayobe/tests/unit/plugins/action/test_kolla_ansible_host_vars.py @@ -41,6 +41,11 @@ def _net_select_bridges(context, names): if (_net_interface(context, name) or "").startswith("br")] +@jinja2.pass_context +def _net_physical_network(context, name): + return context.get(name + '_physical_network') + + class FakeTemplar(object): def __init__(self, variables): @@ -50,6 +55,7 @@ class FakeTemplar(object): self.env.filters['net_parent'] = _net_parent self.env.filters['net_vlan'] = _net_vlan self.env.filters['net_select_bridges'] = _net_select_bridges + self.env.filters['net_physical_network'] = _net_physical_network def template(self, string): template = self.env.from_string(string) @@ -268,6 +274,26 @@ class TestCase(unittest.TestCase): } self.assertEqual(expected, result) + def test_run_external_networks_one_with_physnet(self): + variables = copy.deepcopy(self.variables) + variables["foo_physical_network"] = "custom1" + module = self._create_module(variables) + external_networks = [{ + "network": "foo", + "required": False, + }] + result = module._run([], external_networks) + expected = { + "changed": False, + "ansible_facts": { + "kolla_neutron_bridge_names": "eth0-ovs", + "kolla_neutron_external_interfaces": "eth0", + "kolla_neutron_physical_networks": "custom1", + }, + "_ansible_facts_cacheable": False, + } + self.assertEqual(expected, result) + def test_run_external_networks_two(self): module = self._create_module() external_networks = [{ @@ -288,6 +314,50 @@ class TestCase(unittest.TestCase): } self.assertEqual(expected, result) + def test_run_external_networks_two_with_physnet(self): + variables = copy.deepcopy(self.variables) + variables["foo_physical_network"] = "custom1" + variables["bar_physical_network"] = "custom2" + module = self._create_module(variables) + external_networks = [{ + "network": "foo", + "required": False, + }, { + "network": "bar", + "required": False, + }] + result = module._run([], external_networks) + expected = { + "changed": False, + "ansible_facts": { + "kolla_neutron_bridge_names": "eth0-ovs,eth1-ovs", + "kolla_neutron_external_interfaces": "eth0,eth1", + "kolla_neutron_physical_networks": "custom1,custom2", + }, + "_ansible_facts_cacheable": False, + } + self.assertEqual(expected, result) + + def test_run_external_networks_two_with_one_physnet(self): + variables = copy.deepcopy(self.variables) + variables["foo_physical_network"] = "custom1" + module = self._create_module(variables) + external_networks = [{ + "network": "foo", + "required": False, + }, { + "network": "bar", + "required": False, + }] + result = module._run([], external_networks) + expected = { + "changed": False, + "failed": True, + "msg": ("Some external networks have a 'physical_network' " + "attribute defined but the following do not: bar"), + } + self.assertEqual(expected, result) + def test_run_external_networks_two_same_interface(self): variables = copy.deepcopy(self.variables) variables["bar_interface"] = "eth0" @@ -310,6 +380,54 @@ class TestCase(unittest.TestCase): } self.assertEqual(expected, result) + def test_run_external_networks_two_same_interface_with_physnet(self): + variables = copy.deepcopy(self.variables) + variables["bar_interface"] = "eth0" + variables["foo_physical_network"] = "custom1" + module = self._create_module(variables) + external_networks = [{ + "network": "foo", + "required": False, + }, { + "network": "bar", + "required": False, + }] + result = module._run([], external_networks) + expected = { + "changed": False, + "ansible_facts": { + "kolla_neutron_bridge_names": "eth0-ovs", + "kolla_neutron_external_interfaces": "eth0", + "kolla_neutron_physical_networks": "custom1", + }, + "_ansible_facts_cacheable": False, + } + self.assertEqual(expected, result) + + def test_run_external_networks_two_same_interface_with_different_physnets( + self): + variables = copy.deepcopy(self.variables) + variables["bar_interface"] = "eth0" + variables["foo_physical_network"] = "custom1" + variables["bar_physical_network"] = "custom2" + module = self._create_module(variables) + external_networks = [{ + "network": "foo", + "required": False, + }, { + "network": "bar", + "required": False, + }] + result = module._run([], external_networks) + expected = { + "changed": False, + "failed": True, + "msg": ("Inconsistent 'physical_network' attributes for external " + "networks foo, bar using interface eth0: custom1, " + "custom2"), + } + self.assertEqual(expected, result) + def test_run_external_networks_two_vlans(self): variables = copy.deepcopy(self.variables) variables["foo_interface"] = "eth0.1" diff --git a/releasenotes/notes/neutron-physical-networks-861c2eca701360e8.yaml b/releasenotes/notes/neutron-physical-networks-861c2eca701360e8.yaml new file mode 100644 index 000000000..fb61e44b5 --- /dev/null +++ b/releasenotes/notes/neutron-physical-networks-861c2eca701360e8.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Adds support for customising Neutron physical network names using the + ``physical_network`` network attribute. +upgrade: + - | + The ``physical_network`` attribute must now be applied consistently + to all external networks in Kayobe configuration. If any external + network has the attribute, then all others must also.