diff options
| -rw-r--r-- | etc/ironic/ironic.conf.sample | 5 | ||||
| -rw-r--r-- | ironic/common/pxe_utils.py | 33 | ||||
| -rw-r--r-- | ironic/drivers/modules/agent.py | 45 | ||||
| -rw-r--r-- | ironic/drivers/modules/boot.ipxe | 2 | ||||
| -rw-r--r-- | ironic/drivers/modules/ipmitool.py | 29 | ||||
| -rw-r--r-- | ironic/drivers/modules/iscsi_deploy.py | 9 | ||||
| -rw-r--r-- | ironic/tests/drivers/test_agent.py | 69 | ||||
| -rw-r--r-- | ironic/tests/drivers/test_ipmitool.py | 10 | ||||
| -rw-r--r-- | ironic/tests/drivers/test_iscsi_deploy.py | 19 | ||||
| -rw-r--r-- | ironic/tests/test_pxe_utils.py | 37 |
10 files changed, 208 insertions, 50 deletions
diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index a8a1501a6..ccf368f0d 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=<None> +# 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/common/pxe_utils.py b/ironic/common/pxe_utils.py index a125da034..09528a218 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -80,12 +80,20 @@ def _link_mac_pxe_configs(task): :param task: A TaskManager instance. """ - pxe_config_file_path = get_pxe_config_file_path(task.node.uuid) - for mac in driver_utils.get_node_mac_addresses(task): - mac_path = _get_pxe_mac_path(mac) + + def create_link(mac_path): utils.unlink_without_raise(mac_path) utils.create_link_without_raise(pxe_config_file_path, mac_path) + pxe_config_file_path = get_pxe_config_file_path(task.node.uuid) + for mac in driver_utils.get_node_mac_addresses(task): + create_link(_get_pxe_mac_path(mac)) + # TODO(lucasagomes): Backward compatibility with :hexraw, + # to be removed in M. + # see: https://bugs.launchpad.net/ironic/+bug/1441710 + if CONF.pxe.ipxe_enabled: + create_link(_get_pxe_mac_path(mac, delimiter='')) + def _link_ip_address_pxe_configs(task): """Link each IP address with the PXE configuration file. @@ -110,17 +118,20 @@ def _link_ip_address_pxe_configs(task): ip_address_path) -def _get_pxe_mac_path(mac): +def _get_pxe_mac_path(mac, delimiter=None): """Convert a MAC address into a PXE config file name. :param mac: A MAC address string in the format xx:xx:xx:xx:xx:xx. + :param delimiter: The MAC address delimiter. Defaults to dash ('-'). :returns: the path to the config file. """ - if CONF.pxe.ipxe_enabled: - mac_file_name = mac.replace(':', '').lower() - else: - mac_file_name = "01-" + mac.replace(":", "-").lower() + if delimiter is None: + delimiter = '-' + + mac_file_name = mac.replace(':', delimiter).lower() + if not CONF.pxe.ipxe_enabled: + mac_file_name = '01-' + mac_file_name return os.path.join(get_root_dir(), PXE_CFG_DIR_NAME, mac_file_name) @@ -221,6 +232,12 @@ def clean_up_pxe_config(task): else: for mac in driver_utils.get_node_mac_addresses(task): utils.unlink_without_raise(_get_pxe_mac_path(mac)) + # TODO(lucasagomes): Backward compatibility with :hexraw, + # to be removed in M. + # see: https://bugs.launchpad.net/ironic/+bug/1441710 + if CONF.pxe.ipxe_enabled: + utils.unlink_without_raise(_get_pxe_mac_path(mac, + delimiter='')) utils.rmtree_without_raise(os.path.join(get_root_dir(), task.node.uuid)) 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/drivers/modules/boot.ipxe b/ironic/drivers/modules/boot.ipxe index 25a0ea8dc..3567dc029 100644 --- a/ironic/drivers/modules/boot.ipxe +++ b/ironic/drivers/modules/boot.ipxe @@ -1,7 +1,7 @@ #!ipxe # load the MAC-specific file or fail if it's not found -chain --autofree pxelinux.cfg/${mac:hexraw} || goto error_no_config +chain --autofree pxelinux.cfg/${mac:hexhyp} || goto error_no_config :error_no_config echo PXE boot failed. No configuration found for MAC ${mac} diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 55fc1fa2d..ddebd68ef 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -111,7 +111,11 @@ ipmitool_command_options = { 'dual_bridge': ['ipmitool', '-m', '0', '-b', '0', '-t', '0', '-B', '0', '-T', '0', '-h']} -# Note(TheJulia): This string is hardcoded in ipmitool's lanplus driver. +# Note(TheJulia): This string is hardcoded in ipmitool's lanplus driver +# and is substituted in return for the error code received from the IPMI +# controller. As of 1.8.15, no internationalization support appears to +# be in ipmitool which means the string should always be returned in this +# form regardless of locale. IPMITOOL_RETRYABLE_FAILURES = ['insufficient resources for session'] @@ -348,14 +352,14 @@ def _exec_ipmitool(driver_info, command): args.append('-N') args.append(str(CONF.ipmi.min_command_interval)) - end_time = (_time() + CONF.ipmi.retry_timeout) + end_time = (time.time() + CONF.ipmi.retry_timeout) while True: num_tries = num_tries - 1 # NOTE(deva): ensure that no communications are sent to a BMC more # often than once every min_command_interval seconds. time_till_next_poll = CONF.ipmi.min_command_interval - ( - _time() - LAST_CMD_TIME.get(driver_info['address'], 0)) + time.time() - LAST_CMD_TIME.get(driver_info['address'], 0)) if time_till_next_poll > 0: time.sleep(time_till_next_poll) # Resetting the list that will be utilized so the password arguments @@ -377,21 +381,21 @@ def _exec_ipmitool(driver_info, command): with excutils.save_and_reraise_exception() as ctxt: err_list = [x for x in IPMITOOL_RETRYABLE_FAILURES if x in e.message] - if ((_time() > end_time) or + if ((time.time() > end_time) or (num_tries == 0) or not err_list): - LOG.error(_LE('IPMI Error attempting to execute ' + LOG.error(_LE('IPMI Error while attempting ' '"%(cmd)s" for node %(node)s. ' 'Error: %(error)s'), { - 'node': driver_info['uuid'], - 'cmd': e.cmd, - 'error': e + 'node': driver_info['uuid'], + 'cmd': e.cmd, + 'error': e }) else: ctxt.reraise = False LOG.warning(_LW('IPMI Error encountered, retrying ' - '"%(cmd)s" for node %(node)s ' + '"%(cmd)s" for node %(node)s. ' 'Error: %(error)s'), { 'node': driver_info['uuid'], @@ -399,7 +403,7 @@ def _exec_ipmitool(driver_info, command): 'error': e }) finally: - LAST_CMD_TIME[driver_info['address']] = _time() + LAST_CMD_TIME[driver_info['address']] = time.time() def _sleep_time(iter): @@ -626,11 +630,6 @@ def send_raw(task, raw_bytes): raise exception.IPMIFailure(cmd=cmd) -def _time(): - """Wrapper for time.time() enabling simplified unit testing.""" - return time.time() - - class IPMIPower(base.PowerInterface): def __init__(self): diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 8e39e744b..ac7315d33 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -447,13 +447,20 @@ def build_deploy_ramdisk_options(node): node.instance_info = i_info node.save() + # XXX(jroll) DIB relies on boot_option=local to decide whether or not to + # lay down a bootloader. Hack this for now; fix it for real in Liberty. + # See also bug #1441556. + boot_option = get_boot_option(node) + if node.driver_internal_info.get('is_whole_disk_image'): + boot_option = 'netboot' + deploy_options = { 'deployment_id': node['uuid'], 'deployment_key': deploy_key, 'iscsi_target_iqn': "iqn-%s" % node.uuid, 'ironic_api_url': ironic_api, 'disk': CONF.pxe.disk_devices, - 'boot_option': get_boot_option(node), + 'boot_option': boot_option, 'boot_mode': _get_boot_mode(node), # NOTE: The below entry is a temporary workaround for bug/1433812 'coreos.configdrive': 0, 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') diff --git a/ironic/tests/drivers/test_ipmitool.py b/ironic/tests/drivers/test_ipmitool.py index 0828f2c60..2e0097fba 100644 --- a/ironic/tests/drivers/test_ipmitool.py +++ b/ironic/tests/drivers/test_ipmitool.py @@ -891,21 +891,21 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test__exec_ipmitool_exception_retry_failure_unhandable(self, + def test__exec_ipmitool_exception_non_retryable_failure(self, mock_exec, mock_support, mock_sleep): ipmi.LAST_CMD_TIME = {} mock_support.return_value = False - # Return a retryable error, then a error that cannot - # be retryable thus resulting in a single retry - # attempt by _exec_ipmitool that is successful. + # Return a retryable error, then an error that cannot + # be retried thus resulting in a single retry + # attempt by _exec_ipmitool. mock_exec.side_effect = iter([ processutils.ProcessExecutionError( stderr="insufficient resources for session" ), processutils.ProcessExecutionError( - "Unknown" + stderr="Unknown" ), ]) diff --git a/ironic/tests/drivers/test_iscsi_deploy.py b/ironic/tests/drivers/test_iscsi_deploy.py index a0b1f40ba..4d3803542 100644 --- a/ironic/tests/drivers/test_iscsi_deploy.py +++ b/ironic/tests/drivers/test_iscsi_deploy.py @@ -487,6 +487,25 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self._test_build_deploy_ramdisk_options(mock_alnum, fake_api_url, expected_boot_option=expected) + @mock.patch.object(keystone, 'get_service_url', autospec=True) + @mock.patch.object(utils, 'random_alnum', autospec=True) + def test_build_deploy_ramdisk_options_whole_disk_image(self, mock_alnum, + mock_get_url): + """Tests a hack to boot_option for whole disk images. + + This hack is in place to fix bug #1441556. + """ + self.node.instance_info = {'capabilities': '{"boot_option": "local"}'} + dii = self.node.driver_internal_info + dii['is_whole_disk_image'] = True + self.node.driver_internal_info = dii + self.node.save() + expected = 'netboot' + fake_api_url = 'http://127.0.0.1:6385' + self.config(api_url=fake_api_url, group='conductor') + self._test_build_deploy_ramdisk_options(mock_alnum, fake_api_url, + expected_boot_option=expected) + def test_get_boot_option(self): self.node.instance_info = {'capabilities': '{"boot_option": "local"}'} result = iscsi_deploy.get_boot_option(self.node) diff --git a/ironic/tests/test_pxe_utils.py b/ironic/tests/test_pxe_utils.py index f16cd3e1b..1792d840a 100644 --- a/ironic/tests/test_pxe_utils.py +++ b/ironic/tests/test_pxe_utils.py @@ -144,7 +144,40 @@ class TestPXEUtils(db_base.DbTestCase): ] unlink_calls = [ mock.call('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-66'), - mock.call('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-67') + mock.call('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-67'), + ] + with task_manager.acquire(self.context, self.node.uuid) as task: + pxe_utils._link_mac_pxe_configs(task) + + unlink_mock.assert_has_calls(unlink_calls) + create_link_mock.assert_has_calls(create_link_calls) + + @mock.patch('ironic.common.utils.create_link_without_raise', autospec=True) + @mock.patch('ironic.common.utils.unlink_without_raise', autospec=True) + @mock.patch('ironic.drivers.utils.get_node_mac_addresses', autospec=True) + def test__write_mac_ipxe_configs(self, get_macs_mock, unlink_mock, + create_link_mock): + self.config(ipxe_enabled=True, group='pxe') + macs = [ + '00:11:22:33:44:55:66', + '00:11:22:33:44:55:67' + ] + get_macs_mock.return_value = macs + create_link_calls = [ + mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/httpboot/pxelinux.cfg/00-11-22-33-44-55-66'), + mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/httpboot/pxelinux.cfg/00112233445566'), + mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/httpboot/pxelinux.cfg/00-11-22-33-44-55-67'), + mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/httpboot/pxelinux.cfg/00112233445567'), + ] + unlink_calls = [ + mock.call('/httpboot/pxelinux.cfg/00-11-22-33-44-55-66'), + mock.call('/httpboot/pxelinux.cfg/00112233445566'), + mock.call('/httpboot/pxelinux.cfg/00-11-22-33-44-55-67'), + mock.call('/httpboot/pxelinux.cfg/00112233445567'), ] with task_manager.acquire(self.context, self.node.uuid) as task: pxe_utils._link_mac_pxe_configs(task) @@ -218,7 +251,7 @@ class TestPXEUtils(db_base.DbTestCase): self.config(ipxe_enabled=True, group='pxe') self.config(http_root='/httpboot', group='pxe') mac = '00:11:22:33:AA:BB:CC' - self.assertEqual('/httpboot/pxelinux.cfg/00112233aabbcc', + self.assertEqual('/httpboot/pxelinux.cfg/00-11-22-33-aa-bb-cc', pxe_utils._get_pxe_mac_path(mac)) def test__get_pxe_ip_address_path(self): |