diff --git a/doc/source/install/configure-pxe.rst b/doc/source/install/configure-pxe.rst index 4549640058..0d084d839e 100644 --- a/doc/source/install/configure-pxe.rst +++ b/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. diff --git a/ironic/common/exception.py b/ironic/common/exception.py index c4c42aa93e..b3dcdc673c 100644 --- a/ironic/common/exception.py +++ b/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") diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index cd5a570848..c886360d3f 100644 --- a/ironic/common/pxe_utils.py +++ b/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) diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index 43a3ff51e3..935429a029 100644 --- a/ironic/conductor/base_manager.py +++ b/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 - in a transitory power state and nodes that are presently locked by - the hostname of this conductor. + Prepares the conductor for basic operation by removing any + existing transitory node power states and reservations which + 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. diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py index e6672aabe9..81aef8e74d 100644 --- a/ironic/conf/pxe.py +++ b/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 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')), ] diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index c191686cb3..3b2ff0cf38 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/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), + ]) diff --git a/releasenotes/notes/bootloader-copy-for-network-boot-190c713cb5e872d8.yaml b/releasenotes/notes/bootloader-copy-for-network-boot-190c713cb5e872d8.yaml new file mode 100644 index 0000000000..047f46771e --- /dev/null +++ b/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