Merge pull request #337 from stackhpc/feature/2024.1/physnet-names

Add support for customising Neutron physical network names

ansible/inventory/group_vars/all/kolla

@@ -402,6 +402,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
@@ -432,6 +433,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

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

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``.

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

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

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"

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.