Merge "Remove _net_address_search from dynamic inventory"

playbooks/inventory/dynamic_inventory.py

@@ -69,6 +69,20 @@ class MultipleHostsWithOneIPError(Exception):
         return self.message
 
 
+class ProviderNetworkMisconfiguration(Exception):
+    def __init__(self, queue_name):
+        self.queue_name = queue_name
+
+        error_msg = ("Provider network with queue '{queue}' "
+                     "requires 'is_container_address' and "
+                     "'is_ssh_address' to be set to True.")
+
+        self.message = error_msg.format(queue=self.queue_name)
+
+    def __str__(self):
+        return self.message
+
+
 class MultipleIpForHostError(Exception):
     def __init__(self, hostname, current_ip, new_ip):
         self.hostname = hostname
@@ -664,27 +678,6 @@ def _add_additional_networks(key, inventory, ip_q, q_name, netmask, interface,
                 networks[old_address]['static_routes'].append(route)
 
 
-def _net_address_search(provider_networks, main_network, key):
-    """Set the key network type to the main network if not specified.
-
-    :param provider_networks: ``list`` Network list of ``dict``s
-    :param main_network: ``str`` The name of the main network bridge.
-    :param key: ``str`` The name of the key to set true.
-    """
-    for pn in provider_networks:
-        # p_net are the provider_network values
-        p_net = pn.get('network')
-        if p_net:
-            # Check for the key
-            if p_net.get(key):
-                break
-            else:
-                if p_net.get('container_bridge') == main_network:
-                    p_net[key] = True
-
-    return provider_networks
-
-
 def container_skel_load(container_skel, inventory, config):
     """Build out all containers as defined in the environment file.
 
@@ -718,18 +711,6 @@ def container_skel_load(container_skel, inventory, config):
         overrides = config['global_overrides']
         # iterate over a list of provider_networks, var=pn
         pns = overrides.get('provider_networks', list())
-        pns = _net_address_search(
-            provider_networks=pns,
-            main_network=config['global_overrides']['management_bridge'],
-            key='is_ssh_address'
-        )
-
-        pns = _net_address_search(
-            provider_networks=pns,
-            main_network=config['global_overrides']['management_bridge'],
-            key='is_container_address'
-        )
-
         for pn in pns:
             # p_net are the provider_network values
             p_net = pn.get('network')
@@ -1006,6 +987,12 @@ def _check_config_settings(cidr_networks, config, container_skel):
                     raise SystemExit(
                         "can't find " + q_name + " in cidr_networks"
                     )
+                if (p_net.get('container_bridge') ==
+                        overrides.get('management_bridge')):
+                    if (not p_net.get('is_ssh_address') or
+                            not p_net.get('is_container_address')):
+                        raise ProviderNetworkMisconfiguration(q_name)
+
     # look for same ip address assigned to different hosts
     _check_same_ip_to_multiple_host(config)
 

releasenotes/notes/management_network_config_check-66778387f38b9e0c.yaml

@@ -0,0 +1,8 @@
+---
+upgrade:
+  - The dynamic_inventory script previously set the provider network
+    attributes ``is_container_address`` and ``is_ssh_address`` to True
+    for the management network regardless of whether a deployer had them
+    configured this way or not. Now, these attributes must be configured
+    by deployers and the dynamic_inventory script will fail if they are
+    missing or not True.

tests/test_inventory.py

@@ -376,6 +376,14 @@ class TestConfigChecks(unittest.TestCase):
         del self.user_defined_config['cidr_networks'][net_name]
         self.write_config()
 
+    def delete_provider_network_key(self, net_name, key):
+        pns = self.user_defined_config['global_overrides']['provider_networks']
+        for net in pns:
+            if 'ip_from_q' in net['network']:
+                if net['network']['ip_from_q'] == net_name:
+                    if key in net['network']:
+                        del net['network'][key]
+
     def write_config(self):
         self.config_changed = True
         # rename temporarily our user_config_file so we can use the new one
@@ -398,6 +406,19 @@ class TestConfigChecks(unittest.TestCase):
                        "user config.")
         self.assertEqual(context.exception.message, expectedLog)
 
+    def test_management_network_malformed(self):
+        self.delete_provider_network_key('container', 'is_container_address')
+        self.delete_provider_network_key('container', 'is_ssh_address')
+        self.write_config()
+
+        with self.assertRaises(di.ProviderNetworkMisconfiguration) as context:
+            get_inventory()
+        expectedLog = ("Provider network with queue 'container' "
+                       "requires 'is_container_address' and "
+                       "'is_ssh_address' to be set to True.")
+        self.assertEqual(context.exception.message, expectedLog)
+        self.restore_config()
+
     def test_missing_cidr_network_present_in_provider(self):
         self.delete_provider_network('storage')
         with self.assertRaises(SystemExit) as context:
@@ -622,33 +643,6 @@ class TestStaticRouteConfig(TestConfigChecks):
         self.assertEqual(exception.message, self.expectedMsg)
 
 
-class TestNetAddressSearch(unittest.TestCase):
-    def test_net_address_search_key_not_found(self):
-        pns = [
-            {'network': {'container_bridge': 'br-mgmt'}}
-        ]
-        new_pns = di._net_address_search(pns, 'br-mgmt', 'is_ssh_address')
-
-        self.assertTrue(new_pns[0]['network']['is_ssh_address'])
-
-    def test_net_address_search_key_not_found_bridge_doesnt_match(self):
-        pns = [
-            {'network': {'container_bridge': 'lxcbr0'}}
-        ]
-        new_pns = di._net_address_search(pns, 'br-mgmt', 'is_ssh_address')
-
-        self.assertNotIn('is_ssh_address', new_pns[0]['network'])
-
-    def test_net_address_search_key_found(self):
-        pns = [
-            {'network': {'container_bridge': 'br-mgmt',
-                         'is_ssh_address': True}}
-        ]
-        new_pns = di._net_address_search(pns, 'br-mgmt', 'is_ssh_address')
-
-        self.assertEqual(pns, new_pns)
-
-
 class TestGlobalOverridesConfigDeletion(TestConfigChecks):
     def setUp(self):
         super(TestGlobalOverridesConfigDeletion, self).setUp()