From 6fc33b0a1ba384e7632c90f520bfa9b0464a1ec9 Mon Sep 17 00:00:00 2001 From: Bill Peck Date: Wed, 7 Sep 2022 14:01:03 -0400 Subject: [PATCH] Add support for custom boot loaders This allows for custom boot loaders to be used per installation. Newer versions of RHEL may require newer versions of grub or the ppc64le boot loader to function properly. --- Common/bkr/common/schema/beaker-job.rng | 9 ++ .../src/bkr/inttest/server/test_jobs.py | 37 ++++++++ .../src/bkr/labcontroller/netboot.py | 94 +++++++++++++------ .../src/bkr/labcontroller/provision.py | 3 +- .../src/bkr/labcontroller/test_netboot.py | 10 +- ...36_add_image_path_to_installation_table.py | 28 ++++++ Server/bkr/server/jobs.py | 4 +- Server/bkr/server/labcontroller.py | 6 ++ Server/bkr/server/model/installation.py | 43 +++++++-- 9 files changed, 193 insertions(+), 41 deletions(-) create mode 100644 Server/bkr/server/alembic/versions/140c5eea2836_add_image_path_to_installation_table.py diff --git a/Common/bkr/common/schema/beaker-job.rng b/Common/bkr/common/schema/beaker-job.rng index 5fedccc6b..bf7922d99 100644 --- a/Common/bkr/common/schema/beaker-job.rng +++ b/Common/bkr/common/schema/beaker-job.rng @@ -589,6 +589,15 @@ the Free Software Foundation; either version 2 of the License, or + + + + Location of the installer netboot image. May be specified as + an absolute URL or as a path relative to the installation tree URL. + + + + CPU architecture that the distro is built for. diff --git a/IntegrationTests/src/bkr/inttest/server/test_jobs.py b/IntegrationTests/src/bkr/inttest/server/test_jobs.py index 9e1e508a7..391f5bdb6 100644 --- a/IntegrationTests/src/bkr/inttest/server/test_jobs.py +++ b/IntegrationTests/src/bkr/inttest/server/test_jobs.py @@ -104,6 +104,42 @@ def test_job_with_custom_distro_without_optional_attributes_can_be_roundtripped( self.assertIn('', roundtripped_xml) self.assertIn('', roundtripped_xml) + # https://bugzilla.redhat.com/show_bug.cgi?id=911515 + def test_job_with_custom_distro_with_image_url_can_be_roundtripped(self): + complete_job_xml = ''' + + + so pretty + + + + + + + + + + + + + + + + + ''' + xmljob = lxml.etree.fromstring(complete_job_xml) + job = testutil.call(self.controller.process_xmljob, xmljob, self.user) + roundtripped_xml = lxml.etree.tostring(job.to_xml(clone=True), pretty_print=True, encoding='utf8') + self.assertIn( + '', + roundtripped_xml + ) + self.assertIn('', roundtripped_xml) + self.assertIn('', roundtripped_xml) + self.assertIn('', roundtripped_xml) + self.assertIn('', roundtripped_xml) + self.assertIn('', roundtripped_xml) + def test_complete_job_results(self): complete_job_xml = pkg_resources.resource_string('bkr.inttest', 'complete-job.xml') xmljob = lxml.etree.fromstring(complete_job_xml) @@ -334,6 +370,7 @@ def test_distro_metadata_stored_at_job_submission_time_for_traditional_distro(se self.assertIsNone(recipe.installation.tree_url) self.assertIsNone(recipe.installation.initrd_path) self.assertIsNone(recipe.installation.kernel_path) + self.assertIsNone(recipe.installation.image_path) self.assertEqual(recipe.installation.arch.arch, u'i386') self.assertEqual(recipe.installation.distro_name, u'BlueShoeLinux5-5') self.assertEqual(recipe.installation.osmajor, u'BlueShoeLinux5') diff --git a/LabController/src/bkr/labcontroller/netboot.py b/LabController/src/bkr/labcontroller/netboot.py index 1effe5c60..5c6c519db 100644 --- a/LabController/src/bkr/labcontroller/netboot.py +++ b/LabController/src/bkr/labcontroller/netboot.py @@ -102,6 +102,16 @@ def copy_default_loader_images(): '/usr/share/syslinux/menu.c32') +def fetch_bootloader_image(fqdn, fqdn_dir, distro_tree_id, image_url): + timeout = get_conf().get('IMAGE_FETCH_TIMEOUT') + logger.debug('Fetching bootloader image %s for %s', image_url, fqdn) + with atomically_replaced_file(os.path.join(fqdn_dir, 'image')) as dest: + try: + siphon(urllib2.urlopen(image_url, timeout=timeout), dest) + except Exception as e: + raise ImageFetchingError(image_url, distro_tree_id, e) + + def fetch_images(distro_tree_id, kernel_url, initrd_url, fqdn): """ Creates references to kernel and initrd files at: @@ -181,12 +191,13 @@ def extract_arg(arg, kernel_options): def configure_grub2(fqdn, default_config_loc, config_file, kernel_options, devicetree=''): + grub2_postfix, kernel_options = extract_arg('grub2_postfix=', kernel_options) config = """\ -linux /images/%s/kernel %s netboot_method=grub2 -initrd /images/%s/initrd +linux%s /images/%s/kernel %s netboot_method=grub2 +initrd%s /images/%s/initrd %s boot -""" % (fqdn, kernel_options, fqdn, devicetree) +""" % (grub2_postfix or '', fqdn, kernel_options, grub2_postfix or '', fqdn, devicetree) with atomically_replaced_file(config_file) as f: f.write(config) # We also ensure a default config exists that exits @@ -203,17 +214,28 @@ def configure_aarch64(fqdn, kernel_options, basedir): Creates PXE bootloader files for aarch64 Linux /aarch64/grub.cfg- + /EFI/BOOT/grub.cfg- + /EFI/BOOT/grub.cfg """ + grub2_conf = "grub.cfg-%s" % pxe_basename(fqdn) pxe_base = os.path.join(basedir, 'aarch64') makedirs_ignore(pxe_base, mode=0o755) + + efi_conf_dir = os.path.join(basedir, 'EFI', 'BOOT') + makedirs_ignore(efi_conf_dir, mode=0o755) + devicetree, kernel_options = extract_arg('devicetree=', kernel_options) if devicetree: devicetree = 'devicetree %s' % devicetree else: devicetree = '' - basename = "grub.cfg-%s" % pxe_basename(fqdn) - logger.debug('Writing aarch64 config for %s as %s', fqdn, basename) - grub_cfg_file = os.path.join(pxe_base, basename) + + grub_cfg_file = os.path.join(efi_conf_dir, grub2_conf) + logger.debug('Writing aarch64 config for %s as %s', fqdn, grub_cfg_file) + configure_grub2(fqdn, efi_conf_dir, grub_cfg_file, kernel_options, devicetree) + + grub_cfg_file = os.path.join(pxe_base, grub2_conf) + logger.debug('Writing aarch64 config for %s as %s', fqdn, grub_cfg_file) configure_grub2(fqdn, pxe_base, grub_cfg_file, kernel_options, devicetree) @@ -356,7 +378,7 @@ def configure_ipxe(fqdn, kernel_options, basedir): /ipxe/default """ ipxe_dir = os.path.join(basedir, 'ipxe') - makedirs_ignore(ipxe_dir, mode=0755) + makedirs_ignore(ipxe_dir, mode=0o755) basename = pxe_basename(fqdn).lower() # Unfortunately the initrd kernel arg needs some special handling. It can be @@ -580,15 +602,24 @@ def configure_x86_64(fqdn, kernel_options, basedir): Calls configure_grub2() to create the machine config files to GRUB2 boot loader. + /EFI/BOOT/grub.cfg- + /EFI/BOOT/grub.cfg /x86_64/grub.cfg- /x86_64/grub.cfg /boot/grub2/grub.cfg- /boot/grub2/grub.cfg """ - x86_64_dir = os.path.join(basedir, 'x86_64') - makedirs_ignore(x86_64_dir, mode=0o755) + grub2_conf = "grub.cfg-%s" % pxe_basename(fqdn) + efi_conf_dir = os.path.join(basedir, 'EFI', 'BOOT') + makedirs_ignore(efi_conf_dir, mode=0o755) + grub_cfg_file = os.path.join(efi_conf_dir, grub2_conf) + logger.debug('Writing grub2/x86_64 config for %s as %s', fqdn, grub_cfg_file) + configure_grub2(fqdn, efi_conf_dir, grub_cfg_file, kernel_options) + + x86_64_dir = os.path.join(basedir, 'x86_64') + makedirs_ignore(x86_64_dir, mode=0o755) grub_cfg_file = os.path.join(x86_64_dir, grub2_conf) logger.debug('Writing grub2/x86_64 config for %s as %s', fqdn, grub_cfg_file) configure_grub2(fqdn, x86_64_dir, grub_cfg_file, kernel_options) @@ -768,22 +799,14 @@ def add_bootloader(name, configure, clear, arches=None): # Custom bootloader stuff -def configure_netbootloader_directory(fqdn, kernel_options, netbootloader): - tftp_root = get_tftp_root() - if netbootloader: - fqdn_dir = os.path.join(tftp_root, 'bootloader', fqdn) - logger.debug('Creating custom netbootloader tree for %s in %s', fqdn, fqdn_dir) - makedirs_ignore(fqdn_dir, mode=0o755) - grub2_cfg_file = os.path.join(fqdn_dir, 'grub.cfg-%s'%pxe_basename(fqdn)) - configure_grub2(fqdn, fqdn_dir, grub2_cfg_file, kernel_options) - configure_pxelinux(fqdn, kernel_options, fqdn_dir, symlink=True) - configure_ipxe(fqdn, kernel_options, fqdn_dir) - configure_yaboot(fqdn, kernel_options, fqdn_dir, yaboot_symlink=False) - - # create the symlink to the specified bootloader w.r.t the tftp_root - if netbootloader.startswith('/'): - netbootloader = netbootloader.lstrip('/') - atomic_symlink(os.path.join('../../', netbootloader), os.path.join(fqdn_dir, 'image')) +def configure_netbootloader_directory(fqdn, fqdn_dir, kernel_options): + logger.debug('Creating custom netbootloader tree for %s in %s', fqdn, fqdn_dir) + makedirs_ignore(fqdn_dir, mode=0o755) + grub2_cfg_file = os.path.join(fqdn_dir, 'grub.cfg-%s' % pxe_basename(fqdn)) + configure_grub2(fqdn, fqdn_dir, grub2_cfg_file, kernel_options) + configure_pxelinux(fqdn, kernel_options, fqdn_dir, symlink=True) + configure_ipxe(fqdn, kernel_options, fqdn_dir) + configure_yaboot(fqdn, kernel_options, fqdn_dir, yaboot_symlink=False) def clear_netbootloader_directory(fqdn): @@ -798,7 +821,8 @@ def clear_netbootloader_directory(fqdn): def configure_all(fqdn, arch, distro_tree_id, - kernel_url, initrd_url, kernel_options, basedir=None): + kernel_url, initrd_url, kernel_options, + image_url, basedir=None): """Configure images and all bootloader files for given fqdn""" fetch_images(distro_tree_id, kernel_url, initrd_url, fqdn) if not basedir: @@ -812,7 +836,23 @@ def configure_all(fqdn, arch, distro_tree_id, bootloader.configure(fqdn, kernel_options, basedir) if arch == 's390' or arch == 's390x': configure_zpxe(fqdn, kernel_url, initrd_url, kernel_options, basedir) - configure_netbootloader_directory(fqdn, kernel_options, netbootloader) + + # Custom boot loader code + tftp_root = get_tftp_root() + fqdn_dir = os.path.join(tftp_root, 'bootloader', fqdn) + + if image_url or netbootloader: + configure_netbootloader_directory(fqdn, fqdn_dir, kernel_options) + + if image_url: + fetch_bootloader_image(fqdn, fqdn_dir, distro_tree_id, image_url) + else: + # create the symlink to the specified bootloader w.r.t the tftp_root + if netbootloader.startswith("/"): + netbootloader = netbootloader.lstrip("/") + atomic_symlink( + os.path.join("../../", netbootloader), os.path.join(fqdn_dir, "image") + ) def clear_all(fqdn, basedir=None): diff --git a/LabController/src/bkr/labcontroller/provision.py b/LabController/src/bkr/labcontroller/provision.py index 4fc74b029..bb8ff924a 100644 --- a/LabController/src/bkr/labcontroller/provision.py +++ b/LabController/src/bkr/labcontroller/provision.py @@ -228,7 +228,8 @@ def handle_configure_netboot(command): command['netboot']['distro_tree_id'], command['netboot']['kernel_url'], command['netboot']['initrd_url'], - command['netboot']['kernel_options']) + command['netboot']['kernel_options'], + command['netboot']['image_url']) def handle_clear_netboot(command): netboot.clear_all(command['fqdn']) diff --git a/LabController/src/bkr/labcontroller/test_netboot.py b/LabController/src/bkr/labcontroller/test_netboot.py index b86035574..2bb3b3ce1 100644 --- a/LabController/src/bkr/labcontroller/test_netboot.py +++ b/LabController/src/bkr/labcontroller/test_netboot.py @@ -146,6 +146,10 @@ def setUp(self): for _ in xrange(8 * 1024): self.initrd.write(chr(random.randrange(0, 256)) * 1024) self.initrd.flush() + self.image = tempfile.NamedTemporaryFile(prefix='test_netboot', suffix='image') + for _ in xrange(4 * 1024): + self.image.write(chr(random.randrange(0, 256)) * 1024) + self.image.flush() class ImagesTest(ImagesBaseTestCase): @@ -185,7 +189,8 @@ class ArchBasedConfigTest(ImagesBaseTestCase): def configure(self, arch): netboot.configure_all(TEST_FQDN, arch, 1234, 'file://%s' % self.kernel.name, - 'file://%s' % self.initrd.name, "", self.tftp_root) + 'file://%s' % self.initrd.name, "", + 'file://%s' % self.image.name, self.tftp_root) def get_categories(self, arch): this = self.common_categories @@ -636,7 +641,8 @@ def test_configure_then_clear(self): netboot.configure_all(TEST_FQDN, 'ppc64', 1234, 'file://%s' % self.kernel.name, 'file://%s' % self.initrd.name, - 'netbootloader=myawesome/netbootloader' + 'netbootloader=myawesome/netbootloader', + None ) bootloader_config_symlink = os.path.join(self.tftp_root, 'bootloader', TEST_FQDN, 'image') self.assertTrue(os.path.lexists(bootloader_config_symlink)) diff --git a/Server/bkr/server/alembic/versions/140c5eea2836_add_image_path_to_installation_table.py b/Server/bkr/server/alembic/versions/140c5eea2836_add_image_path_to_installation_table.py new file mode 100644 index 000000000..ead19d7db --- /dev/null +++ b/Server/bkr/server/alembic/versions/140c5eea2836_add_image_path_to_installation_table.py @@ -0,0 +1,28 @@ +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +""" +add image_path to Installation table + +Revision ID: 140c5eea2836 +Revises: 4b3a6065eba2 +Create Date: 2022-09-01 18:06:05.437563 +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '140c5eea2836' +down_revision = '4b3a6065eba2' + + +def upgrade(): + op.add_column('installation', sa.Column('image_path', sa.UnicodeText(), nullable=True)) + + +def downgrade(): + op.drop_column('installation', 'image_path') diff --git a/Server/bkr/server/jobs.py b/Server/bkr/server/jobs.py index 1412a301f..a7b98cf37 100644 --- a/Server/bkr/server/jobs.py +++ b/Server/bkr/server/jobs.py @@ -737,12 +737,14 @@ def handle_distro(distro): tree_url = distro.find("tree").get("url") initrd_path = distro.find("initrd").get("url") kernel_path = distro.find("kernel").get("url") + image_path = distro.find("image").get("url") if distro.find("image") is not None else None osmajor = distro.find("osversion").get("major") osminor = distro.find("osversion").get("minor", "0") name = distro.find("name").get("value") if distro.find("name") is not None else None variant = distro.find("variant").get("value") if distro.find("variant") is not None else None return Installation(tree_url=tree_url, initrd_path=initrd_path, kernel_path=kernel_path, - arch=arch, distro_name=name, osmajor=osmajor, osminor=osminor, variant=variant) + arch=arch, distro_name=name, osmajor=osmajor, osminor=osminor, + variant=variant, image_path=image_path) @expose('json') def update_recipe_set_response(self, recipe_set_id, response_id): diff --git a/Server/bkr/server/labcontroller.py b/Server/bkr/server/labcontroller.py index 434785dd1..2a4eb6c99 100644 --- a/Server/bkr/server/labcontroller.py +++ b/Server/bkr/server/labcontroller.py @@ -449,6 +449,12 @@ def get_queued_command_details(self): 'initrd_url': urlparse.urljoin(distro_tree_url, installation.initrd_path), 'kernel_options': installation.kernel_options or '', } + if installation.image_path: + d["netboot"]["image_url"] = urlparse.urljoin( + distro_tree_url, installation.image_path + ) + else: + d['netboot']['image_url'] = None if distro_tree: d['netboot']['distro_tree_id'] = distro_tree.id else: diff --git a/Server/bkr/server/model/installation.py b/Server/bkr/server/model/installation.py index 3f6c40af7..6843f884e 100644 --- a/Server/bkr/server/model/installation.py +++ b/Server/bkr/server/model/installation.py @@ -61,6 +61,7 @@ class Installation(DeclarativeMappedObject): osmajor = Column(UnicodeText) osminor = Column(UnicodeText) variant = Column(UnicodeText) + image_path = Column(UnicodeText) def distro_to_xml(self): distro_xml = E.distro( @@ -70,6 +71,8 @@ def distro_to_xml(self): E.arch(value=self.arch.arch), E.osversion(major=self.osmajor, minor=self.osminor) ) + if self.image_path: + distro_xml.append(E.image(url=self.image_path)) if self.distro_name: distro_xml.append(E.name(value=self.distro_name)) if self.variant: @@ -77,16 +80,35 @@ def distro_to_xml(self): return distro_xml def __repr__(self): - return ('%s(created=%r, system=%r, distro_tree=%r, kernel_options=%r, ' - 'rendered_kickstart=%r, rebooted=%r, install_started=%r, ' - 'install_finished=%r, postinstall_finished=%r, tree_url=%r,' - ' initrd_path=%r, kernel_path=%r, arch=%r, distro_name=%r, osmajor=%r, osminor=%r,' - ' variant=%r)' % (self.__class__.__name__, self.created, self.system, - self.distro_tree, self.kernel_options, self.rendered_kickstart, - self.rebooted, self.install_started, self.install_finished, - self.postinstall_finished, self.tree_url, self.initrd_path, - self.kernel_path, self.arch, self.distro_name, self.osmajor, self.osminor, - self.variant)) + return ( + "%s(created=%r, system=%r, distro_tree=%r, kernel_options=%r, " + "rendered_kickstart=%r, rebooted=%r, install_started=%r, " + "install_finished=%r, postinstall_finished=%r, tree_url=%r," + " initrd_path=%r, kernel_path=%r, arch=%r, distro_name=%r," + " osmajor=%r, osminor=%r, image_path=%r," + " variant=%r)" + % ( + self.__class__.__name__, + self.created, + self.system, + self.distro_tree, + self.kernel_options, + self.rendered_kickstart, + self.rebooted, + self.install_started, + self.install_finished, + self.postinstall_finished, + self.tree_url, + self.initrd_path, + self.kernel_path, + self.arch, + self.distro_name, + self.osmajor, + self.osminor, + self.image_path, + self.variant, + ) + ) def __json__(self): return { @@ -102,6 +124,7 @@ def __json__(self): 'tree_url': self.tree_url, 'initrd_path': self.initrd_path, 'kernel_path': self.kernel_path, + 'image_path': self.image_path, 'arch': self.arch, 'distro_name': self.distro_name, 'osmajor': self.osmajor,