From b464a195bb066cc78a05bf27752a1773672e73e5 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Thu, 28 Jun 2018 11:32:16 -0700 Subject: [PATCH] Change PXE logic to always link macs with UEFI Our logic for elilo and grub excluded the MAC address based links from being created, which are handy for stand-alone use cases. This patch changes the default to always write MAC address based links, and allows the interface to tolerate an exception being raised if the ``[dhcp]dhcp_provider`` is set to ``none``. This effectively fixes the issue where users that do not use neutron were unable to use UEFI. Story: #2002887 Task: #22848 Change-Id: Ib8b9b2e3281184cd4e8d9a1a33a012f93edf237c --- ironic/common/pxe_utils.py | 40 +++++-- ironic/drivers/modules/master_grub_cfg.txt | 2 +- ironic/tests/unit/common/test_pxe_utils.py | 110 +++++++++++++++++- ...efault-change-to-mac-1e301a96c49acec4.yaml | 22 ++++ 4 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/grub-default-change-to-mac-1e301a96c49acec4.yaml diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index c3ce89a370..cb8036d7bd 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -19,6 +19,7 @@ import os from ironic_lib import utils as ironic_utils from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import excutils from oslo_utils import fileutils from ironic.common import dhcp_factory @@ -90,7 +91,10 @@ def _link_mac_pxe_configs(task): pxe_config_file_path = get_pxe_config_file_path(task.node.uuid) for port in task.ports: client_id = port.extra.get('client-id') + # Syslinux, ipxe, depending on settings. create_link(_get_pxe_mac_path(port.address, client_id=client_id)) + # Grub2 MAC address only + create_link(_get_pxe_grub_mac_path(port.address)) def _link_ip_address_pxe_configs(task, hex_form): @@ -121,6 +125,10 @@ def _link_ip_address_pxe_configs(task, hex_form): ip_address_path) +def _get_pxe_grub_mac_path(mac): + return os.path.join(get_root_dir(), mac + '.conf') + + def _get_pxe_mac_path(mac, delimiter='-', client_id=None): """Convert a MAC address into a PXE config file name. @@ -137,7 +145,6 @@ def _get_pxe_mac_path(mac, delimiter='-', client_id=None): if client_id: hw_type = '20-' mac_file_name = hw_type + mac_file_name - return os.path.join(get_root_dir(), PXE_CFG_DIR_NAME, mac_file_name) @@ -259,7 +266,21 @@ def create_pxe_config(task, pxe_options, template=None): utils.write_to_file(pxe_config_file_path, pxe_config) if is_uefi_boot_mode and not CONF.pxe.ipxe_enabled: - _link_ip_address_pxe_configs(task, hex_form) + # Always write the mac addresses + _link_mac_pxe_configs(task) + try: + _link_ip_address_pxe_configs(task, hex_form) + # NOTE(TheJulia): The IP address support will fail if the + # dhcp_provider interface is set to none. This will result + # in the MAC addresses and DHCP files being written, and + # we can remove IP address creation for the grub use + # case, considering that will ease removal of elilo support. + except exception.FailedToGetIPaddressesOnPort as e: + if CONF.dhcp.dhcp_provider != 'none': + with excutils.save_and_reraise_exception(): + LOG.error('Unable to create boot config, IP address ' + 'was unable to be retrieved. %(error)s', + {'error': e}) else: _link_mac_pxe_configs(task) @@ -308,16 +329,21 @@ def clean_up_pxe_config(task): True) except exception.InvalidIPv4Address: continue + except exception.FailedToGetIPAddressOnPort: + continue # Cleaning up config files created for grub2. ironic_utils.unlink_without_raise(ip_address_path) # Cleaning up config files created for elilo. ironic_utils.unlink_without_raise(hex_ip_path) - else: - for port in task.ports: - client_id = port.extra.get('client-id') - ironic_utils.unlink_without_raise( - _get_pxe_mac_path(port.address, client_id=client_id)) + for port in task.ports: + client_id = port.extra.get('client-id') + # syslinux, ipxe, etc. + ironic_utils.unlink_without_raise( + _get_pxe_mac_path(port.address, client_id=client_id)) + # Grub2 MAC address based confiuration + ironic_utils.unlink_without_raise( + _get_pxe_grub_mac_path(port.address)) utils.rmtree_without_raise(os.path.join(get_root_dir(), task.node.uuid)) diff --git a/ironic/drivers/modules/master_grub_cfg.txt b/ironic/drivers/modules/master_grub_cfg.txt index 14d506d477..82847b81ee 100644 --- a/ironic/drivers/modules/master_grub_cfg.txt +++ b/ironic/drivers/modules/master_grub_cfg.txt @@ -3,5 +3,5 @@ set timeout=5 set hidden_timeout_quiet=false menuentry "master" { -configfile /tftpboot/$net_default_ip.conf +configfile /tftpboot/$net_default_mac.conf } diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index a2677d7c37..5f21a75dae 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -21,6 +21,7 @@ from oslo_config import cfg from oslo_utils import uuidutils import six +from ironic.common import exception from ironic.common import pxe_utils from ironic.common import utils from ironic.conductor import task_manager @@ -255,12 +256,18 @@ class TestPXEUtils(db_base.DbTestCase): create_link_calls = [ mock.call(u'../1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', '/tftpboot/pxelinux.cfg/01-11-22-33-44-55-66'), + mock.call(u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/tftpboot/11:22:33:44:55:66.conf'), mock.call(u'../1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', - '/tftpboot/pxelinux.cfg/01-11-22-33-44-55-67') + '/tftpboot/pxelinux.cfg/01-11-22-33-44-55-67'), + mock.call(u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/tftpboot/11:22:33:44:55:67.conf') ] unlink_calls = [ mock.call('/tftpboot/pxelinux.cfg/01-11-22-33-44-55-66'), + mock.call('/tftpboot/11:22:33:44:55:66.conf'), mock.call('/tftpboot/pxelinux.cfg/01-11-22-33-44-55-67'), + mock.call('/tftpboot/11:22:33:44:55:67.conf') ] with task_manager.acquire(self.context, self.node.uuid) as task: task.ports = [port_1, port_2] @@ -288,12 +295,18 @@ class TestPXEUtils(db_base.DbTestCase): create_link_calls = [ mock.call(u'../1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', '/tftpboot/pxelinux.cfg/20-11-22-33-44-55-66'), + mock.call(u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/tftpboot/11:22:33:44:55:66.conf'), mock.call(u'../1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', - '/tftpboot/pxelinux.cfg/20-11-22-33-44-55-67') + '/tftpboot/pxelinux.cfg/20-11-22-33-44-55-67'), + mock.call(u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/tftpboot/11:22:33:44:55:67.conf') ] unlink_calls = [ mock.call('/tftpboot/pxelinux.cfg/20-11-22-33-44-55-66'), + mock.call('/tftpboot/11:22:33:44:55:66.conf'), mock.call('/tftpboot/pxelinux.cfg/20-11-22-33-44-55-67'), + mock.call('/tftpboot/11:22:33:44:55:67.conf') ] with task_manager.acquire(self.context, self.node.uuid) as task: task.ports = [port_1, port_2] @@ -315,12 +328,18 @@ class TestPXEUtils(db_base.DbTestCase): create_link_calls = [ mock.call(u'../1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', '/httpboot/pxelinux.cfg/11-22-33-44-55-66'), + mock.call(u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/httpboot/11:22:33:44:55:66.conf'), mock.call(u'../1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', '/httpboot/pxelinux.cfg/11-22-33-44-55-67'), + mock.call(u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config', + '/httpboot/11:22:33:44:55:67.conf') ] unlink_calls = [ mock.call('/httpboot/pxelinux.cfg/11-22-33-44-55-66'), + mock.call('/httpboot/11:22:33:44:55:66.conf'), mock.call('/httpboot/pxelinux.cfg/11-22-33-44-55-67'), + mock.call('/httpboot/11:22:33:44:55:67.conf'), ] with task_manager.acquire(self.context, self.node.uuid) as task: task.ports = [port_1, port_2] @@ -505,6 +524,46 @@ class TestPXEUtils(db_base.DbTestCase): write_mock.assert_called_with(pxe_cfg_file_path, render_mock.return_value) + @mock.patch.object(os, 'chmod', autospec=True) + @mock.patch('ironic.common.pxe_utils._link_mac_pxe_configs', + autospec=True) + @mock.patch('ironic.common.pxe_utils._link_ip_address_pxe_configs', + autospec=True) + @mock.patch('ironic.common.utils.write_to_file', autospec=True) + @mock.patch('ironic.common.utils.render_template', autospec=True) + @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) + def test_create_pxe_config_uefi_mac_address( + self, ensure_tree_mock, render_mock, + write_mock, link_ip_configs_mock, + link_mac_pxe_configs_mock, chmod_mock): + # TODO(TheJulia): We should... like... fix the template to + # enable mac address usage..... + grub_tmplte = "ironic/drivers/modules/pxe_grub_config.template" + link_ip_configs_mock.side_efect = exception.FailedToGetIPAddressOnPort( + port_id='blah') + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node.properties['capabilities'] = 'boot_mode:uefi' + pxe_utils.create_pxe_config(task, self.pxe_options, + grub_tmplte) + + ensure_calls = [ + mock.call(os.path.join(CONF.pxe.tftp_root, self.node.uuid)), + mock.call(os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg')), + ] + ensure_tree_mock.assert_has_calls(ensure_calls) + chmod_mock.assert_not_called() + render_mock.assert_called_with( + grub_tmplte, + {'pxe_options': self.pxe_options, + 'ROOT': '(( ROOT ))', + 'DISK_IDENTIFIER': '(( DISK_IDENTIFIER ))'}) + link_mac_pxe_configs_mock.assert_called_once_with(task) + link_ip_configs_mock.assert_called_once_with(task, False) + + pxe_cfg_file_path = pxe_utils.get_pxe_config_file_path(self.node.uuid) + write_mock.assert_called_with(pxe_cfg_file_path, + render_mock.return_value) + @mock.patch.object(os, 'chmod', autospec=True) @mock.patch('ironic.common.pxe_utils._link_mac_pxe_configs', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True) @@ -547,8 +606,13 @@ class TestPXEUtils(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid) as task: pxe_utils.clean_up_pxe_config(task) - unlink_mock.assert_called_once_with("/tftpboot/pxelinux.cfg/01-%s" - % address.replace(':', '-')) + ensure_calls = [ + mock.call("/tftpboot/pxelinux.cfg/01-%s" + % address.replace(':', '-')), + mock.call("/tftpboot/%s.conf" % address) + ] + + unlink_mock.assert_has_calls(ensure_calls) rmtree_mock.assert_called_once_with( os.path.join(CONF.pxe.tftp_root, self.node.uuid)) @@ -801,6 +865,35 @@ class TestPXEUtils(db_base.DbTestCase): rmtree_mock.assert_called_once_with( os.path.join(CONF.pxe.tftp_root, self.node.uuid)) + @mock.patch('ironic.common.utils.rmtree_without_raise', autospec=True) + @mock.patch('ironic_lib.utils.unlink_without_raise', autospec=True) + @mock.patch('ironic.common.dhcp_factory.DHCPFactory.provider', + autospec=True) + def test_clean_up_pxe_config_uefi_mac_address( + self, provider_mock, unlink_mock, rmtree_mock): + ip_address = '10.10.0.1' + address = "aa:aa:aa:aa:aa:aa" + properties = {'capabilities': 'boot_mode:uefi'} + object_utils.create_test_port(self.context, node_id=self.node.id, + address=address) + + provider_mock.get_ip_addresses.return_value = [ip_address] + + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node.properties = properties + pxe_utils.clean_up_pxe_config(task) + + unlink_calls = [ + mock.call('/tftpboot/10.10.0.1.conf'), + mock.call('/tftpboot/0A0A0001.conf'), + mock.call('/tftpboot/pxelinux.cfg/01-%s' % + address.replace(':', '-')) + ] + + unlink_mock.assert_has_calls(unlink_calls) + rmtree_mock.assert_called_once_with( + os.path.join(CONF.pxe.tftp_root, self.node.uuid)) + @mock.patch('ironic.common.utils.rmtree_without_raise', autospec=True) @mock.patch('ironic_lib.utils.unlink_without_raise', autospec=True) @mock.patch('ironic.common.dhcp_factory.DHCPFactory.provider', @@ -840,8 +933,13 @@ class TestPXEUtils(db_base.DbTestCase): task.node.properties = properties pxe_utils.clean_up_pxe_config(task) - unlink_mock.assert_called_once_with( - '/httpboot/pxelinux.cfg/aa-aa-aa-aa-aa-aa') + ensure_calls = [ + mock.call("/httpboot/pxelinux.cfg/%s" + % address.replace(':', '-')), + mock.call("/httpboot/%s.conf" % address) + ] + + unlink_mock.assert_has_calls(ensure_calls) rmtree_mock.assert_called_once_with( os.path.join(CONF.deploy.http_root, self.node.uuid)) diff --git a/releasenotes/notes/grub-default-change-to-mac-1e301a96c49acec4.yaml b/releasenotes/notes/grub-default-change-to-mac-1e301a96c49acec4.yaml new file mode 100644 index 0000000000..4fb51bd65d --- /dev/null +++ b/releasenotes/notes/grub-default-change-to-mac-1e301a96c49acec4.yaml @@ -0,0 +1,22 @@ +--- +upgrade: + - | + Operators utilizing ``grub`` for PXE booting, typically with UEFI, should + change their deployed master PXE configuration file provided for nodes PXE + booting using grub. Ironic 11.1 now writes both MAC address and IP address + based PXE confiuration links for network booting via ``grub``. + The grub variable should be changed from ``$net_default_ip`` to + ``$net_default_mac``. IP address support is deprecated and will be removed + in the Stein release. +deprecations: + - | + Support for ironic to link PXE boot configuration files via the assigned + interface IP address has been deprecated. This option was only the case + when ``[pxe]ipxe_enabled`` was set to ``false`` and the node was + being deployed using UEFI. +fixes: + - | + Fixes support for ``grub`` based UEFI PXE booting by enabling links to the + PXE configuration files to be written using the MAC address of the node in + addition to the interface IP address. If the ``[dhcp]dhcp_provider`` + option is set to ``none``, only the MAC based links will be created.