From b6c89debdd61a463ecf55b34708da641783e8794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Fri, 24 Jul 2020 15:44:29 +0200 Subject: [PATCH] Implement add_finalizer and remove_finalizer This commit adds helper client methods that will aid in working with finalizers of the CRDs and other stuff. Also one place where we remove the finalizer already is updated to use the methods. Change-Id: I665e03f80102a08b2c3ec412a4417c3a32f9384b --- .../controller/handlers/kuryrnetwork.py | 6 +- kuryr_kubernetes/exceptions.py | 5 ++ kuryr_kubernetes/k8s_client.py | 82 +++++++++++++++++++ .../controller/handlers/test_kuryrnetwork.py | 10 +-- 4 files changed, 94 insertions(+), 9 deletions(-) diff --git a/kuryr_kubernetes/controller/handlers/kuryrnetwork.py b/kuryr_kubernetes/controller/handlers/kuryrnetwork.py index 6e163504f..5275d50e2 100644 --- a/kuryr_kubernetes/controller/handlers/kuryrnetwork.py +++ b/kuryr_kubernetes/controller/handlers/kuryrnetwork.py @@ -119,10 +119,8 @@ class KuryrNetworkHandler(k8s_base.ResourceEventHandler): kubernetes = clients.get_kubernetes_client() LOG.debug('Removing finalizer for KuryrNet CRD %s', kuryrnet_crd) try: - kubernetes.patch_crd('metadata', - kuryrnet_crd['metadata']['selfLink'], - 'finalizers', - action='remove') + kubernetes.remove_finalizer(kuryrnet_crd, + constants.KURYRNETWORK_FINALIZER) except k_exc.K8sClientException: LOG.exception('Error removing kuryrnetwork CRD finalizer for %s', kuryrnet_crd) diff --git a/kuryr_kubernetes/exceptions.py b/kuryr_kubernetes/exceptions.py index 1806a35b1..8a02bad8c 100644 --- a/kuryr_kubernetes/exceptions.py +++ b/kuryr_kubernetes/exceptions.py @@ -34,6 +34,11 @@ class K8sResourceNotFound(K8sClientException): "found: %r" % resource) +class K8sConflict(K8sClientException): + def __init__(self, message): + super(K8sConflict, self).__init__("Conflict: %r" % message) + + class InvalidKuryrNetworkAnnotation(Exception): pass diff --git a/kuryr_kubernetes/k8s_client.py b/kuryr_kubernetes/k8s_client.py index 98127f947..a228d8b07 100644 --- a/kuryr_kubernetes/k8s_client.py +++ b/kuryr_kubernetes/k8s_client.py @@ -81,6 +81,8 @@ class K8sClient(object): def _raise_from_response(self, response): if response.status_code == requests.codes.not_found: raise exc.K8sResourceNotFound(response.text) + if response.status_code == requests.codes.conflict: + raise exc.K8sConflict(response.text) if not response.ok: raise exc.K8sClientException(response.text) @@ -211,6 +213,86 @@ class K8sClient(object): self._raise_from_response(response) return response.json() + # TODO(dulek): add_finalizer() and remove_finalizer() have some code + # duplication, but I don't see a nice way to avoid it. + def add_finalizer(self, obj, finalizer): + if finalizer in obj['metadata'].get('finalizers', []): + return obj + + path = obj['metadata']['selfLink'] + LOG.debug(f"Add finalizer {finalizer} to {path}") + url, headers = self._get_url_and_header( + path, 'application/merge-patch+json') + + for i in range(3): # Let's make sure it's not infinite loop + finalizers = obj['metadata'].get('finalizers', []).copy() + finalizers.append(finalizer) + + data = { + 'metadata': { + 'finalizers': finalizers, + 'resourceVersion': obj['metadata']['resourceVersion'], + }, + } + + response = self.session.patch(url, json=data, headers=headers, + cert=self.cert, + verify=self.verify_server) + + if response.ok: + return response.json() + + try: + self._raise_from_response(response) + except exc.K8sConflict: + obj = self.get(path) + if finalizer in obj['metadata'].get('finalizers', []): + # Finalizer is there, return. + return obj + + # If after 3 iterations there's still conflict, just raise. + self._raise_from_response(response) + + def remove_finalizer(self, obj, finalizer): + path = obj['metadata']['selfLink'] + LOG.debug(f"Remove finalizer {finalizer} from {path}") + url, headers = self._get_url_and_header( + path, 'application/merge-patch+json') + + for i in range(3): # Let's make sure it's not infinite loop + finalizers = obj['metadata'].get('finalizers', []).copy() + try: + finalizers.remove(finalizer) + except ValueError: + # Finalizer is not there, return. + return obj + + data = { + 'metadata': { + 'finalizers': finalizers, + 'resourceVersion': obj['metadata']['resourceVersion'], + }, + } + + response = self.session.patch(url, json=data, headers=headers, + cert=self.cert, + verify=self.verify_server) + + if response.ok: + return response.json() + + try: + try: + self._raise_from_response(response) + except exc.K8sConflict: + obj = self.get(path) + except exc.K8sResourceNotFound: + # Object is gone already, stop. + return None + + # If after 3 iterations there's still conflict, just raise. + self._raise_from_response(response) + def annotate(self, path, annotations, resource_version=None): """Pushes a resource annotation to the K8s API resource diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_kuryrnetwork.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_kuryrnetwork.py index b455fdfa3..d52845b00 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_kuryrnetwork.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_kuryrnetwork.py @@ -210,7 +210,7 @@ class TestKuryrNetworkHandler(test_base.TestCase): self._delete_ns_sg_rules.assert_called_once() m_get_svc.assert_called_once() self._handler._update_services.assert_called_once() - kubernetes.patch_crd.assert_called_once() + kubernetes.remove_finalizer.assert_called_once() @mock.patch.object(driver_utils, 'get_services') def test_on_finalize_no_network(self, m_get_svc): @@ -228,7 +228,7 @@ class TestKuryrNetworkHandler(test_base.TestCase): self._delete_ns_sg_rules.assert_called_once() m_get_svc.assert_called_once() self._handler._update_services.assert_called_once() - kubernetes.patch_crd.assert_called_once() + kubernetes.remove_finalizer.assert_called_once() @mock.patch.object(driver_utils, 'get_services') def test_on_finalize_no_sg_enforce(self, m_get_svc): @@ -254,7 +254,7 @@ class TestKuryrNetworkHandler(test_base.TestCase): self._delete_ns_sg_rules.assert_called_once() m_get_svc.assert_not_called() self._handler._update_services.assert_not_called() - kubernetes.patch_crd.assert_called_once() + kubernetes.remove_finalizer.assert_called_once() @mock.patch.object(driver_utils, 'get_services') def test_on_finalize_finalizer_exception(self, m_get_svc): @@ -266,7 +266,7 @@ class TestKuryrNetworkHandler(test_base.TestCase): self._delete_ns_sg_rules.return_value = [crd_selector] m_get_svc.return_value = [] kubernetes = self.useFixture(k_fix.MockK8sClient()).client - kubernetes.patch_crd.side_effect = k_exc.K8sClientException + kubernetes.remove_finalizer.side_effect = k_exc.K8sClientException self.assertRaises( k_exc.K8sClientException, @@ -278,4 +278,4 @@ class TestKuryrNetworkHandler(test_base.TestCase): self._delete_ns_sg_rules.assert_called_once() m_get_svc.assert_called_once() self._handler._update_services.assert_called_once() - kubernetes.patch_crd.assert_called_once() + kubernetes.remove_finalizer.assert_called_once()