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()