From fd3f4c55d7c6d65404e2a9cf06b3c4cc3714ee1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Tue, 3 Mar 2020 16:18:56 +0100 Subject: [PATCH] Raise K8sResourceNotFound for all client methods Seems like K8sClient wasn't raising K8sResourceNotFound exception on 404 from all the methods, e.g. patch. This commit makes sure that is no longer the case and any 404 results in K8sResourceNotFound exception. Change-Id: I23b947ef5f8c56429b5c69b73489fec19c44bd7e Closes-Bug: 1865893 --- kuryr_kubernetes/k8s_client.py | 49 ++++++++----------- .../tests/unit/test_k8s_client.py | 20 ++++++++ 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/kuryr_kubernetes/k8s_client.py b/kuryr_kubernetes/k8s_client.py index 12391371f..86e2d3751 100644 --- a/kuryr_kubernetes/k8s_client.py +++ b/kuryr_kubernetes/k8s_client.py @@ -76,6 +76,12 @@ class K8sClient(object): else: self.verify_server = ca_crt_file + def _raise_from_response(self, response): + if response.status_code == requests.codes.not_found: + raise exc.K8sResourceNotFound(response.text) + if not response.ok: + raise exc.K8sClientException(response.text) + def get(self, path, json=True, headers=None): LOG.debug("Get %(path)s", {'path': path}) url = self._base_url + path @@ -87,10 +93,7 @@ class K8sClient(object): response = self.session.get(url, cert=self.cert, verify=self.verify_server, headers=header) - if response.status_code == requests.codes.not_found: - raise exc.K8sResourceNotFound(response.text) - if not response.ok: - raise exc.K8sClientException(response.text) + self._raise_from_response(response) result = response.json() if json else response.text return result @@ -113,9 +116,8 @@ class K8sClient(object): response = self.session.patch(url, json={field: data}, headers=header, cert=self.cert, verify=self.verify_server) - if response.ok: - return response.json().get('status') - raise exc.K8sClientException(response.text) + self._raise_from_response(response) + return response.json().get('status') def patch_crd(self, field, path, data): content_type = 'application/json-patch+json' @@ -132,9 +134,8 @@ class K8sClient(object): response = self.session.patch(url, data=jsonutils.dumps(data), headers=header, cert=self.cert, verify=self.verify_server) - if response.ok: - return response.json().get('status') - raise exc.K8sClientException(response.text) + self._raise_from_response(response) + return response.json().get('status') def patch_node_annotations(self, node, annotation_name, value): content_type = 'application/json-patch+json' @@ -149,9 +150,8 @@ class K8sClient(object): response = self.session.patch(url, data=jsonutils.dumps(data), headers=header, cert=self.cert, verify=self.verify_server) - if response.ok: - return response.json().get('status') - raise exc.K8sClientException(response.text) + self._raise_from_response(response) + return response.json().get('status') def remove_node_annotations(self, node, annotation_name): content_type = 'application/json-patch+json' @@ -164,9 +164,8 @@ class K8sClient(object): response = self.session.patch(url, data=jsonutils.dumps(data), headers=header, cert=self.cert, verify=self.verify_server) - if response.ok: - return response.json().get('status') - raise exc.K8sClientException(response.text) + self._raise_from_response(response) + return response.json().get('status') def post(self, path, body): LOG.debug("Post %(path)s: %(body)s", {'path': path, 'body': body}) @@ -177,9 +176,8 @@ class K8sClient(object): response = self.session.post(url, json=body, cert=self.cert, verify=self.verify_server, headers=header) - if response.ok: - return response.json() - raise exc.K8sClientException(response) + self._raise_from_response(response) + return response.json() def delete(self, path): LOG.debug("Delete %(path)s", {'path': path}) @@ -191,12 +189,8 @@ class K8sClient(object): response = self.session.delete(url, cert=self.cert, verify=self.verify_server, headers=header) - if response.ok: - return response.json() - else: - if response.status_code == requests.codes.not_found: - raise exc.K8sResourceNotFound(response.text) - raise exc.K8sClientException(response) + self._raise_from_response(response) + return response.json() def annotate(self, path, annotations, resource_version=None): """Pushes a resource annotation to the K8s API resource @@ -245,10 +239,7 @@ class K8sClient(object): % {'headers': response.headers, 'content': response.content, 'text': response.text}) - if response.status_code == requests.codes.not_found: - raise exc.K8sResourceNotFound(response.text) - else: - raise exc.K8sClientException(response.text) + self._raise_from_response(response) def watch(self, path): url = self._base_url + path diff --git a/kuryr_kubernetes/tests/unit/test_k8s_client.py b/kuryr_kubernetes/tests/unit/test_k8s_client.py index 78500e730..9aedd9c07 100644 --- a/kuryr_kubernetes/tests/unit/test_k8s_client.py +++ b/kuryr_kubernetes/tests/unit/test_k8s_client.py @@ -434,3 +434,23 @@ class TestK8sClient(test_base.TestCase): self.assertRaises(exc.K8sClientException, self.client.delete, path) + + def test__raise_from_response(self): + m_resp = mock.MagicMock() + m_resp.ok = True + m_resp.status_code = 202 + self.client._raise_from_response(m_resp) + + def test__raise_from_response_404(self): + m_resp = mock.MagicMock() + m_resp.ok = False + m_resp.status_code = 404 + self.assertRaises(exc.K8sResourceNotFound, + self.client._raise_from_response, m_resp) + + def test__raise_from_response_500(self): + m_resp = mock.MagicMock() + m_resp.ok = False + m_resp.status_code = 500 + self.assertRaises(exc.K8sClientException, + self.client._raise_from_response, m_resp)