bug: Cluster should be creatable w/o fixed subnet

Without this patch, it is impossible to create a cluster without
defining a fixed_network or a fixed_subnet that already exists since we
get a Fixed{Network,Subnet}NotFound error, and Heat is unable to create
these for us.

Story: 2002652
Task: 37201

Change-Id: I0e26682b0b6093b215393eb4ce8e94eae8e5e8f7
Signed-off-by: Bharat Kunwar <brtknr@bath.edu>

magnum/common/neutron.py

@@ -81,7 +81,7 @@ def get_network(context, network, source, target, external):
 
 
 def get_external_network_id(context, network):
-    if uuidutils.is_uuid_like(network):
+    if network and uuidutils.is_uuid_like(network):
         return network
     else:
         return get_network(context, network, source='name',
@@ -89,7 +89,7 @@ def get_external_network_id(context, network):
 
 
 def get_fixed_network_name(context, network):
-    if uuidutils.is_uuid_like(network):
+    if network and uuidutils.is_uuid_like(network):
         return get_network(context, network, source='id',
                            target='name', external=False)
     else:
@@ -119,8 +119,7 @@ def get_subnet(context, subnet, source, target):
 
 
 def get_fixed_subnet_id(context, subnet):
-    if uuidutils.is_uuid_like(subnet):
-        return subnet
+    if subnet and not uuidutils.is_uuid_like(subnet):
+        return get_subnet(context, subnet, source='name', target='id')
     else:
-        return get_subnet(context, subnet, source='name',
-                          target='id')
+        return subnet

magnum/drivers/heat/k8s_template_def.py

@@ -160,46 +160,50 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition):
               self).update_outputs(stack, cluster_template, cluster,
                                    nodegroups=nodegroups)
 
-    def get_params(self, context, cluster_template, cluster, **kwargs):
-        extra_params = kwargs.pop('extra_params', {})
-
-        extra_params['discovery_url'] = self.get_discovery_url(cluster)
-        osc = self.get_osc(context)
-        extra_params['magnum_url'] = osc.magnum_url()
-
-        if cluster_template.tls_disabled:
-            extra_params['loadbalancing_protocol'] = 'HTTP'
-            extra_params['kubernetes_port'] = 8080
-
-        extra_params['octavia_enabled'] = keystone.is_octavia_enabled()
-
+    def get_net_params(self, context, cluster_template, cluster):
+        extra_params = dict()
         # NOTE(lxkong): Convert external network name to UUID, the template
         # field name is confused. If external_network_id is not specified in
         # cluster template use 'public' as the default value, which is the same
         # with the heat template default value as before.
-        external_network = cluster_template.external_network_id or "public"
-        extra_params['external_network'] = \
-            neutron.get_external_network_id(context, external_network)
+        external_network = cluster_template.external_network_id
+        ext_net_id = neutron.get_external_network_id(context, external_network)
+        extra_params['external_network'] = ext_net_id
 
         # NOTE(brtknr): Convert fixed network UUID to name if the given network
         # name is UUID like because OpenStack Cloud Controller Manager only
         # accepts a name as an argument to internal-network-name in the
         # cloud-config file provided to it. The default fixed network name is
         # the same as that defined in the heat template.
-        fixed_network = (cluster.fixed_network or
-                         cluster_template.fixed_network or
-                         "private")
-        extra_params['fixed_network_name'] = \
-            neutron.get_fixed_network_name(context, fixed_network)
+        fixed_network = cluster.fixed_network
+        net_name = neutron.get_fixed_network_name(context, fixed_network)
+        if net_name:
+            extra_params['fixed_network_name'] = net_name
 
         # NOTE(brtknr): Convert fixed subnet name to UUID. If fixed_subnet
         # is not specified in cluster template use 'private' as the default
         # value, which is the same as the heat template default value.
-        fixed_subnet = (cluster.fixed_subnet or
-                        cluster_template.fixed_subnet or
-                        "private")
-        extra_params['fixed_subnet'] = \
-            neutron.get_fixed_subnet_id(context, fixed_subnet)
+        fixed_subnet = cluster.fixed_subnet
+        subnet_id = neutron.get_fixed_subnet_id(context, fixed_subnet)
+        if subnet_id:
+            extra_params['fixed_subnet'] = subnet_id
+        return extra_params
+
+    def get_params(self, context, cluster_template, cluster, **kwargs):
+        extra_params = kwargs.pop('extra_params', {})
+
+        extra_params['discovery_url'] = self.get_discovery_url(cluster)
+        osc = self.get_osc(context)
+        extra_params['magnum_url'] = osc.magnum_url()
+
+        if cluster_template.tls_disabled:
+            extra_params['loadbalancing_protocol'] = 'HTTP'
+            extra_params['kubernetes_port'] = 8080
+
+        extra_params['octavia_enabled'] = keystone.is_octavia_enabled()
+
+        net_params = self.get_net_params(context, cluster_template, cluster)
+        extra_params.update(net_params)
 
         label_list = ['flannel_network_cidr', 'flannel_backend',
                       'flannel_network_subnetlen',

magnum/tests/unit/drivers/test_template_definition.py

@@ -441,12 +441,11 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase):
         mock_cluster_template.network_driver = 'flannel'
         external_network_id = '17e4e301-b7f3-4996-b3dd-97b3a700174b'
         mock_cluster_template.external_network_id = external_network_id
+        mock_cluster = mock.MagicMock()
         fixed_network_name = 'fixed_network'
         mock_get_fixed_network_name.return_value = fixed_network_name
         fixed_network = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52'
-        mock_cluster_template.fixed_network = fixed_network
-        mock_cluster = mock.MagicMock()
-        mock_cluster.fixed_network = None
+        mock_cluster.fixed_network = fixed_network
         mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52'
         fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53'
         mock_cluster.fixed_subnet = fixed_subnet
@@ -678,7 +677,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase):
         )
         mock_get_fixed_network_name.assert_called_once_with(
             mock_context,
-            mock_cluster_template.fixed_network
+            mock_cluster.fixed_network
         )
 
     @mock.patch('magnum.common.neutron.get_external_network_id')
@@ -880,10 +879,9 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase):
         mock_cluster_template.network_driver = 'calico'
         external_network_id = '17e4e301-b7f3-4996-b3dd-97b3a700174b'
         mock_cluster_template.external_network_id = external_network_id
-        fixed_network_name = 'fixed_network'
-        mock_cluster_template.fixed_network = fixed_network_name
         mock_cluster = mock.MagicMock()
-        mock_cluster.fixed_network = None
+        fixed_network_name = 'fixed_network'
+        mock_cluster.fixed_network = fixed_network_name
         mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52'
         fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53'
         mock_cluster.fixed_subnet = fixed_subnet