From fa1b90255b822e120545432e6f1a1122275316f8 Mon Sep 17 00:00:00 2001 From: Ashwin Agate Date: Tue, 28 Mar 2017 15:48:17 -0700 Subject: [PATCH] Ceilosca pagination hanging problem If enable_api_pagination is turned on ceilometer statistics api call with group by did not return any results. This review fixes problem with indentation. Also fixing similar problems with metric_list and measurement list calls in monasca client. Also added unit tests to test metrics list, measurement list and statistics list when pagination is enabled and also when disabled. Change-Id: I2394da8c0c32d8516ac8136fcaaef87e546966b0 --- ceilosca/ceilometer/monasca_client.py | 70 +++--- .../tests/unit/test_monascaclient.py | 235 ++++++++++++++++++ 2 files changed, 270 insertions(+), 35 deletions(-) diff --git a/ceilosca/ceilometer/monasca_client.py b/ceilosca/ceilometer/monasca_client.py index c5fda4b..8661174 100644 --- a/ceilosca/ceilometer/monasca_client.py +++ b/ceilosca/ceilometer/monasca_client.py @@ -73,7 +73,7 @@ class Client(object): def __init__(self, parsed_url): self._retry_interval = cfg.CONF.database.retry_interval * 1000 self._max_retries = cfg.CONF.database.max_retries or 1 - # nable monasca api pagination + # enable monasca api pagination self._enable_api_pagination = cfg.CONF.monasca.enable_api_pagination # NOTE(zqfan): There are many concurrency requests while using # Ceilosca, to save system resource, we don't retry too many times. @@ -176,16 +176,16 @@ class Client(object): """ search_args = copy.deepcopy(kwargs) metrics = self.call_func(self._mon_client.metrics.list, **search_args) - # check of api pagination is enabled + # check if api pagination is enabled if self._enable_api_pagination: # page through monasca results while metrics: for metric in metrics: yield metric - # offset for metircs is the last metric's id - search_args['offset'] = metric['id'] - metrics = self.call_func(self._mon_client.metrics.list, - **search_args) + # offset for metrics is the last metric's id + search_args['offset'] = metric['id'] + metrics = self.call_func(self._mon_client.metrics.list, + **search_args) else: for metric in metrics: yield metric @@ -204,18 +204,18 @@ class Client(object): measurements = self.call_func( self._mon_client.metrics.list_measurements, **search_args) - # check of api pagination is enabled + # check if api pagination is enabled if self._enable_api_pagination: while measurements: for measurement in measurements: yield measurement - # offset for measurements is measurement id composited with - # the last measurement's timestamp - search_args['offset'] = '%s_%s' % ( - measurement['id'], measurement['measurements'][-1][0]) - measurements = self.call_func( - self._mon_client.metrics.list_measurements, - **search_args) + # offset for measurements is measurement id composited with + # the last measurement's timestamp + search_args['offset'] = '%s_%s' % ( + measurement['id'], measurement['measurements'][-1][0]) + measurements = self.call_func( + self._mon_client.metrics.list_measurements, + **search_args) else: for measurement in measurements: yield measurement @@ -229,31 +229,31 @@ class Client(object): search_args = copy.deepcopy(kwargs) statistics = self.call_func(self._mon_client.metrics.list_statistics, **search_args) - # check of api pagination is enabled + # check if api pagination is enabled if self._enable_api_pagination: while statistics: for statistic in statistics: yield statistic - # with groupby, the offset is unpredictable to me, we don't - # support pagination for it now. - if kwargs.get('group_by'): - break - # offset for statistics is statistic id composited with - # the last statistic's timestamp - search_args['offset'] = '%s_%s' % ( - statistic['id'], statistic['statistics'][-1][0]) - statistics = self.call_func( - self._mon_client.metrics.list_statistics, - **search_args) - # unlike metrics.list and metrics.list_measurements - # return whole new data, metrics.list_statistics - # next page will use last page's final statistic - # data as the first one, so we need to pop it here. - # I think Monasca should treat this as a bug and fix it. - if statistics: - statistics[0]['statistics'].pop(0) - if len(statistics[0]['statistics']) == 0: - statistics.pop(0) + # with groupby, the offset is unpredictable to me, we don't + # support pagination for it now. + if kwargs.get('group_by'): + break + # offset for statistics is statistic id composited with + # the last statistic's timestamp + search_args['offset'] = '%s_%s' % ( + statistic['id'], statistic['statistics'][-1][0]) + statistics = self.call_func( + self._mon_client.metrics.list_statistics, + **search_args) + # unlike metrics.list and metrics.list_measurements + # return whole new data, metrics.list_statistics + # next page will use last page's final statistic + # data as the first one, so we need to pop it here. + # I think Monasca should treat this as a bug and fix it. + if statistics: + statistics[0]['statistics'].pop(0) + if len(statistics[0]['statistics']) == 0: + statistics.pop(0) else: for statistic in statistics: yield statistic diff --git a/ceilosca/ceilometer/tests/unit/test_monascaclient.py b/ceilosca/ceilometer/tests/unit/test_monascaclient.py index 0326d71..6603ef5 100644 --- a/ceilosca/ceilometer/tests/unit/test_monascaclient.py +++ b/ceilosca/ceilometer/tests/unit/test_monascaclient.py @@ -183,3 +183,238 @@ class TestMonascaClient(base.BaseTestCase): self.assertRaises( monasca_client.MonascaInvalidParametersException, self.mc.metrics_create) + + def test_metrics_list_with_pagination(self): + + metric_list_pages = [[{u'dimensions': {}, + u'measurements': [ + [u'2015-04-14T17:52:31Z', + 1.0, {}]], + u'id': u'2015-04-14T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test1'}], + [{u'dimensions': {}, + u'measurements': [ + [u'2015-04-15T17:52:31Z', + 2.0, {}]], + u'id': u'2015-04-15T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test2'}], None] + + expected_page_count = len(metric_list_pages) + expected_metric_names = ["test1", "test2"] + + self.conf.set_override('enable_api_pagination', + True, group='monasca') + # get a new ceilosca mc + mc = self._get_client() + with mock.patch.object( + mc._mon_client.metrics, 'list', + side_effect=metric_list_pages) as mocked_metrics_list: + returned_metrics = mc.metrics_list() + returned_metric_names_list = [metric["name"] + for metric in returned_metrics] + self.assertListEqual(expected_metric_names, + returned_metric_names_list) + self.assertEqual(expected_page_count, + mocked_metrics_list.call_count) + self.assertEqual(True, mocked_metrics_list.called) + + def test_metrics_list_without_pagination(self): + + metric_list_pages = [[{u'dimensions': {}, + u'measurements': [ + [u'2015-04-14T17:52:31Z', + 1.0, {}]], + u'id': u'2015-04-14T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test1'}], + [{u'dimensions': {}, + u'measurements': [ + [u'2015-04-15T17:52:31Z', + 2.0, {}]], + u'id': u'2015-04-15T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test2'}], None] + + # first page only + expected_page_count = 1 + expected_metric_names = ["test1"] + + self.conf.set_override('enable_api_pagination', + False, group='monasca') + # get a new ceilosca mc + mc = self._get_client() + with mock.patch.object( + mc._mon_client.metrics, 'list', + side_effect=metric_list_pages) as mocked_metrics_list: + returned_metrics = mc.metrics_list() + returned_metric_names_list = [metric["name"] + for metric in returned_metrics] + self.assertListEqual(expected_metric_names, + returned_metric_names_list) + self.assertEqual(expected_page_count, + mocked_metrics_list.call_count) + self.assertEqual(True, mocked_metrics_list.called) + + def test_measurement_list_with_pagination(self): + + measurement_list_pages = [[{u'dimensions': {}, + u'measurements': [ + [u'2015-04-14T17:52:31Z', + 1.0, {}]], + u'id': u'2015-04-14T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test1'}], + [{u'dimensions': {}, + u'measurements': [ + [u'2015-04-15T17:52:31Z', + 2.0, {}]], + u'id': u'2015-04-15T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test2'}], None] + + expected_page_count = len(measurement_list_pages) + expected_metric_names = ["test1", "test2"] + + self.conf.set_override('enable_api_pagination', + True, group='monasca') + # get a new ceilosca mc + mc = self._get_client() + with mock.patch.object( + mc._mon_client.metrics, 'list_measurements', + side_effect=measurement_list_pages) as mocked_metrics_list: + returned_metrics = mc.measurements_list() + returned_metric_names_list = [metric["name"] + for metric in returned_metrics] + self.assertListEqual(expected_metric_names, + returned_metric_names_list) + self.assertEqual(expected_page_count, + mocked_metrics_list.call_count) + self.assertEqual(True, mocked_metrics_list.called) + + def test_measurement_list_without_pagination(self): + + measurement_list_pages = [[{u'dimensions': {}, + u'measurements': [ + [u'2015-04-14T17:52:31Z', + 1.0, {}]], + u'id': u'2015-04-14T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test1'}], + [{u'dimensions': {}, + u'measurements': [ + [u'2015-04-15T17:52:31Z', + 2.0, {}]], + u'id': u'2015-04-15T18:42:31Z', + u'columns': [u'timestamp', u'value', + u'value_meta'], + u'name': u'test2'}], None] + + # first page only + expected_page_count = 1 + expected_metric_names = ["test1"] + + self.conf.set_override('enable_api_pagination', + False, group='monasca') + # get a new ceilosca mc + mc = self._get_client() + with mock.patch.object( + mc._mon_client.metrics, 'list_measurements', + side_effect=measurement_list_pages) as mocked_metrics_list: + returned_metrics = mc.measurements_list() + returned_metric_names_list = [metric["name"] + for metric in returned_metrics] + self.assertListEqual(expected_metric_names, + returned_metric_names_list) + self.assertEqual(expected_page_count, + mocked_metrics_list.call_count) + self.assertEqual(True, mocked_metrics_list.called) + + def test_statistics_list_with_pagination(self): + + statistics_list_pages = [[{u'dimensions': {}, + u'statistics': [ + [u'2015-04-14T17:52:31Z', + 1.0, 10.0], + [u'2015-04-15T17:52:31Z', + 1.0, 10.0]], + u'id': u'2015-04-14T18:42:31Z', + u'columns': [u'timestamp', u'avg', + u'max'], + u'name': u'test1'}], + [{u'dimensions': {}, + u'statistics': [ + [u'2015-04-16T17:52:31Z', + 2.0, 20.0], + [u'2015-04-17T17:52:31Z', + 2.0, 20.0]], + u'id': u'2015-04-15T18:42:31Z', + u'columns': [u'timestamp', u'avg', + u'max'], + u'name': u'test2'}], None] + + expected_page_count = len(statistics_list_pages) + expected_metric_names = ["test1", "test2"] + + self.conf.set_override('enable_api_pagination', + True, group='monasca') + # get a new ceilosca mc + mc = self._get_client() + with mock.patch.object( + mc._mon_client.metrics, 'list_statistics', + side_effect=statistics_list_pages) as mocked_metrics_list: + + returned_metrics = mc.statistics_list() + returned_metric_names_list = [metric["name"] + for metric in returned_metrics] + self.assertListEqual(expected_metric_names, + returned_metric_names_list) + self.assertEqual(expected_page_count, + mocked_metrics_list.call_count) + self.assertEqual(True, mocked_metrics_list.called) + + def test_statistics_list_without_pagination(self): + + statistics_list_pages = [[{u'dimensions': {}, + u'statistics': [ + [u'2015-04-14T17:52:31Z', + 1.0, 10.0]], + u'id': u'2015-04-14T18:42:31Z', + u'columns': [u'timestamp', u'avg', + u'max'], + u'name': u'test1'}], + [{u'dimensions': {}, + u'statistics': [ + [u'2015-04-15T17:52:31Z', + 2.0, 20.0]], + u'id': u'2015-04-15T18:42:31Z', + u'columns': [u'timestamp', u'avg', + u'max'], + u'name': u'test2'}], None] + # first page only + expected_page_count = 1 + expected_metric_names = ["test1"] + + self.conf.set_override('enable_api_pagination', + False, group='monasca') + # get a new ceilosca mc + mc = self._get_client() + with mock.patch.object( + mc._mon_client.metrics, 'list_statistics', + side_effect=statistics_list_pages) as mocked_metrics_list: + returned_metrics = mc.statistics_list() + returned_metric_names_list = [metric["name"] + for metric in returned_metrics] + self.assertListEqual(expected_metric_names, + returned_metric_names_list) + self.assertEqual(expected_page_count, + mocked_metrics_list.call_count) + self.assertEqual(True, mocked_metrics_list.called)