diff --git a/releasenotes/notes/960265-optional-pool-fields-storage-xml-26701a4ce5e54f2b.yaml b/releasenotes/notes/960265-optional-pool-fields-storage-xml-26701a4ce5e54f2b.yaml new file mode 100644 index 000000000..af8cc6ce1 --- /dev/null +++ b/releasenotes/notes/960265-optional-pool-fields-storage-xml-26701a4ce5e54f2b.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed the errors showing in the decision engine logs when generating the + xml representation of the storage model when using some storage backends + other than lvms (like ceph or nfs). Those drivers do not report pool + fields like total_volumes or free_capacity_gb that the lvm driver does, + so assuming those fields are always reported by the cinder api calls is + wrong. + + See: https://bugs.launchpad.net/watcher/+bug/2121147 for more details. diff --git a/watcher/decision_engine/model/element/base.py b/watcher/decision_engine/model/element/base.py index 3cdc23b0e..f4c802a97 100644 --- a/watcher/decision_engine/model/element/base.py +++ b/watcher/decision_engine/model/element/base.py @@ -50,16 +50,19 @@ class Element(base.WatcherObject, base.WatcherObjectDictCompat, def as_xml_element(self): sorted_fieldmap = [] + element_name = self.__class__.__name__ for field in self.fields: try: value = str(self[field]) sorted_fieldmap.append((field, value)) + except NotImplementedError: + LOG.debug("Attribute %s for object %s: %s is not provided", + field, element_name, self) except Exception as exc: LOG.exception(exc) attrib = collections.OrderedDict(sorted_fieldmap) - element_name = self.__class__.__name__ instance_el = etree.Element(element_name, attrib=attrib) return instance_el diff --git a/watcher/decision_engine/model/model_root.py b/watcher/decision_engine/model/model_root.py index 9de819c77..ebfeb5203 100644 --- a/watcher/decision_engine/model/model_root.py +++ b/watcher/decision_engine/model/model_root.py @@ -268,7 +268,14 @@ class ModelRoot(nx.DiGraph, base.Model): in_dict = {} for field in cn.fields: new_name = "node_"+str(field) - in_dict[new_name] = cn[field] + try: + in_dict[new_name] = cn[field] + # NotImplementedError means field not assigned + # which is really not an issue as it can be an + # optional field + except NotImplementedError: + LOG.debug("Attribute %s for Compute: %s is not provided", + field, cn.hostname) node_instances = self.get_node_instances(cn) if not node_instances: deep_in_dict = in_dict.copy() @@ -277,7 +284,11 @@ class ModelRoot(nx.DiGraph, base.Model): for instance in sorted(node_instances, key=lambda x: x.uuid): for field in instance.fields: new_name = "server_"+str(field) - in_dict[new_name] = instance[field] + try: + in_dict[new_name] = instance[field] + except NotImplementedError: + LOG.debug("Attribute %s for Instance: %s is not " + "provided", field, instance.uuid) if in_dict != {}: deep_in_dict = in_dict.copy() ret_list.append(deep_in_dict) diff --git a/watcher/tests/unit/decision_engine/model/test_model.py b/watcher/tests/unit/decision_engine/model/test_model.py index 952d103f5..7d67e9da6 100644 --- a/watcher/tests/unit/decision_engine/model/test_model.py +++ b/watcher/tests/unit/decision_engine/model/test_model.py @@ -65,18 +65,21 @@ class TestModel(base.TestCase): @mock.patch.object(model_root.ModelRoot, 'get_all_compute_nodes') @mock.patch.object(model_root.ModelRoot, 'get_node_instances') - def test_get_model_to_list(self, mock_instances, mock_nodes): - fake_compute_node = mock.MagicMock( - uuid='fake_node_uuid', - fields=['uuid']) - fake_instance = mock.MagicMock( - uuid='fake_instance_uuid', - fields=['uuid']) + @mock.patch('watcher.decision_engine.model.model_root.LOG') + def test_get_model_to_list(self, mock_log, mock_instances, mock_nodes): + fake_compute_node = element.ComputeNode( + uuid='fake_node_uuid', hostname="hostname0") + fake_instance = element.Instance( + uuid='fake_instance_uuid') mock_nodes.return_value = {'fake_node_uuid': fake_compute_node} mock_instances.return_value = [fake_instance] - expected_keys = ['server_uuid', 'node_uuid'] + expected_keys = [ + 'server_uuid', 'node_uuid', 'node_state', 'node_hostname', + 'node_status', 'server_flavor_extra_specs', 'server_locked', + 'server_pinned_az', 'server_state', 'server_watcher_exclude' + ] result = model_root.ModelRoot().to_list() self.assertEqual(1, len(result)) @@ -84,10 +87,20 @@ class TestModel(base.TestCase): result_keys = result[0].keys() self.assertEqual(sorted(expected_keys), sorted(result_keys)) + mock_log.debug.assert_any_call( + 'Attribute %s for Instance: %s is not provided', 'name', + 'fake_instance_uuid' + ) + mock_log.debug.assert_any_call( + 'Attribute %s for Compute: %s is not provided', 'memory', + 'hostname0' + ) + # test compute node has no instance mock_instances.return_value = [] - expected_keys = ['node_uuid'] + expected_keys = ['node_hostname', 'node_status', + 'node_state', 'node_uuid'] result = model_root.ModelRoot().to_list() self.assertEqual(1, len(result)) @@ -95,6 +108,25 @@ class TestModel(base.TestCase): result_keys = result[0].keys() self.assertEqual(expected_keys, list(result_keys)) + def test_model_to_list_all_fields(self): + struct_str = self.load_data('scenario_1.xml') + model = model_root.ModelRoot.from_xml(struct_str) + model_list = model.to_list() + self.assertIsInstance(model_list, list) + self.assertEqual(len(model_list), 8) + self.assertEqual( + model_list[0]['server_uuid'], + 'd000ef1f-dc19-4982-9383-087498bfde03') + + def test_model_to_xml_all_fields(self): + struct_str = self.load_data('scenario_2_with_metrics.xml') + + model = model_root.ModelRoot.from_xml(struct_str) + model_xml = model.to_xml() + self.assertIsInstance(model_xml, str) + self.assertIn('Instance', model_xml) + self.assertIn('Compute', model_xml) + def test_get_node_by_instance_uuid(self): model = model_root.ModelRoot() uuid_ = f"{uuidutils.generate_uuid()}"