Catch exceptions for deleted pod.

It might happen, that during adding and instantly removing pod, vif
handler will not be able to set the finalizer for such object, resulting
in error. In this patch we prevent for such case.

Closes-Bug: 1894212
Change-Id: I5b3b4b36ae0c2fca7c16153f75c62607efbc3ce4
This commit is contained in:
Roman Dobosz 2020-09-04 12:40:19 +02:00
parent 23eae26136
commit f2e3ffb585
5 changed files with 50 additions and 12 deletions

View File

@ -133,8 +133,13 @@ class KuryrPortHandler(k8s_base.ResourceEventHandler):
LOG.error("Pod %s/%s doesn't exists, deleting orphaned KuryrPort", LOG.error("Pod %s/%s doesn't exists, deleting orphaned KuryrPort",
namespace, name) namespace, name)
# TODO(gryf): Free resources # TODO(gryf): Free resources
self.k8s.remove_finalizer(kuryrport_crd, try:
constants.KURYRPORT_FINALIZER) 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 return
if 'deletionTimestamp' not in pod['metadata']: if 'deletionTimestamp' not in pod['metadata']:

View File

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

View File

@ -75,7 +75,15 @@ class VIFHandler(k8s_base.ResourceEventHandler):
# subsequent updates of the pod, add_finalizer will ignore this if # subsequent updates of the pod, add_finalizer will ignore this if
# finalizer exists. # finalizer exists.
k8s = clients.get_kubernetes_client() 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): if self._move_annotations_to_crd(pod):
return return
@ -114,7 +122,11 @@ class VIFHandler(k8s_base.ResourceEventHandler):
kp = k8s.get(KURYRPORT_URI.format(ns=pod["metadata"]["namespace"], kp = k8s.get(KURYRPORT_URI.format(ns=pod["metadata"]["namespace"],
crd=pod["metadata"]["name"])) crd=pod["metadata"]["name"]))
except k_exc.K8sResourceNotFound: except k_exc.K8sResourceNotFound:
k8s.remove_finalizer(pod, constants.POD_FINALIZER) 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 return
if 'deletionTimestamp' in kp['metadata']: if 'deletionTimestamp' in kp['metadata']:

View File

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

View File

@ -308,6 +308,23 @@ class TestVIFHandler(test_base.TestCase):
self._request_additional_vifs.assert_not_called() self._request_additional_vifs.assert_not_called()
self._activate_vif.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.clients.get_kubernetes_client')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.get_kuryrport') @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_kuryrport')
def test_on_finalize_crd(self, m_get_kuryrport, m_get_k8s_client): def test_on_finalize_crd(self, m_get_kuryrport, m_get_k8s_client):