From 9bc33d9b786c1be7afb2d07f6cf066edcbda3549 Mon Sep 17 00:00:00 2001 From: Roman Dobosz Date: Fri, 8 Jan 2021 17:25:03 +0100 Subject: [PATCH] Fix k8s client for handling empty list in response. Implements: blueprint selflink Change-Id: I0db851889017cfe19cc84acab36bdc67b04a9145 --- kuryr_kubernetes/k8s_client.py | 10 ++++++++- .../tests/unit/test_k8s_client.py | 21 +++++++++++++++++++ kuryr_kubernetes/watcher.py | 6 ++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/kuryr_kubernetes/k8s_client.py b/kuryr_kubernetes/k8s_client.py index 220cd8322..f2a3c3779 100644 --- a/kuryr_kubernetes/k8s_client.py +++ b/kuryr_kubernetes/k8s_client.py @@ -133,12 +133,20 @@ class K8sClient(object): # for custom resources there are both kind and apiVersion.. if kind.endswith('List'): kind = kind[:-4] + + # NOTE(gryf): In case we get null/None for items from the API, + # we need to convert it to the empty list, otherwise it might + # be propagated to the consumers of this method and sent back + # to the Kubernetes as is, and fail as a result. + if result['items'] is None: + result['items'] = [] + for item in result['items']: if not item.get('kind'): item['kind'] = kind if not item.get('apiVersion'): item['apiVersion'] = api_version - else: + if not result.get('apiVersion'): result['apiVersion'] = api_version else: diff --git a/kuryr_kubernetes/tests/unit/test_k8s_client.py b/kuryr_kubernetes/tests/unit/test_k8s_client.py index b73dbf829..ea9d00260 100644 --- a/kuryr_kubernetes/tests/unit/test_k8s_client.py +++ b/kuryr_kubernetes/tests/unit/test_k8s_client.py @@ -155,6 +155,27 @@ class TestK8sClient(test_base.TestCase): self.assertRaises(exc.K8sClientException, self.client.get, path) + @mock.patch('requests.sessions.Session.get') + def test_get_null_on_items_list(self, m_get): + path = '/test' + + req = {'kind': 'PodList', + 'apiVersion': 'v1', + 'metadata': {}, + 'items': None} + + ret = {'kind': 'PodList', + 'apiVersion': 'v1', + 'metadata': {}, + 'items': []} + + m_resp = mock.MagicMock() + m_resp.ok = True + m_resp.json.return_value = req + m_get.return_value = m_resp + + self.assertEqual(self.client.get(path), ret) + @mock.patch('itertools.count') @mock.patch('requests.sessions.Session.patch') def test_annotate(self, m_patch, m_count): diff --git a/kuryr_kubernetes/watcher.py b/kuryr_kubernetes/watcher.py index 39d176f7f..c17fa3f0e 100644 --- a/kuryr_kubernetes/watcher.py +++ b/kuryr_kubernetes/watcher.py @@ -143,6 +143,12 @@ class Watcher(health.HealthHandler): LOG.exception(f'Error getting path when reconciling.') return + # NOTE(gryf): For some resources (like pods) we could observe that + # 'items' is set to None. I'm not sure if that's a K8s issue, since + # accroding to the documentation is should be list. + if not resources: + return + for resource in resources: event = { 'type': 'MODIFIED',