From 933bc59b3912b5bac1550c5260974d83f38033ff Mon Sep 17 00:00:00 2001 From: Dantali0n Date: Thu, 11 Jul 2019 20:38:35 +0200 Subject: [PATCH] Baseclass for ModelBuilder with audit scope This lets all the ModelBuilder classes use one baseclass and forces ClusterDataModelCollector's to pass the scope. The scopes are still unused in the case of Ironic and Cinder. The idea is to do several follow ups to this and in the end have a similar method to query_retry in the datasources baseclass. Change-Id: Ibbdedd3087fef5298d7f4c9d3abdba05d1fbb2f0 --- .../decision_engine/model/collector/base.py | 14 +- .../decision_engine/model/collector/cinder.py | 19 +- .../decision_engine/model/collector/ironic.py | 20 +- .../decision_engine/model/collector/nova.py | 5 +- .../cluster/test_cinder_cdmc.py | 2 + .../cluster/test_model_builder.py | 233 ------------------ .../decision_engine/cluster/test_nova_cdmc.py | 196 ++++++++++++++- 7 files changed, 241 insertions(+), 248 deletions(-) delete mode 100644 watcher/tests/decision_engine/cluster/test_model_builder.py diff --git a/watcher/decision_engine/model/collector/base.py b/watcher/decision_engine/model/collector/base.py index 055d618db..9016054e7 100644 --- a/watcher/decision_engine/model/collector/base.py +++ b/watcher/decision_engine/model/collector/base.py @@ -126,9 +126,9 @@ class BaseClusterDataModelCollector(loadable.LoadableSingleton): def __init__(self, config, osc=None): super(BaseClusterDataModelCollector, self).__init__(config) self.osc = osc if osc else clients.OpenStackClients() - self._cluster_data_model = None self.lock = threading.RLock() self._audit_scope_handler = None + self._cluster_data_model = None self._data_model_scope = None @property @@ -190,3 +190,15 @@ class BaseClusterDataModelCollector(loadable.LoadableSingleton): with the existing cluster data model """ self.cluster_data_model = self.execute() + + +class BaseModelBuilder(object): + + @abc.abstractmethod + def execute(self, model_scope): + """Build the cluster data model limited to the scope and return it + + Builds the cluster data model with respect to the supplied scope. The + schema of this scope will depend on the type of ModelBuilder. + """ + raise NotImplementedError() diff --git a/watcher/decision_engine/model/collector/cinder.py b/watcher/decision_engine/model/collector/cinder.py index c1fa508e4..f6a774b8c 100644 --- a/watcher/decision_engine/model/collector/cinder.py +++ b/watcher/decision_engine/model/collector/cinder.py @@ -137,17 +137,27 @@ class CinderClusterDataModelCollector(base.BaseClusterDataModelCollector): def get_audit_scope_handler(self, audit_scope): self._audit_scope_handler = storage_scope.StorageScope( audit_scope, self.config) + if self._data_model_scope is None or ( + len(self._data_model_scope) > 0 and ( + self._data_model_scope != audit_scope)): + self._data_model_scope = audit_scope + self._cluster_data_model = None + LOG.debug("audit scope %s", audit_scope) return self._audit_scope_handler def execute(self): """Build the storage cluster data model""" LOG.debug("Building latest Cinder cluster data model") - builder = ModelBuilder(self.osc) - return builder.execute() + if self._audit_scope_handler is None: + LOG.debug("No audit, Don't Build storage data model") + return + + builder = CinderModelBuilder(self.osc) + return builder.execute(self._data_model_scope) -class ModelBuilder(object): +class CinderModelBuilder(base.BaseModelBuilder): """Build the graph-based model This model builder adds the following data" @@ -292,12 +302,13 @@ class ModelBuilder(object): return element.Volume(**volume_attributes) - def execute(self): + def execute(self, model_scope): """Instantiates the graph with the openstack cluster data. The graph is populated along 2 layers: virtual and physical. As each new layer is built connections are made back to previous layers. """ + # TODO(Dantali0n): Use scope to limit size of model self._add_physical_layer() self._add_virtual_layer() return self.model diff --git a/watcher/decision_engine/model/collector/ironic.py b/watcher/decision_engine/model/collector/ironic.py index dccad8287..4ac3ec09c 100644 --- a/watcher/decision_engine/model/collector/ironic.py +++ b/watcher/decision_engine/model/collector/ironic.py @@ -48,17 +48,27 @@ class BaremetalClusterDataModelCollector(base.BaseClusterDataModelCollector): def get_audit_scope_handler(self, audit_scope): self._audit_scope_handler = baremetal_scope.BaremetalScope( audit_scope, self.config) + if self._data_model_scope is None or ( + len(self._data_model_scope) > 0 and ( + self._data_model_scope != audit_scope)): + self._data_model_scope = audit_scope + self._cluster_data_model = None + LOG.debug("audit scope %s", audit_scope) return self._audit_scope_handler def execute(self): """Build the baremetal cluster data model""" LOG.debug("Building latest Baremetal cluster data model") - builder = ModelBuilder(self.osc) - return builder.execute() + if self._audit_scope_handler is None: + LOG.debug("No audit, Don't Build Baremetal data model") + return + + builder = BareMetalModelBuilder(self.osc) + return builder.execute(self._data_model_scope) -class ModelBuilder(object): +class BareMetalModelBuilder(base.BaseModelBuilder): """Build the graph-based model This model builder adds the following data" @@ -93,8 +103,8 @@ class ModelBuilder(object): ironic_node = element.IronicNode(**node_attributes) return ironic_node - def execute(self): - + def execute(self, model_scope): + # TODO(Dantali0n): Use scope to limit size of model for node in self.ironic_helper.get_ironic_node_list(): self.add_ironic_node(node) return self.model diff --git a/watcher/decision_engine/model/collector/nova.py b/watcher/decision_engine/model/collector/nova.py index e1b2c4a07..9d408d81a 100644 --- a/watcher/decision_engine/model/collector/nova.py +++ b/watcher/decision_engine/model/collector/nova.py @@ -180,17 +180,16 @@ class NovaClusterDataModelCollector(base.BaseClusterDataModelCollector): LOG.debug("No audit, Don't Build compute data model") return - builder = ModelBuilder(self.osc) + builder = NovaModelBuilder(self.osc) return builder.execute(self._data_model_scope) -class ModelBuilder(object): +class NovaModelBuilder(base.BaseModelBuilder): """Build the graph-based model This model builder adds the following data" - Compute-related knowledge (Nova) - - TODO(v-francoise): Storage-related knowledge (Cinder) - TODO(v-francoise): Network-related knowledge (Neutron) NOTE(v-francoise): This model builder is meant to be extended in the future diff --git a/watcher/tests/decision_engine/cluster/test_cinder_cdmc.py b/watcher/tests/decision_engine/cluster/test_cinder_cdmc.py index e539b2bfc..3d197e066 100644 --- a/watcher/tests/decision_engine/cluster/test_cinder_cdmc.py +++ b/watcher/tests/decision_engine/cluster/test_cinder_cdmc.py @@ -85,6 +85,7 @@ class TestCinderClusterDataModelCollector(base.TestCase): cinder_cdmc = cinder.CinderClusterDataModelCollector( config=m_config, osc=m_osc) + cinder_cdmc.get_audit_scope_handler([]) model = cinder_cdmc.execute() storage_nodes = model.get_all_storage_nodes() @@ -149,5 +150,6 @@ class TestCinderClusterDataModelCollector(base.TestCase): cinder_cdmc = cinder.CinderClusterDataModelCollector( config=m_config, osc=m_osc) + cinder_cdmc.get_audit_scope_handler([]) self.assertRaises(exception.InvalidPoolAttributeValue, cinder_cdmc.execute) diff --git a/watcher/tests/decision_engine/cluster/test_model_builder.py b/watcher/tests/decision_engine/cluster/test_model_builder.py deleted file mode 100644 index bb18d2b8e..000000000 --- a/watcher/tests/decision_engine/cluster/test_model_builder.py +++ /dev/null @@ -1,233 +0,0 @@ -# -*- encoding: utf-8 -*- -# Copyright (c) 2019 European Organization for Nuclear Research (CERN) -# -# Authors: Corne Lukken -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or -# implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import mock - -from oslo_config import cfg -from oslo_log import log - -from watcher.common import nova_helper -from watcher.decision_engine.model.collector import nova -from watcher.tests import base - -CONF = cfg.CONF -LOG = log.getLogger(__name__) - - -class TestModelBuilder(base.BaseTestCase): - """Test the collector ModelBuilder - - Objects under test are preceded with t_ and mocked objects are preceded - with m_ , additionally, patched objects are preceded with p_ no object - under test should be created in setUp this can influence the results. - """ - - def setUp(self): - super(TestModelBuilder, self).setUp() - - def test_check_model(self): - """Initialize collector ModelBuilder and test check model""" - - m_scope = [{"compute": [ - {"host_aggregates": [{"id": 5}]}, - {"availability_zones": [{"name": "av_a"}]} - ]}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - self.assertTrue(t_nova_cluster._check_model_scope(m_scope)) - - def test_check_model_update_false(self): - """Initialize check model with multiple identical scopes - - The seconds check_model should return false as the models are the same - """ - - m_scope = [{"compute": [ - {"host_aggregates": [{"id": 5}]}, - {"availability_zones": [{"name": "av_a"}]} - ]}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - self.assertTrue(t_nova_cluster._check_model_scope(m_scope)) - self.assertFalse(t_nova_cluster._check_model_scope(m_scope)) - - def test_check_model_update_true(self): - """Initialize check model with multiple different scopes - - Since the models differ both should return True for the update flag - """ - - m_scope_one = [{"compute": [ - {"host_aggregates": [{"id": 5}]}, - {"availability_zones": [{"name": "av_a"}]} - ]}] - - m_scope_two = [{"compute": [ - {"host_aggregates": [{"id": 2}]}, - {"availability_zones": [{"name": "av_b"}]} - ]}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - self.assertTrue(t_nova_cluster._check_model_scope(m_scope_one)) - self.assertTrue(t_nova_cluster._check_model_scope(m_scope_two)) - - def test_merge_compute_scope(self): - """""" - - m_scope_one = [ - {"host_aggregates": [{"id": 5}]}, - {"availability_zones": [{"name": "av_a"}]} - ] - - m_scope_two = [ - {"host_aggregates": [{"id": 4}]}, - {"availability_zones": [{"name": "av_b"}]} - ] - - reference = {'availability_zones': - [{'name': 'av_a'}, {'name': 'av_b'}], - 'host_aggregates': - [{'id': 5}, {'id': 4}]} - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - t_nova_cluster._merge_compute_scope(m_scope_one) - t_nova_cluster._merge_compute_scope(m_scope_two) - - self.assertEqual(reference, t_nova_cluster.model_scope) - - @mock.patch.object(nova_helper, 'NovaHelper') - def test_collect_aggregates(self, m_nova): - """""" - - m_nova.return_value.get_aggregate_list.return_value = \ - [mock.Mock(id=1, name='example'), - mock.Mock(id=5, name='example', hosts=['hostone', 'hosttwo'])] - - m_nova.return_value.get_compute_node_by_name.return_value = False - - m_scope = [{'id': 5}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - result = set() - t_nova_cluster._collect_aggregates(m_scope, result) - - self.assertEqual(set(['hostone', 'hosttwo']), result) - - @mock.patch.object(nova_helper, 'NovaHelper') - def test_collect_zones(self, m_nova): - """""" - - m_nova.return_value.get_service_list.return_value = \ - [mock.Mock(zone='av_b'), - mock.Mock(zone='av_a', host='hostone')] - - m_nova.return_value.get_compute_node_by_name.return_value = False - - m_scope = [{'name': 'av_a'}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - result = set() - t_nova_cluster._collect_zones(m_scope, result) - - self.assertEqual(set(['hostone']), result) - - @mock.patch.object(nova_helper, 'NovaHelper') - def test_add_physical_layer(self, m_nova): - """""" - - m_nova.return_value.get_aggregate_list.return_value = \ - [mock.Mock(id=1, name='example'), - mock.Mock(id=5, name='example', hosts=['hostone', 'hosttwo'])] - - m_nova.return_value.get_service_list.return_value = \ - [mock.Mock(zone='av_b', host='hostthree'), - mock.Mock(zone='av_a', host='hostone')] - - m_nova.return_value.get_compute_node_by_name.return_value = False - - m_scope = [{"compute": [ - {"host_aggregates": [{"id": 5}]}, - {"availability_zones": [{"name": "av_a"}]} - ]}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - t_nova_cluster.execute(m_scope) - m_nova.return_value.get_compute_node_by_name.assert_any_call( - 'hostone', servers=True, detailed=True) - m_nova.return_value.get_compute_node_by_name.assert_any_call( - 'hosttwo', servers=True, detailed=True) - self.assertEqual( - m_nova.return_value.get_compute_node_by_name.call_count, 2) - - @mock.patch.object(nova_helper, 'NovaHelper') - def test_add_physical_layer_with_baremetal_node(self, m_nova): - """""" - - m_nova.return_value.get_aggregate_list.return_value = \ - [mock.Mock(id=1, name='example'), - mock.Mock(id=5, name='example', hosts=['hostone', 'hosttwo'])] - - m_nova.return_value.get_service_list.return_value = \ - [mock.Mock(zone='av_b', host='hostthree'), - mock.Mock(zone='av_a', host='hostone')] - - compute_node = mock.Mock( - id='796fee99-65dd-4262-aa-fd2a1143faa6', - hypervisor_hostname='hostone', - hypervisor_type='QEMU', - state='TEST_STATE', - status='TEST_STATUS', - memory_mb=333, - free_disk_gb=222, - local_gb=111, - vcpus=4, - servers=[ - {'name': 'fake_instance', - 'uuid': 'ef500f7e-dac8-470f-960c-169486fce71b'} - ], - service={'id': 123, 'host': 'hostone', - 'disabled_reason': ''}, - ) - - baremetal_node = mock.Mock( - id='5f2d1b3d-4099-4623-b9-05148aefd6cb', - hypervisor_hostname='hosttwo', - hypervisor_type='ironic', - state='TEST_STATE', - status='TEST_STATUS', - ) - - m_nova.return_value.get_compute_node_by_name.side_effect = [ - [compute_node], [baremetal_node]] - - m_scope = [{"compute": [ - {"host_aggregates": [{"id": 5}]}, - {"availability_zones": [{"name": "av_a"}]} - ]}] - - t_nova_cluster = nova.ModelBuilder(mock.Mock()) - model = t_nova_cluster.execute(m_scope) - - compute_nodes = model.get_all_compute_nodes() - self.assertEqual(1, len(compute_nodes)) - m_nova.return_value.get_compute_node_by_name.assert_any_call( - 'hostone', servers=True, detailed=True) - m_nova.return_value.get_compute_node_by_name.assert_any_call( - 'hosttwo', servers=True, detailed=True) - self.assertEqual( - m_nova.return_value.get_compute_node_by_name.call_count, 2) diff --git a/watcher/tests/decision_engine/cluster/test_nova_cdmc.py b/watcher/tests/decision_engine/cluster/test_nova_cdmc.py index 55df67aec..80b1a1056 100644 --- a/watcher/tests/decision_engine/cluster/test_nova_cdmc.py +++ b/watcher/tests/decision_engine/cluster/test_nova_cdmc.py @@ -119,11 +119,11 @@ class TestNovaClusterDataModelCollector(base.TestCase): filters={'host': fake_compute_node.service['host']}, limit=1) -class TestModelBuilder(base.TestCase): +class TestNovaModelBuilder(base.TestCase): @mock.patch.object(nova_helper, 'NovaHelper', mock.MagicMock()) def test_add_instance_node(self): - model_builder = nova.ModelBuilder(osc=mock.MagicMock()) + model_builder = nova.NovaModelBuilder(osc=mock.MagicMock()) model_builder.model = mock.MagicMock() mock_node = mock.MagicMock() mock_host = mock_node.service["host"] @@ -139,3 +139,195 @@ class TestModelBuilder(base.TestCase): model_builder.add_instance_node(mock_node, mock_instances) model_builder.nova_helper.get_instance_list.assert_called_with( filters={'host': mock_host}, limit=-1) + + def test_check_model(self): + """Initialize collector ModelBuilder and test check model""" + + m_scope = [{"compute": [ + {"host_aggregates": [{"id": 5}]}, + {"availability_zones": [{"name": "av_a"}]} + ]}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + self.assertTrue(t_nova_cluster._check_model_scope(m_scope)) + + def test_check_model_update_false(self): + """Initialize check model with multiple identical scopes + + The seconds check_model should return false as the models are the same + """ + + m_scope = [{"compute": [ + {"host_aggregates": [{"id": 5}]}, + {"availability_zones": [{"name": "av_a"}]} + ]}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + self.assertTrue(t_nova_cluster._check_model_scope(m_scope)) + self.assertFalse(t_nova_cluster._check_model_scope(m_scope)) + + def test_check_model_update_true(self): + """Initialize check model with multiple different scopes + + Since the models differ both should return True for the update flag + """ + + m_scope_one = [{"compute": [ + {"host_aggregates": [{"id": 5}]}, + {"availability_zones": [{"name": "av_a"}]} + ]}] + + m_scope_two = [{"compute": [ + {"host_aggregates": [{"id": 2}]}, + {"availability_zones": [{"name": "av_b"}]} + ]}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + self.assertTrue(t_nova_cluster._check_model_scope(m_scope_one)) + self.assertTrue(t_nova_cluster._check_model_scope(m_scope_two)) + + def test_merge_compute_scope(self): + """""" + + m_scope_one = [ + {"host_aggregates": [{"id": 5}]}, + {"availability_zones": [{"name": "av_a"}]} + ] + + m_scope_two = [ + {"host_aggregates": [{"id": 4}]}, + {"availability_zones": [{"name": "av_b"}]} + ] + + reference = {'availability_zones': + [{'name': 'av_a'}, {'name': 'av_b'}], + 'host_aggregates': + [{'id': 5}, {'id': 4}]} + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + t_nova_cluster._merge_compute_scope(m_scope_one) + t_nova_cluster._merge_compute_scope(m_scope_two) + + self.assertEqual(reference, t_nova_cluster.model_scope) + + @mock.patch.object(nova_helper, 'NovaHelper') + def test_collect_aggregates(self, m_nova): + """""" + + m_nova.return_value.get_aggregate_list.return_value = \ + [mock.Mock(id=1, name='example'), + mock.Mock(id=5, name='example', hosts=['hostone', 'hosttwo'])] + + m_nova.return_value.get_compute_node_by_name.return_value = False + + m_scope = [{'id': 5}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + result = set() + t_nova_cluster._collect_aggregates(m_scope, result) + + self.assertEqual(set(['hostone', 'hosttwo']), result) + + @mock.patch.object(nova_helper, 'NovaHelper') + def test_collect_zones(self, m_nova): + """""" + + m_nova.return_value.get_service_list.return_value = \ + [mock.Mock(zone='av_b'), + mock.Mock(zone='av_a', host='hostone')] + + m_nova.return_value.get_compute_node_by_name.return_value = False + + m_scope = [{'name': 'av_a'}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + result = set() + t_nova_cluster._collect_zones(m_scope, result) + + self.assertEqual(set(['hostone']), result) + + @mock.patch.object(nova_helper, 'NovaHelper') + def test_add_physical_layer(self, m_nova): + """""" + + m_nova.return_value.get_aggregate_list.return_value = \ + [mock.Mock(id=1, name='example'), + mock.Mock(id=5, name='example', hosts=['hostone', 'hosttwo'])] + + m_nova.return_value.get_service_list.return_value = \ + [mock.Mock(zone='av_b', host='hostthree'), + mock.Mock(zone='av_a', host='hostone')] + + m_nova.return_value.get_compute_node_by_name.return_value = False + + m_scope = [{"compute": [ + {"host_aggregates": [{"id": 5}]}, + {"availability_zones": [{"name": "av_a"}]} + ]}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + t_nova_cluster.execute(m_scope) + m_nova.return_value.get_compute_node_by_name.assert_any_call( + 'hostone', servers=True, detailed=True) + m_nova.return_value.get_compute_node_by_name.assert_any_call( + 'hosttwo', servers=True, detailed=True) + self.assertEqual( + m_nova.return_value.get_compute_node_by_name.call_count, 2) + + @mock.patch.object(nova_helper, 'NovaHelper') + def test_add_physical_layer_with_baremetal_node(self, m_nova): + """""" + + m_nova.return_value.get_aggregate_list.return_value = \ + [mock.Mock(id=1, name='example'), + mock.Mock(id=5, name='example', hosts=['hostone', 'hosttwo'])] + + m_nova.return_value.get_service_list.return_value = \ + [mock.Mock(zone='av_b', host='hostthree'), + mock.Mock(zone='av_a', host='hostone')] + + compute_node = mock.Mock( + id='796fee99-65dd-4262-aa-fd2a1143faa6', + hypervisor_hostname='hostone', + hypervisor_type='QEMU', + state='TEST_STATE', + status='TEST_STATUS', + memory_mb=333, + free_disk_gb=222, + local_gb=111, + vcpus=4, + servers=[ + {'name': 'fake_instance', + 'uuid': 'ef500f7e-dac8-470f-960c-169486fce71b'} + ], + service={'id': 123, 'host': 'hostone', + 'disabled_reason': ''}, + ) + + baremetal_node = mock.Mock( + id='5f2d1b3d-4099-4623-b9-05148aefd6cb', + hypervisor_hostname='hosttwo', + hypervisor_type='ironic', + state='TEST_STATE', + status='TEST_STATUS', + ) + + m_nova.return_value.get_compute_node_by_name.side_effect = [ + [compute_node], [baremetal_node]] + + m_scope = [{"compute": [ + {"host_aggregates": [{"id": 5}]}, + {"availability_zones": [{"name": "av_a"}]} + ]}] + + t_nova_cluster = nova.NovaModelBuilder(mock.Mock()) + model = t_nova_cluster.execute(m_scope) + + compute_nodes = model.get_all_compute_nodes() + self.assertEqual(1, len(compute_nodes)) + m_nova.return_value.get_compute_node_by_name.assert_any_call( + 'hostone', servers=True, detailed=True) + m_nova.return_value.get_compute_node_by_name.assert_any_call( + 'hosttwo', servers=True, detailed=True) + self.assertEqual( + m_nova.return_value.get_compute_node_by_name.call_count, 2)