Merge "Change PXE logic to always link macs with UEFI"

4 files changed, 160 insertions, 14 deletions

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index c3ce89a37..cb8036d7b 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 14d506d47..82847b81e 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 bc174d11d..edf884b59 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
@@ -256,12 +257,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]
@@ -289,12 +296,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]
@@ -316,12 +329,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]
@@ -507,6 +526,46 @@ class TestPXEUtils(db_base.DbTestCase):
                                       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)
     @mock.patch('ironic.common.utils.render_template', autospec=True)
@@ -548,8 +607,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))
 
@@ -806,6 +870,35 @@ class TestPXEUtils(db_base.DbTestCase):
     @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',
+                autospec=True)
     def test_clean_up_pxe_config_uefi_instance_info(self,
                                                     provider_mock, unlink_mock,
                                                     rmtree_mock):
@@ -841,8 +934,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 000000000..4fb51bd65
--- /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.