From 49baa2cc8bdd88341f062a16bdf237108080e011 Mon Sep 17 00:00:00 2001 From: Nisha Agarwal Date: Tue, 27 Oct 2015 02:13:39 -0700 Subject: take_over() for iscsi_ilo doesnt recreate boot iso When using web server for hosting images for ilo driver, take_over should recreate the boot ISO, if it was created by Ironic. If the boot iso is not recreated, then the node doesn't boot up successfully when a node is powered off and powered on. The fix is available in master branch via I5481e705892f66cb3184ac16ff56c123f5c69087 in file boot.py from lines 117 to 122. The test case for the fix is available via https://review.openstack.org/#/c/239627 in master branch. The master branch patch I5481e705892f66cb3184ac16ff56c123f5c69087 implements a feature (~2K lines of change) while the fix for this issue is just 5-6 lines of change. Closes-bug: #1510425 Change-Id: I6b7f3901f773a5aba906c50dcbd574412f7b6af3 --- ironic/drivers/modules/ilo/deploy.py | 7 +++- ironic/tests/drivers/ilo/test_deploy.py | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index 388ca6e07..799dcf405 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -156,7 +156,12 @@ def _get_boot_iso(task, root_uuid): # Option 1 - Check if user has provided ilo_boot_iso in node's # instance_info - if task.node.instance_info.get('ilo_boot_iso'): + driver_internal_info = task.node.driver_internal_info + boot_iso_created_in_web_server = ( + driver_internal_info.get('boot_iso_created_in_web_server')) + + if (task.node.instance_info.get('ilo_boot_iso') + and not boot_iso_created_in_web_server): LOG.debug("Using ilo_boot_iso provided in node's instance_info") boot_iso = task.node.instance_info['ilo_boot_iso'] if not service_utils.is_glance_image(boot_iso): diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index df886cd09..c632ae012 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -281,6 +281,73 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): copy_file_mock.assert_called_once_with(fileobj_mock.name, 'abcdef') + @mock.patch.object(ilo_common, 'copy_image_to_web_server', spec_set=True, + autospec=True) + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(ilo_deploy, '_get_boot_iso_object_name', spec_set=True, + autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + autospec=True) + @mock.patch.object(images, 'get_image_properties', spec_set=True, + autospec=True) + @mock.patch.object(ilo_deploy, '_parse_deploy_info', spec_set=True, + autospec=True) + def test__get_boot_iso_recreate_boot_iso_use_webserver( + self, deploy_info_mock, image_props_mock, + capability_mock, boot_object_name_mock, + create_boot_iso_mock, tempfile_mock, + copy_file_mock): + CONF.ilo.use_web_server_for_images = True + CONF.deploy.http_url = "http://10.10.1.30/httpboot" + CONF.deploy.http_root = "/httpboot" + CONF.pxe.pxe_append_params = 'kernel-params' + + fileobj_mock = mock.MagicMock(spec=file) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=file) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + + ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" + kernel_href = "http://10.10.1.30/httpboot/kernel" + deploy_info_mock.return_value = {'image_source': 'image-uuid', + 'ilo_deploy_iso': 'deploy_iso_uuid'} + image_props_mock.return_value = {'boot_iso': None, + 'kernel_id': kernel_href, + 'ramdisk_id': ramdisk_href} + boot_object_name_mock.return_value = 'new_boot_iso' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'uefi' + copy_file_mock.return_value = "http://10.10.1.30/httpboot/new_boot_iso" + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + driver_internal_info = task.node.driver_internal_info + driver_internal_info['boot_iso_created_in_web_server'] = True + instance_info = task.node.instance_info + old_boot_iso = 'http://10.10.1.30/httpboot/old_boot_iso' + instance_info['ilo_boot_iso'] = old_boot_iso + boot_iso_actual = ilo_deploy._get_boot_iso(task, 'root-uuid') + deploy_info_mock.assert_called_once_with(task.node) + image_props_mock.assert_called_once_with( + task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) + boot_object_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with(task.context, + 'tmpfile', + kernel_href, + ramdisk_href, + 'deploy_iso_uuid', + 'root-uuid', + 'kernel-params', + 'uefi') + boot_iso_expected = 'http://10.10.1.30/httpboot/new_boot_iso' + self.assertEqual(boot_iso_expected, boot_iso_actual) + copy_file_mock.assert_called_once_with(fileobj_mock.name, + 'new_boot_iso') + @mock.patch.object(ilo_deploy, '_get_boot_iso_object_name', spec_set=True, autospec=True) @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) -- cgit v1.2.1