From f63761a804b240dd0e33832e1e57d9cdb3873277 Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Tue, 15 Jan 2019 15:02:52 +1300 Subject: [PATCH] [k8s_fedora_atomic] Delete floating ip for load balancer When user creates LoadBalancer type service in k8s cluster, a floating ip may be created and associated with the load balancer VIP. Magnum now could delete the load balancers automatically in the cluster pre-delete method, should also remove the floating ip as needed. This patch depends on the github PR for cloud-provider-openstack: https://github.com/kubernetes/cloud-provider-openstack/pull/433 Story: 2004836 Change-Id: Ia553aff4e66033346c6bfe120a72992bec79e136 --- magnum/common/neutron.py | 55 +++++++ magnum/common/octavia.py | 5 + magnum/tests/unit/common/test_neutron.py | 138 ++++++++++++++++++ magnum/tests/unit/common/test_octavia.py | 43 ++++-- .../k8s-delete-vip-fip-b2ddf61ddbc080bc.yaml | 6 + 5 files changed, 238 insertions(+), 9 deletions(-) create mode 100644 magnum/common/neutron.py create mode 100644 magnum/tests/unit/common/test_neutron.py create mode 100644 releasenotes/notes/k8s-delete-vip-fip-b2ddf61ddbc080bc.yaml diff --git a/magnum/common/neutron.py b/magnum/common/neutron.py new file mode 100644 index 0000000000..06b3472def --- /dev/null +++ b/magnum/common/neutron.py @@ -0,0 +1,55 @@ +# Copyright 2019 Catalyst Cloud 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 re + +from oslo_log import log as logging + +from magnum.common import clients +from magnum.common import exception + +LOG = logging.getLogger(__name__) + + +def delete_floatingip(context, fix_port_id, cluster): + """Deletes the floating IP associated with the fix_port_id. + + Only delete the floating IP if it's created and associated with the + LoadBalancer type service in Kubernetes cluster. + + This method only works with the Kubernetes cluster with + cloud-provider-openstack controller manager deployed, patched with + this PR: + https://github.com/kubernetes/cloud-provider-openstack/pull/433 + """ + pattern = (r'Floating IP for Kubernetes external service \w+ from cluster ' + r'%s$' % cluster.uuid) + + try: + n_client = clients.OpenStackClients(context).neutron() + fips = n_client.list_floatingips(port_id=fix_port_id) + if len(fips["floatingips"]) == 0: + return + + # Liberty Neutron doesn't support description field, although Liberty + # is no longer supported, we write good code here. + desc = fips["floatingips"][0].get("description", "") + id = fips["floatingips"][0]["id"] + + if re.match(pattern, desc): + LOG.debug("Deleting floating ip %s for cluster %s", id, + cluster.uuid) + n_client.delete_floatingip(id) + except Exception as e: + raise exception.PreDeletionFailed(cluster_uuid=cluster.uuid, + msg=str(e)) diff --git a/magnum/common/octavia.py b/magnum/common/octavia.py index 8bff56737b..0aab5ba6fd 100644 --- a/magnum/common/octavia.py +++ b/magnum/common/octavia.py @@ -19,6 +19,7 @@ import time from magnum.common import clients from magnum.common import exception +from magnum.common import neutron LOG = logging.getLogger(__name__) CONF = cfg.CONF @@ -74,6 +75,10 @@ def delete_loadbalancers(context, cluster): invalids.add(lb["id"]) continue if lb["provisioning_status"] in ["ACTIVE", "ERROR"]: + # Delete VIP floating ip if needed. + neutron.delete_floatingip(context, lb["vip_port_id"], + cluster) + LOG.debug("Deleting load balancer %s for cluster %s", lb["id"], cluster.uuid) o_client.load_balancer_delete(lb["id"], cascade=True) diff --git a/magnum/tests/unit/common/test_neutron.py b/magnum/tests/unit/common/test_neutron.py new file mode 100644 index 0000000000..87438759d7 --- /dev/null +++ b/magnum/tests/unit/common/test_neutron.py @@ -0,0 +1,138 @@ +# Copyright 2019 Catalyst Cloud 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 mock + +from magnum.common import exception +from magnum.common import neutron +from magnum import objects +from magnum.tests import base +from magnum.tests.unit.db import utils + + +class NeutronTest(base.TestCase): + def setUp(self): + super(NeutronTest, self).setUp() + + cluster_dict = utils.get_test_cluster(node_count=1) + self.cluster = objects.Cluster(self.context, **cluster_dict) + + @mock.patch('magnum.common.clients.OpenStackClients') + def test_delete_floatingip(self, mock_clients): + mock_nclient = mock.MagicMock() + fake_port_id = "b4518944-c2cf-4c69-a1e3-774041fd5d14" + fake_fip_id = "0f8c6849-af85-424c-aa8e-745ade9a46a7" + mock_nclient.list_floatingips.return_value = { + 'floatingips': [ + { + 'router_id': '6ed4f7ef-b8c3-4711-93cf-d53cf0e8bdf5', + 'status': 'ACTIVE', + 'description': 'Floating IP for Kubernetes external ' + 'service ad3080723f1c211e88adbfa163ee1203 ' + 'from cluster %s' % self.cluster.uuid, + 'tags': [], + 'tenant_id': 'cd08a539b7c845ddb92c5d08752101d1', + 'floating_network_id': 'd0b9a8c5-33e5-4ce1-869a-1e2ec7c2f' + '74b', + 'port_details': { + 'status': 'ACTIVE', + 'name': 'test-k8s-master', + 'admin_state_up': True, + 'network_id': '7b9110b5-90a2-40bc-b892-07d641387760 ', + 'device_owner': 'compute:nova', + 'mac_address': 'fa:16:3e:6f:ad:6c', + 'device_id': 'a5c1689f-dd76-4164-8562-6990071701cd' + }, + 'fixed_ip_address': '10.0.0.4', + 'floating_ip_address': '172.24.4.74', + 'revision_number': 14, + 'project_id': 'cd08a539b7c845ddb92c5d08752101d1', + 'port_id': fake_port_id, + 'id': fake_fip_id + } + ] + } + + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.neutron.return_value = mock_nclient + + neutron.delete_floatingip(self.context, fake_port_id, self.cluster) + + mock_nclient.delete_floatingip.assert_called_once_with(fake_fip_id) + + @mock.patch('magnum.common.clients.OpenStackClients') + def test_delete_floatingip_empty(self, mock_clients): + mock_nclient = mock.MagicMock() + fake_port_id = "b4518944-c2cf-4c69-a1e3-774041fd5d14" + mock_nclient.list_floatingips.return_value = { + 'floatingips': [] + } + + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.neutron.return_value = mock_nclient + + neutron.delete_floatingip(self.context, fake_port_id, self.cluster) + + self.assertFalse(mock_nclient.delete_floatingip.called) + + @mock.patch('magnum.common.clients.OpenStackClients') + def test_delete_floatingip_exception(self, mock_clients): + mock_nclient = mock.MagicMock() + fake_port_id = "b4518944-c2cf-4c69-a1e3-774041fd5d14" + fake_fip_id = "0f8c6849-af85-424c-aa8e-745ade9a46a7" + mock_nclient.list_floatingips.return_value = { + 'floatingips': [ + { + 'router_id': '6ed4f7ef-b8c3-4711-93cf-d53cf0e8bdf5', + 'status': 'ACTIVE', + 'description': 'Floating IP for Kubernetes external ' + 'service ad3080723f1c211e88adbfa163ee1203 ' + 'from cluster %s' % self.cluster.uuid, + 'tags': [], + 'tenant_id': 'cd08a539b7c845ddb92c5d08752101d1', + 'floating_network_id': 'd0b9a8c5-33e5-4ce1-869a-1e2ec7c2f' + '74b', + 'port_details': { + 'status': 'ACTIVE', + 'name': 'test-k8s-master', + 'admin_state_up': True, + 'network_id': '7b9110b5-90a2-40bc-b892-07d641387760 ', + 'device_owner': 'compute:nova', + 'mac_address': 'fa:16:3e:6f:ad:6c', + 'device_id': 'a5c1689f-dd76-4164-8562-6990071701cd' + }, + 'fixed_ip_address': '10.0.0.4', + 'floating_ip_address': '172.24.4.74', + 'revision_number': 14, + 'project_id': 'cd08a539b7c845ddb92c5d08752101d1', + 'port_id': fake_port_id, + 'id': fake_fip_id + } + ] + } + mock_nclient.delete_floatingip.side_effect = Exception + + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.neutron.return_value = mock_nclient + + self.assertRaises( + exception.PreDeletionFailed, + neutron.delete_floatingip, + self.context, + fake_port_id, + self.cluster + ) diff --git a/magnum/tests/unit/common/test_octavia.py b/magnum/tests/unit/common/test_octavia.py index 8ba3c84fed..901251707f 100644 --- a/magnum/tests/unit/common/test_octavia.py +++ b/magnum/tests/unit/common/test_octavia.py @@ -28,8 +28,9 @@ class OctaviaTest(base.TestCase): cluster_dict = utils.get_test_cluster(node_count=1) self.cluster = objects.Cluster(self.context, **cluster_dict) + @mock.patch("magnum.common.neutron.delete_floatingip") @mock.patch('magnum.common.clients.OpenStackClients') - def test_delete_loadbalancers(self, mock_clients): + def test_delete_loadbalancers(self, mock_clients, mock_delete_fip): mock_lbs = { "loadbalancers": [ { @@ -38,7 +39,8 @@ class OctaviaTest(base.TestCase): "ad3080723f1c211e88adbfa163ee1203 from " "cluster %s" % self.cluster.uuid, "name": "fake_name_1", - "provisioning_status": "ACTIVE" + "provisioning_status": "ACTIVE", + "vip_port_id": "b4ca07d1-a31e-43e2-891a-7d14f419f342" }, { "id": "fake_id_2", @@ -46,7 +48,8 @@ class OctaviaTest(base.TestCase): "a9f9ba08cf28811e89547fa163ea824f from " "cluster %s" % self.cluster.uuid, "name": "fake_name_2", - "provisioning_status": "ERROR" + "provisioning_status": "ERROR", + "vip_port_id": "c17c1a6e-1868-11e9-84cd-00224d6b7bc1" }, ] } @@ -67,7 +70,24 @@ class OctaviaTest(base.TestCase): mock_octavie_client.load_balancer_delete.assert_has_calls(calls) @mock.patch('magnum.common.clients.OpenStackClients') - def test_delete_loadbalancers_with_invalid_lb(self, mock_clients): + def test_delete_loadbalancers_no_candidate(self, mock_clients): + mock_lbs = { + "loadbalancers": [] + } + mock_octavie_client = mock.MagicMock() + mock_octavie_client.load_balancer_list.return_value = mock_lbs + osc = mock.MagicMock() + mock_clients.return_value = osc + osc.octavia.return_value = mock_octavie_client + + octavia.delete_loadbalancers(self.context, self.cluster) + + self.assertFalse(mock_octavie_client.load_balancer_delete.called) + + @mock.patch("magnum.common.neutron.delete_floatingip") + @mock.patch('magnum.common.clients.OpenStackClients') + def test_delete_loadbalancers_with_invalid_lb(self, mock_clients, + mock_delete_fip): osc = mock.MagicMock() mock_clients.return_value = osc mock_octavie_client = mock.MagicMock() @@ -81,7 +101,8 @@ class OctaviaTest(base.TestCase): "ad3080723f1c211e88adbfa163ee1203 from " "cluster %s" % self.cluster.uuid, "name": "fake_name_1", - "provisioning_status": "ACTIVE" + "provisioning_status": "ACTIVE", + "vip_port_id": "c17c1a6e-1868-11e9-84cd-00224d6b7bc1" }, { "id": "fake_id_2", @@ -89,7 +110,8 @@ class OctaviaTest(base.TestCase): "a9f9ba08cf28811e89547fa163ea824f from " "cluster %s" % self.cluster.uuid, "name": "fake_name_2", - "provisioning_status": "PENDING_UPDATE" + "provisioning_status": "PENDING_UPDATE", + "vip_port_id": "b4ca07d1-a31e-43e2-891a-7d14f419f342" }, ] } @@ -104,8 +126,9 @@ class OctaviaTest(base.TestCase): mock_octavie_client.load_balancer_delete.assert_called_once_with( "fake_id_1", cascade=True) + @mock.patch("magnum.common.neutron.delete_floatingip") @mock.patch('magnum.common.clients.OpenStackClients') - def test_delete_loadbalancers_timeout(self, mock_clients): + def test_delete_loadbalancers_timeout(self, mock_clients, mock_delete_fip): osc = mock.MagicMock() mock_clients.return_value = osc mock_octavie_client = mock.MagicMock() @@ -119,7 +142,8 @@ class OctaviaTest(base.TestCase): "ad3080723f1c211e88adbfa163ee1203 from " "cluster %s" % self.cluster.uuid, "name": "fake_name_1", - "provisioning_status": "ACTIVE" + "provisioning_status": "ACTIVE", + "vip_port_id": "b4ca07d1-a31e-43e2-891a-7d14f419f342" }, { "id": "fake_id_2", @@ -127,7 +151,8 @@ class OctaviaTest(base.TestCase): "a9f9ba08cf28811e89547fa163ea824f from " "cluster %s" % self.cluster.uuid, "name": "fake_name_2", - "provisioning_status": "ACTIVE" + "provisioning_status": "ACTIVE", + "vip_port_id": "c17c1a6e-1868-11e9-84cd-00224d6b7bc1" }, ] } diff --git a/releasenotes/notes/k8s-delete-vip-fip-b2ddf61ddbc080bc.yaml b/releasenotes/notes/k8s-delete-vip-fip-b2ddf61ddbc080bc.yaml new file mode 100644 index 0000000000..be863815ec --- /dev/null +++ b/releasenotes/notes/k8s-delete-vip-fip-b2ddf61ddbc080bc.yaml @@ -0,0 +1,6 @@ +fixes: + - | + In kubernetes cluster, a floating IP is created and associated with the vip + of a load balancer which is created corresponding to the service of + LoadBalancer type inside kubernetes, it should be deleted when the cluster + is deleted.