Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grub/kernel/buildsys: Enable Boot Configuration usage #1980

Merged
merged 6 commits into from Apr 11, 2022

Conversation

zmrow
Copy link
Contributor

@zmrow zmrow commented Mar 3, 2022

Description of changes:

This set of changes provides the foundation for enabling Boot Configuration in Bottlerocket. Boot Configuration allows the user to provide additional parameters to the kernel command line at runtime. It does this by encoding a file and attaching it to the end of the initrd. In grub, the bootconfig parameter lets the kernel know it should expect bootconfig to exist at the end of the specified initrd.

It should be noted that Bottlerocket does not use an initrd. The "initrd" in our case is empty save for the Boot Configuration encoded into the otherwise empty file. By default, we build into the images a Boot Configuration file with no settings.

We expect the Boot Configuration to reside on the BOTTLEROCKET_PRIVATE partition, so additional commands have been added to grub to find that partition, and populate a variable private with its location.

Because we do not currently update grub during OS updates, we need to conditionally add Boot Config support (and its associated grub config changes) to variants we know to have been built with the appropriate support in grub. Currently that limits Boot Configuration usage to metal variants and other new variants to come. In order to make this possible, an additional parameter has been added to the variant definition, which gets fed through buildsys to rpm2img, where we write the appropriate grub configuration.

The changes in this PR only enable the feature; additional changes are forthcoming that will provide the API interface.

    grub: Set `private` variable via search
    
    We currently carry a few patches to enable `search` by partition label
    and uuid, but don't include the `search` module.  This commit adds the
    module to enable this functionality in our grub config.
    
    This commit also adds an additional command to our grub configs.  The
    command uses the `search` module to set the `private` variable to the
    location of the `BOTTLEROCKET-PRIVATE` partition.  The `private`
    variable will be used to populate the grub menuentry that points to the
    bootconfig file.
    kernel 5.10: Add CONFIG_BOOT_CONFIG
    
    Adds `CONFIG_BOOT_CONFIG` to the kernel 5.10 config which enables the
    use of the "Boot config" mechanism.  This allows a user to pass a
    configuration file attached to the end of the initrd which is used to
    extend the kernel command line at runtime.
    buildsys: Add `grub-features` property to variant config
    
    This change adds a `grub-features` property to the supported configuration
    that can be set in a variant's `Cargo.toml`.  This property is meant to gate
    certain features of grub and it's config to variants that support it.  Since
    grub isn't updated during OS update, it's important to ensure that variants
    which are known to support a certain grub feature receive the proper grub
    config.
    
    Currently, the only supported parameter is `set-private-var`.  This
    parameter means the variant has a grub config that understands how to search
    for and find the BOTTLEROCKET_PRIVATE partition, and set the corresponding
    `$private` variable
    rpm2img: Conditionally add bootconfig parameters to grub menuentry
    
    This change conditionally adds the `bootconfig` parameter to the kernel
    command line if `set-private-var` is added to a variants
    `grub-features`.  The `bootconfig` parameter signals the kernel to look
    for the Boot Configuration at the end of the specified initrd.  This
    change also adds the corresponding initrd entry to the grub menuentry
    which points at the bootconfig file on the `BOTTLEROCKET-PRIVATE`
    partition.  Bottlerocket technically doesn't use an initrd, so this
    initrd will only contain Boot Configuration data.  If the file doesn't
    contain Boot Config data, the kernel logs a message but does not fail to
    boot.
    
    An empty `bootconfig.data` file is created and added to the
    `BOTTLEROCKET_PRIVATE` partition to keep grub from pausing and printing
    an error that the file doesn't exist
    rpm2img: Reorganize kernel/init command line parameters
    
    This change organizes the kernel and init command line parameters and
    moves the init parameters to the section after the "--" where they
    belong
    variants: Add `set-private-var` to grub-features for metal variants
    
    This change adds the `set-private-var` parameter to new `grub-features`
    section of the variant config for all metal variants

Testing done:

  • Boot an aws-k8s-1.21 and vmware-k8s-1.21 image. Both instances booted correctly, were able to join the cluster and run pods correctly. Neither contain any references to bootconfig or initrd in their grub configuration:
bash-5.1# cat /boot/grub/grub.cfg 
set default="0"
set timeout="0"

menuentry "Bottlerocket OS 1.7.0" {
   linux ($root)/vmlinuz \
       console=tty0 console=ttyS0,115200n8 \
        \
       root=/dev/dm-0 rootwait ro \
       raid=noautodetect \
       random.trust_cpu=on selinux=1 enforcing=1 \
       dm_verity.max_bios=-1 dm_verity.dev_wait=1 \
       dm-mod.create="root,,,ro,0 1884160 verity 1 PARTUUID=$boot_uuid/PARTNROFF=1 PARTUUID=$boot_uuid/PARTNROFF=2 \
       4096 4096 235520 1 sha256 4e7e561679e6b2f2352da6e9030e130e62b2c95a8a59215e4b4e0f9aaae9476f ae41adafebee7385f163d4e0e1ef52bd7fde8893e90abc758daca975c2a8c69c 1 restart_on_corruption" \
       -- \
       systemd.log_target=journal-or-kmsg systemd.log_color=0 \
       net.ifnames=0 biosdevname=0
   
}
  • Booted a metal-dev under BIOS/EFI on a Supermicro SYS-E200-8D. The image boots without issue, grub does not log any errors with the included empty Boot Configuration file:
bash-5.0# cat /proc/bootconfig 
kernel = ""
init = ""
  • Also booted a metal-dev image without the console settings on the kernel command line on a Supermicro SYS-E200-8D. At provisioning time I encoded the following to bootconfig.data and replaced the default bootconfig.data file on the BOTTLEROCKET-PRIVATE partition.
kernel {
    console = tty0, "ttyS1,115200n8"
}
init {
    systemd.log_level = debug
}

During boot time, the console showed output on both local and SOL. I was able to see the Boot Config data in both /proc/bootconfig and /proc/cmdline. Querying systemd showed log level was set to debug as directed by the Boot config params.

bash-5.0# cat /proc/bootconfig 
kernel.console = "tty0", "ttyS1,115200n8"
init.systemd.log_level = "debug"

bash-5.0# systemctl log-level
debug

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@zmrow zmrow requested review from bcressey and etungsten March 3, 2022 16:27
Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Copy link
Contributor

@arnaldo2792 arnaldo2792 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment, and a few nits in commit messages:

  • grub: Set private variable via search
    • There is an extra space after each .
  • kernel 5.10: Add CONFIG_BOOT_CONFIG
    • The commit subject should use the package name, with the - in it
    • Same spacing problem
  • rpm2img: Add bootconfig parameters to grub menuentry
    • Same spacing problem
    • Using initrd and "initrd" is somewhat confusing, I don't understand why one has the " and the other doesn't, or if they refer to the same concept
    • I haven't seen commit messages with markdown syntax in them, in the commit _only_ will be rendered as it is, and won't be formatted
  • rpm2img: Reorganize kernel/init command line parameters
    • There is an extra " at the end of the paragraph.

packages/grub/bios.cfg Show resolved Hide resolved
@zmrow
Copy link
Contributor Author

zmrow commented Mar 3, 2022

😄 @arnaldo2792 Re: "2 spaces after periods": I guess I'm old school and this is how I was taught to write. I'm not aware of convention for this in commit messages so I'll leave them as they're comparable to all my other commits.

I'll fix up the other bits shortly!

@zmrow
Copy link
Contributor Author

zmrow commented Mar 3, 2022

^ Fixes the commit message for kernel-5.10, removes quotes from initrd and removes the trailing " from the commit message for rpm2img.

Copy link
Contributor

@arnaldo2792 arnaldo2792 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👢

tools/rpm2img Outdated Show resolved Hide resolved
@bcressey bcressey self-requested a review March 28, 2022 22:23
We currently carry a few patches to enable `search` by partition label
and uuid, but don't include the `search` module.  This commit adds the
module to enable this functionality in our grub config.

This commit also adds an additional command to our grub configs.  The
command uses the `search` module to set the `private` variable to the
location of the `BOTTLEROCKET-PRIVATE` partition.  The `private`
variable will be used to populate the grub menuentry that points to the
bootconfig file.
Adds `CONFIG_BOOT_CONFIG` to the kernel 5.10 config which enables the
use of the "Boot config" mechanism.  This allows a user to pass a
configuration file attached to the end of the initrd which is used to
extend the kernel command line at runtime.
@zmrow
Copy link
Contributor Author

zmrow commented Mar 29, 2022

^ rebase on develop

@zmrow zmrow changed the title grub/kernel: Enable Boot Configuration usage grub/kernel/buildsys: Enable Boot Configuration usage Mar 31, 2022
@zmrow
Copy link
Contributor Author

zmrow commented Mar 31, 2022

^ The above adds the new grub-features option and it's single parameter set-private-var to the variant config in buildsys. This conditionally adds the Boot Config bits to the grub menuentry config. It also builds an empty bootconfig file into the image to keep grub from pausing and printing an error in it's absence.

Finally, we use the grub-features option to limit Boot Configuration usage to metal variants as they will be built with the latest version of the grub configuration.

@@ -190,6 +203,11 @@ impl ManifestInfo {
.and_then(|b| b.kernel_parameters.as_ref())
}

/// Convenience method to return the kernel parameters for this variant.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
/// Convenience method to return the kernel parameters for this variant.
/// Convenience method to return the GRUB features for this variant.

tools/rpm2img Outdated
Comment on lines 259 to 263
# If GRUB_SET_PRIVATE_VAR is set, include the parameters that support Boot Config
if [[ "${GRUB_FEATURES_ARRAY[*]}" =~ "GRUB_SET_PRIVATE_VAR" ]]; then
BOOTCONFIG='bootconfig'
INITRD='initrd ($private)/bootconfig.data'
else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might be better to test each element specifically, in the unlikely event that one is a substring of another:

Suggested change
# If GRUB_SET_PRIVATE_VAR is set, include the parameters that support Boot Config
if [[ "${GRUB_FEATURES_ARRAY[*]}" =~ "GRUB_SET_PRIVATE_VAR" ]]; then
BOOTCONFIG='bootconfig'
INITRD='initrd ($private)/bootconfig.data'
else
# If GRUB_SET_PRIVATE_VAR is set, include the parameters that support Boot Config
if printf '%s\n' "${GRUB_FEATURES_ARRAY[@]}" | grep -Fxq 'GRUB_SET_PRIVATE_VAR' ; then
BOOTCONFIG='bootconfig'
INITRD='initrd ($private)/bootconfig.data'
else

This change adds a `grub-features` property to the supported configuration
that can be set in a variant's `Cargo.toml`.  This property is meant to gate
certain features of grub and it's config to variants that support it.  Since
grub isn't updated during OS update, it's important to ensure that variants
which are known to support a certain grub feature receive the proper grub
config.

Currently, the only supported parameter is `set-private-var`.  This
parameter means the variant has a grub config that understands how to search
for and find the BOTTLEROCKET_PRIVATE partition, and set the corresponding
`$private` variable.
This change conditionally adds the `bootconfig` parameter to the kernel
command line if `set-private-var` is added to a variants
`grub-features`.  The `bootconfig` parameter signals the kernel to look
for the Boot Configuration at the end of the specified initrd.  This
change also adds the corresponding initrd entry to the grub menuentry
which points at the bootconfig file on the `BOTTLEROCKET-PRIVATE`
partition.  Bottlerocket technically doesn't use an initrd, so this
initrd will only contain Boot Configuration data.  If the file doesn't
contain Boot Config data, the kernel logs a message but does not fail to
boot.

An empty `bootconfig.data` file is created and added to the
`BOTTLEROCKET_PRIVATE` partition to keep grub from pausing and printing
an error that the file doesn't exist.
This change organizes the kernel and init command line parameters and
moves the init parameters to the section after the "--" where they
belong.
This change adds the `set-private-var` parameter to new `grub-features`
section of the variant config for all metal variants.
@zmrow
Copy link
Contributor Author

zmrow commented Apr 7, 2022

^Addresses @bcressey 's comments

@zmrow zmrow requested a review from bcressey April 7, 2022 16:03
@zmrow zmrow merged commit 758a708 into bottlerocket-os:develop Apr 11, 2022
@zmrow zmrow deleted the grub-search-bootconfig branch April 11, 2022 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants