From 9dead366ec73a7c1d0f2039877640876660e2831 Mon Sep 17 00:00:00 2001 From: Josh Gachnang Date: Mon, 6 Apr 2015 13:17:48 -0700 Subject: Agent driver fails without Ironic-managed TFTP There are a lot of assumptions that Ironic has local PXE files. This is untrue for deployments with separate DHCP servers and agent images not hosted in Glance. Adds a new config option 'manage_tftp' in the DCHP section to disable TFTP management. Closes-Bug: #1440896 Change-Id: Ibf5a415e3beee13d12e65e177816256df57cb423 --- etc/ironic/ironic.conf.sample | 5 +++ ironic/drivers/modules/agent.py | 45 +++++++++++++++---------- ironic/tests/drivers/test_agent.py | 69 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 18 deletions(-) diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index bc38835b5..cf75c9b1f 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -360,6 +360,11 @@ # set to 0, will not run during cleaning. (integer value) #agent_erase_devices_priority= +# Whether Ironic will manage TFTP files for the deploy +# ramdisks. If set to False, you will need to configure your +# own TFTP server that allows booting the deploy ramdisks. +# (boolean value) +#manage_tftp=true # # Options defined in ironic.drivers.modules.agent_base_vendor diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index c257d2c77..6a590c8c4 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -57,7 +57,14 @@ agent_opts = [ 'Python Agent ramdisk. If unset, will use the priority ' 'set in the ramdisk (defaults to 10 for the ' 'GenericHardwareManager). If set to 0, will not run ' - 'during cleaning.') + 'during cleaning.'), + cfg.BoolOpt('manage_tftp', + default=True, + help='Whether Ironic will manage TFTP files for the deploy ' + 'ramdisks. If set to False, you will need to configure ' + 'your own TFTP server that allows booting the deploy ' + 'ramdisks.' + ), ] CONF = cfg.CONF @@ -196,12 +203,13 @@ def build_instance_info_for_deploy(task): def _prepare_pxe_boot(task): """Prepare the files required for PXE booting the agent.""" - pxe_info = _get_tftp_image_info(task.node) - pxe_options = _build_pxe_config_options(task.node, pxe_info) - pxe_utils.create_pxe_config(task, - pxe_options, - CONF.agent.agent_pxe_config_template) - _cache_tftp_images(task.context, task.node, pxe_info) + if CONF.agent.manage_tftp: + pxe_info = _get_tftp_image_info(task.node) + pxe_options = _build_pxe_config_options(task.node, pxe_info) + pxe_utils.create_pxe_config(task, + pxe_options, + CONF.agent.agent_pxe_config_template) + _cache_tftp_images(task.context, task.node, pxe_info) def _do_pxe_boot(task, ports=None): @@ -220,13 +228,13 @@ def _do_pxe_boot(task, ports=None): def _clean_up_pxe(task): """Clean up left over PXE and DHCP files.""" - pxe_info = _get_tftp_image_info(task.node) - for label in pxe_info: - path = pxe_info[label][1] - utils.unlink_without_raise(path) - AgentTFTPImageCache().clean_up() - - pxe_utils.clean_up_pxe_config(task) + if CONF.agent.manage_tftp: + pxe_info = _get_tftp_image_info(task.node) + for label in pxe_info: + path = pxe_info[label][1] + utils.unlink_without_raise(path) + AgentTFTPImageCache().clean_up() + pxe_utils.clean_up_pxe_config(task) class AgentDeploy(base.DeployInterface): @@ -251,10 +259,11 @@ class AgentDeploy(base.DeployInterface): """ node = task.node params = {} - params['driver_info.deploy_kernel'] = node.driver_info.get( - 'deploy_kernel') - params['driver_info.deploy_ramdisk'] = node.driver_info.get( - 'deploy_ramdisk') + if CONF.agent.manage_tftp: + params['driver_info.deploy_kernel'] = node.driver_info.get( + 'deploy_kernel') + params['driver_info.deploy_ramdisk'] = node.driver_info.get( + 'deploy_ramdisk') image_source = node.instance_info.get('image_source') params['instance_info.image_source'] = image_source error_msg = _('Node %s failed to validate deploy image info. Some ' diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index b9ea638e0..bb9675f3f 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -164,6 +164,14 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertIn('driver_info.deploy_ramdisk', str(e)) self.assertIn('driver_info.deploy_kernel', str(e)) + def test_validate_driver_info_manage_tftp_false(self): + self.config(manage_tftp=False, group='agent') + self.node.driver_info = {} + self.node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.driver.validate(task) + def test_validate_instance_info_missing_params(self): self.node.instance_info = {} self.node.save() @@ -199,6 +207,37 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, task.driver.deploy.validate, task) + @mock.patch.object(agent, '_cache_tftp_images') + @mock.patch.object(pxe_utils, 'create_pxe_config') + @mock.patch.object(agent, '_build_pxe_config_options') + @mock.patch.object(agent, '_get_tftp_image_info') + def test__prepare_pxe_boot(self, pxe_info_mock, options_mock, + create_mock, cache_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + agent._prepare_pxe_boot(task) + pxe_info_mock.assert_called_once_with(task.node) + options_mock.assert_called_once_with(task.node, mock.ANY) + create_mock.assert_called_once_with( + task, mock.ANY, CONF.agent.agent_pxe_config_template) + cache_mock.assert_called_once_with(task.context, task.node, + mock.ANY) + + @mock.patch.object(agent, '_cache_tftp_images') + @mock.patch.object(pxe_utils, 'create_pxe_config') + @mock.patch.object(agent, '_build_pxe_config_options') + @mock.patch.object(agent, '_get_tftp_image_info') + def test__prepare_pxe_boot_manage_tftp_false( + self, pxe_info_mock, options_mock, create_mock, cache_mock): + self.config(manage_tftp=False, group='agent') + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + agent._prepare_pxe_boot(task) + self.assertFalse(pxe_info_mock.called) + self.assertFalse(options_mock.called) + self.assertFalse(create_mock.called) + self.assertFalse(cache_mock.called) + @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp') @mock.patch('ironic.conductor.utils.node_set_boot_device') @mock.patch('ironic.conductor.utils.node_power_action') @@ -221,6 +260,36 @@ class TestAgentDeploy(db_base.DbTestCase): power_mock.assert_called_once_with(task, states.POWER_OFF) self.assertEqual(driver_return, states.DELETED) + @mock.patch.object(pxe_utils, 'clean_up_pxe_config') + @mock.patch.object(agent, 'AgentTFTPImageCache') + @mock.patch('ironic.common.utils.unlink_without_raise') + @mock.patch.object(agent, '_get_tftp_image_info') + def test__clean_up_pxe(self, info_mock, unlink_mock, cache_mock, + clean_mock): + info_mock.return_value = {'label': ['fake1', 'fake2']} + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + agent._clean_up_pxe(task) + info_mock.assert_called_once_with(task.node) + unlink_mock.assert_called_once_with('fake2') + clean_mock.assert_called_once_with(task) + + @mock.patch.object(pxe_utils, 'clean_up_pxe_config') + @mock.patch.object(agent.AgentTFTPImageCache, 'clean_up') + @mock.patch('ironic.common.utils.unlink_without_raise') + @mock.patch.object(agent, '_get_tftp_image_info') + def test__clean_up_pxe_manage_tftp_false( + self, info_mock, unlink_mock, cache_mock, clean_mock): + self.config(manage_tftp=False, group='agent') + info_mock.return_value = {'label': ['fake1', 'fake2']} + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + agent._clean_up_pxe(task) + self.assertFalse(info_mock.called) + self.assertFalse(unlink_mock.called) + self.assertFalse(cache_mock.called) + self.assertFalse(clean_mock.called) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports') @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.create_cleaning_ports') @mock.patch('ironic.drivers.modules.agent._do_pxe_boot') -- cgit v1.2.1