[stable-only] Add report_ironic_standard_resource_class_inventory option

Since Pike it has been possible to schedule ironic nodes
using custom resource classes. As part of that change, existing
ironic instances needed to undergo a data migration and until
that data migration was complete, ironic compute services needed
to continue reporting standard resource class inventory.

Once the data migration is complete, the problem with continuing
to report standard resource class inventory is non-baremetal
flavors can get scheduled to ironic nodes.

The standard resource class inventory reporting was removed
from the ironic driver in Stein:

  If2b8c1a76d7dbabbac7bb359c9e572cfed510800

Therefore as a stable-only workaround, this change adds an
option for operators to disable reporting standard resource
class inventory for ironic nodes once they have confirmed that
their ironic instance data migrations are complete, which they
can do via the related "nova-status upgrade check" added in
change Ifd22325e849db2353b1b1eedfe998e3d6a79591c.

Change-Id: Id3c74c019da29070811ffc368351e2238b3f6da5
Closes-Bug: #1796920

nova/conf/workarounds.py

@@ -184,6 +184,39 @@ deploying the new code during an upgrade.
 Related options:
 
 * ``[consoleauth]/token_ttl``
+"""),
+
+    cfg.BoolOpt(
+        'report_ironic_standard_resource_class_inventory',
+        default=True,
+        help="""
+Starting in the 16.0.0 Pike release, ironic nodes can be scheduled using
+custom resource classes rather than the standard resource classes VCPU,
+MEMORY_MB and DISK_GB:
+
+https://docs.openstack.org/ironic/rocky/install/configure-nova-flavors.html
+
+However, existing ironic instances require a data migration which can be
+achieved either by restarting ``nova-compute`` services managing ironic nodes
+or running ``nova-manage db ironic_flavor_migration``. The completion of the
+data migration can be checked by running the ``nova-status upgrade check``
+command and checking the "Ironic Flavor Migration" result.
+
+Once all data migrations are complete, you can set this option to False to
+stop reporting VCPU, MEMORY_MB and DISK_GB resource class inventory to the
+Placement service so that scheduling will only rely on the custom resource
+class for each ironic node, as described in the document above.
+
+Note that this option does not apply starting in the 19.0.0 Stein release
+since the ironic compute driver no longer reports standard resource class
+inventory regardless of configuration.
+
+Alternatives to this workaround would be unsetting ``memory_mb`` and/or
+``vcpus`` properties from ironic nodes, or using host aggregates to segregate
+VM from BM compute hosts and restrict flavors to those aggregates, but those
+alternatives might not be feasible at large scale.
+
+See related bug https://bugs.launchpad.net/nova/+bug/1796920 for more details.
 """),
 ]
 

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

@@ -833,6 +833,49 @@ class IronicDriverTestCase(test.NoDBTestCase):
         result = self.ptree.data(mock.sentinel.nodename).inventory
         self.assertEqual(expected, result)
 
+    @mock.patch.object(ironic_driver.IronicDriver,
+                       '_node_resources_used', return_value=False)
+    @mock.patch.object(ironic_driver.IronicDriver,
+                       '_node_resources_unavailable', return_value=False)
+    @mock.patch.object(ironic_driver.IronicDriver, '_node_resource')
+    @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache')
+    def test_update_provider_tree_no_report_standard_res_classes(
+            self, mock_nfc, mock_nr, mock_res_unavail, mock_res_used):
+        """Ensures that if
+        [workarounds]/report_ironic_standard_resource_class_inventory=False we
+        only report the custom resource class inventory.
+        """
+        self.flags(report_ironic_standard_resource_class_inventory=False,
+                   group='workarounds')
+        mock_nr.return_value = {
+            'vcpus': 24,
+            'vcpus_used': 0,
+            'memory_mb': 1024,
+            'memory_mb_used': 0,
+            'local_gb': 100,
+            'local_gb_used': 0,
+            'resource_class': 'iron-nfv',
+        }
+
+        self.driver.update_provider_tree(self.ptree, mock.sentinel.nodename)
+
+        expected = {
+            'CUSTOM_IRON_NFV': {
+                'total': 1,
+                'reserved': 0,
+                'min_unit': 1,
+                'max_unit': 1,
+                '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)
     @mock.patch.object(ironic_driver.IronicDriver,

nova/virt/ironic/driver.py

@@ -804,20 +804,25 @@ class IronicDriver(virt_driver.ComputeDriver):
 
         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,
-                }
+        # Only report standard resource class inventory if configured to do so.
+        # This allows operators to cut-over to ironic custom resource class
+        # based scheduled as soon as they have completed the ironic instance
+        # flavor data migrations.
+        if CONF.workarounds.report_ironic_standard_resource_class_inventory:
+            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:

releasenotes/notes/bug-1796920-report_ironic_standard_resource_class_inventory-85c1fc629a08038d.yaml

@@ -0,0 +1,35 @@
+---
+fixes:
+  - |
+    A new ``[workarounds]/report_ironic_standard_resource_class_inventory``
+    configuration option has been added.
+
+    Starting in the 16.0.0 Pike release, ironic nodes can be scheduled using
+    custom resource classes rather than the standard resource classes VCPU,
+    MEMORY_MB and DISK_GB:
+
+    https://docs.openstack.org/ironic/rocky/install/configure-nova-flavors.html
+
+    However, existing ironic instances require a data migration which can be
+    achieved either by restarting ``nova-compute`` services managing ironic
+    nodes or running ``nova-manage db ironic_flavor_migration``. The completion
+    of the data migration can be checked by running the
+    ``nova-status upgrade check`` command and checking the
+    "Ironic Flavor Migration" result.
+
+    Once all data migrations are complete, you can set this option to False to
+    stop reporting VCPU, MEMORY_MB and DISK_GB resource class inventory to the
+    Placement service so that scheduling will only rely on the custom resource
+    class for each ironic node, as described in the document above.
+
+    Note that this option does not apply starting in the 19.0.0 Stein release
+    since the ironic compute driver no longer reports standard resource class
+    inventory regardless of configuration.
+
+    Alternatives to this workaround would be unsetting ``memory_mb`` and/or
+    ``vcpus`` properties from ironic nodes, or using host aggregates to
+    segregate VM from BM compute hosts and restrict flavors to those
+    aggregates, but those alternatives might not be feasible at large scale.
+
+    See related bug https://bugs.launchpad.net/nova/+bug/1796920 for more
+    details.