Merge "Catch exceptions for deleted pod."

kuryr_kubernetes/controller/handlers/kuryrport.py

@@ -133,8 +133,13 @@ class KuryrPortHandler(k8s_base.ResourceEventHandler):
             LOG.error("Pod %s/%s doesn't exists, deleting orphaned KuryrPort",
                       namespace, name)
             # TODO(gryf): Free resources
+            try:
                 self.k8s.remove_finalizer(kuryrport_crd,
                                           constants.KURYRPORT_FINALIZER)
+            except k_exc.K8sClientException as ex:
+                LOG.exception("Failed to remove finalizer from KuryrPort %s",
+                              ex)
+                raise
             return
 
         if 'deletionTimestamp' not in pod['metadata']:

kuryr_kubernetes/controller/handlers/lbaas.py

@@ -56,7 +56,8 @@ class ServiceHandler(k8s_base.ResourceEventHandler):
         k8s = clients.get_kubernetes_client()
         loadbalancer_crd = k8s.get_loadbalancer_crd(service)
         try:
-            self._patch_service_finalizer(service)
+            if not self._patch_service_finalizer(service):
+                return
         except k_exc.K8sClientException as ex:
             LOG.exception("Failed to set service finalizer: %s", ex)
             raise
@@ -100,7 +101,7 @@ class ServiceHandler(k8s_base.ResourceEventHandler):
 
     def _patch_service_finalizer(self, service):
         k8s = clients.get_kubernetes_client()
-        k8s.add_finalizer(service, k_const.SERVICE_FINALIZER)
+        return k8s.add_finalizer(service, k_const.SERVICE_FINALIZER)
 
     def on_finalize(self, service):
         k8s = clients.get_kubernetes_client()

kuryr_kubernetes/controller/handlers/vif.py

@@ -75,7 +75,15 @@ class VIFHandler(k8s_base.ResourceEventHandler):
         # subsequent updates of the pod, add_finalizer will ignore this if
         # finalizer exists.
         k8s = clients.get_kubernetes_client()
-        k8s.add_finalizer(pod, constants.POD_FINALIZER)
+
+        try:
+            if not k8s.add_finalizer(pod, constants.POD_FINALIZER):
+                # NOTE(gryf) It might happen that pod will be deleted even
+                # before we got here.
+                return
+        except k_exc.K8sClientException as ex:
+            LOG.exception("Failed to add finalizer to pod object: %s", ex)
+            raise
 
         if self._move_annotations_to_crd(pod):
             return
@@ -114,7 +122,11 @@ class VIFHandler(k8s_base.ResourceEventHandler):
             kp = k8s.get(KURYRPORT_URI.format(ns=pod["metadata"]["namespace"],
                                               crd=pod["metadata"]["name"]))
         except k_exc.K8sResourceNotFound:
+            try:
                 k8s.remove_finalizer(pod, constants.POD_FINALIZER)
+            except k_exc.K8sClientException as ex:
+                LOG.exception('Failed to remove finalizer from pod: %s', ex)
+                raise
             return
 
         if 'deletionTimestamp' in kp['metadata']:

kuryr_kubernetes/k8s_client.py

@@ -227,7 +227,7 @@ class K8sClient(object):
     #              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
+            return True
 
         path = obj['metadata']['selfLink']
         LOG.debug(f"Add finalizer {finalizer} to {path}")
@@ -250,15 +250,18 @@ class K8sClient(object):
                                           verify=self.verify_server)
 
             if response.ok:
-                return response.json()
+                return True
 
             try:
                 self._raise_from_response(response)
+            except (exc.K8sForbidden, exc.K8sResourceNotFound):
+                # Object is being deleting or gone. Return.
+                return False
             except exc.K8sConflict:
                 obj = self.get(path)
                 if finalizer in obj['metadata'].get('finalizers', []):
                     # Finalizer is there, return.
-                    return obj
+                    return True
 
         # If after 3 iterations there's still conflict, just raise.
         self._raise_from_response(response)
@@ -275,7 +278,7 @@ class K8sClient(object):
                 finalizers.remove(finalizer)
             except ValueError:
                 # Finalizer is not there, return.
-                return obj
+                return True
 
             data = {
                 'metadata': {
@@ -289,7 +292,7 @@ class K8sClient(object):
                                           verify=self.verify_server)
 
             if response.ok:
-                return response.json()
+                return True
 
             try:
                 try:
@@ -298,7 +301,7 @@ class K8sClient(object):
                     obj = self.get(path)
             except exc.K8sResourceNotFound:
                 # Object is gone already, stop.
-                return None
+                return False
 
         # If after 3 iterations there's still conflict, just raise.
         self._raise_from_response(response)

kuryr_kubernetes/tests/unit/controller/handlers/test_vif.py

@@ -308,6 +308,23 @@ class TestVIFHandler(test_base.TestCase):
         self._request_additional_vifs.assert_not_called()
         self._activate_vif.assert_not_called()
 
+    @mock.patch('kuryr_kubernetes.clients.get_kubernetes_client')
+    @mock.patch('kuryr_kubernetes.controller.drivers.utils.is_host_network')
+    def test_on_present_pod_finalizer_exception(self, m_host_network,
+                                                m_get_k8s_client):
+        m_host_network.return_value = False
+        self._matc.return_value = True
+        k8s = mock.MagicMock()
+        k8s.add_finalizer.side_effect = k_exc.K8sClientException
+        m_get_k8s_client.return_value = k8s
+
+        self.assertRaises(k_exc.K8sClientException,
+                          h_vif.VIFHandler.on_present, self._handler,
+                          self._pod)
+
+        k8s.add_finalizer.assert_called_once_with(self._pod,
+                                                  k_const.POD_FINALIZER)
+
     @mock.patch('kuryr_kubernetes.clients.get_kubernetes_client')
     @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_kuryrport')
     def test_on_finalize_crd(self, m_get_kuryrport, m_get_k8s_client):