From 97dbd49d8207fdac49605fe138e4c68dfb9eb0d1 Mon Sep 17 00:00:00 2001 From: Bharat Kunwar Date: Tue, 24 Sep 2019 12:51:44 +0000 Subject: [PATCH] Convert fixed_subnet name to uuid for OCCM Since OpenStack Cloud Controller Manager only accepts fixed_subnet uuid, convert fixed_subnet name to uuid when a cluster is created. Without this patch, there is a chance OCCM fails to start in come cases when fixed_subnet is rendered as name. Story: 2002652 Task: 28816 Change-Id: Ie70bc00f5617ef94c39c9faea7d39617ee01b07b --- magnum/common/exception.py | 6 ++ magnum/common/neutron.py | 36 ++++++++- magnum/drivers/heat/k8s_template_def.py | 10 ++- magnum/tests/unit/common/test_neutron.py | 77 +++++++++++++++++++ .../handlers/test_k8s_cluster_conductor.py | 16 ++-- .../unit/drivers/test_template_definition.py | 9 +++ 6 files changed, 142 insertions(+), 12 deletions(-) diff --git a/magnum/common/exception.py b/magnum/common/exception.py index acda4fc2a7..22f6882fc9 100755 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -350,6 +350,12 @@ class FixedNetworkNotFound(ResourceNotFound): code = 400 +class FixedSubnetNotFound(ResourceNotFound): + """The code here changed to 400 according to the latest document.""" + message = _("Unable to find fixed subnet %(subnet)s.") + code = 400 + + class ExternalNetworkNotFound(ResourceNotFound): """The code here changed to 400 according to the latest document.""" """"Ensure the network is not private.""" diff --git a/magnum/common/neutron.py b/magnum/common/neutron.py index 242ea35401..5d6c4b9929 100644 --- a/magnum/common/neutron.py +++ b/magnum/common/neutron.py @@ -58,10 +58,10 @@ def delete_floatingip(context, fix_port_id, cluster): def get_network(context, network, source, target, external): nets = [] n_client = clients.OpenStackClients(context).neutron() - ext_filter = {'router:external': external} + filters = {source: network, 'router:external': external} + networks = n_client.list_networks(**filters).get('networks') - networks = n_client.list_networks(**ext_filter) - for net in networks.get('networks'): + for net in networks: if net.get(source) == network: nets.append(net) @@ -94,3 +94,33 @@ def get_fixed_network_name(context, network): target='name', external=False) else: return network + + +def get_subnet(context, subnet, source, target): + nets = [] + n_client = clients.OpenStackClients(context).neutron() + filters = {source: subnet} + subnets = n_client.list_subnets(**filters).get('subnets', []) + + for net in subnets: + if net.get(source) == subnet: + nets.append(net) + + if len(nets) == 0: + raise exception.FixedSubnetNotFound(subnet=subnet) + + if len(nets) > 1: + raise exception.Conflict( + "Multiple subnets exist with same name '%s'. Please use the " + "subnet ID instead." % subnet + ) + + return nets[0][target] + + +def get_fixed_subnet_id(context, subnet): + if uuidutils.is_uuid_like(subnet): + return subnet + else: + return get_subnet(context, subnet, source='name', + target='id') diff --git a/magnum/drivers/heat/k8s_template_def.py b/magnum/drivers/heat/k8s_template_def.py index e2a86b1a5b..44c06c8744 100644 --- a/magnum/drivers/heat/k8s_template_def.py +++ b/magnum/drivers/heat/k8s_template_def.py @@ -189,10 +189,18 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): 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) + label_list = ['flannel_network_cidr', 'flannel_backend', 'flannel_network_subnetlen', 'system_pods_initial_delay', diff --git a/magnum/tests/unit/common/test_neutron.py b/magnum/tests/unit/common/test_neutron.py index 9641539dbe..6630c2f291 100644 --- a/magnum/tests/unit/common/test_neutron.py +++ b/magnum/tests/unit/common/test_neutron.py @@ -272,3 +272,80 @@ class NeutronTest(base.TestCase): self.context, another_fake_id ) + + @mock.patch('magnum.common.clients.OpenStackClients') + def test_get_fixed_subnet_id(self, mock_clients): + fake_name = "fake_subnet" + fake_id = "35ee5da0-1ac0-11e9-84cd-00224d6b7bc1" + mock_nclient = mock.MagicMock() + mock_nclient.list_subnets.return_value = { + 'subnets': [ + { + 'id': fake_id, + 'name': fake_name, + } + ] + } + + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.neutron.return_value = mock_nclient + + subnet_id = neutron.get_fixed_subnet_id(self.context, fake_name) + + self.assertEqual(fake_id, subnet_id) + + @mock.patch('magnum.common.clients.OpenStackClients') + def test_get_fixed_subnet_id_notfound(self, mock_clients): + fake_name = "fake_subnet" + fake_id = "35ee5da0-1ac0-11e9-84cd-00224d6b7bc1" + mock_nclient = mock.MagicMock() + mock_nclient.list_subnets.return_value = { + 'subnets': [ + { + 'id': fake_id, + 'name': fake_name, + } + ] + } + + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.neutron.return_value = mock_nclient + + self.assertRaises( + exception.FixedSubnetNotFound, + neutron.get_fixed_subnet_id, + self.context, + "another_subnet" + ) + + @mock.patch('magnum.common.clients.OpenStackClients') + def test_get_fixed_subnet_id_conflict(self, mock_clients): + fake_name = "fake_subnet" + fake_id_1 = "35ee5da0-1ac0-11e9-84cd-00224d6b7bc1" + fake_id_2 = "93781f82-1ac0-11e9-84cd-00224d6b7bc1" + mock_nclient = mock.MagicMock() + mock_nclient.list_subnets.return_value = { + 'subnets': [ + { + 'id': fake_id_1, + 'name': fake_name, + }, + { + 'id': fake_id_2, + 'name': fake_name, + } + ] + } + + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.neutron.return_value = mock_nclient + + self.assertRaises( + exception.Conflict, + neutron.get_fixed_subnet_id, + self.context, + fake_name + ) diff --git a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py index 90222e625f..a79cf22df0 100644 --- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py @@ -44,7 +44,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'dns_nameserver': 'dns_nameserver', 'external_network_id': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'fixed_network': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'network_driver': 'network_driver', 'volume_driver': 'volume_driver', 'docker_volume_size': 20, @@ -121,7 +121,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'project_id': 'project_id', 'keystone_auth_default_policy': self.keystone_auth_default_policy, 'fixed_network': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'floating_ip_enabled': False, } self.worker_ng_dict = { @@ -301,7 +301,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'external_network': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'fixed_network': 'fixed_network', 'fixed_network_name': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'network_driver': 'network_driver', 'volume_driver': 'volume_driver', 'dns_nameserver': 'dns_nameserver', @@ -454,7 +454,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'external_network': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'fixed_network': 'fixed_network', 'fixed_network_name': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'flannel_backend': 'vxlan', 'flannel_network_cidr': '10.101.0.0/16', 'flannel_network_subnetlen': '26', @@ -593,7 +593,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'minion_flavor': 'flavor_id', 'fixed_network': 'fixed_network', 'fixed_network_name': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'external_network': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'flannel_backend': 'vxlan', 'flannel_network_cidr': '10.101.0.0/16', @@ -697,7 +697,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'external_network': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'fixed_network': 'fixed_network', 'fixed_network_name': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'availability_zone': 'az_1', 'nodes_affinity_policy': 'soft-anti-affinity', 'dns_nameserver': 'dns_nameserver', @@ -805,7 +805,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'external_network': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'fixed_network': 'fixed_network', 'fixed_network_name': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'dns_nameserver': 'dns_nameserver', 'docker_storage_driver': u'devicemapper', 'docker_volume_size': 20, @@ -1025,7 +1025,7 @@ class TestClusterConductorWithK8s(base.TestCase): 'external_network': 'e2a6c8b0-a3c2-42a3-b3f4-01400a30896e', 'fixed_network': 'fixed_network', 'fixed_network_name': 'fixed_network', - 'fixed_subnet': 'fixed_subnet', + 'fixed_subnet': 'c2a6c8b0-a3c2-42a3-b3f4-01400a30896f', 'dns_nameserver': 'dns_nameserver', 'master_image': 'image_id', 'minion_image': 'image_id', diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py index 9fbdfb6893..3ef1bbd4dd 100644 --- a/magnum/tests/unit/drivers/test_template_definition.py +++ b/magnum/tests/unit/drivers/test_template_definition.py @@ -448,6 +448,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster = mock.MagicMock() mock_cluster.fixed_network = None mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' + fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53' + mock_cluster.fixed_subnet = fixed_subnet del mock_cluster.stack_id mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' @@ -604,6 +606,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): 'etcd_tag': etcd_tag, 'coredns_tag': coredns_tag, 'fixed_network_name': fixed_network_name, + 'fixed_subnet': fixed_subnet, 'flannel_tag': flannel_tag, 'flannel_cni_tag': flannel_cni_tag, 'container_infra_prefix': container_infra_prefix, @@ -719,6 +722,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster.labels = {} mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' mock_cluster.project_id = 'e2a6c8b0-a3c2-42a3-b3f4-1f639a523a52' + mock_cluster.fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53' mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' @@ -776,6 +780,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster.labels = {"ingress_controller": "octavia"} mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' mock_cluster.project_id = 'e2a6c8b0-a3c2-42a3-b3f4-1f639a523a52' + mock_cluster.fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53' mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' @@ -830,6 +835,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster.labels = {"ingress_controller": "octavia"} mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' mock_cluster.project_id = 'e2a6c8b0-a3c2-42a3-b3f4-1f639a523a52' + mock_cluster.fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53' mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' @@ -879,6 +885,8 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster = mock.MagicMock() mock_cluster.fixed_network = None mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' + fixed_subnet = 'f2a6c8b0-a3c2-42a3-b3f4-1f639a523a53' + mock_cluster.fixed_subnet = fixed_subnet del mock_cluster.stack_id mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' @@ -1013,6 +1021,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): 'system_pods_initial_delay': system_pods_initial_delay, 'system_pods_timeout': system_pods_timeout, 'fixed_network_name': fixed_network_name, + 'fixed_subnet': fixed_subnet, 'admission_control_list': admission_control_list, 'prometheus_monitoring': prometheus_monitoring, 'grafana_admin_passwd': grafana_admin_passwd,