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

@@ -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

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 @@ 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

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"

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.