From c5dfa1bd9fe39eedbb60be537b5b3d83b1cde15b Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 10 Sep 2019 15:51:03 +0200 Subject: [PATCH] Foundation for boot/network management for in-band inspection This change the required base driver interface additions and inspector interface changes to support in-band inspection driven by ironic. Change-Id: Ibf9a80d0f72d5f128bf46ddca4cb9762c9a8191b Story: #1528920 Task: #10404 --- ironic/common/utils.py | 28 ++ ironic/conf/inspector.py | 17 ++ ironic/drivers/base.py | 43 +++ ironic/drivers/modules/inspector.py | 188 ++++++++++-- ironic/drivers/modules/network/noop.py | 7 + .../unit/drivers/modules/test_inspector.py | 270 +++++++++++++++++- ...pection-boot-network-59fd23ca62b09e81.yaml | 19 ++ 7 files changed, 546 insertions(+), 26 deletions(-) create mode 100644 releasenotes/notes/inspection-boot-network-59fd23ca62b09e81.yaml diff --git a/ironic/common/utils.py b/ironic/common/utils.py index fc61fd02fe..f170dabdf8 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -532,3 +532,31 @@ def validate_conductor_group(conductor_group): raise exception.InvalidConductorGroup(group=conductor_group) if not re.match(r'^[a-zA-Z0-9_\-\.]*$', conductor_group): raise exception.InvalidConductorGroup(group=conductor_group) + + +def set_node_nested_field(node, collection, field, value): + """Set a value in a dictionary field of a node. + + :param node: Node object. + :param collection: Name of the field with the dictionary. + :param field: Nested field name. + :param value: New value. + """ + col = getattr(node, collection) + col[field] = value + setattr(node, collection, col) + + +def pop_node_nested_field(node, collection, field, default=None): + """Pop a value from a dictionary field of a node. + + :param node: Node object. + :param collection: Name of the field with the dictionary. + :param field: Nested field name. + :param default: The default value to return. + :return: The removed value or the default. + """ + col = getattr(node, collection) + result = col.pop(field, default) + setattr(node, collection, col) + return result diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py index da63687b4d..290d4ed414 100644 --- a/ironic/conf/inspector.py +++ b/ironic/conf/inspector.py @@ -21,6 +21,23 @@ opts = [ cfg.IntOpt('status_check_period', default=60, help=_('period (in seconds) to check status of nodes ' 'on inspection')), + cfg.StrOpt('extra_kernel_params', default='', + help=_('extra kernel parameters to pass to the inspection ' + 'ramdisk when boot is managed by ironic (not ' + 'ironic-inspector). Pairs key=value separated by ' + 'spaces.')), + cfg.BoolOpt('power_off', default=True, + help=_('whether to power off a node after inspection ' + 'finishes')), + cfg.StrOpt('callback_endpoint_override', + help=_('endpoint to use as a callback for posting back ' + 'introspection data when boot is managed by ironic. ' + 'Standard keystoneauth options are used by default.')), + cfg.BoolOpt('require_managed_boot', default=False, + help=_('require that the in-band inspection boot is fully ' + 'managed by ironic. Set this to True if your ' + 'installation of ironic-inspector does not have a ' + 'separate PXE boot environment.')), ] diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 81ee2006cf..7508adbb84 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -559,6 +559,17 @@ class BootInterface(BaseInterface): raise exception.UnsupportedDriverExtension( driver=task.node.driver, extension='validate_rescue') + def validate_inspection(self, task): + """Validate that the node has required properties for inspection. + + :param task: A TaskManager instance with the node being checked + :raises: MissingParameterValue if node is missing one or more required + parameters + :raises: UnsupportedDriverExtension + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='validate_inspection') + class PowerInterface(BaseInterface): """Interface for power-related actions.""" @@ -1493,6 +1504,38 @@ class NetworkInterface(BaseInterface): """ pass + def validate_inspection(self, task): + """Validate that the node has required properties for inspection. + + :param task: A TaskManager instance with the node being checked + :raises: MissingParameterValue if node is missing one or more required + parameters + :raises: UnsupportedDriverExtension + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='validate_inspection') + + def add_inspection_network(self, task): + """Add the inspection network to the node. + + :param task: A TaskManager instance. + :returns: a dictionary in the form {port.uuid: neutron_port['id']} + :raises: NetworkError + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + """ + return {} + + def remove_inspection_network(self, task): + """Removes the inspection network from a node. + + :param task: A TaskManager instance. + :raises: NetworkError + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + :raises: MissingParameterValue, if some parameters are missing. + """ + def need_power_on(self, task): """Check if ironic node must be powered on before applying network changes diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 5774079300..6339c59cba 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -15,6 +15,8 @@ Modules required to work with ironic_inspector: https://pypi.org/project/ironic-inspector """ +import shlex + import eventlet from futurist import periodics import openstack @@ -24,7 +26,9 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import keystone from ironic.common import states +from ironic.common import utils from ironic.conductor import task_manager +from ironic.conductor import utils as cond_utils from ironic.conf import CONF from ironic.drivers import base @@ -32,6 +36,8 @@ from ironic.drivers import base LOG = logging.getLogger(__name__) _INSPECTOR_SESSION = None +# Internal field to mark whether ironic or inspector manages boot for the node +_IRONIC_MANAGES_BOOT = 'inspector_manage_boot' def _get_inspector_session(**kwargs): @@ -62,6 +68,124 @@ def _get_client(context): oslo_conf=conf).baremetal_introspection +def _get_callback_endpoint(client): + root = CONF.inspector.callback_endpoint_override or client.get_endpoint() + if root == 'mdns': + return root + root = root.rstrip('/') + # NOTE(dtantsur): the IPA side is quite picky about the exact format. + if root.endswith('/v1'): + return '%s/continue' % root + else: + return '%s/v1/continue' % root + + +def _tear_down_managed_boot(task): + errors = [] + + ironic_manages_boot = utils.pop_node_nested_field( + task.node, 'driver_internal_info', _IRONIC_MANAGES_BOOT) + if not ironic_manages_boot: + return errors + + try: + task.driver.boot.clean_up_ramdisk(task) + except Exception as exc: + errors.append(_('unable to clean up ramdisk boot: %s') % exc) + LOG.exception('Unable to clean up ramdisk boot for node %s', + task.node.uuid) + try: + task.driver.network.remove_inspection_network(task) + except Exception as exc: + errors.append(_('unable to remove inspection ports: %s') % exc) + LOG.exception('Unable to remove inspection network for node %s', + task.node.uuid) + + if CONF.inspector.power_off: + try: + cond_utils.node_power_action(task, states.POWER_OFF) + except Exception as exc: + errors.append(_('unable to power off the node: %s') % exc) + LOG.exception('Unable to power off node %s', task.node.uuid) + + return errors + + +def _inspection_error_handler(task, error, raise_exc=False, clean_up=True): + if clean_up: + _tear_down_managed_boot(task) + + task.node.last_error = error + if raise_exc: + task.node.save() + raise exception.HardwareInspectionFailure(error=error) + else: + task.process_event('fail') + + +def _ironic_manages_boot(task, raise_exc=False): + """Whether ironic should manage boot for this node.""" + try: + task.driver.boot.validate_inspection(task) + except exception.UnsupportedDriverExtension as e: + LOG.debug('The boot interface %(iface)s of the node %(node)s does ' + 'not support managed boot for in-band inspection or ' + 'the required options are not populated: %(exc)s', + {'node': task.node.uuid, + 'iface': task.node.boot_interface, + 'exc': e}) + if raise_exc: + raise + return False + + try: + task.driver.network.validate_inspection(task) + except exception.UnsupportedDriverExtension as e: + LOG.debug('The network interface %(iface)s of the node %(node)s does ' + 'not support managed boot for in-band inspection or ' + 'the required options are not populated: %(exc)s', + {'node': task.node.uuid, + 'iface': task.node.network_interface, + 'exc': e}) + if raise_exc: + raise + return False + + return True + + +def _parse_kernel_params(): + """Parse kernel params from the configuration.""" + result = {} + for s in shlex.split(CONF.inspector.extra_kernel_params): + try: + key, value = s.split('=', 1) + except ValueError: + raise exception.InvalidParameterValue( + _('Invalid key-value pair in extra_kernel_params: %s') % s) + result[key] = value + return result + + +def _start_managed_inspection(task): + """Start inspection managed by ironic.""" + try: + client = _get_client(task.context) + endpoint = _get_callback_endpoint(client) + params = dict(_parse_kernel_params(), + **{'ipa-inspection-callback-url': endpoint}) + + task.driver.network.add_inspection_network(task) + task.driver.boot.prepare_ramdisk(task, ramdisk_params=params) + client.start_introspection(task.node.uuid, manage_boot=False) + cond_utils.node_power_action(task, states.REBOOT) + except Exception as exc: + LOG.exception('Unable to start managed inspection for node %(uuid)s: ' + '%(err)s', {'uuid': task.node.uuid, 'err': exc}) + error = _('unable to start inspection: %s') % exc + _inspection_error_handler(task, error, raise_exc=True) + + class Inspector(base.InspectInterface): """In-band inspection via ironic-inspector project.""" @@ -78,10 +202,11 @@ class Inspector(base.InspectInterface): If invalid, raises an exception; otherwise returns None. :param task: a task from TaskManager. + :raises: UnsupportedDriverExtension """ - # NOTE(deva): this is not callable if inspector is disabled - # so don't raise an exception -- just pass. - pass + _parse_kernel_params() + if CONF.inspector.require_managed_boot: + _ironic_manages_boot(task, raise_exc=True) def inspect_hardware(self, task): """Inspect hardware to obtain the hardware properties. @@ -91,14 +216,29 @@ class Inspector(base.InspectInterface): :param task: a task from TaskManager. :returns: states.INSPECTWAIT + :raises: HardwareInspectionFailure on failure """ - LOG.debug('Starting inspection for node %(uuid)s using ' - 'ironic-inspector', {'uuid': task.node.uuid}) + ironic_manages_boot = _ironic_manages_boot( + task, raise_exc=CONF.inspector.require_managed_boot) - # NOTE(dtantsur): we're spawning a short-living green thread so that - # we can release a lock as soon as possible and allow ironic-inspector - # to operate on a node. - eventlet.spawn_n(_start_inspection, task.node.uuid, task.context) + utils.set_node_nested_field(task.node, 'driver_internal_info', + _IRONIC_MANAGES_BOOT, + ironic_manages_boot) + task.node.save() + + LOG.debug('Starting inspection for node %(uuid)s using ' + 'ironic-inspector, booting is managed by %(project)s', + {'uuid': task.node.uuid, + 'project': 'ironic' if ironic_manages_boot + else 'ironic-inspector'}) + + if ironic_manages_boot: + _start_managed_inspection(task) + else: + # NOTE(dtantsur): spawning a short-living green thread so that + # we can release a lock as soon as possible and allow + # ironic-inspector to operate on the node. + eventlet.spawn_n(_start_inspection, task.node.uuid, task.context) return states.INSPECTWAIT def abort(self, task): @@ -133,16 +273,16 @@ def _start_inspection(node_uuid, context): try: _get_client(context).start_introspection(node_uuid) except Exception as exc: - LOG.exception('Exception during contacting ironic-inspector ' - 'for inspection of node %(node)s: %(err)s', - {'node': node_uuid, 'err': exc}) + LOG.error('Error contacting ironic-inspector for inspection of node ' + '%(node)s: %(cls)s: %(err)s', + {'node': node_uuid, 'cls': type(exc).__name__, 'err': exc}) # NOTE(dtantsur): if acquire fails our last option is to rely on # timeout lock_purpose = 'recording hardware inspection error' with task_manager.acquire(context, node_uuid, purpose=lock_purpose) as task: - task.node.last_error = _('Failed to start inspection: %s') % exc - task.process_event('fail') + error = _('Failed to start inspection: %s') % exc + _inspection_error_handler(task, error) else: LOG.info('Node %s was sent to inspection to ironic-inspector', node_uuid) @@ -180,9 +320,21 @@ def _check_status(task): if status.error: LOG.error('Inspection failed for node %(uuid)s with error: %(err)s', {'uuid': node.uuid, 'err': status.error}) - node.last_error = (_('ironic-inspector inspection failed: %s') - % status.error) - task.process_event('fail') + error = _('ironic-inspector inspection failed: %s') % status.error + _inspection_error_handler(task, error) elif status.is_finished: - LOG.info('Inspection finished successfully for node %s', node.uuid) + _clean_up(task) + + +def _clean_up(task): + errors = _tear_down_managed_boot(task) + if errors: + errors = ', '.join(errors) + LOG.error('Inspection clean up failed for node %(uuid)s: %(err)s', + {'uuid': task.node.uuid, 'err': errors}) + msg = _('Inspection clean up failed: %s') % errors + _inspection_error_handler(task, msg, raise_exc=False, clean_up=False) + else: + LOG.info('Inspection finished successfully for node %s', + task.node.uuid) task.process_event('done') diff --git a/ironic/drivers/modules/network/noop.py b/ironic/drivers/modules/network/noop.py index 5f50162ad3..abe1fcb064 100644 --- a/ironic/drivers/modules/network/noop.py +++ b/ironic/drivers/modules/network/noop.py @@ -119,3 +119,10 @@ class NoopNetwork(base.NetworkInterface): :param task: A TaskManager instance. """ pass + + def validate_inspection(self, task): + """Validate that the node has required properties for inspection. + + :param task: A TaskManager instance with the node being checked + """ + pass diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index 2b33211010..f894fcc91e 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -15,13 +15,18 @@ import mock import openstack from ironic.common import context +from ironic.common import exception from ironic.common import states +from ironic.common import utils from ironic.conductor import task_manager from ironic.drivers.modules import inspector from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils +CONF = inspector.CONF + + @mock.patch('ironic.common.keystone.get_auth', autospec=True, return_value=mock.sentinel.auth) @mock.patch('ironic.common.keystone.get_session', autospec=True, @@ -57,14 +62,17 @@ class GetClientTestCase(db_base.DbTestCase): class BaseTestCase(db_base.DbTestCase): def setUp(self): super(BaseTestCase, self).setUp() - self.node = obj_utils.get_test_node(self.context, - inspect_interface='inspector') + self.node = obj_utils.create_test_node(self.context, + inspect_interface='inspector') self.iface = inspector.Inspector() self.task = mock.MagicMock(spec=task_manager.TaskManager) self.task.context = self.context self.task.shared = False self.task.node = self.node - self.task.driver = mock.Mock(spec=['inspect'], inspect=self.iface) + self.task.driver = mock.Mock( + spec=['boot', 'network', 'inspect', 'power'], + inspect=self.iface) + self.driver = self.task.driver class CommonFunctionsTestCase(BaseTestCase): @@ -75,25 +83,165 @@ class CommonFunctionsTestCase(BaseTestCase): res = self.iface.get_properties() self.assertEqual({}, res) + def test_get_callback_endpoint(self): + for catalog_endp in ['http://192.168.0.42:5050', + 'http://192.168.0.42:5050/v1', + 'http://192.168.0.42:5050/']: + client = mock.Mock() + client.get_endpoint.return_value = catalog_endp + self.assertEqual('http://192.168.0.42:5050/v1/continue', + inspector._get_callback_endpoint(client)) + + def test_get_callback_endpoint_override(self): + CONF.set_override('callback_endpoint_override', 'http://url', + group='inspector') + client = mock.Mock() + self.assertEqual('http://url/v1/continue', + inspector._get_callback_endpoint(client)) + self.assertFalse(client.get_endpoint.called) + + def test_get_callback_endpoint_mdns(self): + CONF.set_override('callback_endpoint_override', 'mdns', + group='inspector') + client = mock.Mock() + self.assertEqual('mdns', inspector._get_callback_endpoint(client)) + self.assertFalse(client.get_endpoint.called) + @mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw)) @mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) class InspectHardwareTestCase(BaseTestCase): - def test_ok(self, mock_client): + def test_validate_ok(self, mock_client): + self.iface.validate(self.task) + + def test_validate_invalid_kernel_params(self, mock_client): + CONF.set_override('extra_kernel_params', 'abcdef', group='inspector') + self.assertRaises(exception.InvalidParameterValue, + self.iface.validate, self.task) + + def test_validate_require_managed_boot(self, mock_client): + CONF.set_override('require_managed_boot', True, group='inspector') + self.driver.boot.validate_inspection.side_effect = ( + exception.UnsupportedDriverExtension('')) + self.assertRaises(exception.UnsupportedDriverExtension, + self.iface.validate, self.task) + + def test_unmanaged_ok(self, mock_client): + self.driver.boot.validate_inspection.side_effect = ( + exception.UnsupportedDriverExtension('')) mock_introspect = mock_client.return_value.start_introspection self.assertEqual(states.INSPECTWAIT, self.iface.inspect_hardware(self.task)) mock_introspect.assert_called_once_with(self.node.uuid) + self.assertFalse(self.driver.boot.prepare_ramdisk.called) + self.assertFalse(self.driver.network.add_inspection_network.called) + self.assertFalse(self.driver.power.reboot.called) + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) @mock.patch.object(task_manager, 'acquire', autospec=True) - def test_error(self, mock_acquire, mock_client): + def test_unmanaged_error(self, mock_acquire, mock_client): + mock_acquire.return_value.__enter__.return_value = self.task + self.driver.boot.validate_inspection.side_effect = ( + exception.UnsupportedDriverExtension('')) mock_introspect = mock_client.return_value.start_introspection mock_introspect.side_effect = RuntimeError('boom') self.iface.inspect_hardware(self.task) mock_introspect.assert_called_once_with(self.node.uuid) - task = mock_acquire.return_value.__enter__.return_value - self.assertIn('boom', task.node.last_error) - task.process_event.assert_called_once_with('fail') + self.assertIn('boom', self.task.node.last_error) + self.task.process_event.assert_called_once_with('fail') + self.assertFalse(self.driver.boot.prepare_ramdisk.called) + self.assertFalse(self.driver.network.add_inspection_network.called) + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) + + def test_require_managed_boot(self, mock_client): + CONF.set_override('require_managed_boot', True, group='inspector') + self.driver.boot.validate_inspection.side_effect = ( + exception.UnsupportedDriverExtension('')) + mock_introspect = mock_client.return_value.start_introspection + self.assertRaises(exception.UnsupportedDriverExtension, + self.iface.inspect_hardware, self.task) + self.assertFalse(mock_introspect.called) + self.assertFalse(self.driver.boot.prepare_ramdisk.called) + self.assertFalse(self.driver.network.add_inspection_network.called) + self.assertFalse(self.driver.power.reboot.called) + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) + + def test_managed_ok(self, mock_client): + endpoint = 'http://192.169.0.42:5050/v1' + mock_client.return_value.get_endpoint.return_value = endpoint + mock_introspect = mock_client.return_value.start_introspection + self.assertEqual(states.INSPECTWAIT, + self.iface.inspect_hardware(self.task)) + mock_introspect.assert_called_once_with(self.node.uuid, + manage_boot=False) + self.driver.boot.prepare_ramdisk.assert_called_once_with( + self.task, ramdisk_params={ + 'ipa-inspection-callback-url': endpoint + '/continue', + }) + self.driver.network.add_inspection_network.assert_called_once_with( + self.task) + self.driver.power.reboot.assert_called_once_with( + self.task, timeout=None) + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) + + def test_managed_custom_params(self, mock_client): + CONF.set_override('extra_kernel_params', + 'ipa-inspection-collectors=default,logs ' + 'ipa-collect-dhcp=1', + group='inspector') + endpoint = 'http://192.169.0.42:5050/v1' + mock_client.return_value.get_endpoint.return_value = endpoint + mock_introspect = mock_client.return_value.start_introspection + self.iface.validate(self.task) + self.assertEqual(states.INSPECTWAIT, + self.iface.inspect_hardware(self.task)) + mock_introspect.assert_called_once_with(self.node.uuid, + manage_boot=False) + self.driver.boot.prepare_ramdisk.assert_called_once_with( + self.task, ramdisk_params={ + 'ipa-inspection-callback-url': endpoint + '/continue', + 'ipa-inspection-collectors': 'default,logs', + 'ipa-collect-dhcp': '1', + }) + self.driver.network.add_inspection_network.assert_called_once_with( + self.task) + self.driver.power.reboot.assert_called_once_with( + self.task, timeout=None) + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) + + @mock.patch.object(task_manager, 'acquire', autospec=True) + def test_managed_error(self, mock_acquire, mock_client): + endpoint = 'http://192.169.0.42:5050/v1' + mock_client.return_value.get_endpoint.return_value = endpoint + mock_acquire.return_value.__enter__.return_value = self.task + mock_introspect = mock_client.return_value.start_introspection + mock_introspect.side_effect = RuntimeError('boom') + self.assertRaises(exception.HardwareInspectionFailure, + self.iface.inspect_hardware, self.task) + mock_introspect.assert_called_once_with(self.node.uuid, + manage_boot=False) + self.assertIn('boom', self.task.node.last_error) + self.driver.boot.prepare_ramdisk.assert_called_once_with( + self.task, ramdisk_params={ + 'ipa-inspection-callback-url': endpoint + '/continue', + }) + self.driver.network.add_inspection_network.assert_called_once_with( + self.task) + self.driver.network.remove_inspection_network.assert_called_once_with( + self.task) + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) + self.driver.power.set_power_state.assert_called_once_with( + self.task, 'power off', timeout=None) @mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) @@ -144,6 +292,43 @@ class CheckStatusTestCase(BaseTestCase): inspector._check_status(self.task) mock_get.assert_called_once_with(self.node.uuid) self.task.process_event.assert_called_once_with('done') + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) + + def test_status_ok_managed(self, mock_client): + utils.set_node_nested_field(self.node, 'driver_internal_info', + 'inspector_manage_boot', True) + self.node.save() + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error=None, + spec=['is_finished', 'error']) + inspector._check_status(self.task) + mock_get.assert_called_once_with(self.node.uuid) + self.task.process_event.assert_called_once_with('done') + self.driver.network.remove_inspection_network.assert_called_once_with( + self.task) + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) + self.driver.power.set_power_state.assert_called_once_with( + self.task, 'power off', timeout=None) + + def test_status_ok_managed_no_power_off(self, mock_client): + CONF.set_override('power_off', False, group='inspector') + utils.set_node_nested_field(self.node, 'driver_internal_info', + 'inspector_manage_boot', True) + self.node.save() + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error=None, + spec=['is_finished', 'error']) + inspector._check_status(self.task) + mock_get.assert_called_once_with(self.node.uuid) + self.task.process_event.assert_called_once_with('done') + self.driver.network.remove_inspection_network.assert_called_once_with( + self.task) + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) + self.assertFalse(self.driver.power.set_power_state.called) def test_status_error(self, mock_client): mock_get = mock_client.return_value.get_introspection @@ -154,6 +339,75 @@ class CheckStatusTestCase(BaseTestCase): mock_get.assert_called_once_with(self.node.uuid) self.task.process_event.assert_called_once_with('fail') self.assertIn('boom', self.node.last_error) + self.assertFalse(self.driver.network.remove_inspection_network.called) + self.assertFalse(self.driver.boot.clean_up_ramdisk.called) + self.assertFalse(self.driver.power.set_power_state.called) + + def test_status_error_managed(self, mock_client): + utils.set_node_nested_field(self.node, 'driver_internal_info', + 'inspector_manage_boot', True) + self.node.save() + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error='boom', + spec=['is_finished', 'error']) + inspector._check_status(self.task) + mock_get.assert_called_once_with(self.node.uuid) + self.task.process_event.assert_called_once_with('fail') + self.assertIn('boom', self.node.last_error) + self.driver.network.remove_inspection_network.assert_called_once_with( + self.task) + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) + self.driver.power.set_power_state.assert_called_once_with( + self.task, 'power off', timeout=None) + + def test_status_error_managed_no_power_off(self, mock_client): + CONF.set_override('power_off', False, group='inspector') + utils.set_node_nested_field(self.node, 'driver_internal_info', + 'inspector_manage_boot', True) + self.node.save() + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error='boom', + spec=['is_finished', 'error']) + inspector._check_status(self.task) + mock_get.assert_called_once_with(self.node.uuid) + self.task.process_event.assert_called_once_with('fail') + self.assertIn('boom', self.node.last_error) + self.driver.network.remove_inspection_network.assert_called_once_with( + self.task) + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) + self.assertFalse(self.driver.power.set_power_state.called) + + def _test_status_clean_up_failed(self, mock_client): + utils.set_node_nested_field(self.node, 'driver_internal_info', + 'inspector_manage_boot', True) + self.node.save() + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error=None, + spec=['is_finished', 'error']) + inspector._check_status(self.task) + mock_get.assert_called_once_with(self.node.uuid) + self.task.process_event.assert_called_once_with('fail') + self.assertIn('boom', self.node.last_error) + + def test_status_boot_clean_up_failed(self, mock_client): + self.driver.boot.clean_up_ramdisk.side_effect = RuntimeError('boom') + + self._test_status_clean_up_failed(mock_client) + + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) + + def test_status_network_clean_up_failed(self, mock_client): + self.driver.network.remove_inspection_network.side_effect = \ + RuntimeError('boom') + + self._test_status_clean_up_failed(mock_client) + + self.driver.network.remove_inspection_network.assert_called_once_with( + self.task) + self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) @mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) diff --git a/releasenotes/notes/inspection-boot-network-59fd23ca62b09e81.yaml b/releasenotes/notes/inspection-boot-network-59fd23ca62b09e81.yaml new file mode 100644 index 0000000000..aa4b96972f --- /dev/null +++ b/releasenotes/notes/inspection-boot-network-59fd23ca62b09e81.yaml @@ -0,0 +1,19 @@ +--- +features: + - | + It's now possible to force booting for in-band inspection to be managed + by ironic by setting the new ``[inspector]require_managed_boot`` option + to ``True``. In-band inspection will fail if the node's driver does not + support managing boot for it. +other: + - | + Boot and network interface implementations can now manage boot for in-band + inspection by implementing the new methods: + + * ``BootInterface.validate_inspection`` + * ``NetworkInterface.validate_inspection`` + * ``NetworkInterface.add_inspection_network`` + * ``NetworkInterface.remove_inspection_network`` + + Previously only ironic-inspector itself could manage boot for it. This + change opens a way for non-PXE implementations of in-band inspection.