Ironic: report 0 for vcpus/memory_mb/disk_gb resources

Ironic nodes should all be using resource classes for scheduling by now,
which means reporting CPU/RAM/disk isn't useful. Report these as zero so
they cannot be scheduled.

Since we now require resource classes, raise an exception in
update_provider_tree for any nodes that don't have a resource class set
on the node.

Change-Id: If2b8c1a76d7dbabbac7bb359c9e572cfed510800

Author: jimrollenhagen

nova/exception.py

@@ -2364,3 +2364,7 @@ def __init__(self, message=None, **kwargs):
         self.rc = results.get('rc')
         self.rs = results.get('rs')
         self.errmsg = results.get('errmsg')
+
+
+class NoResourceClass(NovaException):
+    msg_fmt = _("Resource class not found for Ironic node %(node)s.")

nova/tests/unit/virt/ironic/test_driver.py

@@ -286,12 +286,12 @@ def test__node_resource_with_instance_uuid(self):
         gotkeys = sorted(result.keys())
         self.assertEqual(wantkeys, gotkeys)
 
-        self.assertEqual(props['cpus'], result['vcpus'])
-        self.assertEqual(result['vcpus'], result['vcpus_used'])
-        self.assertEqual(props['memory_mb'], result['memory_mb'])
-        self.assertEqual(result['memory_mb'], result['memory_mb_used'])
-        self.assertEqual(props['local_gb'], result['local_gb'])
-        self.assertEqual(result['local_gb'], result['local_gb_used'])
+        self.assertEqual(0, result['vcpus'])
+        self.assertEqual(0, result['vcpus_used'])
+        self.assertEqual(0, result['memory_mb'])
+        self.assertEqual(0, result['memory_mb_used'])
+        self.assertEqual(0, result['local_gb'])
+        self.assertEqual(0, result['local_gb_used'])
         self.assertEqual(node_uuid, result['uuid'])
         self.assertEqual(node_uuid, result['hypervisor_hostname'])
         self.assertEqual(stats, result['stats'])
@@ -354,11 +354,11 @@ def test__node_resource_available(self):
             provision_state=ironic_states.AVAILABLE)
 
         result = self.driver._node_resource(node)
-        self.assertEqual(props['cpus'], result['vcpus'])
+        self.assertEqual(0, result['vcpus'])
         self.assertEqual(0, result['vcpus_used'])
-        self.assertEqual(props['memory_mb'], result['memory_mb'])
+        self.assertEqual(0, result['memory_mb'])
         self.assertEqual(0, result['memory_mb_used'])
-        self.assertEqual(props['local_gb'], result['local_gb'])
+        self.assertEqual(0, result['local_gb'])
         self.assertEqual(0, result['local_gb_used'])
         self.assertEqual(node_uuid, result['hypervisor_hostname'])
         self.assertEqual(stats, result['stats'])
@@ -374,12 +374,12 @@ def test__node_resource_unavailable_node_res(self, mock_res_unavail):
                 uuid=node_uuid, instance_uuid=None, properties=props)
 
         result = self.driver._node_resource(node)
-        self.assertEqual(props['cpus'], result['vcpus'])
-        self.assertEqual(result['vcpus'], result['vcpus_used'])
-        self.assertEqual(props['memory_mb'], result['memory_mb'])
-        self.assertEqual(result['memory_mb'], result['memory_mb_used'])
-        self.assertEqual(props['local_gb'], result['local_gb'])
-        self.assertEqual(result['local_gb'], result['local_gb_used'])
+        self.assertEqual(0, result['vcpus'])
+        self.assertEqual(0, result['vcpus_used'])
+        self.assertEqual(0, result['memory_mb'])
+        self.assertEqual(0, result['memory_mb_used'])
+        self.assertEqual(0, result['local_gb'])
+        self.assertEqual(0, result['local_gb_used'])
         self.assertEqual(node_uuid, result['hypervisor_hostname'])
         self.assertEqual(stats, result['stats'])
 
@@ -399,12 +399,12 @@ def test__node_resource_used_node_res(self, mock_res_used):
             instance_info=instance_info)
 
         result = self.driver._node_resource(node)
-        self.assertEqual(props['cpus'], result['vcpus'])
-        self.assertEqual(result['vcpus'], result['vcpus_used'])
-        self.assertEqual(props['memory_mb'], result['memory_mb'])
-        self.assertEqual(result['memory_mb'], result['memory_mb_used'])
-        self.assertEqual(props['local_gb'], result['local_gb'])
-        self.assertEqual(result['local_gb'], result['local_gb_used'])
+        self.assertEqual(0, result['vcpus'])
+        self.assertEqual(0, result['vcpus_used'])
+        self.assertEqual(0, result['memory_mb'])
+        self.assertEqual(0, result['memory_mb_used'])
+        self.assertEqual(0, result['local_gb'])
+        self.assertEqual(0, result['local_gb_used'])
         self.assertEqual(node_uuid, result['hypervisor_hostname'])
         self.assertEqual(stats, result['stats'])
 
@@ -733,40 +733,14 @@ def test_update_provider_tree_no_rc(self, mock_nfc, mock_nr,
             'resource_class': None,
         }
 
-        self.driver.update_provider_tree(self.ptree, mock.sentinel.nodename)
+        self.assertRaises(exception.NoResourceClass,
+                          self.driver.update_provider_tree,
+                          self.ptree, mock.sentinel.nodename)
 
-        expected = {
-            fields.ResourceClass.VCPU: {
-                'total': 24,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 24,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.MEMORY_MB: {
-                'total': 1024,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 1024,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.DISK_GB: {
-                'total': 100,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 100,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-        }
         mock_nfc.assert_called_once_with(mock.sentinel.nodename)
         mock_nr.assert_called_once_with(mock_nfc.return_value)
         mock_res_used.assert_called_once_with(mock_nfc.return_value)
         mock_res_unavail.assert_called_once_with(mock_nfc.return_value)
-        result = self.ptree.data(mock.sentinel.nodename).inventory
-        self.assertEqual(expected, result)
 
     @mock.patch.object(ironic_driver.IronicDriver,
                        '_node_resources_used', return_value=False)
@@ -793,30 +767,6 @@ def test_update_provider_tree_with_rc(self, mock_nfc, mock_nr,
         self.driver.update_provider_tree(self.ptree, mock.sentinel.nodename)
 
         expected = {
-            fields.ResourceClass.VCPU: {
-                'total': 24,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 24,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.MEMORY_MB: {
-                'total': 1024,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 1024,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.DISK_GB: {
-                'total': 100,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 100,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
             'CUSTOM_IRON_NFV': {
                 'total': 1,
                 'reserved': 0,
@@ -899,30 +849,6 @@ def test_update_provider_tree_with_rc_occupied(self, mock_nfc, mock_nr,
         self.driver.update_provider_tree(self.ptree, mock.sentinel.nodename)
 
         expected = {
-            fields.ResourceClass.VCPU: {
-                'total': 24,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 24,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.MEMORY_MB: {
-                'total': 1024,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 1024,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.DISK_GB: {
-                'total': 100,
-                'reserved': 0,
-                'min_unit': 1,
-                'max_unit': 100,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
             'CUSTOM_IRON_NFV': {
                 'total': 1,
                 'reserved': 0,
@@ -964,30 +890,6 @@ def test_update_provider_tree_disabled_node(self, mock_nfc, mock_nr,
         self.driver.update_provider_tree(self.ptree, mock.sentinel.nodename)
 
         expected = {
-            fields.ResourceClass.VCPU: {
-                'total': 24,
-                'reserved': 24,
-                'min_unit': 1,
-                'max_unit': 24,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.MEMORY_MB: {
-                'total': 1024,
-                'reserved': 1024,
-                'min_unit': 1,
-                'max_unit': 1024,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
-            fields.ResourceClass.DISK_GB: {
-                'total': 100,
-                'reserved': 100,
-                'min_unit': 1,
-                'max_unit': 100,
-                'step_size': 1,
-                'allocation_ratio': 1.0,
-            },
             'CUSTOM_IRON_NFV': {
                 'total': 1,
                 'reserved': 1,

nova/virt/ironic/driver.py

@@ -261,9 +261,6 @@ def _node_resource(self, node):
         """Helper method to create resource dict from node stats."""
         properties = self._parse_node_properties(node)
 
-        vcpus = properties['cpus']
-        memory_mb = properties['memory_mb']
-        local_gb = properties['local_gb']
         raw_cpu_arch = properties['raw_cpu_arch']
         cpu_arch = properties['cpu_arch']
 
@@ -296,17 +293,9 @@ def _node_resource(self, node):
                     LOG.warning("Ignoring malformed capability '%s'. "
                                 "Format should be 'key:val'.", capability)
 
-        vcpus_used = 0
-        memory_mb_used = 0
-        local_gb_used = 0
-
-        if (self._node_resources_used(node)
-                or self._node_resources_unavailable(node)):
-            # Node is deployed, or is in a state when deployment can not start.
-            # Report all of its resources as in use.
-            vcpus_used = vcpus
-            memory_mb_used = memory_mb
-            local_gb_used = local_gb
+        vcpus = vcpus_used = 0
+        memory_mb = memory_mb_used = 0
+        local_gb = local_gb_used = 0
 
         dic = {
             'uuid': str(node.uuid),
@@ -788,10 +777,6 @@ def update_provider_tree(self, provider_tree, nodename):
         # nodename is the ironic node's UUID.
         node = self._node_from_cache(nodename)
         reserved = False
-        # TODO(jaypipes): Completely remove the reporting of VCPU, MEMORY_MB,
-        # and DISK_GB resource classes in early Queens when Ironic nodes will
-        # *always* return the custom resource class that represents the
-        # baremetal node class in an atomic, singular unit.
         if (not self._node_resources_used(node) and
                 self._node_resources_unavailable(node)):
             LOG.debug('Node %(node)s is not ready for a deployment, '
@@ -804,35 +789,21 @@ def update_provider_tree(self, provider_tree, nodename):
 
         info = self._node_resource(node)
         result = {}
-        for rc, field in [(rc_fields.ResourceClass.VCPU, 'vcpus'),
-                          (rc_fields.ResourceClass.MEMORY_MB, 'memory_mb'),
-                          (rc_fields.ResourceClass.DISK_GB, 'local_gb')]:
-            # NOTE(dtantsur): any of these fields can be zero starting with
-            # the Pike release.
-            if info[field]:
-                result[rc] = {
-                    'total': info[field],
-                    'reserved': info[field] if reserved else 0,
-                    'min_unit': 1,
-                    'max_unit': info[field],
-                    'step_size': 1,
-                    'allocation_ratio': 1.0,
-                }
 
         rc_name = info.get('resource_class')
-        if rc_name is not None:
-            # TODO(jaypipes): Raise an exception in Queens if Ironic doesn't
-            # report a resource class for the node
-            norm_name = rc_fields.ResourceClass.normalize_name(rc_name)
-            if norm_name is not None:
-                result[norm_name] = {
-                    'total': 1,
-                    'reserved': int(reserved),
-                    'min_unit': 1,
-                    'max_unit': 1,
-                    'step_size': 1,
-                    'allocation_ratio': 1.0,
-                }
+        if rc_name is None:
+            raise exception.NoResourceClass(node=nodename)
+
+        norm_name = rc_fields.ResourceClass.normalize_name(rc_name)
+        if norm_name is not None:
+            result[norm_name] = {
+                'total': 1,
+                'reserved': int(reserved),
+                'min_unit': 1,
+                'max_unit': 1,
+                'step_size': 1,
+                'allocation_ratio': 1.0,
+            }
 
         provider_tree.update_inventory(nodename, result)
         # TODO(efried): *Unset* (remove_traits) if "owned" by ironic virt but

releasenotes/notes/ironic-remove-properties-scheduling-7555eb8e5e25f18d.yaml

@@ -0,0 +1,10 @@
+---
+upgrade:
+  - |
+    Ironic nodes are now only scheduled using the ``resource_class`` field set
+    on the node. CPUs, RAM, and disks are not reported to the resource tracker.
+    Ironic nodes must have the ``resource_class`` field set before upgrading.
+    Flavors must also be configured to use resource classes instead of node
+    properties. See the `ironic flavor configuration guide
+    <https://docs.openstack.org/ironic/latest/install/configure-nova-flavors.html>`_
+    for more information on doing this.