From c4d3abcca8186e26db872ac49759d2ad7289cb45 Mon Sep 17 00:00:00 2001 From: Ramakrishnan G Date: Fri, 19 Jun 2015 00:05:51 -0700 Subject: [PATCH] Clear ilo_boot_iso before deploy for glance images This commit clears instance_info['ilo_boot_iso'] in iscsi_ilo driver for glance images before beginning deploy. This will make sure that in rebuild event with glance images, we don't end up using previously generated (or used) boot ISOs. It also ejects any virtual media devices connected to make sure that deploy images can be connected later on during deploy. The proliantutils version is updated to 2.1.1, which makes sure that we silently return on ilo_client.eject_vmedia_device() when virtual media is not connected (instead of raising an exception). Change-Id: I68fcfc2b62cb58dd1c16f312c3a6a2ebd383c170 Closes-Bug: 1466729 --- doc/source/drivers/ilo.rst | 4 +- driver-requirements.txt | 2 +- ironic/drivers/modules/ilo/common.py | 34 ++++++++++++----- ironic/drivers/modules/ilo/deploy.py | 20 +++++++++- ironic/tests/drivers/ilo/test_common.py | 35 +++++++++++++++--- ironic/tests/drivers/ilo/test_deploy.py | 49 +++++++++++++++++++++---- 6 files changed, 117 insertions(+), 27 deletions(-) diff --git a/doc/source/drivers/ilo.rst b/doc/source/drivers/ilo.rst index 5804a7019d..387e9ee7a7 100644 --- a/doc/source/drivers/ilo.rst +++ b/doc/source/drivers/ilo.rst @@ -40,9 +40,9 @@ Prerequisites managing HP Proliant hardware. Install ``proliantutils`` [2]_ module on the Ironic conductor node. Minimum - version required is 2.1.0.:: + version required is 2.1.1.:: - $ pip install "proliantutils>=2.1.0" + $ pip install "proliantutils>=2.1.1" * ``ipmitool`` command must be present on the service node(s) where ``ironic-conductor`` is running. On most distros, this is provided as part diff --git a/driver-requirements.txt b/driver-requirements.txt index 5064e884e0..84eaca3891 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -4,7 +4,7 @@ # python projects they should package as optional dependencies for Ironic. # These are available on pypi -proliantutils>=2.1.0 +proliantutils>=2.1.1 pyghmi pysnmp python-ironic-inspector-client diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 34da277816..8aab409ad0 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -414,6 +414,29 @@ def setup_vmedia_for_boot(task, boot_iso, parameters=None): attach_vmedia(task.node, 'CDROM', boot_iso_url or boot_iso) +def eject_vmedia_devices(task): + """Ejects virtual media devices. + + This method ejects virtual media devices. It ignores + any exception encountered in the process. + + :param task: a TaskManager instance containing the node to act on. + :returns: None + """ + ilo_object = get_ilo_object(task.node) + for device in ('FLOPPY', 'CDROM'): + try: + ilo_object.eject_virtual_media(device) + except ilo_error.IloError as ilo_exception: + LOG.error(_LE("Error while ejecting virtual media %(device)s " + "from node %(uuid)s. Error: %(error)s"), + {'device': device, 'uuid': task.node.uuid, + 'error': ilo_exception}) + operation = _("Eject virtual media %s") % device.lower() + raise exception.IloOperationError(operation=operation, + error=ilo_exception) + + def cleanup_vmedia_boot(task): """Cleans a node after a virtual media boot. @@ -435,16 +458,7 @@ def cleanup_vmedia_boot(task): "%(container)s. Error: %(error)s"), {'object_name': object_name, 'container': container, 'error': e}) - - ilo_object = get_ilo_object(task.node) - for device in ('FLOPPY', 'CDROM'): - try: - ilo_object.eject_virtual_media(device) - except ilo_error.IloError as ilo_exception: - LOG.exception(_LE("Error while ejecting virtual media %(device)s " - "from node %(uuid)s. Error: %(error)s"), - {'device': device, 'uuid': task.node.uuid, - 'error': ilo_exception}) + eject_vmedia_devices(task) def get_secure_boot_mode(task): diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index a2c6c23919..26b8542ccf 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -273,7 +273,10 @@ def _reboot_into(task, iso, ramdisk_options): def _prepare_agent_vmedia_boot(task): - """prepare for vmedia boot.""" + """Ejects virtual media devices and prepares for vmedia boot.""" + # Eject all virtual media devices are we are going to use them + # during deploy. + ilo_common.eject_vmedia_devices(task) deploy_ramdisk_opts = agent.build_agent_options(task.node) deploy_iso = task.node.driver_info['ilo_deploy_iso'] @@ -411,6 +414,20 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface): """ node = task.node + # 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. + if service_utils.is_glance_image( + node.instance_info.get('image_source')): + instance_info = node.instance_info + instance_info.pop('ilo_boot_iso', None) + node.instance_info = instance_info + node.save() + + # Eject all virtual media devices are we are going to use them + # during deploy. + ilo_common.eject_vmedia_devices(task) + iscsi_deploy.cache_instance_image(task.context, node) iscsi_deploy.check_image_size(task) @@ -503,6 +520,7 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface): image. :raises: IloOperationError, if some operation on iLO fails. """ + _prepare_agent_vmedia_boot(task) return states.DEPLOYWAIT diff --git a/ironic/tests/drivers/ilo/test_common.py b/ironic/tests/drivers/ilo/test_common.py index c5c0d98a89..ec6442475c 100644 --- a/ironic/tests/drivers/ilo/test_common.py +++ b/ironic/tests/drivers/ilo/test_common.py @@ -426,18 +426,16 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): attach_vmedia_mock.assert_called_once_with(task.node, 'CDROM', boot_iso) - @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, - autospec=True) + @mock.patch.object(ilo_common, 'eject_vmedia_devices', + spec_set=True, autospec=True) @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) @mock.patch.object(ilo_common, '_get_floppy_image_name', spec_set=True, autospec=True) def test_cleanup_vmedia_boot(self, get_name_mock, swift_api_mock, - get_ilo_object_mock): + eject_mock): swift_obj_mock = swift_api_mock.return_value CONF.ilo.swift_ilo_container = 'ilo_cont' - ilo_object_mock = mock.MagicMock(spec=['eject_virtual_media']) - get_ilo_object_mock.return_value = ilo_object_mock get_name_mock.return_value = 'image-node-uuid' with task_manager.acquire(self.context, self.node.uuid, @@ -445,9 +443,36 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): ilo_common.cleanup_vmedia_boot(task) swift_obj_mock.delete_object.assert_called_once_with( 'ilo_cont', 'image-node-uuid') + eject_mock.assert_called_once_with(task) + + @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, + autospec=True) + def test_eject_vmedia_devices(self, get_ilo_object_mock): + ilo_object_mock = mock.MagicMock(spec=['eject_virtual_media']) + get_ilo_object_mock.return_value = ilo_object_mock + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + ilo_common.eject_vmedia_devices(task) ilo_object_mock.eject_virtual_media.assert_any_call('CDROM') ilo_object_mock.eject_virtual_media.assert_any_call('FLOPPY') + @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, + autospec=True) + def test_eject_vmedia_devices_raises( + self, get_ilo_object_mock): + ilo_object_mock = mock.MagicMock(spec=['eject_virtual_media']) + get_ilo_object_mock.return_value = ilo_object_mock + exc = ilo_error.IloError('error') + ilo_object_mock.eject_virtual_media.side_effect = exc + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises(exception.IloOperationError, + ilo_common.eject_vmedia_devices, + task) + + ilo_object_mock.eject_virtual_media.assert_called_once_with( + 'FLOPPY') + @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, autospec=True) def test_get_secure_boot_mode(self, diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index 51bf7bebd5..16d63c6bab 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -283,18 +283,23 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): boot_devices.CDROM) node_power_action_mock.assert_called_once_with(task, states.REBOOT) + @mock.patch.object(ilo_common, 'eject_vmedia_devices', + spec_set=True, autospec=True) @mock.patch.object(ilo_deploy, '_reboot_into', spec_set=True, autospec=True) @mock.patch.object(agent, 'build_agent_options', spec_set=True, autospec=True) def test__prepare_agent_vmedia_boot(self, build_options_mock, - reboot_into_mock): + reboot_into_mock, eject_mock): deploy_opts = {'a': 'b'} build_options_mock.return_value = deploy_opts with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso-uuid' + ilo_deploy._prepare_agent_vmedia_boot(task) + + eject_mock.assert_called_once_with(task) build_options_mock.assert_called_once_with(task.node) reboot_into_mock.assert_called_once_with(task, 'deploy-iso-uuid', @@ -565,6 +570,8 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): is_glance_image_mock.return_value = False self._test_validate(props_expected=['kernel', 'ramdisk']) + @mock.patch.object(ilo_common, 'eject_vmedia_devices', + spec_set=True, autospec=True) @mock.patch.object(ilo_deploy, '_reboot_into', spec_set=True, autospec=True) @mock.patch.object(deploy_utils, 'get_single_nic_with_vif_port_id', @@ -577,13 +584,23 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(iscsi_deploy, 'cache_instance_image', spec_set=True, autospec=True) - def test_deploy(self, - cache_instance_image_mock, - check_image_size_mock, - build_opts_mock, - agent_options_mock, - get_nic_mock, - reboot_into_mock): + def _test_deploy(self, + cache_instance_image_mock, + check_image_size_mock, + build_opts_mock, + agent_options_mock, + get_nic_mock, + reboot_into_mock, + eject_mock, + ilo_boot_iso, + image_source + ): + 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() + deploy_opts = {'a': 'b'} agent_options_mock.return_value = { 'ipa-api-url': 'http://1.2.3.4:6385'} @@ -595,6 +612,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' returned_state = task.driver.deploy.deploy(task) + eject_mock.assert_called_once_with(task) cache_instance_image_mock.assert_called_once_with(task.context, task.node) check_image_size_mock.assert_called_once_with(task) @@ -607,6 +625,21 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): self.assertEqual(states.DEPLOYWAIT, returned_state) + def test_deploy_glance_image(self): + self._test_deploy( + ilo_boot_iso='swift:abcdef', + image_source='6b2f0c0c-79e8-4db6-842e-43c9764204af') + self.node.refresh() + self.assertNotIn('ilo_boot_iso', self.node.instance_info) + + def test_deploy_not_a_glance_image(self): + self._test_deploy( + ilo_boot_iso='http://mybootiso', + image_source='http://myimage') + self.node.refresh() + self.assertEqual('http://mybootiso', + self.node.instance_info['ilo_boot_iso']) + @mock.patch.object(ilo_deploy, '_update_secure_boot_mode', spec_set=True, autospec=True) @mock.patch.object(manager_utils, 'node_power_action', spec_set=True,