From bba518c04d26cac107905faf775b2508b56797dd Mon Sep 17 00:00:00 2001
From: Justin Ferrieu <jferrieu@objectif-libre.com>
Date: Mon, 3 Jun 2019 13:34:47 +0000
Subject: [PATCH] Fix Prometheus fetcher error

An error was returned by the Prometheus fetcher
due to the fact that Prometheus returns the
aggregated total as {} even when filtering with
groupby parameters causing a KeyError to be triggered.

This is now fixed and the fetcher works as expected.

Change-Id: If0e2d5a4cfd55bd67e0414803bd5b4f3fe57c1f5
---
 cloudkitty/fetcher/prometheus.py             | 24 ++++-----
 cloudkitty/tests/fetchers/test_prometheus.py | 53 ++++++++++++--------
 2 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/cloudkitty/fetcher/prometheus.py b/cloudkitty/fetcher/prometheus.py
index d13ef408..98bd680d 100644
--- a/cloudkitty/fetcher/prometheus.py
+++ b/cloudkitty/fetcher/prometheus.py
@@ -126,22 +126,16 @@ class PrometheusFetcher(fetcher.BaseFetcher):
 
             scope_ids = [
                 item['metric'][scope_attribute] for item in result
-                if item['metric'][scope_attribute]
+                if scope_attribute in item['metric'].keys()
             ]
-        except KeyError as e:
-            missing_key = e.args[0]
-            if missing_key in ['data', 'result', 'metric']:
-                msg = (
-                    'Unexpected Prometheus server response '
-                    '"{}" for "{}"'
-                ).format(
-                    res,
-                    query,
-                )
-            else:
-                msg = '"{}" not found in Prometheus server response'.format(
-                    missing_key
-                )
+        except KeyError:
+            msg = (
+                'Unexpected Prometheus server response '
+                '"{}" for "{}"'
+            ).format(
+                res,
+                query,
+            )
             raise PrometheusFetcherError(msg)
 
         # Returning unique ids
diff --git a/cloudkitty/tests/fetchers/test_prometheus.py b/cloudkitty/tests/fetchers/test_prometheus.py
index 7e73920f..6c1863d6 100644
--- a/cloudkitty/tests/fetchers/test_prometheus.py
+++ b/cloudkitty/tests/fetchers/test_prometheus.py
@@ -60,6 +60,39 @@ class PrometheusFetcherTest(tests.TestCase):
             self.fetcher.get_tenants()
             mock_get.assert_called_once_with(query)
 
+    def test_get_tenants(self):
+        response = mock.patch(
+            'cloudkitty.common.prometheus_client.PrometheusClient.get_instant',
+            return_value={
+                'data': {
+                    'result': [
+                        {
+                            'metric': {},
+                            'value': [42, 1337],
+                        },
+                        {
+                            'metric': {'namespace': 'scope_id1'},
+                            'value': [42, 1337],
+                        },
+                        {
+                            'metric': {'namespace': 'scope_id2'},
+                            'value': [42, 1337],
+                        },
+                        {
+                            'metric': {'namespace': 'scope_id3'},
+                            'value': [42, 1337],
+                        },
+                    ]
+                }
+            },
+        )
+
+        with response:
+            scopes = self.fetcher.get_tenants()
+            self.assertItemsEqual(scopes, [
+                'scope_id1', 'scope_id2', 'scope_id3',
+            ])
+
     def test_get_tenants_raises_exception(self):
         no_response = mock.patch(
             'cloudkitty.common.prometheus_client.PrometheusClient.get_instant',
@@ -73,26 +106,6 @@ class PrometheusFetcherTest(tests.TestCase):
             )
 
     def test_get_tenants_raises_exception2(self):
-        no_response = mock.patch(
-            'cloudkitty.common.prometheus_client.PrometheusClient.get_instant',
-            return_value={
-                'data': {
-                    'result': [{
-                        'metric': {
-                            'foo': 'bar'
-                        }
-                    }]
-                }
-            },
-        )
-
-        with no_response:
-            self.assertRaises(
-                prometheus.PrometheusFetcherError,
-                self.fetcher.get_tenants,
-            )
-
-    def test_get_tenants_raises_exception3(self):
         invalid_response = mock.patch(
             'cloudkitty.common.prometheus_client.PrometheusClient.get_instant',
             side_effect=PrometheusResponseError,