From 55c790bd92dd184244128577273c122359c1cc43 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 4 Mar 2016 12:48:26 +0100 Subject: [PATCH] Allow 'openstack baremetal configure boot' to guess the root device This change simplifies using Ironic root device hints for people who only need to change the default strategy of selecting the default device. E.g. we have a use case for selecting the first device instead of the smallest, so that the root device does not change after upgrade Kilo -> Liberty. Note that this feature does not replace per-node device hints, rather complement them with a more global and easy to use setting. This change introduces 3 new arguments to the command: --root-device states how to find a root devices for a node. If this argument is provided, the client will try to detect the root device based on the stored introspection data. Possible values: * smallest (the same thing as IPA does by default - the smallest device) * largest (the opposite thing - pick the largest device) * otherwise it's treated as a comma-separated list of possible device names, e.g. sda,vda,hda rouchly matches the logic the Kilo ramdisk used. The resulting device WWN or serial (whatever is available) is then recorded in the root device hints (/properties/root_device) for a node. --root-device-minimum-size minimum size of the considered devices The default value of 4 GiB matches what IPA does. --overwrite-root-device-hints allows overwriting root device hints set previously. It's disabled by default to allow more precise control over the root device for some subset of nodes. Note that for these arguments to work, this command should be run after introspection. A separate documentation change will be posted for that. Closes-Bug: #1588433 Change-Id: I9f19554c5e7f34c8f63c1603c32b4d470fb12592 (cherry picked from commit ceabf658b847f9be6f0115e96a7af86a890ba232) --- tripleoclient/exceptions.py | 4 + tripleoclient/tests/v1/baremetal/fakes.py | 10 +- .../tests/v1/baremetal/test_baremetal.py | 214 ++++++++++++++++++ tripleoclient/v1/baremetal.py | 116 ++++++++++ 4 files changed, 343 insertions(+), 1 deletion(-) diff --git a/tripleoclient/exceptions.py b/tripleoclient/exceptions.py index 345118b35..d6d65e33d 100644 --- a/tripleoclient/exceptions.py +++ b/tripleoclient/exceptions.py @@ -64,3 +64,7 @@ class ProfileMatchingError(Exception): class PasswordFileNotFound(Exception): """Password file for the Heat stack not found in the current working dir""" + + +class RootDeviceDetectionError(Exception): + """Failed to detect the root device""" diff --git a/tripleoclient/tests/v1/baremetal/fakes.py b/tripleoclient/tests/v1/baremetal/fakes.py index f59eb5760..7cbf3d12c 100644 --- a/tripleoclient/tests/v1/baremetal/fakes.py +++ b/tripleoclient/tests/v1/baremetal/fakes.py @@ -13,6 +13,7 @@ # under the License. # +import ironic_inspector_client import mock from openstackclient.tests import utils @@ -68,8 +69,9 @@ class FakeBaremetalNodeClient(object): class FakeInspectorClient(object): - def __init__(self, states=None): + def __init__(self, states=None, data=None): self.states = states or {} + self.data = data or {} self.on_introspection = [] def introspect(self, uuid): @@ -78,6 +80,12 @@ class FakeInspectorClient(object): def get_status(self, uuid): return self.states[uuid] + def get_data(self, uuid): + try: + return self.data[uuid] + except KeyError: + raise ironic_inspector_client.ClientError(mock.Mock()) + class TestBaremetal(utils.TestCommand): diff --git a/tripleoclient/tests/v1/baremetal/test_baremetal.py b/tripleoclient/tests/v1/baremetal/test_baremetal.py index 124bd97f4..4d6fceb0c 100644 --- a/tripleoclient/tests/v1/baremetal/test_baremetal.py +++ b/tripleoclient/tests/v1/baremetal/test_baremetal.py @@ -21,6 +21,7 @@ import os import yaml import fixtures +from oslo_utils import units from tripleoclient import exceptions from tripleoclient.tests.v1.baremetal import fakes @@ -1170,6 +1171,219 @@ class TestConfigureBaremetalBoot(fakes.TestBaremetal): ]) +@mock.patch('openstackclient.common.utils.find_resource', autospec=True) +class TestConfigureBaremetalBootRootDeviceDetection(fakes.TestBaremetal): + + def setUp(self): + super(TestConfigureBaremetalBootRootDeviceDetection, self).setUp() + + # Get the command object to test + self.cmd = baremetal.ConfigureBaremetalBoot(self.app, None) + + self.disks = [ + {'name': '/dev/sda', 'size': 11 * units.Gi}, + {'name': '/dev/sdb', 'size': 2 * units.Gi}, + {'name': '/dev/sdc', 'size': 5 * units.Gi}, + {'name': '/dev/sdd', 'size': 21 * units.Gi}, + {'name': '/dev/sde', 'size': 13 * units.Gi}, + ] + for i, disk in enumerate(self.disks): + disk['wwn'] = 'wwn%d' % i + disk['serial'] = 'serial%d' % i + + self.inspector_client = self.app.client_manager.baremetal_introspection + self.inspector_client.data['ABCDEFGH'] = { + 'inventory': {'disks': self.disks} + } + + self.bm_client = self.app.client_manager.baremetal + self.bm_client.node.list.return_value = [ + mock.Mock(uuid="ABCDEFGH"), + ] + + self.node = mock.Mock(uuid="ABCDEFGH", properties={}) + self.bm_client.node.get.return_value = self.node + + def test_smallest(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + + arglist = ['--root-device', 'smallest'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 2) + root_device_args = self.bm_client.node.update.call_args_list[1] + expected_patch = [{'op': 'add', 'path': '/properties/root_device', + 'value': {'wwn': 'wwn2'}}, + {'op': 'add', 'path': '/properties/local_gb', + 'value': 4}] + self.assertEqual(mock.call('ABCDEFGH', expected_patch), + root_device_args) + + def test_largest(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + + arglist = ['--root-device', 'largest'] + verifylist = [('root_device', 'largest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 2) + root_device_args = self.bm_client.node.update.call_args_list[1] + expected_patch = [{'op': 'add', 'path': '/properties/root_device', + 'value': {'wwn': 'wwn3'}}, + {'op': 'add', 'path': '/properties/local_gb', + 'value': 20}] + self.assertEqual(mock.call('ABCDEFGH', expected_patch), + root_device_args) + + def test_no_overwrite(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + self.node.properties['root_device'] = {'foo': 'bar'} + + arglist = ['--root-device', 'smallest'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 1) + + def test_with_overwrite(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + self.node.properties['root_device'] = {'foo': 'bar'} + + arglist = ['--root-device', 'smallest', + '--overwrite-root-device-hints'] + verifylist = [('root_device', 'smallest'), + ('overwrite_root_device_hints', True)] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 2) + root_device_args = self.bm_client.node.update.call_args_list[1] + expected_patch = [{'op': 'add', 'path': '/properties/root_device', + 'value': {'wwn': 'wwn2'}}, + {'op': 'add', 'path': '/properties/local_gb', + 'value': 4}] + self.assertEqual(mock.call('ABCDEFGH', expected_patch), + root_device_args) + + def test_minimum_size(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + + arglist = ['--root-device', 'smallest', + '--root-device-minimum-size', '10'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 2) + root_device_args = self.bm_client.node.update.call_args_list[1] + expected_patch = [{'op': 'add', 'path': '/properties/root_device', + 'value': {'wwn': 'wwn0'}}, + {'op': 'add', 'path': '/properties/local_gb', + 'value': 10}] + self.assertEqual(mock.call('ABCDEFGH', expected_patch), + root_device_args) + + def test_bad_inventory(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + del self.inspector_client.data['ABCDEFGH']['inventory'] + + arglist = ['--root-device', 'smallest'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp(exceptions.RootDeviceDetectionError, + "Malformed introspection data", + self.cmd.take_action, parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 1) + + def test_no_disks(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + self.inspector_client.data['ABCDEFGH']['inventory']['disks'] = [ + {'name': '/dev/sda', 'size': 1 * units.Gi} + ] + + arglist = ['--root-device', 'smallest'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp(exceptions.RootDeviceDetectionError, + "No suitable disks", + self.cmd.take_action, parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 1) + + def test_no_data(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + del self.inspector_client.data['ABCDEFGH'] + + arglist = ['--root-device', 'smallest'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp(exceptions.RootDeviceDetectionError, + "No introspection data", + self.cmd.take_action, parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 1) + + def test_no_wwn_and_serial(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + self.inspector_client.data['ABCDEFGH']['inventory']['disks'] = [ + {'name': '/dev/sda', 'size': 10 * units.Gi} + ] + + arglist = ['--root-device', 'smallest'] + verifylist = [('root_device', 'smallest')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp(exceptions.RootDeviceDetectionError, + "Neither WWN nor serial number are known", + self.cmd.take_action, parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 1) + + def test_device_list(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + + arglist = ['--root-device', 'hda,sda,sdb,sdc'] + verifylist = [('root_device', 'hda,sda,sdb,sdc')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 2) + root_device_args = self.bm_client.node.update.call_args_list[1] + expected_patch = [{'op': 'add', 'path': '/properties/root_device', + 'value': {'wwn': 'wwn0'}}, + {'op': 'add', 'path': '/properties/local_gb', + 'value': 10}] + self.assertEqual(mock.call('ABCDEFGH', expected_patch), + root_device_args) + + def test_device_list_not_found(self, find_resource_mock): + find_resource_mock.return_value = mock.Mock(id="IDIDID") + + arglist = ['--root-device', 'hda'] + verifylist = [('root_device', 'hda')] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp(exceptions.RootDeviceDetectionError, + "Cannot find a disk", + self.cmd.take_action, parsed_args) + + self.assertEqual(self.bm_client.node.update.call_count, 1) + + class TestShowNodeCapabilities(fakes.TestBaremetal): def setUp(self): diff --git a/tripleoclient/v1/baremetal.py b/tripleoclient/v1/baremetal.py index b025ab44a..b4992cc5f 100644 --- a/tripleoclient/v1/baremetal.py +++ b/tripleoclient/v1/baremetal.py @@ -24,8 +24,10 @@ import yaml from cliff import command from cliff import lister +import ironic_inspector_client from openstackclient.common import utils as osc_utils from openstackclient.i18n import _ +from oslo_utils import units from tripleo_common.utils import nodes from tripleoclient import exceptions @@ -438,6 +440,20 @@ class ConfigureBaremetalBoot(command.Command): parser.add_argument('--deploy-ramdisk', default='bm-deploy-ramdisk', help='Image with deploy ramdisk.') + parser.add_argument('--root-device', + help='Define the root device for nodes. ' + 'Can be either a list of device names (without ' + '/dev) to choose from or one of two strategies: ' + 'largest or smallest. For it to work this command ' + 'should be run after the introspection.') + parser.add_argument('--root-device-minimum-size', + type=int, default=4, + help='Minimum size (in GiB) of the detected ' + 'root device. Used with --detect-root-device.') + parser.add_argument('--overwrite-root-device-hints', + action='store_true', + help='Whether to overwrite existing root device ' + 'hints when --detect-root-device is used.') return parser def take_action(self, parsed_args): @@ -521,6 +537,106 @@ class ConfigureBaremetalBoot(command.Command): }, ]) + self._apply_root_device_strategy( + node_detail, parsed_args.root_device, + parsed_args.root_device_minimum_size, + parsed_args.overwrite_root_device_hints) + + def _apply_root_device_strategy(self, node, strategy, minimum_size, + overwrite=False): + if not strategy: + return + + if node.properties.get('root_device') and not overwrite: + # This is a correct situation, we still want to allow people to + # fine-tune the root device setting for a subset of nodes. + # However, issue a warning, so that they know which nodes were not + # updated during this run. + self.log.warning('Root device hints are already set for node %s ' + 'and overwriting is not requested, skipping', + node.uuid) + self.log.warning('You may unset them by running $ ironic ' + 'node-update %s remove properties/root_device', + node.uuid) + return + + inspector_client = self.app.client_manager.baremetal_introspection + try: + data = inspector_client.get_data(node.uuid) + except ironic_inspector_client.ClientError: + raise exceptions.RootDeviceDetectionError( + 'No introspection data found for node %s, ' + 'root device cannot be detected' % node.uuid) + except AttributeError: + raise RuntimeError('Ironic inspector client version 1.2.0 or ' + 'newer is required for detecting root device') + + try: + disks = data['inventory']['disks'] + except KeyError: + raise exceptions.RootDeviceDetectionError( + 'Malformed introspection data for node %s: ' + 'disks list is missing' % node.uuid) + + minimum_size *= units.Gi + disks = [d for d in disks if d.get('size', 0) >= minimum_size] + + if not disks: + raise exceptions.RootDeviceDetectionError( + 'No suitable disks found for node %s' % node.uuid) + + if strategy == 'smallest': + disks.sort(key=lambda d: d['size']) + root_device = disks[0] + elif strategy == 'largest': + disks.sort(key=lambda d: d['size'], reverse=True) + root_device = disks[0] + else: + disk_names = [x.strip() for x in strategy.split(',')] + disks = {d['name']: d for d in disks} + for candidate in disk_names: + try: + root_device = disks['/dev/%s' % candidate] + except KeyError: + continue + else: + break + else: + raise exceptions.RootDeviceDetectionError( + 'Cannot find a disk with any of names %(strategy)s ' + 'for node %(node)s' % + {'strategy': strategy, 'node': node.uuid}) + + hint = None + for hint_name in ('wwn', 'serial'): + if root_device.get(hint_name): + hint = {hint_name: root_device[hint_name]} + break + + if hint is None: + # I don't think it might actually happen, but just in case + raise exceptions.RootDeviceDetectionError( + 'Neither WWN nor serial number are known for device %(dev)s ' + 'on node %(node)s; root device hints cannot be used' % + {'dev': root_device['name'], 'node': node.uuid}) + + # During the introspection process we got local_gb assigned according + # to the default strategy. Now we need to update it. + new_size = root_device['size'] / units.Gi + # This -1 is what we always do to account for partitioning + new_size -= 1 + + bm_client = self.app.client_manager.baremetal + bm_client.node.update( + node.uuid, + [{'op': 'add', 'path': '/properties/root_device', 'value': hint}, + {'op': 'add', 'path': '/properties/local_gb', 'value': new_size}]) + + self.log.info('Updated root device for node %(node)s, new device ' + 'is %(dev)s, new local_gb is %(local_gb)d', + {'node': node.uuid, 'dev': root_device, + 'local_gb': new_size}) + class ShowNodeCapabilities(lister.Lister): """List the capabilities for all Nodes"""