From 2e6c7eaae7a198cc169f060351b54e9c0586de11 Mon Sep 17 00:00:00 2001 From: Yossi Boaron Date: Wed, 21 Feb 2018 23:54:52 +0200 Subject: [PATCH] Services: Rollback openstack resources in case of annotation failure Upon K8S service creation the LBaaS handler creates all LB resources at neutron (LB,Listener,Pool,etc) and store them at K8S resource using annotation. When K8S service is deleted, the LBaaS handler retrieves LB resources details from annotation and release them at neutron. This patch handles the case in which K8S service resource was deleted before LBaaS handler stored openstack resource details. Closes-Bug: 1748890 Change-Id: Iea806d32c99cd3cf51a832b576ff4054fc522bd3 --- kuryr_kubernetes/controller/handlers/lbaas.py | 15 ++++++++--- kuryr_kubernetes/exceptions.py | 6 +++++ kuryr_kubernetes/k8s_client.py | 6 ++++- .../unit/controller/handlers/test_lbaas.py | 25 +++++++++++++++++ .../tests/unit/test_k8s_client.py | 27 +++++++++++++++++++ 5 files changed, 75 insertions(+), 4 deletions(-) diff --git a/kuryr_kubernetes/controller/handlers/lbaas.py b/kuryr_kubernetes/controller/handlers/lbaas.py index 64cf06038..5f46082a0 100644 --- a/kuryr_kubernetes/controller/handlers/lbaas.py +++ b/kuryr_kubernetes/controller/handlers/lbaas.py @@ -246,10 +246,19 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler): # required to deal with such situations (e.g. cleanup, or skip # failing items, or validate configuration) to prevent annotation # being out of sync with the actual Neutron state. - self._set_lbaas_state(endpoints, lbaas_state) + try: + self._set_lbaas_state(endpoints, lbaas_state) + except k_exc.K8sResourceNotFound: + # Note(yboaron) It's impossible to store neutron resources + # in K8S object since object was deleted. In that case + # we should rollback all neutron resources. + LOG.debug("LoadBalancerHandler failed to store Openstack " + "resources in K8S object (not found)") + self.on_deleted(endpoints, lbaas_state) - def on_deleted(self, endpoints): - lbaas_state = self._get_lbaas_state(endpoints) + def on_deleted(self, endpoints, lbaas_state=None): + if lbaas_state is None: + lbaas_state = self._get_lbaas_state(endpoints) if not lbaas_state: return # NOTE(ivc): deleting pool deletes its members diff --git a/kuryr_kubernetes/exceptions.py b/kuryr_kubernetes/exceptions.py index 8889e76a2..82139a1e7 100644 --- a/kuryr_kubernetes/exceptions.py +++ b/kuryr_kubernetes/exceptions.py @@ -28,6 +28,12 @@ class ResourceNotReady(Exception): % resource) +class K8sResourceNotFound(K8sClientException): + def __init__(self, resource): + super(K8sResourceNotFound, self).__init__("Resource not " + "found: %r" % resource) + + class CNIError(Exception): pass diff --git a/kuryr_kubernetes/k8s_client.py b/kuryr_kubernetes/k8s_client.py index 75ab664fd..0c40e6237 100644 --- a/kuryr_kubernetes/k8s_client.py +++ b/kuryr_kubernetes/k8s_client.py @@ -145,7 +145,11 @@ class K8sClient(object): "content: %(content)s, text: %(text)s" % {'headers': response.headers, 'content': response.content, 'text': response.text}) - raise exc.K8sClientException(response.text) + + if response.status_code == requests.codes.not_found: + raise exc.K8sResourceNotFound(response.text) + else: + raise exc.K8sClientException(response.text) def watch(self, path): params = {'watch': 'true'} diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py index 794fba1c2..f940bf108 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_lbaas.py @@ -423,6 +423,31 @@ class TestLoadBalancerHandler(test_base.TestCase): m_handler._set_lbaas_state.assert_called_once_with( endpoints, lbaas_state) + def test_on_present_rollback(self): + lbaas_spec = mock.sentinel.lbaas_spec + lbaas_state = mock.sentinel.lbaas_state + endpoints = mock.sentinel.endpoints + + m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler) + m_handler._get_lbaas_spec.return_value = lbaas_spec + m_handler._should_ignore.return_value = False + m_handler._get_lbaas_state.return_value = lbaas_state + m_handler._sync_lbaas_members.return_value = True + m_handler._set_lbaas_state.side_effect = ( + k_exc.K8sResourceNotFound('ep')) + + h_lbaas.LoadBalancerHandler.on_present(m_handler, endpoints) + + m_handler._get_lbaas_spec.assert_called_once_with(endpoints) + m_handler._should_ignore.assert_called_once_with(endpoints, lbaas_spec) + m_handler._get_lbaas_state.assert_called_once_with(endpoints) + m_handler._sync_lbaas_members.assert_called_once_with( + endpoints, lbaas_state, lbaas_spec) + m_handler._set_lbaas_state.assert_called_once_with( + endpoints, lbaas_state) + m_handler.on_deleted.assert_called_once_with( + endpoints, lbaas_state) + @mock.patch('kuryr_kubernetes.objects.lbaas' '.LBaaSServiceSpec') def test_on_deleted(self, m_svc_spec_ctor): diff --git a/kuryr_kubernetes/tests/unit/test_k8s_client.py b/kuryr_kubernetes/tests/unit/test_k8s_client.py index 91151e878..da5faea2b 100644 --- a/kuryr_kubernetes/tests/unit/test_k8s_client.py +++ b/kuryr_kubernetes/tests/unit/test_k8s_client.py @@ -280,6 +280,33 @@ class TestK8sClient(test_base.TestCase): headers=mock.ANY, cert=(None, None), verify=False) + @mock.patch('itertools.count') + @mock.patch('requests.patch') + def test_annotate_resource_not_found(self, m_patch, m_count): + m_count.return_value = list(range(1, 5)) + path = '/test' + annotations = {'a1': 'v1', 'a2': 'v2'} + resource_version = "123" + annotate_obj = {'metadata': { + 'annotations': annotations, + 'resourceVersion': resource_version}} + annotate_data = jsonutils.dumps(annotate_obj, sort_keys=True) + + m_resp_not_found = mock.MagicMock() + m_resp_not_found.ok = False + m_resp_not_found.status_code = requests.codes.not_found + m_patch.return_value = m_resp_not_found + + self.assertRaises(exc.K8sResourceNotFound, + self.client.annotate, + path, + annotations, + resource_version=resource_version) + m_patch.assert_called_once_with(self.base_url + path, + data=annotate_data, + headers=mock.ANY, + cert=(None, None), verify=False) + @mock.patch('requests.get') def test_watch(self, m_get): path = '/test'