Facilitate asset copy for bootloader ops

Adds capability to copy bootloader assets from the system OS
into the network boot folders on conductor startup.

Change-Id: Ica8f9472d0a2409cf78832166c57f2bb96677833

doc/source/install/configure-pxe.rst

@@ -535,3 +535,28 @@ minutes, allowing two retries after 20 minutes:
 
     [pxe]
     boot_retry_timeout = 1200
+
+PXE artifacts
+-------------
+
+Ironic features the capability to load PXE artifacts into the conductor
+startup, minimizing the need for external installation and configuration
+management tooling from having to do additional work to facilitate.
+
+While this is an advanced feature, and destination file names must match
+existing bootloader configured filenames.
+
+For example, if using iPXE and GRUB across interfaces, you may desire
+a configuration similar to this example.
+
+.. code-block:: ini
+
+   [pxe]
+   loader_file_paths = ipxe.efi:/usr/share/ipxe/ipxe-snponly-x86_64.efi,undionly.kpxe:/usr/share/ipxe/undionly.kpxe,bootx64.efi,/boot/efi/EFI/boot/grubx64.efi,bootx64.efi:/boot/efi/EFI/boot/BOOTX64.EFI
+
+If you choose to use relative paths as part of your destination,
+those paths will be created using configuration parameter
+``[pxe]dir_permission`` where as actual files copied are set with
+the configuration parameter ``[pxe]file_permission``. Absolute destination
+paths are not supported and will result in ironic failing to start up as
+it is a misconfiguration of the deployment.

ironic/common/exception.py

@@ -825,3 +825,8 @@ class InsufficentMemory(IronicException):
 
 class NodeHistoryNotFound(NotFound):
     _msg_fmt = _("Node history record %(history)s could not be found.")
+
+
+class IncorrectConfiguration(IronicException):
+    _msg_fmt = _("Supplied configuration is incorrect and must be fixed. "
+                 "Error: %(error)s")

ironic/common/pxe_utils.py

@@ -16,6 +16,7 @@
 
 import copy
 import os
+import shutil
 import tempfile
 
 from ironic_lib import utils as ironic_utils
@@ -1262,3 +1263,60 @@ def clean_up_pxe_env(task, images_info, ipxe_enabled=False):
 
     clean_up_pxe_config(task, ipxe_enabled=ipxe_enabled)
     TFTPImageCache().clean_up()
+
+
+def place_loaders_for_boot(base_path):
+    """Place configured bootloaders from the host OS.
+
+    Example: grubaa64.efi:/path/to/grub-aarch64.efi,...
+
+    :param base_path: Destination path where files should be copied to.
+    """
+    loaders = CONF.pxe.loader_file_paths
+    if not loaders or not base_path:
+        # Do nothing, return.
+        return
+
+    for dest, src in loaders.items():
+        (head, _tail) = os.path.split(dest)
+        if head:
+            if head.startswith('/'):
+                # NOTE(TheJulia): The intent here is to error if the operator
+                # has put absolute paths in place, as we can and likely should
+                # copy to multiple folders based upon the protocol operation
+                # being used. Absolute paths are problematic there, where
+                # as a relative path is more a "well, that is silly, but okay
+                # misconfiguration.
+                msg = ('File paths configured for [pxe]loader_file_paths '
+                       'must be relative paths. Entry: %s') % dest
+                raise exception.IncorrectConfiguration(msg)
+            else:
+                try:
+                    os.makedirs(
+                        os.path.join(base_path, head),
+                        mode=CONF.pxe.dir_permission or 0o755, exist_ok=True)
+                except OSError as e:
+                    msg = ('Failed to create supplied directories in '
+                           'asset copy paths. Error: %s') % e
+                    raise exception.IncorrectConfiguration(msg)
+
+        full_dest = os.path.join(base_path, dest)
+        if not os.path.isfile(src):
+            msg = ('Was not able to find source path %(src)s '
+                   'to copy to %(dest)s.' %
+                   {'src': src, 'dest': full_dest})
+            LOG.error(msg)
+            raise exception.IncorrectConfiguration(error=msg)
+
+        LOG.debug('Copying bootloader %(dest)s from %(src)s.',
+                  {'src': src, 'dest': full_dest})
+        try:
+            shutil.copy2(src, full_dest)
+            if CONF.pxe.file_permission:
+                os.chmod(full_dest, CONF.pxe.file_permission)
+        except OSError as e:
+            msg = ('Error encountered while attempting to '
+                   'copy a configured bootloader into '
+                   'the requested destination. %s' % e)
+            LOG.error(msg)
+            raise exception.IncorrectConfiguration(error=msg)

ironic/conductor/base_manager.py

@@ -31,6 +31,7 @@ from ironic.common import driver_factory
 from ironic.common import exception
 from ironic.common import hash_ring
 from ironic.common.i18n import _
+from ironic.common import pxe_utils
 from ironic.common import release_mappings as versions
 from ironic.common import rpc
 from ironic.common import states
@@ -87,9 +88,11 @@ class BaseConductorManager(object):
     def prepare_host(self):
         """Prepares host for initialization
 
-        Removes existing database entries involved with node locking for nodes
+        Prepares the conductor for basic operation by removing any
-        in a transitory power state and nodes that are presently locked by
+        existing transitory node power states and reservations which
-        the hostname of this conductor.
+        were previously held by this host. Once that has been completed,
+        bootloader assets, if configured, are staged for network (PXE) boot
+        operations.
 
         Under normal operation, this is also when the initial database
         connectivity is established for the conductor's normal operation.
@@ -109,6 +112,8 @@ class BaseConductorManager(object):
         self.dbapi.clear_node_target_power_state(self.host)
         # clear all locks held by this conductor before registering
         self.dbapi.clear_node_reservations_for_conductor(self.host)
+        pxe_utils.place_loaders_for_boot(CONF.pxe.tftp_root)
+        pxe_utils.place_loaders_for_boot(CONF.deploy.http_root)
 
     def init_host(self, admin_context=None):
         """Initialize the conductor host.

ironic/conf/pxe.py

@@ -17,6 +17,7 @@
 import os
 
 from oslo_config import cfg
+from oslo_config import types as cfg_types
 
 from ironic.common.i18n import _
 
@@ -100,8 +101,15 @@ opts = [
                       "creating files that cannot be read by the TFTP server. "
                       "Setting to <None> will result in the operating "
                       "system's umask to be utilized for the creation of new "
-                      "tftp folders. It is recommended that an octal "
+                      "tftp folders. It is required that an octal "
                       "representation is specified. For example: 0o755")),
+    cfg.IntOpt('file_permission',
+               default=0o644,
+               help=_('The permission which is used on files created as part '
+                      'of configuration and setup of file assets for PXE '
+                      'based operations. Defaults to a value of 0o644.'
+                      'This value must be specified as an octal '
+                      'representation. For example: 0o644')),
     cfg.StrOpt('pxe_bootfile_name',
                default='pxelinux.0',
                help=_('Bootfile DHCP parameter.')),
@@ -178,6 +186,19 @@ opts = [
                        'or with Redfish on machines that cannot do persistent '
                        'boot. Mostly useful for standalone ironic since '
                        'Neutron will prevent incorrect PXE boot.')),
+    cfg.Opt('loader_file_paths',
+            type=cfg_types.Dict(cfg_types.String(quotes=True)),
+            default={},
+            help=_('Dictionary describing the bootloaders to load into '
+                   'conductor PXE/iPXE boot folders values from the host '
+                   'operating system. Formatted as key of destination '
+                   'file name, and value of a full path to a file to be '
+                   'copied. File assets will have [pxe]file_permission '
+                   'applied, if set. If used, the file names should '
+                   'match established bootloader configuration settings '
+                   'for bootloaders. Use example: '
+                   'ipxe.efi:/usr/share/ipxe/ipxe-snponly-x86_64.efi,'
+                   'undionly.kpxe:/usr/share/ipxe/undionly.kpxe')),
 ]
 
 

ironic/tests/unit/common/test_pxe_utils.py

@@ -16,6 +16,7 @@
 
 import collections
 import os
+import shutil
 import tempfile
 from unittest import mock
 
@@ -2130,3 +2131,124 @@ class TFTPImageCacheTestCase(db_base.DbTestCase):
         mock_ensure_tree.assert_not_called()
         self.assertEqual(500 * 1024 * 1024, cache._cache_size)
         self.assertEqual(30 * 60, cache._cache_ttl)
+
+
+@mock.patch.object(os, 'makedirs', autospec=True)
+@mock.patch.object(os.path, 'isfile', autospec=True)
+@mock.patch.object(os, 'chmod', autospec=True)
+@mock.patch.object(shutil, 'copy2', autospec=True)
+class TestPXEUtilsBootloader(db_base.DbTestCase):
+
+    def setUp(self):
+        super(TestPXEUtilsBootloader, self).setUp()
+
+    def test_place_loaders_for_boot_default_noop(self, mock_copy2,
+                                                 mock_chmod, mock_isfile,
+                                                 mock_makedirs):
+        res = pxe_utils.place_loaders_for_boot('/httpboot')
+        self.assertIsNone(res)
+        self.assertFalse(mock_copy2.called)
+        self.assertFalse(mock_chmod.called)
+        self.assertFalse(mock_isfile.called)
+        self.assertFalse(mock_makedirs.called)
+        self.assertFalse(mock_makedirs.called)
+
+    def test_place_loaders_for_boot_no_source(self, mock_copy2,
+                                              mock_chmod, mock_isfile,
+                                              mock_makedirs):
+        self.config(loader_file_paths='grubaa64.efi:/path/to/file',
+                    group='pxe')
+        mock_isfile.return_value = False
+        self.assertRaises(exception.IncorrectConfiguration,
+                          pxe_utils.place_loaders_for_boot,
+                          '/tftpboot')
+        self.assertFalse(mock_copy2.called)
+        self.assertFalse(mock_chmod.called)
+        self.assertFalse(mock_makedirs.called)
+
+    def test_place_loaders_for_boot_two_files(self, mock_copy2,
+                                              mock_chmod, mock_isfile,
+                                              mock_makedirs):
+        self.config(loader_file_paths='bootx64.efi:/path/to/shimx64.efi,'
+                                      'grubx64.efi:/path/to/grubx64.efi',
+                    group='pxe')
+        self.config(file_permission=420, group='pxe')
+        mock_isfile.return_value = True
+        res = pxe_utils.place_loaders_for_boot('/tftpboot')
+        self.assertIsNone(res)
+        mock_isfile.assert_has_calls([
+            mock.call('/path/to/shimx64.efi'),
+            mock.call('/path/to/grubx64.efi'),
+        ])
+        mock_copy2.assert_has_calls([
+            mock.call('/path/to/shimx64.efi', '/tftpboot/bootx64.efi'),
+            mock.call('/path/to/grubx64.efi', '/tftpboot/grubx64.efi')
+        ])
+        mock_chmod.assert_has_calls([
+            mock.call('/tftpboot/bootx64.efi', CONF.pxe.file_permission),
+            mock.call('/tftpboot/grubx64.efi', CONF.pxe.file_permission)
+        ])
+        self.assertFalse(mock_makedirs.called)
+
+    def test_place_loaders_for_boot_two_files_exception_on_copy(
+            self, mock_copy2, mock_chmod, mock_isfile, mock_makedirs):
+        self.config(loader_file_paths='bootx64.efi:/path/to/shimx64.efi,'
+                                      'grubx64.efi:/path/to/grubx64.efi',
+                    group='pxe')
+        self.config(file_permission=420, group='pxe')
+        mock_isfile.side_effect = iter([True, False, True, False])
+        mock_chmod.side_effect = OSError('Chmod not permitted')
+        self.assertRaises(exception.IncorrectConfiguration,
+                          pxe_utils.place_loaders_for_boot,
+                          '/tftpboot')
+        mock_isfile.assert_has_calls([
+            mock.call('/path/to/shimx64.efi'),
+        ])
+        mock_copy2.assert_has_calls([
+            mock.call('/path/to/shimx64.efi', '/tftpboot/bootx64.efi'),
+        ])
+        mock_chmod.assert_has_calls([
+            mock.call('/tftpboot/bootx64.efi', CONF.pxe.file_permission),
+        ])
+        self.assertFalse(mock_makedirs.called)
+
+    def test_place_loaders_for_boot_raises_exception_with_absolute_path(
+            self, mock_copy2, mock_chmod, mock_isfile, mock_makedirs):
+        self.config(
+            loader_file_paths='/tftpboot/bootx64.efi:/path/to/shimx64.efi',
+            group='pxe')
+        exc = self.assertRaises(exception.IncorrectConfiguration,
+                                pxe_utils.place_loaders_for_boot,
+                                '/tftpboot')
+        self.assertEqual('File paths configured for [pxe]loader_file_paths '
+                         'must be relative paths. Entry: '
+                         '/tftpboot/bootx64.efi', str(exc))
+
+    def test_place_loaders_for_boot_two_files_relative_path(
+            self, mock_copy2, mock_chmod, mock_isfile, mock_makedirs):
+        self.config(loader_file_paths='grub/bootx64.efi:/path/to/shimx64.efi,'
+                                      'grub/grubx64.efi:/path/to/grubx64.efi',
+                    group='pxe')
+        self.config(dir_permission=484, group='pxe')
+        self.config(file_permission=420, group='pxe')
+        mock_isfile.return_value = True
+        res = pxe_utils.place_loaders_for_boot('/tftpboot')
+        self.assertIsNone(res)
+        mock_isfile.assert_has_calls([
+            mock.call('/path/to/shimx64.efi'),
+            mock.call('/path/to/grubx64.efi'),
+        ])
+        mock_copy2.assert_has_calls([
+            mock.call('/path/to/shimx64.efi', '/tftpboot/grub/bootx64.efi'),
+            mock.call('/path/to/grubx64.efi', '/tftpboot/grub/grubx64.efi')
+        ])
+        mock_chmod.assert_has_calls([
+            mock.call('/tftpboot/grub/bootx64.efi', CONF.pxe.file_permission),
+            mock.call('/tftpboot/grub/grubx64.efi', CONF.pxe.file_permission)
+        ])
+        mock_makedirs.assert_has_calls([
+            mock.call('/tftpboot/grub', mode=CONF.pxe.dir_permission,
+                      exist_ok=True),
+            mock.call('/tftpboot/grub', mode=CONF.pxe.dir_permission,
+                      exist_ok=True),
+        ])

releasenotes/notes/bootloader-copy-for-network-boot-190c713cb5e872d8.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    Adds a capability to allow bootloaders to be copied into the configured
+    network boot path. This capability can be opted in by using the
+    ``[pxe]loader_file_paths`` by being set to a list of key, value pairs
+    of destination filename, and source file path.
+
+    .. code-block::
+
+       [pxe]
+       loader_file_paths = bootx64.efi:/path/to/shimx64.efi,grubx64.efi:/path/to/grubx64.efi