Fix typo in ceilometer datasource

Change I25b4cb0e1b85379ff0c4da9d0c1474380d75ce3a in
Queens refactored the statistic_aggregation method
and renamed the "aggregate" kwarg to "aggregation",
presumably to match the signature of the GnocchiHelper
statistic_aggregation method (the commit message does
not give details) so a base method could be added to
the parent class for all datasource helpers.

As a result, the CeilometerHelper calls to its
statistic_aggregation started passing the new
"granularity" kwarg but failed to match the rename
to the "aggregation" kwarg, which breaks the
CeilometerHelper. This was missed by the unit tests
because the tests were just asserting the erroneous
call that the runtime code made.

This change fixes the kwarg typo and makes the
tests more robust by using the mock spec kwarg
to define a spec for the statistic_aggregation
mock so that it must be called with the correct
parameters defined in the method signature. The
test is refactored to reduce duplicate mocking.
The same test hardening can and should be done
in the gnocchi and monasca helper tests but that
should be done in a separate change.

Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com>

Closes-Bug: #1829542

Change-Id: Idfd099f718873d9056fdc35a97954771c9ae5762
This commit is contained in:
Zhenyu Zheng
2019-05-17 15:01:37 +08:00
committed by Matt Riedemann
parent ce1d64a16c
commit f92f77f683
2 changed files with 76 additions and 105 deletions

View File

@@ -232,64 +232,64 @@ class CeilometerHelper(base.DataSourceBase):
granularity=None):
meter_name = self.METRIC_MAP.get('host_cpu_usage')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_instance_cpu_usage(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('instance_cpu_usage')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_host_memory_usage(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('host_memory_usage')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_instance_ram_usage(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('instance_ram_usage')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_instance_l3_cache_usage(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('instance_l3_cache_usage')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_instance_ram_allocated(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('instance_ram_allocated')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_instance_root_disk_size(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('instance_root_disk_size')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_host_outlet_temp(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('host_outlet_temp')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_host_inlet_temp(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('host_inlet_temp')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_host_airflow(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('host_airflow')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)
def get_host_power(self, resource_id, period, aggregate,
granularity=None):
meter_name = self.METRIC_MAP.get('host_power')
return self.statistic_aggregation(resource_id, meter_name, period,
granularity, aggregate=aggregate)
granularity, aggregation=aggregate)

View File

@@ -27,6 +27,16 @@ from watcher.tests import base
@mock.patch.object(clients.OpenStackClients, 'ceilometer')
class TestCeilometerHelper(base.BaseTestCase):
def setUp(self):
super(TestCeilometerHelper, self).setUp()
self.osc_mock = mock.Mock()
self.helper = ceilometer_helper.CeilometerHelper(osc=self.osc_mock)
stat_agg_patcher = mock.patch.object(
self.helper, 'statistic_aggregation',
spec=ceilometer_helper.CeilometerHelper.statistic_aggregation)
self.mock_aggregation = stat_agg_patcher.start()
self.addCleanup(stat_agg_patcher.stop)
def test_build_query(self, mock_ceilometer):
mock_ceilometer.return_value = mock.MagicMock()
cm = ceilometer_helper.CeilometerHelper()
@@ -95,110 +105,71 @@ class TestCeilometerHelper(base.BaseTestCase):
val = cm.statistic_list(meter_name="cpu_util")
self.assertEqual(expected_value, val)
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_host_cpu_usage(self, mock_aggregation, mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_host_cpu_usage('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['host_cpu_usage'], 600, None,
aggregate='mean')
def test_get_host_cpu_usage(self, mock_ceilometer):
self.helper.get_host_cpu_usage('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['host_cpu_usage'], 600, None,
aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_instance_cpu_usage(self, mock_aggregation, mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_instance_cpu_usage('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['instance_cpu_usage'], 600, None,
aggregate='mean')
def test_get_instance_cpu_usage(self, mock_ceilometer):
self.helper.get_instance_cpu_usage('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['instance_cpu_usage'], 600,
None, aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_host_memory_usage(self, mock_aggregation, mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_host_memory_usage('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['host_memory_usage'], 600, None,
aggregate='mean')
def test_get_host_memory_usage(self, mock_ceilometer):
self.helper.get_host_memory_usage('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['host_memory_usage'], 600, None,
aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_instance_memory_usage(self, mock_aggregation,
mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_instance_ram_usage('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['instance_ram_usage'], 600, None,
aggregate='mean')
def test_get_instance_memory_usage(self, mock_ceilometer):
self.helper.get_instance_ram_usage('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['instance_ram_usage'], 600,
None, aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_instance_l3_cache_usage(self, mock_aggregation,
mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_instance_l3_cache_usage('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['instance_l3_cache_usage'], 600,
None, aggregate='mean')
def test_get_instance_l3_cache_usage(self, mock_ceilometer):
self.helper.get_instance_l3_cache_usage('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['instance_l3_cache_usage'], 600,
None, aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_instance_ram_allocated(self, mock_aggregation,
mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_instance_ram_allocated('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['instance_ram_allocated'], 600, None,
aggregate='mean')
def test_get_instance_ram_allocated(self, mock_ceilometer):
self.helper.get_instance_ram_allocated('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['instance_ram_allocated'], 600,
None, aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_instance_root_disk_allocated(self, mock_aggregation,
mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_instance_root_disk_size('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['instance_root_disk_size'], 600,
None, aggregate='mean')
def test_get_instance_root_disk_allocated(self, mock_ceilometer):
self.helper.get_instance_root_disk_size('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['instance_root_disk_size'], 600,
None, aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_host_outlet_temperature(self, mock_aggregation,
mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_host_outlet_temp('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['host_outlet_temp'], 600, None,
aggregate='mean')
def test_get_host_outlet_temperature(self, mock_ceilometer):
self.helper.get_host_outlet_temp('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['host_outlet_temp'], 600, None,
aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_host_inlet_temperature(self, mock_aggregation,
mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_host_inlet_temp('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['host_inlet_temp'], 600, None,
aggregate='mean')
def test_get_host_inlet_temperature(self, mock_ceilometer):
self.helper.get_host_inlet_temp('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['host_inlet_temp'], 600, None,
aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_host_airflow(self, mock_aggregation, mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_host_airflow('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['host_airflow'], 600, None,
aggregate='mean')
def test_get_host_airflow(self, mock_ceilometer):
self.helper.get_host_airflow('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['host_airflow'], 600, None,
aggregation='mean')
@mock.patch.object(ceilometer_helper.CeilometerHelper,
'statistic_aggregation')
def test_get_host_power(self, mock_aggregation, mock_ceilometer):
helper = ceilometer_helper.CeilometerHelper()
helper.get_host_power('compute1', 600, 'mean')
mock_aggregation.assert_called_once_with(
'compute1', helper.METRIC_MAP['host_power'], 600, None,
aggregate='mean')
def test_get_host_power(self, mock_ceilometer):
self.helper.get_host_power('compute1', 600, 'mean')
self.mock_aggregation.assert_called_once_with(
'compute1', self.helper.METRIC_MAP['host_power'], 600, None,
aggregation='mean')
def test_check_availability(self, mock_ceilometer):
ceilometer = mock.MagicMock()