From bfa1acf27f54d72e0ad2f3947feb237e764fd83d Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Thu, 18 Jul 2019 16:11:54 +0200 Subject: [PATCH] Ensure namespace network resources are cleaned up In case of kuryr-controller crash/restart while creating a namespace it may happen that some network resources are leftover as the kuryr net crd was not yet created. This may lead to a waste of OpenStack network resources (networks, subnets). This patch ensures the namespace is cleaned up before creating the resources in case a previous attempt was made without completing the kuryr net crd creation. Change-Id: Iddc03090dc8a847abe4afa2bed0900f0c7cf6936 --- kuryr_kubernetes/controller/drivers/base.py | 11 ++ .../controller/drivers/namespace_subnet.py | 67 +++++++--- kuryr_kubernetes/controller/drivers/utils.py | 6 + .../controller/drivers/vif_pool.py | 17 +-- .../controller/handlers/namespace.py | 6 + .../drivers/test_namespace_subnet.py | 25 +--- .../unit/controller/drivers/test_vif_pool.py | 121 +++++++++++------- .../controller/handlers/test_namespace.py | 17 +++ 8 files changed, 176 insertions(+), 94 deletions(-) diff --git a/kuryr_kubernetes/controller/drivers/base.py b/kuryr_kubernetes/controller/drivers/base.py index 127e5f3b7..5354c7897 100644 --- a/kuryr_kubernetes/controller/drivers/base.py +++ b/kuryr_kubernetes/controller/drivers/base.py @@ -190,6 +190,17 @@ class PodSubnetsDriver(DriverBase): """ raise NotImplementedError() + def cleanup_namespace_networks(self, namespace): + """Clean up network leftover on the namespace. + + Due to Kuryr controller restarts it may happen that some network + resources are leftover. This method ensures they are deleted upon + retries. + + :param namespace: name of the Kubernetes namespace object + """ + raise NotImplementedError() + @six.add_metaclass(abc.ABCMeta) class ServiceSubnetsDriver(DriverBase): diff --git a/kuryr_kubernetes/controller/drivers/namespace_subnet.py b/kuryr_kubernetes/controller/drivers/namespace_subnet.py index 9b1c7d8de..392fc6e09 100644 --- a/kuryr_kubernetes/controller/drivers/namespace_subnet.py +++ b/kuryr_kubernetes/controller/drivers/namespace_subnet.py @@ -78,31 +78,47 @@ class NamespacePodSubnetDriver(default_subnet.DefaultPodSubnetDriver): return net_crd['spec']['subnetId'] def delete_namespace_subnet(self, net_crd): - neutron = clients.get_neutron_client() - - router_id = oslo_cfg.CONF.namespace_subnet.pod_router subnet_id = net_crd['spec']['subnetId'] net_id = net_crd['spec']['netId'] - try: - neutron.remove_interface_router(router_id, - {"subnet_id": subnet_id}) - except n_exc.NotFound: - LOG.debug("Subnet %(subnet)s not attached to router %(router)s", - {'subnet': subnet_id, 'router': router_id}) - except n_exc.NeutronClientException: - LOG.exception("Error deleting subnet %(subnet)s from router " - "%(router)s.", {'subnet': subnet_id, 'router': - router_id}) - raise + self._delete_namespace_network_resources(subnet_id, net_id) + + def _delete_namespace_network_resources(self, subnet_id, net_id): + neutron = clients.get_neutron_client() + if subnet_id: + router_id = oslo_cfg.CONF.namespace_subnet.pod_router + try: + neutron.remove_interface_router(router_id, + {"subnet_id": subnet_id}) + except n_exc.NotFound: + LOG.debug("Subnet %(subnet)s not attached to router " + "%(router)s", {'subnet': subnet_id, + 'router': router_id}) + except n_exc.NeutronClientException: + LOG.exception("Error deleting subnet %(subnet)s from router " + "%(router)s.", {'subnet': subnet_id, 'router': + router_id}) + raise try: neutron.delete_network(net_id) except n_exc.NotFound: LOG.debug("Neutron Network not found: %s", net_id) except n_exc.NetworkInUseClient: - LOG.exception("One or more ports in use on the network %s.", - net_id) + LOG.exception("One or more ports in use on the network %s. " + "Deleting leftovers ports before retrying", net_id) + leftover_ports = c_utils.get_ports_by_attrs(status='DOWN', + network_id=net_id) + for leftover_port in leftover_ports: + try: + neutron.delete_port(leftover_port['id']) + except n_exc.PortNotFoundClient: + LOG.debug("Port already deleted.") + except n_exc.NeutronClientException: + LOG.debug("Unexpected error deleting leftover port %s. " + "Skiping it and continue with the other rest.", + leftover_port['id']) + continue raise exceptions.ResourceNotReady(net_id) except n_exc.NeutronClientException: LOG.exception("Error deleting network %s.", net_id) @@ -165,3 +181,22 @@ class NamespacePodSubnetDriver(default_subnet.DefaultPodSubnetDriver): "%(net_id)s, created for the namespace: " "%(namespace)s." % {'net_id': net_crd_spec['netId'], 'namespace': namespace}) + + def cleanup_namespace_networks(self, namespace): + neutron = clients.get_neutron_client() + net_name = 'ns/' + namespace + '-net' + filters = {'name': net_name} + tags = oslo_cfg.CONF.neutron_defaults.resource_tags + if tags: + filters['tags'] = tags + networks = neutron.list_networks(**filters)['networks'] + if networks: + for net in networks: + net_id = net['id'] + subnets = net.get('subnets') + subnet_id = None + if subnets: + # NOTE(ltomasbo): Each network created by kuryr only has + # one subnet + subnet_id = subnets[0] + self._delete_namespace_network_resources(subnet_id, net_id) diff --git a/kuryr_kubernetes/controller/drivers/utils.py b/kuryr_kubernetes/controller/drivers/utils.py index db3387f40..269750006 100644 --- a/kuryr_kubernetes/controller/drivers/utils.py +++ b/kuryr_kubernetes/controller/drivers/utils.py @@ -534,3 +534,9 @@ def get_port_annot_pci_info(nodename, neutron_port): LOG.exception('Exception when reading annotations ' '%s and converting from json', annot_name) return pci_info + + +def get_ports_by_attrs(**attrs): + neutron = clients.get_neutron_client() + ports = neutron.list_ports(**attrs) + return ports['ports'] diff --git a/kuryr_kubernetes/controller/drivers/vif_pool.py b/kuryr_kubernetes/controller/drivers/vif_pool.py index c5e1be069..d13e3cdb2 100644 --- a/kuryr_kubernetes/controller/drivers/vif_pool.py +++ b/kuryr_kubernetes/controller/drivers/vif_pool.py @@ -261,11 +261,6 @@ class BaseVIFPool(base.VIFPoolDriver): def _recover_precreated_ports(self): raise NotImplementedError() - def _get_ports_by_attrs(self, **attrs): - neutron = clients.get_neutron_client() - ports = neutron.list_ports(**attrs) - return ports['ports'] - def _get_in_use_ports(self): kubernetes = clients.get_kubernetes_client() in_use_ports = [] @@ -353,7 +348,7 @@ class BaseVIFPool(base.VIFPoolDriver): tags = config.CONF.neutron_defaults.resource_tags if tags: attrs['tags'] = tags - all_active_ports = self._get_ports_by_attrs(**attrs) + all_active_ports = c_utils.get_ports_by_attrs(**attrs) in_use_ports = self._get_in_use_ports() for port in all_active_ports: @@ -383,7 +378,7 @@ class BaseVIFPool(base.VIFPoolDriver): def _cleanup_leftover_ports(self): neutron = clients.get_neutron_client() attrs = {'device_owner': kl_const.DEVICE_OWNER, 'status': 'DOWN'} - existing_ports = self._get_ports_by_attrs(**attrs) + existing_ports = c_utils.get_ports_by_attrs(**attrs) tags = config.CONF.neutron_defaults.resource_tags if tags: @@ -510,7 +505,7 @@ class NeutronVIFPool(BaseVIFPool): tags = config.CONF.neutron_defaults.resource_tags if tags: attrs['tags'] = tags - kuryr_ports = self._get_ports_by_attrs(**attrs) + kuryr_ports = c_utils.get_ports_by_attrs(**attrs) for port in kuryr_ports: if port['id'] in self._recyclable_ports: sg_current[port['id']] = tuple(sorted( @@ -563,9 +558,9 @@ class NeutronVIFPool(BaseVIFPool): if config.CONF.kubernetes.port_debug: attrs['name'] = constants.KURYR_PORT_NAME - available_ports = self._get_ports_by_attrs(**attrs) + available_ports = c_utils.get_ports_by_attrs(**attrs) else: - kuryr_ports = self._get_ports_by_attrs(**attrs) + kuryr_ports = c_utils.get_ports_by_attrs(**attrs) in_use_ports = self._get_in_use_ports() available_ports = [port for port in kuryr_ports if port['id'] not in in_use_ports] @@ -737,7 +732,7 @@ class NestedVIFPool(BaseVIFPool): tags = config.CONF.neutron_defaults.resource_tags if tags: attrs['tags'] = tags - kuryr_subports = self._get_ports_by_attrs(**attrs) + kuryr_subports = c_utils.get_ports_by_attrs(**attrs) for subport in kuryr_subports: if subport['id'] in self._recyclable_ports: sg_current[subport['id']] = tuple(sorted( diff --git a/kuryr_kubernetes/controller/handlers/namespace.py b/kuryr_kubernetes/controller/handlers/namespace.py index 54d623a42..74593a7c4 100644 --- a/kuryr_kubernetes/controller/handlers/namespace.py +++ b/kuryr_kubernetes/controller/handlers/namespace.py @@ -84,6 +84,12 @@ class NamespaceHandler(k8s_base.ResourceEventHandler): self.on_deleted(namespace, net_crd) raise exceptions.ResourceNotReady(namespace) + # NOTE(ltomasbo): Ensure there is no previously created networks + # leftovers due to a kuryr-controller crash/restart + LOG.debug("Deleting leftovers network resources for namespace: %s", + ns_name) + self._drv_subnets.cleanup_namespace_networks(ns_name) + LOG.debug("Creating network resources for namespace: %s", ns_name) net_crd_spec = self._drv_subnets.create_namespace_network(ns_name, project_id) diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_subnet.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_subnet.py index be7a0123a..4b92c8558 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_subnet.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_subnet.py @@ -177,40 +177,27 @@ class TestNamespacePodSubnetDriver(test_base.TestCase): net_id = mock.sentinel.net_id subnet_id = mock.sentinel.subnet_id - sg_id = mock.sentinel.sg_id - crd = { - 'spec': { - 'subnetId': subnet_id, - 'netId': net_id, - 'sgId': sg_id - } - } neutron = self.useFixture(k_fix.MockNeutronClient()).client - cls.delete_namespace_subnet(m_driver, crd) + cls._delete_namespace_network_resources(m_driver, subnet_id, net_id) neutron.remove_interface_router.assert_called_once() neutron.delete_network.assert_called_once_with(net_id) - def test_delete_namespace_subnet_neutron_exception(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_delete_namespace_subnet_neutron_exception(self, m_get_ports): cls = subnet_drv.NamespacePodSubnetDriver m_driver = mock.MagicMock(spec=cls) net_id = mock.sentinel.net_id subnet_id = mock.sentinel.subnet_id - sg_id = mock.sentinel.sg_id - crd = { - 'spec': { - 'subnetId': subnet_id, - 'netId': net_id, - 'sgId': sg_id - } - } neutron = self.useFixture(k_fix.MockNeutronClient()).client neutron.delete_network.side_effect = n_exc.NetworkInUseClient + m_get_ports.return_value = [] self.assertRaises(k_exc.ResourceNotReady, - cls.delete_namespace_subnet, m_driver, crd) + cls._delete_namespace_network_resources, m_driver, + subnet_id, net_id) neutron.remove_interface_router.assert_called_once() neutron.delete_network.assert_called_once_with(net_id) diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_vif_pool.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_vif_pool.py index f5b4da3df..32d430787 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_vif_pool.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_vif_pool.py @@ -326,7 +326,8 @@ class BaseVIFPool(test_base.TestCase): self.assertEqual(resp, []) - def test_cleanup_leftover_ports(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_cleanup_leftover_ports(self, m_get_ports): cls = vif_pool.BaseVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -336,7 +337,7 @@ class BaseVIFPool(test_base.TestCase): net_id = port['network_id'] tags = 'clusterTest' port['tags'] = [tags] - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] oslo_cfg.CONF.set_override('resource_tags', tags, group='neutron_defaults') @@ -346,7 +347,8 @@ class BaseVIFPool(test_base.TestCase): neutron.list_networks.assert_called() neutron.delete_port.assert_not_called() - def test_cleanup_leftover_ports_different_network(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_cleanup_leftover_ports_different_network(self, m_get_ports): cls = vif_pool.BaseVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -355,7 +357,7 @@ class BaseVIFPool(test_base.TestCase): port = get_port_obj(port_id=port_id) tags = 'clusterTest' port['tags'] = [tags] - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] oslo_cfg.CONF.set_override('resource_tags', tags, group='neutron_defaults') @@ -365,7 +367,8 @@ class BaseVIFPool(test_base.TestCase): neutron.list_networks.assert_called() neutron.delete_port.assert_not_called() - def test_cleanup_leftover_ports_no_binding(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_cleanup_leftover_ports_no_binding(self, m_get_ports): cls = vif_pool.BaseVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -376,7 +379,7 @@ class BaseVIFPool(test_base.TestCase): tags = 'clusterTest' port['tags'] = [tags] port['binding:host_id'] = None - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] oslo_cfg.CONF.set_override('resource_tags', tags, group='neutron_defaults') @@ -386,7 +389,8 @@ class BaseVIFPool(test_base.TestCase): neutron.list_networks.assert_called() neutron.delete_port.assert_called_once_with(port['id']) - def test_cleanup_leftover_ports_no_tags(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_cleanup_leftover_ports_no_tags(self, m_get_ports): cls = vif_pool.BaseVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -395,7 +399,7 @@ class BaseVIFPool(test_base.TestCase): port = get_port_obj(port_id=port_id) net_id = port['network_id'] tags = 'clusterTest' - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] oslo_cfg.CONF.set_override('resource_tags', tags, group='neutron_defaults') @@ -405,14 +409,15 @@ class BaseVIFPool(test_base.TestCase): neutron.list_networks.assert_called() neutron.delete_port.assert_called_once_with(port['id']) - def test_cleanup_leftover_ports_no_tagging(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_cleanup_leftover_ports_no_tagging(self, m_get_ports): cls = vif_pool.BaseVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client port_id = str(uuid.uuid4()) port = get_port_obj(port_id=port_id) - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] oslo_cfg.CONF.set_override('resource_tags', [], group='neutron_defaults') @@ -421,7 +426,8 @@ class BaseVIFPool(test_base.TestCase): neutron.list_networks.assert_not_called() neutron.delete_port.assert_not_called() - def test_cleanup_leftover_ports_no_tagging_no_binding(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test_cleanup_leftover_ports_no_tagging_no_binding(self, m_get_ports): cls = vif_pool.BaseVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -429,7 +435,7 @@ class BaseVIFPool(test_base.TestCase): port_id = str(uuid.uuid4()) port = get_port_obj(port_id=port_id) port['binding:host_id'] = None - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] oslo_cfg.CONF.set_override('resource_tags', [], group='neutron_defaults') @@ -659,8 +665,9 @@ class NeutronVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @ddt.data((0), (10)) - def test__trigger_return_to_pool(self, max_pool): + def test__trigger_return_to_pool(self, max_pool, m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -677,7 +684,7 @@ class NeutronVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('port_debug', True, group='kubernetes') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length @@ -693,8 +700,9 @@ class NeutronVIFPool(test_base.TestCase): }) neutron.delete_port.assert_not_called() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @ddt.data((0), (10)) - def test__trigger_return_to_pool_no_update(self, max_pool): + def test__trigger_return_to_pool_no_update(self, max_pool, m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -711,7 +719,7 @@ class NeutronVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('port_debug', False, group='kubernetes') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group']}] m_driver._get_pool_size.return_value = pool_length @@ -720,7 +728,8 @@ class NeutronVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() neutron.delete_port.assert_not_called() - def test__trigger_return_to_pool_delete_port(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_delete_port(self, m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -736,7 +745,7 @@ class NeutronVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('ports_pool_max', 10, group='vif_pool') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length @@ -745,7 +754,8 @@ class NeutronVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() neutron.delete_port.assert_called_once_with(port_id) - def test__trigger_return_to_pool_update_exception(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_update_exception(self, m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -765,7 +775,7 @@ class NeutronVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('port_debug', True, group='kubernetes') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length neutron.update_port.side_effect = n_exc.NeutronClientException @@ -782,7 +792,8 @@ class NeutronVIFPool(test_base.TestCase): }) neutron.delete_port.assert_not_called() - def test__trigger_return_to_pool_delete_exception(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_delete_exception(self, m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -798,7 +809,7 @@ class NeutronVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('ports_pool_max', 5, group='vif_pool') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length neutron.delete_port.side_effect = n_exc.PortNotFoundClient @@ -808,7 +819,8 @@ class NeutronVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() neutron.delete_port.assert_called_once_with(port_id) - def test__trigger_return_to_pool_delete_key_error(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_delete_key_error(self, m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -823,7 +835,7 @@ class NeutronVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('ports_pool_max', 5, group='vif_pool') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length @@ -832,9 +844,11 @@ class NeutronVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() neutron.delete_port.assert_not_called() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @mock.patch('kuryr_kubernetes.os_vif_util.neutron_to_osvif_vif') @mock.patch('kuryr_kubernetes.utils.get_subnet') - def test__recover_precreated_ports(self, m_get_subnet, m_to_osvif): + def test__recover_precreated_ports(self, m_get_subnet, m_to_osvif, + m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) @@ -848,7 +862,7 @@ class NeutronVIFPool(test_base.TestCase): port_id = str(uuid.uuid4()) port = get_port_obj(port_id=port_id) filtered_ports = [port] - m_driver._get_ports_by_attrs.return_value = filtered_ports + m_get_ports.return_value = filtered_ports vif_plugin = mock.sentinel.plugin m_driver._drv_vif._get_vif_plugin.return_value = vif_plugin @@ -870,7 +884,7 @@ class NeutronVIFPool(test_base.TestCase): cls._recover_precreated_ports(m_driver) - m_driver._get_ports_by_attrs.assert_called_once() + m_get_ports.assert_called_once() m_get_subnet.assert_called_with(subnet_id) m_driver._drv_vif._get_vif_plugin.assert_called_once_with(port) m_to_osvif.assert_called_once_with(vif_plugin, port, subnet) @@ -879,14 +893,16 @@ class NeutronVIFPool(test_base.TestCase): self.assertEqual(m_driver._available_ports_pools[pool_key], {tuple(port['security_groups']): [port_id]}) + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @mock.patch('kuryr_kubernetes.os_vif_util.neutron_to_osvif_vif') @mock.patch('kuryr_kubernetes.utils.get_subnet') - def test__recover_precreated_ports_empty(self, m_get_subnet, m_to_osvif): + def test__recover_precreated_ports_empty(self, m_get_subnet, m_to_osvif, + m_get_ports): cls = vif_pool.NeutronVIFPool m_driver = mock.MagicMock(spec=cls) filtered_ports = [] - m_driver._get_ports_by_attrs.return_value = filtered_ports + m_get_ports.return_value = filtered_ports m_driver._get_trunks_info.return_value = ({}, {}, {}) oslo_cfg.CONF.set_override('port_debug', @@ -895,7 +911,7 @@ class NeutronVIFPool(test_base.TestCase): cls._recover_precreated_ports(m_driver) - m_driver._get_ports_by_attrs.assert_called_once() + m_get_ports.assert_called_once() m_get_subnet.assert_not_called() m_to_osvif.assert_not_called() @@ -1193,8 +1209,9 @@ class NestedVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @ddt.data((0), (10)) - def test__trigger_return_to_pool(self, max_pool): + def test__trigger_return_to_pool(self, max_pool, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -1211,7 +1228,7 @@ class NestedVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('port_debug', True, group='kubernetes') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length @@ -1226,8 +1243,9 @@ class NestedVIFPool(test_base.TestCase): }) neutron.delete_port.assert_not_called() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @ddt.data((0), (10)) - def test__trigger_return_to_pool_no_update(self, max_pool): + def test__trigger_return_to_pool_no_update(self, max_pool, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -1244,7 +1262,7 @@ class NestedVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('port_debug', False, group='kubernetes') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group']}] m_driver._get_pool_size.return_value = pool_length @@ -1253,7 +1271,8 @@ class NestedVIFPool(test_base.TestCase): neutron.update_port.assert_not_called() neutron.delete_port.assert_not_called() - def test__trigger_return_to_pool_delete_port(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_delete_port(self, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -1274,7 +1293,7 @@ class NestedVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('ports_pool_max', 10, group='vif_pool') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length m_driver._get_trunk_id.return_value = trunk_id @@ -1289,7 +1308,8 @@ class NestedVIFPool(test_base.TestCase): trunk_id, port_id) - def test__trigger_return_to_pool_update_exception(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_update_exception(self, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -1306,7 +1326,7 @@ class NestedVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('port_debug', True, group='kubernetes') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length neutron.update_port.side_effect = n_exc.NeutronClientException @@ -1322,7 +1342,8 @@ class NestedVIFPool(test_base.TestCase): }) neutron.delete_port.assert_not_called() - def test__trigger_return_to_pool_delete_exception(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_delete_exception(self, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -1343,7 +1364,7 @@ class NestedVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('ports_pool_max', 5, group='vif_pool') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length m_driver._get_trunk_id.return_value = trunk_id @@ -1359,7 +1380,8 @@ class NestedVIFPool(test_base.TestCase): port_id) neutron.delete_port.assert_called_once_with(port_id) - def test__trigger_return_to_pool_delete_key_error(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__trigger_return_to_pool_delete_key_error(self, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) neutron = self.useFixture(k_fix.MockNeutronClient()).client @@ -1378,7 +1400,7 @@ class NestedVIFPool(test_base.TestCase): oslo_cfg.CONF.set_override('ports_pool_max', 5, group='vif_pool') - m_driver._get_ports_by_attrs.return_value = [ + m_get_ports.return_value = [ {'id': port_id, 'security_groups': ['security_group_modified']}] m_driver._get_pool_size.return_value = pool_length m_driver._known_trunk_ids = {} @@ -1407,8 +1429,9 @@ class NestedVIFPool(test_base.TestCase): self.assertEqual(ip_address, cls._get_parent_port_ip(m_driver, port_id)) + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') @mock.patch('kuryr_kubernetes.utils.get_subnet') - def test__get_trunk_info(self, m_get_subnet): + def test__get_trunk_info(self, m_get_subnet, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) @@ -1426,7 +1449,7 @@ class NestedVIFPool(test_base.TestCase): subport_id = str(uuid.uuid4()) subport = get_port_obj(port_id=subport_id, device_owner='trunk:subport') - m_driver._get_ports_by_attrs.return_value = [trunk_port, subport] + m_get_ports.return_value = [trunk_port, subport] m_driver._get_in_use_ports.return_value = [] subnet = mock.sentinel.subnet m_get_subnet.return_value = subnet @@ -1443,11 +1466,12 @@ class NestedVIFPool(test_base.TestCase): self.assertEqual(r_subports, {subport_id: subport}) self.assertEqual(r_subnets, exp_subnets) - def test__get_trunk_info_empty(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__get_trunk_info_empty(self, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) - m_driver._get_ports_by_attrs.return_value = [] + m_get_ports.return_value = [] m_driver._get_in_use_ports.return_value = [] r_p_ports, r_subports, r_subnets = cls._get_trunks_info(m_driver) @@ -1456,14 +1480,15 @@ class NestedVIFPool(test_base.TestCase): self.assertEqual(r_subports, {}) self.assertEqual(r_subnets, {}) - def test__get_trunk_info_no_trunk_details(self): + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports_by_attrs') + def test__get_trunk_info_no_trunk_details(self, m_get_ports): cls = vif_pool.NestedVIFPool m_driver = mock.MagicMock(spec=cls) port_id = str(uuid.uuid4()) port = get_port_obj(port_id=port_id) port = get_port_obj(port_id=port_id, device_owner='compute:nova') - m_driver._get_ports_by_attrs.return_value = [port] + m_get_ports.return_value = [port] m_driver._get_in_use_ports.return_value = [] r_p_ports, r_subports, r_subnets = cls._get_trunks_info(m_driver) diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_namespace.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_namespace.py index 32a11cd26..7f2c4ab86 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_namespace.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_namespace.py @@ -64,6 +64,8 @@ class TestNamespaceHandler(test_base.TestCase): self._handler._drv_sg.create_namespace_sg) self._delete_sg = ( self._handler._drv_sg.delete_sg) + self._cleanup_namespace_networks = ( + self._handler._drv_subnets.cleanup_namespace_networks) self._get_net_crd = self._handler._get_net_crd self._set_net_crd = self._handler._set_net_crd self._get_net_crd_id = self._handler._get_net_crd_id @@ -128,6 +130,8 @@ class TestNamespaceHandler(test_base.TestCase): self._get_net_crd_id.assert_called_once_with(self._namespace) self._get_net_crd.assert_called_once_with(self._crd_id) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) self._create_namespace_network.assert_called_once_with( self._namespace_name, self._project_id) self._create_namespace_sg.assert_called_once_with( @@ -144,6 +148,7 @@ class TestNamespaceHandler(test_base.TestCase): namespace.NamespaceHandler.on_present(self._handler, self._namespace) self._get_net_crd_id.assert_called_once_with(self._namespace) + self._cleanup_namespace_networks.assert_not_called() self._create_namespace_network.assert_not_called() def test_on_present_create_network_exception(self): @@ -158,6 +163,8 @@ class TestNamespaceHandler(test_base.TestCase): self._get_net_crd_id.assert_called_once_with(self._namespace) self._get_net_crd.assert_called_once_with(self._crd_id) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) self._create_namespace_network.assert_called_once_with( self._namespace_name, self._project_id) self._create_namespace_sg.assert_not_called() @@ -176,6 +183,8 @@ class TestNamespaceHandler(test_base.TestCase): self._get_net_crd_id.assert_called_once_with(self._namespace) self._get_net_crd.assert_called_once_with(self._crd_id) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) self._create_namespace_network.assert_called_once_with( self._namespace_name, self._project_id) self._create_namespace_sg.assert_called_once_with( @@ -194,6 +203,8 @@ class TestNamespaceHandler(test_base.TestCase): self._get_net_crd_id.assert_called_once_with(self._namespace) self._get_net_crd.assert_called_once_with(self._crd_id) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) self._create_namespace_network.assert_called_once_with( self._namespace_name, self._project_id) self._create_namespace_sg.assert_called_once_with( @@ -218,6 +229,10 @@ class TestNamespaceHandler(test_base.TestCase): self._get_net_crd_id.assert_called_once_with(self._namespace) self._get_net_crd.assert_called_once_with(self._crd_id) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) self._create_namespace_network.assert_called_once_with( self._namespace_name, self._project_id) self._create_namespace_sg.assert_called_once_with( @@ -246,6 +261,8 @@ class TestNamespaceHandler(test_base.TestCase): self._get_net_crd_id.assert_called_once_with(self._namespace) self._get_net_crd.assert_called_once_with(self._crd_id) + self._cleanup_namespace_networks.assert_called_once_with( + self._namespace_name) self._create_namespace_network.assert_called_once_with( self._namespace_name, self._project_id) self._create_namespace_sg.assert_called_once_with(