From c1da6651abbc6064304e8e6b4eea6e360931250e 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 (cherry picked from commit 16879753261109defdf41cd9151d70fc22c5a306) --- 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 | 5 + 7 files changed, 211 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 7a55dfd12..8d464415c 100644 --- a/ansible/inventory/group_vars/all/kolla +++ b/ansible/inventory/group_vars/all/kolla @@ -406,6 +406,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 @@ -436,6 +437,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 97bf88df5..e1356fd0d 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 f3d37da74..4a14e59e1 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. @@ -331,6 +333,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 356c7e7ff..b4ee02db8 100644 --- a/kayobe/plugins/action/kolla_ansible_host_vars.py +++ b/kayobe/plugins/action/kolla_ansible_host_vars.py @@ -63,20 +63,27 @@ def _run(self, interfaces, external_networks): 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: @@ -137,11 +144,13 @@ def _get_external_interface(self, net_name, required): 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) @@ -157,8 +166,38 @@ def _get_external_interface_facts(self, external_interfaces): 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 667cd1329..3ae00e1a9 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 @@ def __init__(self, variables): 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) @@ -248,6 +254,26 @@ def test_run_external_networks_one(self): } 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 = [{ @@ -268,6 +294,50 @@ def test_run_external_networks_two(self): } 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" @@ -290,6 +360,54 @@ def test_run_external_networks_two_same_interface(self): } 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..27b4543ac --- /dev/null +++ b/releasenotes/notes/neutron-physical-networks-861c2eca701360e8.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds support for customising Neutron physical network names using the + ``physical_network`` network attribute.