From 6c0730fda48d405522ccddf89c51b32ae81b876c Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Fri, 15 Feb 2019 10:24:42 +0100 Subject: [PATCH] Skip exception in case kuryrnetpolicy CRD is already deleted As kuryrnetpolicy CRD objects are namespaced, when a namespace is deleted, the object is deleted by kubernetes as part of the namespace deletion process. This was making network policy driver failing on releasing the network policy as it could not find the object. This patch ensures kuryr-controller doesn't fail in case kubernetes has already deleted the kuryrnetpolicy object by skipping the exception when trying to delete an already deleted object. Closes-Bug: 1816020 Change-Id: I0443b65e5d6897c5d6673c222fc50101c244cd1e --- .zuul.d/octavia.yaml | 2 +- doc/source/installation/network_policy.rst | 2 +- .../controller/drivers/network_policy.py | 33 ++++++++------ .../controller/handlers/kuryrnetpolicy.py | 37 ++++++++++++++++ .../controller/handlers/policy.py | 44 ++++++++++--------- .../controller/drivers/test_network_policy.py | 16 +++++++ setup.cfg | 1 + 7 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 kuryr_kubernetes/controller/handlers/kuryrnetpolicy.py diff --git a/.zuul.d/octavia.yaml b/.zuul.d/octavia.yaml index 691f3acd2..be726f47b 100644 --- a/.zuul.d/octavia.yaml +++ b/.zuul.d/octavia.yaml @@ -123,7 +123,7 @@ vars: tempest_test_regex: '^(kuryr_tempest_plugin.tests.scenario.test_network_policy.TestNetworkPolicyScenario)' devstack_localrc: - KURYR_ENABLED_HANDLERS: vif,lb,lbaasspec,namespace,pod_label,policy + KURYR_ENABLED_HANDLERS: vif,lb,lbaasspec,namespace,pod_label,policy,kuryrnetpolicy KURYR_SG_DRIVER: policy KURYR_SUBNET_DRIVER: namespace voting: false diff --git a/doc/source/installation/network_policy.rst b/doc/source/installation/network_policy.rst index ae5257d23..8c2405f2c 100644 --- a/doc/source/installation/network_policy.rst +++ b/doc/source/installation/network_policy.rst @@ -7,7 +7,7 @@ handlers at kuryr.conf (further info on how to do this can be found at :doc:`./devstack/containerized`):: [kubernetes] - enabled_handlers=vif,lb,lbaasspec,policy,pod_label,namespace + enabled_handlers=vif,lb,lbaasspec,policy,pod_label,namespace,kuryrnetpolicy After that, enable also the security group drivers for policies:: diff --git a/kuryr_kubernetes/controller/drivers/network_policy.py b/kuryr_kubernetes/controller/drivers/network_policy.py index 32f82dfc9..c332cf4b8 100644 --- a/kuryr_kubernetes/controller/drivers/network_policy.py +++ b/kuryr_kubernetes/controller/drivers/network_policy.py @@ -370,22 +370,24 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): return ingress_sg_rule_body_list, egress_sg_rule_body_list + def delete_np_sg(self, sg_id): + try: + self.neutron.delete_security_group(sg_id) + except n_exc.NotFound: + LOG.debug("Security Group not found: %s", sg_id) + except n_exc.Conflict: + LOG.debug("Security Group already in use: %s", sg_id) + # raising ResourceNotReady to retry this action in case ports + # associated to affected pods are not updated on time, i.e., + # they are still using the security group to be removed + raise exceptions.ResourceNotReady(sg_id) + except n_exc.NeutronClientException: + LOG.exception("Error deleting security group %s.", sg_id) + raise + def release_network_policy(self, netpolicy_crd): if netpolicy_crd is not None: - try: - sg_id = netpolicy_crd['spec']['securityGroupId'] - self.neutron.delete_security_group(sg_id) - except n_exc.NotFound: - LOG.debug("Security Group not found: %s", sg_id) - except n_exc.Conflict: - LOG.debug("Security Group already in use: %s", sg_id) - # raising ResourceNotReady to retry this action in case ports - # associated to affected pods are not updated on time, i.e., - # they are still using the security group to be removed - raise exceptions.ResourceNotReady(sg_id) - except n_exc.NeutronClientException: - LOG.exception("Error deleting security group %s.", sg_id) - raise + self.delete_np_sg(netpolicy_crd['spec']['securityGroupId']) self._del_kuryrnetpolicy_crd( netpolicy_crd['metadata']['name'], netpolicy_crd['metadata']['namespace']) @@ -470,6 +472,9 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): LOG.exception("Kubernetes Client Exception deleting kuryrnetpolicy" " CRD.") raise + except n_exc.NotFound: + LOG.debug("KuryrNetPolicy CRD Object not found: %s", + netpolicy_crd_name) def affected_pods(self, policy, selector=None): if selector: diff --git a/kuryr_kubernetes/controller/handlers/kuryrnetpolicy.py b/kuryr_kubernetes/controller/handlers/kuryrnetpolicy.py new file mode 100644 index 000000000..8c30f7c18 --- /dev/null +++ b/kuryr_kubernetes/controller/handlers/kuryrnetpolicy.py @@ -0,0 +1,37 @@ +# Copyright 2019 Red Hat, Inc. +# +# 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. + +from kuryr_kubernetes import constants +from kuryr_kubernetes.controller.drivers import base as drivers +from kuryr_kubernetes.handlers import k8s_base + + +class KuryrNetPolicyHandler(k8s_base.ResourceEventHandler): + """Controller side of KuryrNetPolicy process for Kubernetes pods. + + `KuryrNetPolicyHandler` runs on the Kuryr-Kubernetes controller and is + responsible for deleting associated security groups upon namespace + deletion. + """ + OBJECT_KIND = constants.K8S_OBJ_KURYRNETPOLICY + OBJECT_WATCH_PATH = constants.K8S_API_CRD_KURYRNETPOLICIES + + def __init__(self): + super(KuryrNetPolicyHandler, self).__init__() + self._drv_policy = drivers.NetworkPolicyDriver.get_instance() + + def on_deleted(self, netpolicy_crd): + crd_sg = netpolicy_crd['spec'].get('securityGroupId') + if crd_sg: + self._drv_policy.delete_np_sg(crd_sg) diff --git a/kuryr_kubernetes/controller/handlers/policy.py b/kuryr_kubernetes/controller/handlers/policy.py index a0a994fd0..2e97782fc 100644 --- a/kuryr_kubernetes/controller/handlers/policy.py +++ b/kuryr_kubernetes/controller/handlers/policy.py @@ -96,29 +96,33 @@ class NetworkPolicyHandler(k8s_base.ResourceEventHandler): project_id = self._drv_project.get_project(policy) pods_to_update = self._drv_policy.affected_pods(policy) netpolicy_crd = self._drv_policy.get_kuryrnetpolicy_crd(policy) - crd_sg = netpolicy_crd['spec'].get('securityGroupId') - for pod in pods_to_update: - if driver_utils.is_host_network(pod): - continue - pod_sgs = self._drv_pod_sg.get_security_groups(pod, project_id) - if crd_sg in pod_sgs: - pod_sgs.remove(crd_sg) - if not pod_sgs: - pod_sgs = oslo_cfg.CONF.neutron_defaults.pod_security_groups + if netpolicy_crd: + crd_sg = netpolicy_crd['spec'].get('securityGroupId') + for pod in pods_to_update: + if driver_utils.is_host_network(pod): + continue + pod_sgs = self._drv_pod_sg.get_security_groups(pod, + project_id) + if crd_sg in pod_sgs: + pod_sgs.remove(crd_sg) if not pod_sgs: - raise oslo_cfg.RequiredOptError('pod_security_groups', - oslo_cfg.OptGroup( - 'neutron_defaults')) - self._drv_vif_pool.update_vif_sgs(pod, pod_sgs) + pod_sgs = ( + oslo_cfg.CONF.neutron_defaults.pod_security_groups) + if not pod_sgs: + raise oslo_cfg.RequiredOptError( + 'pod_security_groups', + oslo_cfg.OptGroup('neutron_defaults')) + self._drv_vif_pool.update_vif_sgs(pod, pod_sgs) - self._drv_policy.release_network_policy(netpolicy_crd) + self._drv_policy.release_network_policy(netpolicy_crd) - services = self._get_services(policy['metadata']['namespace']) - for service in services.get('items'): - if service['metadata']['name'] == 'kubernetes': - continue - sgs = self._drv_svc_sg.get_security_groups(service, project_id) - self._drv_lbaas.update_lbaas_sg(service, sgs) + services = self._get_services(policy['metadata']['namespace']) + for service in services.get('items'): + if service['metadata']['name'] == 'kubernetes': + continue + sgs = self._drv_svc_sg.get_security_groups(service, + project_id) + self._drv_lbaas.update_lbaas_sg(service, sgs) def is_ready(self, quota): if not utils.has_kuryr_crd(k_const.K8S_API_CRD_KURYRNETPOLICIES): diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py index 053e55edc..400298761 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py @@ -96,6 +96,7 @@ class TestNetworkPolicyDriver(test_base.TestCase): self._crd = { 'metadata': {'name': mock.sentinel.name, + 'namespace': u'default', 'selfLink': mock.sentinel.selfLink}, 'spec': { 'egressSgRules': [ @@ -440,3 +441,18 @@ class TestNetworkPolicyDriver(test_base.TestCase): resp = self._driver.namespaced_pods(self._policy) self.assertEqual([], resp) + + @mock.patch.object(network_policy.NetworkPolicyDriver, + '_del_kuryrnetpolicy_crd', return_value=False) + def test_release_network_policy(self, m_del_crd): + self._driver.release_network_policy(self._crd) + self.neutron.delete_security_group.assert_called_once_with( + self._crd['spec']['securityGroupId']) + m_del_crd.assert_called_once_with(self._crd['metadata']['name'], + self._crd['metadata']['namespace']) + + @mock.patch.object(network_policy.NetworkPolicyDriver, + '_del_kuryrnetpolicy_crd', return_value=False) + def test_release_network_policy_removed_crd(self, m_del_crd): + self._driver.release_network_policy(None) + m_del_crd.assert_not_called() diff --git a/setup.cfg b/setup.cfg index 019c1436e..bd74b9a1e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -103,6 +103,7 @@ kuryr_kubernetes.controller.handlers = ocproute = kuryr_kubernetes.platform.ocp.controller.handlers.route:OcpRouteHandler policy = kuryr_kubernetes.controller.handlers.policy:NetworkPolicyHandler pod_label = kuryr_kubernetes.controller.handlers.pod_label:PodLabelHandler + kuryrnetpolicy = kuryr_kubernetes.controller.handlers.kuryrnetpolicy:KuryrNetPolicyHandler test_handler = kuryr_kubernetes.tests.unit.controller.handlers.test_fake_handler:TestHandler kuryr_kubernetes.controller.drivers.multi_vif =