Handle missing fields building storage model XML or list

When generating XML or list representations of the storage model, Watcher assumed that certain fields will always be present. Some fields, like 'total_volumes', are reported by the LVM driver but not by others such as the NFS driver, logging NotImplementedError exceptions that do not prevent the model being built but can be confusing for the user. Add specific exception handling for NotImplementedError in the as_xml_element() method and to_list(), allowing the code to gracefully skip missing fields and continue processing. When an optional field is not available, a debug message is logged indicating which attribute is missing. This approach aligns with the existing error handling in the Cinder collector[1] and ensures that audits can complete successfully regardless of which storage backend is in use. As a follow-up, we can remove fields in the Pool model[2] which are marked as optional in Cinder's volume driver interface[3] and may not be available from all drivers: - total_volumes: Number of volumes in the pool - free_capacity_gb: Free capacity in GB - provisioned_capacity_gb: Provisioned capacity in GB - allocated_capacity_gb: Allocated capacity in GB [1] https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/collector/cinder.py#L250 [2] https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/element/node.py#L87-L93 [3] https://github.com/openstack/cinder/blob/master/cinder/interface/volume_driver.py#L78 Closes-Bug: #2121147 Change-Id: Icce725c05c48b83f7bc4c6d573d3fbe759f5866b Signed-off-by: jgilaber <jgilaber@redhat.com>
2025-09-09 15:40:39 +02:00
parent 7df0931593
commit 7e7a31a1f7
4 changed files with 69 additions and 12 deletions
@@ -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.
@@ -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
@@ -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)
@@ -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()}"