From 628e71cec8b60c50b3fc67b0528beeb1ebd59cbb Mon Sep 17 00:00:00 2001 From: Aparna Date: Wed, 15 Mar 2017 03:48:16 +0000 Subject: [PATCH] Implements validate_rescue() for IloVirtualMediaBoot This commit implements validate_rescue() for 'ilo-virtual-media' boot interface of 'ilo' hardware type. With this it enables 'agent' rescue interface for 'ilo' hardware type when corresponding boot interface being used is 'ilo-virtual-media'. Support already exists for the 'agent' rescue interface with 'ilo-pxe' boot interface. Co-Authored-By: Aparna Co-Authored-By: Shivanand Tendulker Closes-Bug: #1674899 Change-Id: Ia1d879e1356de18c5f14f032ddd195f4b48f0378 --- ironic/drivers/modules/ilo/boot.py | 67 ++++++++++---- .../unit/drivers/modules/ilo/test_boot.py | 89 ++++++++++++++++--- ironic/tests/unit/drivers/test_ilo.py | 34 +++++++ ...or-ilo-hardware-type-2392989d0fef8849.yaml | 7 ++ 4 files changed, 166 insertions(+), 31 deletions(-) create mode 100644 releasenotes/notes/rescue-interface-for-ilo-hardware-type-2392989d0fef8849.yaml diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 85388fe4d8..16300b29b7 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -49,10 +49,15 @@ REQUIRED_PROPERTIES = { 'ilo_deploy_iso': _("UUID (from Glance) of the deployment ISO. " "Required.") } +RESCUE_PROPERTIES = { + 'ilo_rescue_iso': _("UUID (from Glance) of the rescue ISO. Only " + "required if rescue mode is being used and ironic is " + "managing booting the rescue ramdisk.") +} COMMON_PROPERTIES = REQUIRED_PROPERTIES -def parse_driver_info(node): +def parse_driver_info(node, mode='deploy'): """Gets the driver specific Node deployment info. This method validates whether the 'driver_info' property of the @@ -60,16 +65,23 @@ def parse_driver_info(node): deploy images to the node. :param node: a single Node. + :param mode: Label indicating a deploy or rescue operation being + carried out on the node. Supported values are + 'deploy' and 'rescue'. Defaults to 'deploy', indicating + deploy operation is being carried out. :returns: A dict with the driver_info values. :raises: MissingParameterValue, if any of the required parameters are missing. """ info = node.driver_info d_info = {} - d_info['ilo_deploy_iso'] = info.get('ilo_deploy_iso') + if mode == 'rescue': + d_info['ilo_rescue_iso'] = info.get('ilo_rescue_iso') + else: + d_info['ilo_deploy_iso'] = info.get('ilo_deploy_iso') - error_msg = _("Error validating iLO virtual media deploy. Some parameters" - " were missing in node's driver_info") + error_msg = (_("Error validating iLO virtual media for %s. Some " + "parameters were missing in node's driver_info") % mode) deploy_utils.check_for_missing_params(d_info, error_msg) return d_info @@ -386,6 +398,9 @@ class IloVirtualMediaBoot(base.BootInterface): capabilities = ['iscsi_volume_boot'] def get_properties(self): + # TODO(stendulker): COMMON_PROPERTIES should also include rescue + # related properties (RESCUE_PROPERTIES). We can add them in Rocky, + # when classic drivers get removed. return COMMON_PROPERTIES @METRICS.timer('IloVirtualMediaBoot.validate') @@ -410,7 +425,7 @@ class IloVirtualMediaBoot(base.BootInterface): def prepare_ramdisk(self, task, ramdisk_params): """Prepares the boot of deploy ramdisk using virtual media. - This method prepares the boot of the deploy ramdisk after + This method prepares the boot of the deploy or rescue ramdisk after reading relevant information from the node's driver_info and instance_info. @@ -428,23 +443,25 @@ class IloVirtualMediaBoot(base.BootInterface): node = task.node # NOTE(TheJulia): If this method is being called by something - # aside from deployment and clean, such as conductor takeover, we - # should treat this as a no-op and move on otherwise we would modify - # the state of the node due to virtual media operations. - if (node.provision_state != states.DEPLOYING and - node.provision_state != states.CLEANING): + # aside from deployment, clean and rescue, such as conductor takeover, + # we should treat this as a no-op and move on otherwise we would + # modify the state of the node due to virtual media operations. + if node.provision_state not in (states.DEPLOYING, + states.CLEANING, + states.RESCUING): return # Powering off the Node before initiating boot for node cleaning. # If node is in system POST, setting boot device fails. manager_utils.node_power_action(task, states.POWER_OFF) - if task.node.provision_state == states.DEPLOYING: + if node.provision_state in (states.DEPLOYING, + states.RESCUING): prepare_node_for_deploy(task) # Clear ilo_boot_iso if it's a glance image to force recreate # another one again (or use existing one in glance). - # This is mainly for rebuild scenario. + # This is mainly for rebuild and rescue scenario. if service_utils.is_glance_image( node.instance_info.get('image_source')): instance_info = node.instance_info @@ -458,9 +475,12 @@ class IloVirtualMediaBoot(base.BootInterface): deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac - deploy_iso = node.driver_info['ilo_deploy_iso'] + if node.provision_state == states.RESCUING: + iso = node.driver_info['ilo_rescue_iso'] + else: + iso = node.driver_info['ilo_deploy_iso'] - ilo_common.setup_vmedia(task, deploy_iso, ramdisk_params) + ilo_common.setup_vmedia(task, iso, ramdisk_params) @METRICS.timer('IloVirtualMediaBoot.prepare_instance') def prepare_instance(self, task): @@ -552,7 +572,11 @@ class IloVirtualMediaBoot(base.BootInterface): else: _clean_up_boot_iso_for_instance(task.node) driver_internal_info.pop('boot_iso_created_in_web_server', None) - driver_internal_info.pop('root_uuid_or_disk_id', None) + # Need to retain 'root_uuid_or_disk_id' during rescue. It would + # be required if boot iso needs to be created during unrescue + # operation. + if task.node.provision_state != states.RESCUING: + driver_internal_info.pop('root_uuid_or_disk_id', None) ilo_common.cleanup_vmedia_boot(task) task.node.driver_internal_info = driver_internal_info task.node.save() @@ -562,13 +586,12 @@ class IloVirtualMediaBoot(base.BootInterface): """Cleans up the boot of ironic ramdisk. This method cleans up virtual media devices setup for the deploy - ramdisk. + or rescue ramdisk. :param task: a task from TaskManager. :returns: None :raises: IloOperationError, if some operation on iLO failed. """ - ilo_common.cleanup_vmedia_boot(task) def _configure_vmedia_boot(self, task, root_uuid): @@ -599,6 +622,16 @@ class IloVirtualMediaBoot(base.BootInterface): node.instance_info = i_info node.save() + @METRICS.timer('IloVirtualMediaBoot.validate_rescue') + def validate_rescue(self, task): + """Validate that the node has required properties for rescue. + + :param task: a TaskManager instance with the node being checked + :raises: MissingParameterValue if node is missing one or more required + parameters + """ + parse_driver_info(task.node, mode='rescue') + class IloPXEBoot(pxe.PXEBoot): diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 1bce8f432c..63d5ff02c5 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -672,9 +672,19 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): def setUp(self): super(IloVirtualMediaBootTestCase, self).setUp() - mgr_utils.mock_the_extension_manager(driver="iscsi_ilo") + self.config(enabled_hardware_types=['ilo'], + enabled_boot_interfaces=['ilo-virtual-media'], + enabled_console_interfaces=['ilo'], + enabled_deploy_interfaces=['iscsi'], + enabled_inspect_interfaces=['ilo'], + enabled_management_interfaces=['ilo'], + enabled_power_interfaces=['ilo'], + enabled_raid_interfaces=['no-raid'], + enabled_rescue_interfaces=['agent'], + enabled_vendor_interfaces=['no-vendor']) + self.config(enabled_hardware_types=['ilo']) self.node = obj_utils.create_test_node( - self.context, driver='iscsi_ilo', driver_info=INFO_DICT) + self.context, driver='ilo', driver_info=INFO_DICT) @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', autospec=True) @@ -728,35 +738,39 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): eject_mock, node_power_mock, prepare_node_for_deploy_mock, ilo_boot_iso, image_source, - ramdisk_params={'a': 'b'}): + ramdisk_params={'a': 'b'}, + mode='deploy'): instance_info = self.node.instance_info instance_info['ilo_boot_iso'] = ilo_boot_iso instance_info['image_source'] = image_source self.node.instance_info = instance_info self.node.save() + iso = 'provisioning-iso' get_nic_mock.return_value = '12:34:56:78:90:ab' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: driver_info = task.node.driver_info - driver_info['ilo_deploy_iso'] = 'deploy-iso' + driver_info['ilo_%s_iso' % mode] = iso task.node.driver_info = driver_info task.driver.boot.prepare_ramdisk(task, ramdisk_params) node_power_mock.assert_called_once_with(task, states.POWER_OFF) - if task.node.provision_state == states.DEPLOYING: + if task.node.provision_state in (states.DEPLOYING, + states.RESCUING): prepare_node_for_deploy_mock.assert_called_once_with(task) + eject_mock.assert_called_once_with(task) expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab'} get_nic_mock.assert_called_once_with(task) - setup_vmedia_mock.assert_called_once_with(task, 'deploy-iso', + setup_vmedia_mock.assert_called_once_with(task, iso, expected_ramdisk_opts) @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, autospec=True) - def test_prepare_ramdisk_not_deploying_not_cleaning(self, mock_is_image): + def test_prepare_ramdisk_in_takeover(self, mock_is_image): """Ensure deploy ops are blocked when not deploying and not cleaning""" for state in states.STABLE_STATES: @@ -769,6 +783,27 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, None)) self.assertFalse(mock_is_image.called) + def test_prepare_ramdisk_rescue_glance_image(self): + self.node.provision_state = states.RESCUING + self.node.save() + self._test_prepare_ramdisk( + ilo_boot_iso='swift:abcdef', + image_source='6b2f0c0c-79e8-4db6-842e-43c9764204af', + mode='rescue') + self.node.refresh() + self.assertNotIn('ilo_boot_iso', self.node.instance_info) + + def test_prepare_ramdisk_rescue_not_a_glance_image(self): + self.node.provision_state = states.RESCUING + self.node.save() + self._test_prepare_ramdisk( + ilo_boot_iso='http://mybootiso', + image_source='http://myimage', + mode='rescue') + self.node.refresh() + self.assertEqual('http://mybootiso', + self.node.instance_info['ilo_boot_iso']) + def test_prepare_ramdisk_glance_image(self): self.node.provision_state = states.DEPLOYING self.node.save() @@ -865,16 +900,16 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(ilo_boot, '_clean_up_boot_iso_for_instance', spec_set=True, autospec=True) - def test_clean_up_instance(self, cleanup_iso_mock, - cleanup_vmedia_mock, node_power_mock, - update_secure_boot_mode_mock, - is_iscsi_boot_mock): + def _test_clean_up_instance(self, cleanup_iso_mock, + cleanup_vmedia_mock, node_power_mock, + update_secure_boot_mode_mock, + is_iscsi_boot_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + root_uuid = "12312642-09d3-467f-8e09-12385826a123" driver_internal_info = task.node.driver_internal_info driver_internal_info['boot_iso_created_in_web_server'] = False - driver_internal_info['root_uuid_or_disk_id'] = ( - "12312642-09d3-467f-8e09-12385826a123") + driver_internal_info['root_uuid_or_disk_id'] = root_uuid task.node.driver_internal_info = driver_internal_info task.node.save() is_iscsi_boot_mock.return_value = False @@ -884,11 +919,23 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): driver_internal_info = task.node.driver_internal_info self.assertNotIn('boot_iso_created_in_web_server', driver_internal_info) - self.assertNotIn('root_uuid_or_disk_id', driver_internal_info) + if task.node.provision_state != states.RESCUING: + self.assertNotIn('root_uuid_or_disk_id', driver_internal_info) + else: + self.assertEqual(root_uuid, + driver_internal_info['root_uuid_or_disk_id']) node_power_mock.assert_called_once_with(task, states.POWER_OFF) update_secure_boot_mode_mock.assert_called_once_with(task, False) + def test_clean_up_instance_deleting(self): + self.node.provisioning_state = states.DELETING + self._test_clean_up_instance() + + def test_clean_up_instance_rescuing(self): + self.node.provisioning_state = states.RESCUING + self._test_clean_up_instance() + @mock.patch.object(deploy_utils, 'is_iscsi_boot', spec_set=True, autospec=True) @mock.patch.object(ilo_management.IloManagement, 'clear_iscsi_boot_target', @@ -1078,6 +1125,20 @@ class IloVirtualMediaBootTestCase(db_base.DbTestCase): self.assertIsNone(task.node.driver_internal_info.get( 'ilo_uefi_iscsi_boot')) + def test_validate_rescue(self): + driver_info = self.node.driver_info + driver_info['ilo_rescue_iso'] = 'rescue.iso' + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.boot.validate_rescue(task) + + def test_validate_rescue_no_rescue_ramdisk(self): + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaisesRegex(exception.MissingParameterValue, + 'Missing.*ilo_rescue_iso', + task.driver.boot.validate_rescue, task) + class IloPXEBootTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/test_ilo.py b/ironic/tests/unit/drivers/test_ilo.py index 02c681dee8..e6e056a207 100644 --- a/ironic/tests/unit/drivers/test_ilo.py +++ b/ironic/tests/unit/drivers/test_ilo.py @@ -48,6 +48,7 @@ class IloHardwareTestCase(db_base.DbTestCase): enabled_management_interfaces=['ilo'], enabled_power_interfaces=['ilo'], enabled_raid_interfaces=['no-raid', 'agent'], + enabled_rescue_interfaces=['no-rescue', 'agent'], enabled_vendor_interfaces=['ilo', 'no-vendor']) def test_default_interfaces(self): @@ -70,6 +71,8 @@ class IloHardwareTestCase(db_base.DbTestCase): noop.NoRAID) self.assertIsInstance(task.driver.vendor, ilo.vendor.VendorPassthru) + self.assertIsInstance(task.driver.rescue, + noop.NoRescue) def test_override_with_inspector(self): self.config(enabled_inspect_interfaces=['inspector', 'ilo']) @@ -94,6 +97,8 @@ class IloHardwareTestCase(db_base.DbTestCase): ilo.power.IloPower) self.assertIsInstance(task.driver.raid, agent.AgentRAID) + self.assertIsInstance(task.driver.rescue, + noop.NoRescue) self.assertIsInstance(task.driver.vendor, noop.NoVendor) @@ -117,6 +122,35 @@ class IloHardwareTestCase(db_base.DbTestCase): ilo.power.IloPower) self.assertIsInstance(task.driver.raid, agent.AgentRAID) + self.assertIsInstance(task.driver.rescue, + noop.NoRescue) + self.assertIsInstance(task.driver.vendor, + ilo.vendor.VendorPassthru) + + def test_override_with_agent_rescue(self): + self.config(enabled_inspect_interfaces=['inspector', 'ilo']) + node = obj_utils.create_test_node( + self.context, driver='ilo', + deploy_interface='direct', + rescue_interface='agent', + raid_interface='agent') + with task_manager.acquire(self.context, node.id) as task: + self.assertIsInstance(task.driver.boot, + ilo.boot.IloVirtualMediaBoot) + self.assertIsInstance(task.driver.console, + ilo.console.IloConsoleInterface) + self.assertIsInstance(task.driver.deploy, + agent.AgentDeploy) + self.assertIsInstance(task.driver.inspect, + ilo.inspect.IloInspect) + self.assertIsInstance(task.driver.management, + ilo.management.IloManagement) + self.assertIsInstance(task.driver.power, + ilo.power.IloPower) + self.assertIsInstance(task.driver.raid, + agent.AgentRAID) + self.assertIsInstance(task.driver.rescue, + agent.AgentRescue) self.assertIsInstance(task.driver.vendor, ilo.vendor.VendorPassthru) diff --git a/releasenotes/notes/rescue-interface-for-ilo-hardware-type-2392989d0fef8849.yaml b/releasenotes/notes/rescue-interface-for-ilo-hardware-type-2392989d0fef8849.yaml new file mode 100644 index 0000000000..622c90d986 --- /dev/null +++ b/releasenotes/notes/rescue-interface-for-ilo-hardware-type-2392989d0fef8849.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds support for rescue interface ``agent`` for ``ilo`` hardware type + when corresponding boot interface being used is ``ilo-virtual-media``. + The supported values of rescue interface for ``ilo`` hardware type + are ``agent`` and ``no-rescue``. The default value is ``no-rescue``.