From 756a85b70cc7fa1b539ec14ab7d1f0e4294264f8 Mon Sep 17 00:00:00 2001 From: Bharat Kunwar Date: Wed, 16 Oct 2019 13:34:50 +0000 Subject: [PATCH] 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 --- magnum/common/neutron.py | 11 ++-- magnum/drivers/heat/k8s_template_def.py | 58 ++++++++++--------- .../unit/drivers/test_template_definition.py | 12 ++-- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/magnum/common/neutron.py b/magnum/common/neutron.py index 5d6c4b9929..7e62febf21 100644 --- a/magnum/common/neutron.py +++ b/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 diff --git a/magnum/drivers/heat/k8s_template_def.py b/magnum/drivers/heat/k8s_template_def.py index 44c06c8744..d84ea5de2c 100644 --- a/magnum/drivers/heat/k8s_template_def.py +++ b/magnum/drivers/heat/k8s_template_def.py @@ -160,6 +160,35 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): self).update_outputs(stack, cluster_template, cluster, nodegroups=nodegroups) + 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 + 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 + 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 + 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', {}) @@ -173,33 +202,8 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): extra_params['octavia_enabled'] = keystone.is_octavia_enabled() - # 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) - - # 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) - - # 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) + 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', diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py index 3ef1bbd4dd..54adcfa36b 100644 --- a/magnum/tests/unit/drivers/test_template_definition.py +++ b/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