From 4b832517db3678aa98c03a69fea97f29cd5768eb Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Mon, 17 Dec 2018 18:09:08 +0100 Subject: [PATCH] Avoid raising ResourceNotReady exception at pod label handler Pod label handler should not raise ResourceNotReady exception when the pods don't have pod state annotations. The vif handler will add those annotations and then a new on_modified event will be triggered that the pod label will receive. Thus there is no need to ensure the previous event gets re-executed. Closes-Bug: 1808787 Partially Implements: blueprint k8s-network-policies Change-Id: I7be86f7d03241e59164374c0712109e01c5c0842 --- .../controller/handlers/pod_label.py | 11 ++++------ .../controller/handlers/test_pod_label.py | 20 +++++++++---------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/kuryr_kubernetes/controller/handlers/pod_label.py b/kuryr_kubernetes/controller/handlers/pod_label.py index 6d6e29201..44dd47e1f 100644 --- a/kuryr_kubernetes/controller/handlers/pod_label.py +++ b/kuryr_kubernetes/controller/handlers/pod_label.py @@ -20,7 +20,6 @@ from kuryr_kubernetes import clients from kuryr_kubernetes import constants from kuryr_kubernetes.controller.drivers import base as drivers from kuryr_kubernetes.controller.drivers import utils as driver_utils -from kuryr_kubernetes import exceptions from kuryr_kubernetes.handlers import k8s_base LOG = logging.getLogger(__name__) @@ -44,13 +43,11 @@ class PodLabelHandler(k8s_base.ResourceEventHandler): specific_driver='multi_pool') self._drv_vif_pool.set_vif_driver() - def on_modified(self, pod): - if driver_utils.is_host_network(pod): + def on_present(self, pod): + if driver_utils.is_host_network(pod) or not self._has_pod_state(pod): + # NOTE(ltomasbo): The event will be retried once the vif handler + # annotates the pod with the pod state. return - if not self._has_pod_state(pod): - # NOTE(ltomasbo): Ensuring the event is retried and the right - # pod label annotation is added to the pod - raise exceptions.ResourceNotReady(pod) current_pod_labels = pod['metadata'].get('labels') previous_pod_labels = self._get_pod_labels(pod) diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_pod_label.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_pod_label.py index 1b5ced8f4..3db975d81 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_pod_label.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_pod_label.py @@ -17,7 +17,6 @@ import mock from kuryr_kubernetes import constants as k_const from kuryr_kubernetes.controller.drivers import base as drivers from kuryr_kubernetes.controller.handlers import pod_label as p_label -from kuryr_kubernetes import exceptions from kuryr_kubernetes.tests import base as test_base @@ -73,11 +72,11 @@ class TestPodLabelHandler(test_base.TestCase): self.assertEqual(sg_driver, handler._drv_sg) self.assertEqual(vif_pool_driver, handler._drv_vif_pool) - def test_on_modified(self): + def test_on_present(self): self._has_pod_state.return_value = True self._get_pod_labels.return_value = {'test1': 'test'} - p_label.PodLabelHandler.on_modified(self._handler, self._pod) + p_label.PodLabelHandler.on_present(self._handler, self._pod) self._has_pod_state.assert_called_once_with(self._pod) self._get_pod_labels.assert_called_once_with(self._pod) @@ -86,34 +85,33 @@ class TestPodLabelHandler(test_base.TestCase): self._update_vif_sgs.assert_called_once_with(self._pod, [self._sg_id]) self._set_pod_labels.assert_called_once_with(self._pod, None) - def test_on_modified_no_state(self): + def test_on_present_no_state(self): self._has_pod_state.return_value = False - self.assertRaises(exceptions.ResourceNotReady, - p_label.PodLabelHandler.on_modified, self._handler, - self._pod) + resp = p_label.PodLabelHandler.on_present(self._handler, self._pod) + self.assertIsNone(resp) self._has_pod_state.assert_called_once_with(self._pod) self._get_pod_labels.assert_not_called() self._set_pod_labels.assert_not_called() - def test_on_modified_no_labels(self): + def test_on_present_no_labels(self): self._has_pod_state.return_value = True self._get_pod_labels.return_value = None - p_label.PodLabelHandler.on_modified(self._handler, self._pod) + p_label.PodLabelHandler.on_present(self._handler, self._pod) self._has_pod_state.assert_called_once_with(self._pod) self._get_pod_labels.assert_called_once_with(self._pod) self._set_pod_labels.assert_not_called() - def test_on_modified_no_changes(self): + def test_on_present_no_changes(self): self._has_pod_state.return_value = True pod_with_label = self._pod.copy() pod_with_label['metadata']['labels'] = {'test1': 'test'} self._get_pod_labels.return_value = {'test1': 'test'} - p_label.PodLabelHandler.on_modified(self._handler, pod_with_label) + p_label.PodLabelHandler.on_present(self._handler, pod_with_label) self._has_pod_state.assert_called_once_with(pod_with_label) self._get_pod_labels.assert_called_once_with(pod_with_label)