From 2c18f22f9976870b71bfd1e471afa18fd50a7a25 Mon Sep 17 00:00:00 2001 From: Mike Turek Date: Wed, 28 Jun 2017 20:20:42 -0400 Subject: [PATCH] Generate iPXE boot script when deploying with boot from volume This patch moves the boot from volume skip logic for the prepare step of deployment into the boot interface, allowing the template to get generated before skipping the remainder of the step. Partial-Bug: #1559691 Change-Id: Icfea16c62a753c77942107af287880f35f28c404 --- ironic/drivers/modules/agent.py | 18 ++-- ironic/drivers/modules/pxe.py | 2 + .../tests/unit/drivers/modules/test_agent.py | 24 +++++ ironic/tests/unit/drivers/modules/test_pxe.py | 98 +++++++++++++------ 4 files changed, 105 insertions(+), 37 deletions(-) diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 32d0b249e6..3ee0463aaa 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -452,16 +452,18 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): if node.provision_state == states.ACTIVE: # Call is due to conductor takeover task.driver.boot.prepare_instance(task) - elif not task.driver.storage.should_write_image(task): - # We have nothing else to do as this is handled in the - # backend storage system. - return elif node.provision_state != states.ADOPTING: - node.instance_info = deploy_utils.build_instance_info_for_deploy( - task) - node.save() + + if task.driver.storage.should_write_image(task): + instance_info = deploy_utils.build_instance_info_for_deploy( + task) + node.instance_info = instance_info + node.save() if CONF.agent.manage_agent_boot: - deploy_opts = deploy_utils.build_agent_options(node) + if task.driver.storage.should_write_image(task): + deploy_opts = deploy_utils.build_agent_options(node) + else: + deploy_opts = None task.driver.boot.prepare_ramdisk(task, deploy_opts) @METRICS.timer('AgentDeploy.clean_up') diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index d6619d00af..a2d27ef008 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -472,6 +472,8 @@ class PXEBoot(base.BootInterface): if (not os.path.isfile(bootfile_path) or not utils.file_has_content(bootfile_path, boot_script)): utils.write_to_file(bootfile_path, boot_script) + if not task.driver.storage.should_write_image(task): + return dhcp_opts = pxe_utils.dhcp_options_for_instance(task) provider = dhcp_factory.DHCPFactory() diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index a7e637ebaf..fe76121874 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -426,6 +426,30 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(pxe_prepare_ramdisk_mock.called) self.assertFalse(add_provisioning_net_mock.called) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'add_provisioning_network', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') + @mock.patch.object(deploy_utils, 'build_agent_options') + @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy') + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + autospec=True) + def test_prepare_boot_from_volume(self, mock_write, + build_instance_info_mock, + build_options_mock, + pxe_prepare_ramdisk_mock, + add_provisioning_net_mock): + mock_write.return_value = False + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.DEPLOYING + build_instance_info_mock.return_value = {'foo': 'bar'} + build_options_mock.return_value = {'a': 'b'} + + self.driver.prepare(task) + build_instance_info_mock.assert_not_called() + build_options_mock.assert_not_called() + pxe_prepare_ramdisk_mock.assert_called_once_with(task, None) + @mock.patch('ironic.common.dhcp_factory.DHCPFactory._set_dhcp_provider') @mock.patch('ironic.common.dhcp_factory.DHCPFactory.clean_dhcp') @mock.patch.object(pxe.PXEBoot, 'clean_up_instance') diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 2a53368b59..df3d675027 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -808,7 +808,10 @@ class PXEBootTestCase(db_base.DbTestCase): @mock.patch.object(pxe, '_cache_ramdisk_kernel', autospec=True) @mock.patch.object(pxe, '_build_pxe_config_options', autospec=True) @mock.patch.object(pxe_utils, 'create_pxe_config', autospec=True) - def _test_prepare_ramdisk(self, mock_pxe_config, + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + autospec=True) + def _test_prepare_ramdisk(self, mock_should_write_image, + mock_pxe_config, mock_build_pxe, mock_cache_r_k, mock_deploy_img_info, mock_instance_img_info, @@ -817,7 +820,9 @@ class PXEBootTestCase(db_base.DbTestCase): uefi=False, cleaning=False, ipxe_use_swift=False, - whole_disk_image=False): + whole_disk_image=False, + should_write_image=True): + mock_should_write_image.return_value = should_write_image mock_build_pxe.return_value = {} mock_deploy_img_info.return_value = {'deploy_kernel': 'a'} if whole_disk_image: @@ -833,38 +838,51 @@ class PXEBootTestCase(db_base.DbTestCase): self.node.driver_internal_info = driver_internal_info self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: - dhcp_opts = pxe_utils.dhcp_options_for_instance(task) + if should_write_image: + dhcp_opts = pxe_utils.dhcp_options_for_instance(task) task.driver.boot.prepare_ramdisk(task, {'foo': 'bar'}) - mock_deploy_img_info.assert_called_once_with(task.node) - provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts) - set_boot_device_mock.assert_called_once_with(task, - boot_devices.PXE, - persistent=False) - if ipxe_use_swift: - if whole_disk_image: - self.assertFalse(mock_cache_r_k.called) + if should_write_image: + mock_deploy_img_info.assert_called_once_with(task.node) + provider_mock.update_dhcp.assert_called_once_with(task, + dhcp_opts) + set_boot_device_mock.assert_called_once_with(task, + boot_devices.PXE, + persistent=False) + if ipxe_use_swift: + if whole_disk_image: + self.assertFalse(mock_cache_r_k.called) + else: + mock_cache_r_k.assert_called_once_with( + self.context, task.node, + {'kernel': 'b'}) + mock_instance_img_info.assert_called_once_with( + task.node, self.context) + elif cleaning is False: + mock_cache_r_k.assert_called_once_with( + self.context, task.node, + {'deploy_kernel': 'a', 'kernel': 'b'}) + mock_instance_img_info.assert_called_once_with( + task.node, self.context) else: mock_cache_r_k.assert_called_once_with( self.context, task.node, - {'kernel': 'b'}) - mock_instance_img_info.assert_called_once_with(task.node, - self.context) - elif cleaning is False: - mock_cache_r_k.assert_called_once_with( - self.context, task.node, - {'deploy_kernel': 'a', 'kernel': 'b'}) - mock_instance_img_info.assert_called_once_with(task.node, - self.context) + {'deploy_kernel': 'a'}) + if uefi: + mock_pxe_config.assert_called_once_with( + task, {'foo': 'bar'}, + CONF.pxe.uefi_pxe_config_template) + else: + mock_pxe_config.assert_called_once_with( + task, {'foo': 'bar'}, CONF.pxe.pxe_config_template) else: - mock_cache_r_k.assert_called_once_with( - self.context, task.node, - {'deploy_kernel': 'a'}) - if uefi: - mock_pxe_config.assert_called_once_with( - task, {'foo': 'bar'}, CONF.pxe.uefi_pxe_config_template) - else: - mock_pxe_config.assert_called_once_with( - task, {'foo': 'bar'}, CONF.pxe.pxe_config_template) + # When booting from volume we return early. + # Check that nothing was called + self.assertFalse(mock_pxe_config.called) + self.assertFalse(mock_build_pxe.called) + self.assertFalse(mock_cache_r_k.called) + self.assertFalse(mock_deploy_img_info.called) + self.assertFalse(mock_instance_img_info.called) + self.assertFalse(dhcp_factory_mock.called) def test_prepare_ramdisk(self): self.node.provision_state = states.DEPLOYING @@ -926,6 +944,28 @@ class PXEBootTestCase(db_base.DbTestCase): CONF.pxe.ipxe_boot_script, {'ipxe_for_mac_uri': 'pxelinux.cfg/'}) + @mock.patch.object(os.path, 'isfile', autospec=True) + @mock.patch('ironic.common.utils.file_has_content', autospec=True) + @mock.patch('ironic.common.utils.write_to_file', autospec=True) + @mock.patch('ironic.common.utils.render_template', autospec=True) + def test_prepare_ramdisk_ipxe_boot_from_volume( + self, render_mock, write_mock, cmp_mock, isfile_mock): + self.node.provision_state = states.DEPLOYING + self.node.save() + self.config(group='pxe', ipxe_enabled=True) + isfile_mock.return_value = False + render_mock.return_value = 'foo' + self._test_prepare_ramdisk(should_write_image=False) + self.assertFalse(cmp_mock.called) + write_mock.assert_called_once_with( + os.path.join( + CONF.deploy.http_root, + os.path.basename(CONF.pxe.ipxe_boot_script)), + 'foo') + render_mock.assert_called_once_with( + CONF.pxe.ipxe_boot_script, + {'ipxe_for_mac_uri': 'pxelinux.cfg/'}) + @mock.patch.object(os.path, 'isfile', autospec=True) @mock.patch('ironic.common.utils.file_has_content', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True)