Switch to scheduling based on resource classes

This is needed for upgrade. In the future we may support custom
resource classes, but let's at least make sure we have one.

Depends-On: I027ee4ccf5db51729f036aceab047f2b65d0b368
Change-Id: I5d0ef61e1527927882802f01c4f5c82b1f495cdd
Closes-Bug: #1708653

instack_undercloud/tests/test_undercloud.py

@@ -681,7 +681,9 @@ def test_configure_ssh_keys_missing(self, mock_eui, _):
 
 
 class TestPostConfig(base.BaseTestCase):
+    @mock.patch('instack_undercloud.undercloud._ensure_node_resource_classes')
     @mock.patch('instack_undercloud.undercloud._member_role_exists')
+    @mock.patch('ironicclient.client.get_client', autospec=True)
     @mock.patch('novaclient.client.Client', autospec=True)
     @mock.patch('swiftclient.client.Connection', autospec=True)
     @mock.patch('mistralclient.api.client.client', autospec=True)
@@ -694,8 +696,8 @@ class TestPostConfig(base.BaseTestCase):
     def test_post_config(self, mock_post_config_mistral, mock_ensure_flavor,
                          mock_configure_ssh_keys, mock_get_auth_values,
                          mock_copy_stackrc, mock_delete, mock_mistral_client,
-                         mock_swift_client, mock_nova_client,
-                         mock_member_role_exists):
+                         mock_swift_client, mock_nova_client, mock_ir_client,
+                         mock_member_role_exists, mock_resource_classes):
         instack_env = {
             'UNDERCLOUD_ENDPOINT_MISTRAL_PUBLIC':
                 'http://192.168.24.1:8989/v2',
@@ -708,22 +710,33 @@ def test_post_config(self, mock_post_config_mistral, mock_ensure_flavor,
         mock_swift_client.return_value = mock_instance_swift
         mock_instance_mistral = mock.Mock()
         mock_mistral_client.return_value = mock_instance_mistral
+        mock_instance_ironic = mock_ir_client.return_value
+        flavors = [mock.Mock(spec=['name']),
+                   mock.Mock(spec=['name'])]
+        # The mock library treats "name" attribute differently, and we cannot
+        # pass it through __init__
+        flavors[0].name = 'baremetal'
+        flavors[1].name = 'ceph-storage'
+        mock_instance_nova.flavors.list.return_value = flavors
+
         undercloud._post_config(instack_env)
         mock_nova_client.assert_called_with(
             2, 'aturing', '3nigma', project_name='hut8',
             auth_url='http://bletchley:5000/')
         self.assertTrue(mock_copy_stackrc.called)
         mock_configure_ssh_keys.assert_called_with(mock_instance_nova)
-        calls = [mock.call(mock_instance_nova, 'baremetal'),
-                 mock.call(mock_instance_nova, 'control', 'control'),
-                 mock.call(mock_instance_nova, 'compute', 'compute'),
-                 mock.call(mock_instance_nova, 'ceph-storage', 'ceph-storage'),
-                 mock.call(mock_instance_nova,
+        calls = [mock.call(mock_instance_nova, flavors[0], 'baremetal', None),
+                 mock.call(mock_instance_nova, None, 'control', 'control'),
+                 mock.call(mock_instance_nova, None, 'compute', 'compute'),
+                 mock.call(mock_instance_nova, flavors[1],
+                           'ceph-storage', 'ceph-storage'),
+                 mock.call(mock_instance_nova, None,
                            'block-storage', 'block-storage'),
-                 mock.call(mock_instance_nova,
+                 mock.call(mock_instance_nova, None,
                            'swift-storage', 'swift-storage'),
                  ]
         mock_ensure_flavor.assert_has_calls(calls)
+        mock_resource_classes.assert_called_once_with(mock_instance_ironic)
         mock_post_config_mistral.assert_called_once_with(
             instack_env, mock_instance_mistral, mock_instance_swift)
 
@@ -892,24 +905,69 @@ def _create_flavor_mocks(self):
 
     def test_ensure_flavor_no_profile(self):
         mock_nova, mock_flavor = self._create_flavor_mocks()
-        undercloud._ensure_flavor(mock_nova, 'test')
+        undercloud._ensure_flavor(mock_nova, None, 'test')
         mock_nova.flavors.create.assert_called_with('test', 4096, 1, 40)
-        keys = {'capabilities:boot_option': 'local'}
+        keys = {'capabilities:boot_option': 'local',
+                'resources:CUSTOM_BAREMETAL': '1',
+                'resources:DISK_GB': '0',
+                'resources:MEMORY_MB': '0',
+                'resources:VCPU': '0'}
         mock_flavor.set_keys.assert_called_with(keys)
 
     def test_ensure_flavor_profile(self):
         mock_nova, mock_flavor = self._create_flavor_mocks()
-        undercloud._ensure_flavor(mock_nova, 'test', 'test')
+        undercloud._ensure_flavor(mock_nova, None, 'test', 'test')
         mock_nova.flavors.create.assert_called_with('test', 4096, 1, 40)
         keys = {'capabilities:boot_option': 'local',
-                'capabilities:profile': 'test'}
+                'capabilities:profile': 'test',
+                'resources:CUSTOM_BAREMETAL': '1',
+                'resources:DISK_GB': '0',
+                'resources:MEMORY_MB': '0',
+                'resources:VCPU': '0'}
         mock_flavor.set_keys.assert_called_with(keys)
 
     def test_ensure_flavor_exists(self):
         mock_nova, mock_flavor = self._create_flavor_mocks()
         mock_nova.flavors.create.side_effect = exceptions.Conflict(None)
-        undercloud._ensure_flavor(mock_nova, 'test')
-        mock_flavor.set_keys.assert_not_called()
+        flavor = mock.Mock(spec=['name', 'get_keys', 'set_keys'])
+        flavor.get_keys.return_value = {'foo': 'bar'}
+
+        undercloud._ensure_flavor(mock_nova, flavor, 'test')
+
+        keys = {'foo': 'bar',
+                'resources:CUSTOM_BAREMETAL': '1',
+                'resources:DISK_GB': '0',
+                'resources:MEMORY_MB': '0',
+                'resources:VCPU': '0'}
+        flavor.set_keys.assert_called_with(keys)
+        mock_nova.flavors.create.assert_not_called()
+
+    @mock.patch.object(undercloud.LOG, 'warning', autospec=True)
+    def test_ensure_flavor_exists_conflicting_rc(self, mock_warn):
+        mock_nova, mock_flavor = self._create_flavor_mocks()
+        mock_nova.flavors.create.side_effect = exceptions.Conflict(None)
+        flavor = mock.Mock(spec=['name', 'get_keys', 'set_keys'])
+        flavor.get_keys.return_value = {'foo': 'bar',
+                                        'resources:CUSTOM_FOO': '42'}
+
+        undercloud._ensure_flavor(mock_nova, flavor, 'test')
+
+        flavor.set_keys.assert_not_called()
+        mock_warn.assert_called_once_with(mock.ANY, flavor.name,
+                                          'resources:CUSTOM_FOO')
+        mock_nova.flavors.create.assert_not_called()
+
+    def test_ensure_node_resource_classes(self):
+        nodes = [mock.Mock(uuid='1', resource_class=None),
+                 mock.Mock(uuid='2', resource_class='foobar')]
+        ironic_mock = mock.Mock()
+        ironic_mock.node.list.return_value = nodes
+
+        undercloud._ensure_node_resource_classes(ironic_mock)
+
+        ironic_mock.node.update.assert_called_once_with(
+            '1', [{'path': '/resource_class', 'op': 'add',
+                   'value': 'baremetal'}])
 
     @mock.patch('instack_undercloud.undercloud._extract_from_stackrc')
     @mock.patch('instack_undercloud.undercloud._run_command')

instack_undercloud/undercloud.py

@@ -31,6 +31,7 @@
 import uuid
 import yaml
 
+from ironicclient import client as ir_client
 from keystoneauth1 import session
 from keystoneauth1 import exceptions as ks_exceptions
 from keystoneclient import discover
@@ -78,6 +79,7 @@ def WORKBOOK_PATH(self):
 PATHS = Paths()
 DEFAULT_LOG_LEVEL = logging.DEBUG
 DEFAULT_LOG_FORMAT = '%(asctime)s %(levelname)s: %(message)s'
+DEFAULT_NODE_RESOURCE_CLASS = 'baremetal'
 LOG = None
 CONF = cfg.CONF
 COMPLETION_MESSAGE = """
@@ -1366,18 +1368,63 @@ def _delete_default_flavors(nova):
             nova.flavors.delete(f.id)
 
 
-def _ensure_flavor(nova, name, profile=None):
-    try:
+def _ensure_flavor(nova, existing, name, profile=None):
+    rc_key_name = 'resources:CUSTOM_%s' % DEFAULT_NODE_RESOURCE_CLASS.upper()
+    keys = {
+        # First, make it request the default resource class
+        rc_key_name: "1",
+        # Then disable scheduling based on everything else
+        "resources:DISK_GB": "0",
+        "resources:MEMORY_MB": "0",
+        "resources:VCPU": "0"
+    }
+
+    if existing is None:
         flavor = nova.flavors.create(name, 4096, 1, 40)
-    except exceptions.Conflict:
+
+        keys['capabilities:boot_option'] = 'local'
+        if profile is not None:
+            keys['capabilities:profile'] = profile
+        flavor.set_keys(keys)
+        message = 'Created flavor "%s" with profile "%s"'
+
+        LOG.info(message, name, profile)
+    else:
         LOG.info('Not creating flavor "%s" because it already exists.', name)
-        return
-    keys = {'capabilities:boot_option': 'local'}
-    if profile is not None:
-        keys['capabilities:profile'] = profile
-    flavor.set_keys(keys)
-    message = 'Created flavor "%s" with profile "%s"'
-    LOG.info(message, name, profile)
+
+        # NOTE(dtantsur): it is critical to ensure that the flavors request
+        # the correct resource class, otherwise scheduling will fail.
+        old_keys = existing.get_keys()
+        for key in old_keys:
+            if key.startswith('resources:CUSTOM_') and key != rc_key_name:
+                LOG.warning('Not updating flavor %s, as it already has a '
+                            'custom resource class %s. Make sure you have '
+                            'enough nodes with this resource class.',
+                            existing.name, key)
+                return
+
+        # Keep existing values
+        keys.update(old_keys)
+        existing.set_keys(keys)
+        LOG.info('Flavor %s updated to use custom resource class %s',
+                 name, DEFAULT_NODE_RESOURCE_CLASS)
+
+
+def _ensure_node_resource_classes(ironic):
+    for node in ironic.node.list(limit=-1, fields=['uuid', 'resource_class']):
+        if node.resource_class:
+            if node.resource_class != DEFAULT_NODE_RESOURCE_CLASS:
+                LOG.warning('Node %s is using a resource class %s instead '
+                            'of the default %s. Make sure you use the correct '
+                            'flavor for it.', node.uuid, node.resource_class,
+                            DEFAULT_NODE_RESOURCE_CLASS)
+            continue
+
+        ironic.node.update(node.uuid,
+                           [{'path': '/resource_class', 'op': 'add',
+                             'value': DEFAULT_NODE_RESOURCE_CLASS}])
+        LOG.info('Node %s resource class was set to %s',
+                 node.uuid, DEFAULT_NODE_RESOURCE_CLASS)
 
 
 def _copy_stackrc():
@@ -1524,15 +1571,22 @@ def _post_config(instack_env):
         nova = novaclient.Client(2, user, password, auth_url=auth_url,
                                  project_name=project)
 
+    ironic = ir_client.get_client(1, session=sess,
+                                  os_ironic_api_version='1.21')
+
     _configure_ssh_keys(nova)
     _delete_default_flavors(nova)
 
-    _ensure_flavor(nova, 'baremetal')
-    _ensure_flavor(nova, 'control', 'control')
-    _ensure_flavor(nova, 'compute', 'compute')
-    _ensure_flavor(nova, 'ceph-storage', 'ceph-storage')
-    _ensure_flavor(nova, 'block-storage', 'block-storage')
-    _ensure_flavor(nova, 'swift-storage', 'swift-storage')
+    _ensure_node_resource_classes(ironic)
+
+    all_flavors = {f.name: f for f in nova.flavors.list()}
+    for name, profile in [('baremetal', None),
+                          ('control', 'control'),
+                          ('compute', 'compute'),
+                          ('ceph-storage', 'ceph-storage'),
+                          ('block-storage', 'block-storage'),
+                          ('swift-storage', 'swift-storage')]:
+        _ensure_flavor(nova, all_flavors.get(name), name, profile)
 
     mistral_url = instack_env['UNDERCLOUD_ENDPOINT_MISTRAL_PUBLIC']
     mistral = mistralclient.client(

releasenotes/notes/resource-class-init-e11b6a630bc47bed.yaml

@@ -0,0 +1,18 @@
+---
+upgrade:
+  - |
+    This release replaces node scheduling based on properties (CPU count,
+    memory and disk) with scheduling based on *custom resource classes*.
+    As part of this change during the upgrade:
+
+    * The ``resource_class`` field is set to ``baremetal``, if empty.
+    * The standard flavors are adjusted to request one instance of the
+      ``baremetal`` resource class and to **not** request the standard
+      properties. Flavors that already have a resource class attached are
+      not changed.
+
+    All non-standard custom flavors have to be changed in a similar way.
+
+    See the `ironic flavor documentation
+    <https://docs.openstack.org/ironic/latest/install/configure-nova-flavors.html#scheduling-based-on-resource-classes>`_
+    for details.

requirements.txt

@@ -2,6 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 six>=1.9.0 # MIT
+python-ironicclient>=1.14.0 # Apache-2.0
 python-keystoneclient>=3.8.0 # Apache-2.0
 python-novaclient>=9.0.0 # Apache-2.0
 python-mistralclient>=3.1.0 # Apache-2.0