From bf75b8709b93cc41779f538249347a0dd07b6b14 Mon Sep 17 00:00:00 2001 From: Devananda van der Veen Date: Thu, 12 Feb 2015 11:38:04 -0800 Subject: [PATCH] Correctly rebuild the PXE file during takeover of ACTIVE nodes During a conductor take-over, eg. from another failed conductor, it rebuilds the PXE configuration of nodes that are mapped to it. This calls node.driver.deploy.prepare(). However, in the PXE driver, prepare() was not properly handling the case where it was called on a node with an ACTIVE deploy state. In such a situation, prepare() needs to switch from the "deploy" config to the "boot" config. Furthermore, the PXE boot file must specify the node's root disk UUID. This was previously not available outside of the _continue_deploy() method, and so is now saved on node.driver_internal_info when the initial deploy is completed. Co-Authored-By: David Shrewsbury Co-Authored-By: Chris Krelle Change-Id: I6adc65b756e22c5c78d30231187fe2e8ccf2594d Closes-bug: #1421370 --- ironic/drivers/modules/pxe.py | 29 ++++++++++++++ ironic/tests/drivers/test_pxe.py | 67 ++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 912c7813b3..246a558fe4 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -413,6 +413,26 @@ class PXEDeploy(base.DeployInterface): # the image kernel and ramdisk (Or even require it). _cache_ramdisk_kernel(task.context, task.node, pxe_info) + # NOTE(deva): prepare may be called from conductor._do_takeover + # in which case, it may need to regenerate the PXE config file for an + # already-active deployment. + if task.node.provision_state == states.ACTIVE: + try: + # this should have been stashed when the deploy was done + # but let's guard, just in case it's missing + root_uuid = task.node.driver_internal_info['root_uuid'] + except KeyError: + LOG.warn(_LW("The UUID for the root partition can't be found, " + "unable to switch the pxe config from deployment " + "mode to service (boot) mode for node %(node)s"), + {"node": task.node.uuid}) + else: + pxe_config_path = pxe_utils.get_pxe_config_file_path( + task.node.uuid) + deploy_utils.switch_pxe_config( + pxe_config_path, root_uuid, + driver_utils.get_node_capability(task.node, 'boot_mode')) + def clean_up(self, task): """Clean up the deployment environment for the task's node. @@ -497,6 +517,15 @@ class VendorPassthru(base.VendorInterface): if not root_uuid: return + # save the node's root disk UUID so that another conductor could + # rebuild the PXE config file. Due to a shortcoming in Nova objects, + # we have to assign to node.driver_internal_info so the node knows it + # has changed. + driver_internal_info = node.driver_internal_info + driver_internal_info['root_uuid'] = root_uuid + node.driver_internal_info = driver_internal_info + node.save() + try: if iscsi_deploy.get_boot_option(node) == "local": try_set_boot_device(task, boot_devices.DISK) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index b4bf3d7e52..dbed24dd4f 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -38,6 +38,7 @@ from ironic.conductor import utils as manager_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import iscsi_deploy from ironic.drivers.modules import pxe +from ironic.drivers import utils as driver_utils from ironic.openstack.common import fileutils from ironic.tests.conductor import utils as mgr_utils from ironic.tests.db import base as db_base @@ -504,6 +505,71 @@ class PXEDriverTestCase(db_base.DbTestCase): mock_cache_r_k.assert_called_once_with(self.context, task.node, None) + @mock.patch.object(pxe, '_get_image_info') + @mock.patch.object(pxe, '_cache_ramdisk_kernel') + @mock.patch.object(pxe, '_build_pxe_config_options') + @mock.patch.object(pxe_utils, 'create_pxe_config') + @mock.patch.object(pxe_utils, 'get_pxe_config_file_path') + @mock.patch.object(deploy_utils, 'switch_pxe_config') + def test_prepare_node_active_missing_root_uuid(self, + mock_switch, + mock_pxe_get_cfg, + mock_pxe_config, + mock_build_pxe, + mock_cache_r_k, + mock_img_info): + mock_build_pxe.return_value = None + mock_img_info.return_value = None + self.node.provision_state = states.ACTIVE + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.deploy.prepare(task) + mock_img_info.assert_called_once_with(task.node, + self.context) + mock_pxe_config.assert_called_once_with( + task, None, CONF.pxe.pxe_config_template) + mock_cache_r_k.assert_called_once_with(self.context, + task.node, None) + self.assertFalse(mock_pxe_get_cfg.called) + self.assertFalse(mock_switch.called) + + @mock.patch.object(pxe, '_get_image_info') + @mock.patch.object(pxe, '_cache_ramdisk_kernel') + @mock.patch.object(pxe, '_build_pxe_config_options') + @mock.patch.object(pxe_utils, 'create_pxe_config') + @mock.patch.object(pxe_utils, 'get_pxe_config_file_path') + @mock.patch.object(deploy_utils, 'switch_pxe_config') + @mock.patch.object(driver_utils, 'get_node_capability') + def test_prepare_node_active(self, + mock_get_cap, + mock_switch, + mock_pxe_get_cfg, + mock_pxe_config, + mock_build_pxe, + mock_cache_r_k, + mock_img_info): + mock_build_pxe.return_value = None + mock_img_info.return_value = None + mock_pxe_get_cfg.return_value = '/path' + mock_get_cap.return_value = None + + self.node.provision_state = states.ACTIVE + self.node.driver_internal_info = {'root_uuid': 'abcd'} + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.deploy.prepare(task) + mock_img_info.assert_called_once_with(task.node, + self.context) + mock_pxe_config.assert_called_once_with( + task, None, CONF.pxe.pxe_config_template) + mock_cache_r_k.assert_called_once_with(self.context, + task.node, None) + + mock_pxe_get_cfg.assert_called_once_with(task.node.uuid) + mock_switch.assert_called_once_with('/path', 'abcd', None) + @mock.patch.object(keystone, 'token_expires_soon') @mock.patch.object(deploy_utils, 'get_image_mb') @mock.patch.object(iscsi_deploy, '_get_image_file_path') @@ -656,6 +722,7 @@ class PXEDriverTestCase(db_base.DbTestCase): self.assertEqual(states.ACTIVE, self.node.provision_state) self.assertEqual(states.NOSTATE, self.node.target_provision_state) self.assertEqual(states.POWER_ON, self.node.power_state) + self.assertIn('root_uuid', self.node.driver_internal_info) self.assertIsNone(self.node.last_error) self.assertFalse(os.path.exists(token_path)) mock_image_cache.assert_called_once_with()