Merge "Performance: avoid set_fact in Kolla Ansible host vars"

ansible/action_plugins/kolla_ansible_host_vars.py

@@ -0,0 +1,19 @@
+# Copyright (c) 2020 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.
+
+__metaclass__ = type
+
+import kayobe.plugins.action.kolla_ansible_host_vars
+
+ActionModule = kayobe.plugins.action.kolla_ansible_host_vars.ActionModule

ansible/kolla-ansible.yml

@@ -127,30 +127,19 @@
     - kolla-ansible
   gather_facts: False
   tasks:
-    - name: Set bifrost network interface
-      set_fact:
-        kolla_bifrost_network_interface: "{{ provision_oc_net_name | net_interface | replace('-', '_') }}"
-      when: provision_oc_net_name in network_interfaces
-
-    - name: Validate seed Kolla Ansible network configuration
-      fail:
-        msg: >
-          The Kolla Ansible variable {{ item.var_name }}
-          ({{ item.description }}) is invalid. Value:
-          "{{ hostvars[inventory_hostname][item.var_name] | default('<undefined>') }}".
-      when:
-        - item.required | bool
-        - hostvars[inventory_hostname][item.var_name] is not defined or not hostvars[inventory_hostname][item.var_name]
-      with_items:
-        - var_name: "kolla_bifrost_network_interface"
-          description: "Bifrost network interface name"
-          required: True
-
-    # Strictly api_interface is not required but kolla-ansible currently
-    # references it in prechecks.
-    - name: Set API network interface
-      set_fact:
-        kolla_api_interface: "{{ kolla_bifrost_network_interface }}"
+    - name: Set Kolla Ansible host variables
+      kolla_ansible_host_vars:
+        interfaces:
+          - var_name: "kolla_bifrost_network_interface"
+            description: "Bifrost provisioning network"
+            network: "{{ provision_oc_net_name }}"
+            required: True
+          # Strictly api_interface is not required but kolla-ansible currently
+          # references it in prechecks.
+          - var_name: "kolla_api_interface"
+            description: "Bifrost provisioning network"
+            network: "{{ provision_oc_net_name }}"
+            required: True
 
     - import_role:
         name: kolla-ansible-host-vars
@@ -166,152 +155,76 @@
     - config-validation
     - kolla-ansible
   gather_facts: False
-  vars:
-    require_provider_networks: >-
-      {{ kolla_enable_neutron | bool and
-         (inventory_hostname in groups['network'] or
-          (kolla_enable_neutron_provider_networks | bool and inventory_hostname in groups['compute'])) }}
   tasks:
-    - name: Set API network interface
-      set_fact:
-        kolla_network_interface: "{{ internal_net_name | net_interface | replace('-', '_')  }}"
-        kolla_api_interface: "{{ internal_net_name | net_interface | replace('-', '_')  }}"
-      when: internal_net_name in network_interfaces
-
-    - name: Set storage network interface
-      set_fact:
-        kolla_storage_interface: "{{ storage_net_name | net_interface | replace('-', '_')  }}"
-      when: storage_net_name in network_interfaces
-
-    - name: Set cluster network interface
-      set_fact:
-        kolla_cluster_interface: "{{ storage_mgmt_net_name | net_interface | replace('-', '_')  }}"
-      when: storage_mgmt_net_name in network_interfaces
-
-    - name: Set Swift storage network interface
-      set_fact:
-        kolla_swift_storage_interface: "{{ swift_storage_net_name | net_interface | replace('-', '_')  }}"
-      when: swift_storage_net_name in network_interfaces
-
-    - name: Set Swift cluster network interface
-      set_fact:
-        kolla_swift_replication_interface: "{{ swift_storage_replication_net_name | net_interface | replace('-', '_')  }}"
-      when: swift_storage_replication_net_name in network_interfaces
-
-    - name: Set provision network interface
-      set_fact:
-        kolla_provision_interface: "{{ provision_wl_net_name | net_interface | replace('-', '_')  }}"
-      when: provision_wl_net_name in network_interfaces
-
-    - name: Set inspector dnsmasq network interface
-      set_fact:
-        kolla_inspector_dnsmasq_interface: "{{ inspection_net_name | net_interface | replace('-', '_')  }}"
-      when: inspection_net_name in network_interfaces
-
-    - name: Set DNS network interface
-      set_fact:
-        kolla_dns_interface: "{{ public_net_name | net_interface | replace('-', '_')  }}"
-      when: public_net_name in network_interfaces
-
-    - name: Set tunnel network interface
-      set_fact:
-        kolla_tunnel_interface: "{{ tunnel_net_name | net_interface | replace('-', '_') }}"
-      when: tunnel_net_name in network_interfaces
-
-    - name: Set external VIP interface
-      set_fact:
-        kolla_external_vip_interface: "{{ public_net_name | net_interface | replace('-', '_') }}"
-      when: public_net_name in network_interfaces
-
-    - name: Initialise facts containing the network host interfaces
-      set_fact:
-        # Initialise the following lists.
-        kolla_neutron_interfaces: []
-        kolla_neutron_bridge_names: []
-        kolla_neutron_external_interfaces: []
-
-    # When these networks are VLANs, we need to use the underlying tagged
-    # bridge interface rather than the untagged interface. We therefore
-    # strip the .<vlan> suffix of the interface name. We use a union here
-    # as a single tagged interface may be shared between these networks.
-    - name: Set a fact containing the interfaces to be plugged to the Neutron OVS bridges
-      set_fact:
-        kolla_neutron_interfaces: >
-          {{ kolla_neutron_interfaces |
-             union([item | net_interface | replace('.' ~ item | net_vlan | default('!nomatch!'), '')]) |
-             list }}
-      with_items: "{{ [provision_wl_net_name, cleaning_net_name] + external_net_names | unique | list }}"
-      when: item in network_interfaces
-
-    - name: Set facts containing the Neutron bridge and interface names
+    - name: Set Kolla Ansible host variables
       vars:
-        is_bridge: "{{ item in (network_interfaces | net_select_bridges | map('net_interface')) }}"
-        # For a bridge, use a veth pair connected to the bridge. Otherwise use
-        # the interface directly.
-        external_interface_name: "{{ (network_patch_prefix ~ item ~ network_patch_suffix_ovs) if is_bridge else item }}"
-      set_fact:
-        kolla_neutron_bridge_names: >
-          {{ kolla_neutron_bridge_names +
-             [item ~ network_bridge_suffix_ovs] }}
-        kolla_neutron_external_interfaces: >
-          {{ kolla_neutron_external_interfaces +
-             [external_interface_name] }}
-      with_items: "{{ kolla_neutron_interfaces }}"
-
-    - name: Validate overcloud host Kolla Ansible network configuration
-      fail:
-        msg: >
-          The Kolla Ansible variable {{ item.var_name }}
-          ({{ item.description }}) is invalid. Value:
-          "{{ hostvars[inventory_hostname][item.var_name] | default('<undefined>') }}".
-      when:
-        - item.required | bool
-        - hostvars[inventory_hostname][item.var_name] is not defined or not hostvars[inventory_hostname][item.var_name]
-      with_items:
-        - var_name: "kolla_api_interface"
-          description: "API network interface name"
-          required: True
-        - var_name: "kolla_external_vip_interface"
-          description: "External network interface name"
-          required: "{{ inventory_hostname in groups['network'] }}"
-        - var_name: "kolla_provision_interface"
-          description: "Bare metal provisioning network interface name"
-          required: "{{ kolla_enable_ironic | bool and inventory_hostname in groups['controllers'] }}"
-        - var_name: "kolla_inspector_dnsmasq_interface"
-          description: "Bare metal introspection network interface name"
-          required: "{{ kolla_enable_ironic | bool and inventory_hostname in groups['controllers'] }}"
-        - var_name: "kolla_neutron_bridge_names"
-          description: "List of Neutron bridge names"
-          required: "{{ require_provider_networks }}"
-        - var_name: "kolla_neutron_external_interfaces"
-          description: "List of Neutron interface names"
-          required: "{{ require_provider_networks }}"
-
-    - name: Validate Kolla Ansible Neutron bridge and interface configuration
-      fail:
-        msg: >
-          The Kolla Ansible variable {{ item.0.var_name }}
-          ({{ item.0.description }}) is invalid. Value:
-          "{{ item.1 | default('<undefined>') }}".
-      when:
-        - item.0.required | bool
-        - item.1 is not defined or not item.1
-      with_subelements:
-        - - var_name: "kolla_neutron_bridge_names"
-            value: "{{ kolla_neutron_bridge_names }}"
-            description: "List of Neutron bridge names"
-            required: "{{ require_provider_networks }}"
-          - var_name: "kolla_neutron_external_interfaces"
-            value: "{{ kolla_neutron_external_interfaces }}"
-            description: "List of Neutron interface names"
-            required: "{{ require_provider_networks }}"
-        - value
-
-    # Kolla ansible expects these variables to be comma-separated lists.
-    - name: Update facts containing the Neutron bridge and interface names
-      set_fact:
-        kolla_neutron_bridge_names: "{{ kolla_neutron_bridge_names | join(',') }}"
-        kolla_neutron_external_interfaces: "{{ kolla_neutron_external_interfaces | join(',') }}"
+        require_ironic_networks: >-
+          {{ kolla_enable_ironic | bool and
+             inventory_hostname in groups['controllers'] }}
+        ironic_networks:
+          - network: "{{ provision_wl_net_name }}"
+            required: "{{ require_ironic_networks }}"
+          - network: "{{ cleaning_net_name }}"
+            required: "{{ require_ironic_networks }}"
+        require_provider_networks: >-
+          {{ kolla_enable_neutron | bool and
+             (inventory_hostname in groups['network'] or
+              (kolla_enable_neutron_provider_networks | bool and inventory_hostname in groups['compute'])) }}
+        # This expression generates a list containing an item for each network
+        # in external_net_names, in the format required by the
+        # external_networks argument of the kolla_ansible_host_vars action
+        # plugin.
+        provider_networks: >-
+          {{ dict(external_net_names |
+                  zip_longest([], fillvalue=require_provider_networks)) |
+             dict2items(key_name='network', value_name='required') }}
+      kolla_ansible_host_vars:
+        interfaces:
+          - var_name: "kolla_network_interface"
+            description: "Default network"
+            network: "{{ internal_net_name }}"
+            required: True
+          - var_name: "kolla_api_interface"
+            description: "API network"
+            network: "{{ internal_net_name }}"
+            required: True
+          - var_name: "kolla_storage_interface"
+            description: "Storage network"
+            network: "{{ storage_net_name }}"
+            required: False
+          - var_name: "kolla_cluster_interface"
+            description: "Cluster network"
+            network: "{{ storage_mgmt_net_name }}"
+            required: False
+          - var_name: "kolla_swift_storage_interface"
+            description: "Swift storage network"
+            network: "{{ swift_storage_net_name }}"
+            required: False
+          - var_name: "kolla_swift_replication_interface"
+            description: "Swift storage replication network"
+            network: "{{ swift_storage_replication_net_name }}"
+            required: False
+          - var_name: "kolla_provision_interface"
+            description: "Bare metal provisioning network"
+            network: "{{ provision_wl_net_name }}"
+            required: "{{ kolla_enable_ironic | bool and inventory_hostname in groups['controllers'] }}"
+          - var_name: "kolla_inspector_dnsmasq_interface"
+            description: "Bare metal introspection network"
+            network: "{{ inspection_net_name }}"
+            required: "{{ kolla_enable_ironic | bool and inventory_hostname in groups['controllers'] }}"
+          - var_name: "kolla_dns_interface"
+            description: "DNS network"
+            network: "{{ public_net_name }}"
+            required: False
+          - var_name: "kolla_tunnel_interface"
+            description: "Tunnel network"
+            network: "{{ tunnel_net_name }}"
+            required: False
+          - var_name: "kolla_external_vip_interface"
+            description: "External network"
+            network: "{{ public_net_name }}"
+            required: "{{ inventory_hostname in groups['network'] }}"
+        external_networks: "{{ ironic_networks + provider_networks }}"
 
     - import_role:
         name: kolla-ansible-host-vars
@@ -319,6 +232,3 @@
         kolla_ansible_pass_through_host_vars: "{{ kolla_overcloud_inventory_pass_through_host_vars }}"
         kolla_ansible_pass_through_host_vars_map: "{{ kolla_overcloud_inventory_pass_through_host_vars_map }}"
         kolla_ansible_inventory_path: "{{ kolla_config_path }}/inventory/overcloud"
-        # Kolla ansible expects these variables to be comma-separated lists.
-        kolla_neutron_bridge_names: "{{ kolla_neutron_bridge_names | join(',') }}"
-        kolla_neutron_external_interfaces: "{{ kolla_neutron_external_interfaces | join(',') }}"

kayobe/plugins/action/kolla_ansible_host_vars.py

@@ -0,0 +1,161 @@
+# Copyright (c) 2020 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.
+
+from ansible.plugins.action import ActionBase
+
+
+class ConfigError(Exception):
+    pass
+
+
+class ActionModule(ActionBase):
+    """Kolla Ansible host vars action plugin
+
+    This class provides an Ansible action module that returns facts
+    representing host variables to be passed to Kolla Ansible.
+    """
+
+    TRANSFERS_FILES = False
+
+    def run(self, tmp=None, task_vars=None):
+        if task_vars is None:
+            task_vars = dict()
+
+        result = super(ActionModule, self).run(tmp, task_vars)
+        del tmp  # tmp no longer has any effect
+
+        # Module arguments:
+        # interfaces: a list of dicts, each containing 'network', 'required',
+        #             and 'description' keys. Each describes a Kolla Ansible
+        #             interface variable.
+        # external_networks: a list of dicts, each containing 'network', and
+        #                    'required' keys. Each describes an external
+        #                    network.
+        interfaces = self._task.args["interfaces"]
+        external_networks = self._task.args.get('external_networks', [])
+        result.update(self._run(interfaces, external_networks))
+        return result
+
+    def _run(self, interfaces, external_networks):
+        result = {}
+        facts = {}
+        errors = []
+
+        # Kolla Ansible interface facts.
+        for interface in interfaces:
+            try:
+                iface = self._get_interface_fact(interface["network"],
+                                                 interface["required"],
+                                                 interface["description"])
+                if iface:
+                    facts[interface["var_name"]] = iface
+            except ConfigError as e:
+                errors.append(str(e))
+
+        # Build a list of external network interfaces.
+        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)
+            except ConfigError as e:
+                errors.append(str(e))
+
+        if external_interfaces:
+            facts.update(self._get_external_interface_facts(
+                external_interfaces))
+
+        result['changed'] = False
+        if errors:
+            result['failed'] = True
+            result['msg'] = "; ".join(errors)
+        else:
+            result['ansible_facts'] = facts
+            result['_ansible_facts_cacheable'] = False
+        return result
+
+    def _get_interface_fact(self, net_name, required, description):
+        # Check whether the network is mapped to this host.
+        condition = "{{ '%s' in network_interfaces }}" % net_name
+        condition = self._templar.template(condition)
+        if condition:
+            # Get the network interface for this network.
+            iface = ("{{ '%s' | net_interface }}" % net_name)
+            iface = self._templar.template(iface)
+            if iface:
+                # Ansible fact names replace dashes with underscores.
+                # FIXME(mgoddard): Is this still required?
+                iface = iface.replace('-', '_')
+            if required and not iface:
+                msg = ("Required network '%s' (%s) does not have an interface "
+                       "configured for this host" % (net_name, description))
+                raise ConfigError(msg)
+            return iface
+        elif required:
+            msg = ("Required network '%s' (%s) is not mapped to this host" %
+                   (net_name, description))
+            raise ConfigError(msg)
+
+    def _get_external_interface(self, net_name, required):
+        condition = "{{ '%s' in network_interfaces }}" % net_name
+        condition = self._templar.template(condition)
+        if condition:
+            iface = self._templar.template("{{ '%s' | net_interface }}" %
+                                           net_name)
+            if iface:
+                # When these networks are VLANs, we need to use the
+                # underlying tagged bridge interface rather than the
+                # untagged interface. We therefore strip the .<vlan> suffix
+                # of the interface name. We use a union here as a single
+                # tagged interface may be shared between these networks.
+                vlan = self._templar.template("{{ '%s' | net_vlan }}" %
+                                              net_name)
+                if vlan and iface.endswith(".%s" % vlan):
+                    iface = iface.replace(".%s" % vlan, "")
+                return iface
+            elif required:
+                raise ConfigError("Required external network '%s' does not "
+                                  "have an interface configured for this host"
+                                  % net_name)
+        elif required:
+            raise ConfigError("Required external network '%s' is not mapped "
+                              "to this host" % net_name)
+
+    def _get_external_interface_facts(self, external_interfaces):
+        neutron_bridge_names = []
+        neutron_external_interfaces = []
+        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:
+            is_bridge = ("{{ '%s' in (network_interfaces |"
+                         "net_select_bridges |"
+                         "map('net_interface')) }}" % interface)
+            is_bridge = self._templar.template(is_bridge)
+            neutron_bridge_names.append(interface + bridge_suffix)
+            # For a bridge, use a veth pair connected to the bridge. Otherwise
+            # use the interface directly.
+            if is_bridge:
+                external_interface = patch_prefix + interface + patch_suffix
+            else:
+                external_interface = interface
+            neutron_external_interfaces.append(external_interface)
+        return {
+            "kolla_neutron_bridge_names": ",".join(neutron_bridge_names),
+            "kolla_neutron_external_interfaces": ",".join(
+                neutron_external_interfaces),
+        }

kayobe/tests/unit/plugins/action/test_kolla_ansible_host_vars.py

@@ -0,0 +1,450 @@
+# Copyright (c) 2020 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.action import kolla_ansible_host_vars
+
+
+@jinja2.contextfilter
+def _net_interface(context, name):
+    return context.get(name + '_interface')
+
+
+@jinja2.contextfilter
+def _net_vlan(context, name):
+    return context.get(name + '_vlan')
+
+
+@jinja2.contextfilter
+def _net_select_bridges(context, names):
+    return [name for name in names
+            if (_net_interface(context, name) or "").startswith("br")]
+
+
+class FakeTemplar(object):
+
+    def __init__(self, variables):
+        self.variables = variables
+        self.env = jinja2.Environment()
+        self.env.filters['net_interface'] = _net_interface
+        self.env.filters['net_vlan'] = _net_vlan
+        self.env.filters['net_select_bridges'] = _net_select_bridges
+
+    def template(self, string):
+        template = self.env.from_string(string)
+        result = template.render(**self.variables)
+        return {
+            "None": None,
+            "True": True,
+            "False": False,
+        }.get(result, result)
+
+
+class TestCase(unittest.TestCase):
+
+    variables = {
+        "network_interfaces": [
+            "foo",
+            "bar",
+        ],
+        "foo_interface": "eth0",
+        "foo_vlan": 1,
+        "bar_interface": "eth1",
+        "bar_vlan": 2,
+        "network_bridge_suffix_ovs": "-ovs",
+        "network_patch_prefix": "p-",
+        "network_patch_suffix_ovs": "-ovs",
+    }
+
+    def _create_module(self, variables=None):
+        if not variables:
+            variables = self.variables
+        templar = FakeTemplar(variables)
+        return kolla_ansible_host_vars.ActionModule(None, None, None, None,
+                                                    templar, None)
+
+    def test__run_empty_args(self):
+        module = self._create_module()
+        result = module._run([], [])
+        expected = {
+            "changed": False,
+            "ansible_facts": {},
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_one_interface(self):
+        module = self._create_module()
+        interfaces = [{
+            "var_name": "kolla_foo_interface",
+            "network": "foo",
+            "description": "Foo network",
+            "required": False,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "ansible_facts": {
+                "kolla_foo_interface": "eth0",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_two_interfaces(self):
+        module = self._create_module()
+        interfaces = [{
+            "var_name": "kolla_foo_interface",
+            "network": "foo",
+            "description": "Foo network",
+            "required": False,
+        }, {
+            "var_name": "kolla_bar_interface",
+            "network": "bar",
+            "description": "Bar network",
+            "required": False,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "ansible_facts": {
+                "kolla_foo_interface": "eth0",
+                "kolla_bar_interface": "eth1",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_one_with_dashes(self):
+        variables = copy.deepcopy(self.variables)
+        variables["foo_interface"] = "eth-0"
+        module = self._create_module(variables)
+        interfaces = [{
+            "var_name": "kolla_foo_interface",
+            "network": "foo",
+            "description": "Foo network",
+            "required": False,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "ansible_facts": {
+                "kolla_foo_interface": "eth_0",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_interface_not_mapped(self):
+        module = self._create_module()
+        interfaces = [{
+            "var_name": "kolla_baz_interface",
+            "network": "baz",
+            "description": "Baz network",
+            "required": True,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "failed": True,
+            "msg": ("Required network 'baz' (Baz network) is not mapped to "
+                    "this host"),
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_interface_not_mapped_not_required(self):
+        module = self._create_module()
+        interfaces = [{
+            "var_name": "kolla_baz_interface",
+            "network": "baz",
+            "description": "Baz network",
+            "required": False,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "ansible_facts": {},
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_interface_no_interface(self):
+        variables = copy.deepcopy(self.variables)
+        del variables["bar_interface"]
+        module = self._create_module(variables)
+        interfaces = [{
+            "var_name": "kolla_bar_interface",
+            "network": "bar",
+            "description": "Bar network",
+            "required": True,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "failed": True,
+            "msg": ("Required network 'bar' (Bar network) does not have an "
+                    "interface configured for this host"),
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_interface_no_interface_not_required(self):
+        variables = copy.deepcopy(self.variables)
+        del variables["bar_interface"]
+        module = self._create_module(variables)
+        interfaces = [{
+            "var_name": "kolla_bar_interface",
+            "network": "bar",
+            "description": "Bar network",
+            "required": False,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "ansible_facts": {},
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test__run_interface_no_interface_not_mapped(self):
+        variables = copy.deepcopy(self.variables)
+        del variables["bar_interface"]
+        module = self._create_module(variables)
+        interfaces = [{
+            "var_name": "kolla_bar_interface",
+            "network": "bar",
+            "description": "Bar network",
+            "required": True,
+        }, {
+            "var_name": "kolla_baz_interface",
+            "network": "baz",
+            "description": "Baz network",
+            "required": True,
+        }]
+        result = module._run(interfaces, [])
+        expected = {
+            "changed": False,
+            "failed": True,
+            "msg": ("Required network 'bar' (Bar network) does not have an "
+                    "interface configured for this host; Required network "
+                    "'baz' (Baz network) is not mapped to this host"),
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_one(self):
+        module = self._create_module()
+        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",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_two(self):
+        module = self._create_module()
+        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",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_two_same_interface(self):
+        variables = copy.deepcopy(self.variables)
+        variables["bar_interface"] = "eth0"
+        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",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_two_vlans(self):
+        variables = copy.deepcopy(self.variables)
+        variables["foo_interface"] = "eth0.1"
+        variables["bar_interface"] = "eth0.2"
+        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",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_bridge(self):
+        variables = copy.deepcopy(self.variables)
+        variables["foo_interface"] = "breth0"
+        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": "breth0-ovs",
+                "kolla_neutron_external_interfaces": "p-breth0-ovs",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_bridge_vlan(self):
+        variables = copy.deepcopy(self.variables)
+        variables["foo_interface"] = "breth0.1"
+        variables["bar_interface"] = "breth0"
+        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": "breth0-ovs",
+                "kolla_neutron_external_interfaces": "p-breth0-ovs",
+            },
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_not_mapped(self):
+        module = self._create_module()
+        external_networks = [{
+            "network": "baz",
+            "required": True,
+        }]
+        result = module._run([], external_networks)
+        expected = {
+            "changed": False,
+            "failed": True,
+            "msg": ("Required external network 'baz' is not mapped to "
+                    "this host"),
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_not_mapped_not_required(self):
+        module = self._create_module()
+        external_networks = [{
+            "network": "baz",
+            "required": False,
+        }]
+        result = module._run([], external_networks)
+        expected = {
+            "changed": False,
+            "ansible_facts": {},
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_no_interface(self):
+        variables = copy.deepcopy(self.variables)
+        del variables["bar_interface"]
+        module = self._create_module(variables)
+        external_networks = [{
+            "network": "bar",
+            "required": True,
+        }]
+        result = module._run([], external_networks)
+        expected = {
+            "changed": False,
+            "failed": True,
+            "msg": ("Required external network 'bar' does not have an "
+                    "interface configured for this host"),
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_no_interface_not_required(self):
+        variables = copy.deepcopy(self.variables)
+        del variables["bar_interface"]
+        module = self._create_module(variables)
+        external_networks = [{
+            "network": "bar",
+            "required": False,
+        }]
+        result = module._run([], external_networks)
+        expected = {
+            "changed": False,
+            "ansible_facts": {},
+            "_ansible_facts_cacheable": False,
+        }
+        self.assertEqual(expected, result)
+
+    def test_run_external_networks_not_mapped_no_interface(self):
+        variables = copy.deepcopy(self.variables)
+        del variables["bar_interface"]
+        module = self._create_module(variables)
+        external_networks = [{
+            "network": "baz",
+            "required": True,
+        }, {
+            "network": "bar",
+            "required": True,
+        }]
+        result = module._run([], external_networks)
+        expected = {
+            "changed": False,
+            "failed": True,
+            "msg": ("Required external network 'baz' is not mapped to "
+                    "this host; Required external network 'bar' does not "
+                    "have an interface configured for this host"),
+        }
+        self.assertEqual(expected, result)