From 16961722c5b87766b4aed536589a20c2f05e1170 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 4 Aug 2017 14:48:22 +0200 Subject: [PATCH] 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 | 86 ++++++++++++++++--- instack_undercloud/undercloud.py | 86 +++++++++++++++---- .../resource-class-init-e11b6a630bc47bed.yaml | 18 ++++ requirements.txt | 1 + 4 files changed, 161 insertions(+), 30 deletions(-) create mode 100644 releasenotes/notes/resource-class-init-e11b6a630bc47bed.yaml diff --git a/instack_undercloud/tests/test_undercloud.py b/instack_undercloud/tests/test_undercloud.py index 24f6656c4..2bf65b628 100644 --- a/instack_undercloud/tests/test_undercloud.py +++ b/instack_undercloud/tests/test_undercloud.py @@ -681,7 +681,9 @@ class TestConfigureSshKeys(base.BaseTestCase): 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 @@ class TestPostConfig(base.BaseTestCase): 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 @@ class TestPostConfig(base.BaseTestCase): 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') diff --git a/instack_undercloud/undercloud.py b/instack_undercloud/undercloud.py index 89020f68d..bc4c3aeee 100644 --- a/instack_undercloud/undercloud.py +++ b/instack_undercloud/undercloud.py @@ -31,6 +31,7 @@ import time 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 @@ class Paths(object): 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( diff --git a/releasenotes/notes/resource-class-init-e11b6a630bc47bed.yaml b/releasenotes/notes/resource-class-init-e11b6a630bc47bed.yaml new file mode 100644 index 000000000..2e34c7d42 --- /dev/null +++ b/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 + `_ + for details. diff --git a/requirements.txt b/requirements.txt index 1d8264861..af6d92caf 100644 --- a/requirements.txt +++ b/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