openstack/ironic
Code Issues Proposed changes

Generate iPXE boot script on start up

Browse Source 
This is a revert of [0], which added some problematic short circuit
logic to prepare_ramdisk. This patch proposes a better fix that was
suggested in the reviews of [0], which is to generate the boot script
on start up rather than during prepare_ramdisk. The iPXE boot script
is not dependent on node properties, or anything that is determined
during deployment, so generating the boot script on start up should
not harm any current functionality.

[0] 2c18f22f9976870b71bfd1e471afa18fd50a7a25

Change-Id: Id545d6cf93227cf1fc2ff0c05dbdceb8fb6aa5b9
This commit is contained in:
Michael Turek 2017-07-14 16:04:09 -04:00 committed by Dao Cong Tien
parent 9d161f6922
commit bc60c21d63
6 changed files with 111 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic/common/pxe_utils.py
View File

@ -241,6 +241,22 @@ def create_pxe_config(task, pxe_options, template=None):
_link_mac_pxe_configs(task)
def create_ipxe_boot_script():
"""Render the iPXE boot script into the HTTP root directory"""
boot_script = utils.render_template(
CONF.pxe.ipxe_boot_script,
{'ipxe_for_mac_uri': PXE_CFG_DIR_NAME + '/'})
bootfile_path = os.path.join(
CONF.deploy.http_root,
os.path.basename(CONF.pxe.ipxe_boot_script))
# NOTE(pas-ha) to prevent unneeded writes,
# only write to file if its content is different from required,
# which should be rather rare
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)
def clean_up_pxe_config(task):
"""Clean up the TFTP environment for the task's node.

18
ironic/drivers/modules/agent.py
View File

@ -449,21 +449,19 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
task.driver.network.add_provisioning_network(task)
# Signal to storage driver to attach volumes
task.driver.storage.attach_volumes(task)
if not task.driver.storage.should_write_image(task):
# We have nothing else to do as this is handled in the
# backend storage system.
return
if node.provision_state == states.ACTIVE:
# Call is due to conductor takeover
task.driver.boot.prepare_instance(task)
elif node.provision_state != states.ADOPTING:
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()
node.instance_info = deploy_utils.build_instance_info_for_deploy(
task)
node.save()
if CONF.agent.manage_agent_boot:
if task.driver.storage.should_write_image(task):
deploy_opts = deploy_utils.build_agent_options(node)
else:
deploy_opts = None
deploy_opts = deploy_utils.build_agent_options(node)
task.driver.boot.prepare_ramdisk(task, deploy_opts)
@METRICS.timer('AgentDeploy.clean_up')

25
ironic/drivers/modules/pxe.py
View File

@ -31,7 +31,6 @@ from ironic.common import image_service as service
from ironic.common import images
from ironic.common import pxe_utils
from ironic.common import states
from ironic.common import utils
from ironic.conductor import utils as manager_utils
from ironic.conf import CONF
from ironic.drivers import base
@ -376,6 +375,8 @@ class PXEBoot(base.BootInterface):
def __init__(self):
self.capabilities = ['iscsi_volume_boot']
if CONF.pxe.ipxe_enabled:
pxe_utils.create_ipxe_boot_script()
def get_properties(self):
"""Return the properties of the interface.
@ -458,22 +459,12 @@ class PXEBoot(base.BootInterface):
node = task.node
if CONF.pxe.ipxe_enabled:
# Render the iPXE boot script template and save it
# to HTTP root directory
boot_script = utils.render_template(
CONF.pxe.ipxe_boot_script,
{'ipxe_for_mac_uri': pxe_utils.PXE_CFG_DIR_NAME + '/'})
bootfile_path = os.path.join(
CONF.deploy.http_root,
os.path.basename(CONF.pxe.ipxe_boot_script))
# NOTE(pas-ha) to prevent unneeded writes,
# only write to file if its content is different from required,
# which should be rather rare
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
# NOTE(mjturek): At this point, the ipxe boot script should
# already exist as it is created at startup time. However, we
# call the boot script create method here to assert its
# existence and handle the unlikely case that it wasn't created
# or was deleted.
pxe_utils.create_ipxe_boot_script()
dhcp_opts = pxe_utils.dhcp_options_for_instance(task)
provider = dhcp_factory.DHCPFactory()

49
ironic/tests/unit/common/test_pxe_utils.py
View File

@ -532,6 +532,55 @@ class TestPXEUtils(db_base.DbTestCase):
rmtree_mock.assert_called_once_with(
os.path.join(CONF.pxe.tftp_root, self.node.uuid))
@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_create_ipxe_boot_script(self, render_mock, write_mock,
cmp_mock, isfile_mock):
isfile_mock.return_value = False
render_mock.return_value = 'foo'
self.assertFalse(cmp_mock.called)
pxe_utils.create_ipxe_boot_script()
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)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_create_ipxe_boot_script_copy_file_different(
self, render_mock, write_mock, cmp_mock, isfile_mock):
isfile_mock.return_value = True
cmp_mock.return_value = False
render_mock.return_value = 'foo'
self.assertFalse(cmp_mock.called)
pxe_utils.create_ipxe_boot_script()
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)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_create_ipxe_boot_script_already_exists(self, render_mock,
write_mock, cmp_mock,
isfile_mock):
isfile_mock.return_value = True
cmp_mock.return_value = True
pxe_utils.create_ipxe_boot_script()
self.assertFalse(write_mock.called)
def test__get_pxe_mac_path(self):
mac = '00:11:22:33:44:55:66'
self.assertEqual('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-66',

2
ironic/tests/unit/drivers/modules/test_agent.py
View File

@ -448,7 +448,7 @@ class TestAgentDeploy(db_base.DbTestCase):
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)
pxe_prepare_ramdisk_mock.assert_not_called()
@mock.patch('ironic.common.dhcp_factory.DHCPFactory._set_dhcp_provider')
@mock.patch('ironic.common.dhcp_factory.DHCPFactory.clean_dhcp')

98
ironic/tests/unit/drivers/modules/test_pxe.py
View File

@ -808,10 +808,7 @@ 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)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
def _test_prepare_ramdisk(self, mock_should_write_image,
mock_pxe_config,
def _test_prepare_ramdisk(self, mock_pxe_config,
mock_build_pxe, mock_cache_r_k,
mock_deploy_img_info,
mock_instance_img_info,
@ -820,9 +817,7 @@ class PXEBootTestCase(db_base.DbTestCase):
uefi=False,
cleaning=False,
ipxe_use_swift=False,
whole_disk_image=False,
should_write_image=True):
mock_should_write_image.return_value = should_write_image
whole_disk_image=False):
mock_build_pxe.return_value = {}
mock_deploy_img_info.return_value = {'deploy_kernel': 'a'}
if whole_disk_image:
@ -838,51 +833,38 @@ 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:
if should_write_image:
dhcp_opts = pxe_utils.dhcp_options_for_instance(task)
dhcp_opts = pxe_utils.dhcp_options_for_instance(task)
task.driver.boot.prepare_ramdisk(task, {'foo': 'bar'})
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)
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,
{'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)
{'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:
# 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)
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)
def test_prepare_ramdisk(self):
self.node.provision_state = states.DEPLOYING
@ -944,28 +926,6 @@ 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)