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.

Sign up for GitHub

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

Jump to bottom

Automated reconciliation of boot settings #2375

Merged
merged 6 commits into from
Sep 23, 2022
Merged

Automated reconciliation of boot settings #2375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abf0c0c
models: add reboot-to-reconcile boot setting
markusboehme Aug 29, 2022
8de8bd4
migrations: add boot.reboot-to-reconcile setting
markusboehme Aug 31, 2022
16cef4b
prairiedog: extract function to read /proc/bootconfig
markusboehme Jul 28, 2022
955aa97
prairiedog: extract a helper to create BootSettings for unit tests
markusboehme Sep 9, 2022
bbc3324
prairiedog: add reboot-if-required command
markusboehme Aug 12, 2022
d07baac
os, release: reboot host if needed after reaching configured.target
markusboehme Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,16 +716,20 @@ Here are the metrics settings:

*Please note that boot settings currently only exist for the bare metal variants and \*-k8s-1.23 variants. Boot settings will be added to any future variant introduced after Bottlerocket v1.8.0.*

Specifying either of the following settings will generate a kernel boot config file to be loaded on subsequent boots:
Specifying any of the following settings will generate a kernel boot config file to be loaded on subsequent boots:

* `settings.boot.kernel-parameters`: This allows additional kernel parameters to be specified on the kernel command line during boot.
* `settings.boot.init-parameters`: This allows additional init parameters to be specified on the kernel command line during boot.
* `settings.boot.reboot-to-reconcile`: If set to `true`, Bottlerocket will automatically reboot again during boot if either the `settings.boot.kernel-parameters` or `settings.boot.init-parameters` were changed via user data or a bootstrap container so that these changes may take effect.

You can learn more about kernel boot configuration [here](https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html).

Example user data for specifying boot settings:

```toml
[settings.boot]
reboot-to-reconcile = true

[settings.boot.kernel-parameters]
"console" = [
"tty0",
Expand Down
1 change: 1 addition & 0 deletions Release.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,5 @@ version = "1.10.0"
"(1.9.2, 1.10.0)" = [
"migrate_v1.10.0_dns-settings.lz4",
"migrate_v1.10.0_dns-settings-metadata.lz4",
"migrate_v1.10.0_reboot-to-reconcile-setting.lz4",
]
4 changes: 3 additions & 1 deletion packages/os/os.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Source116: load-kernel-modules.service.in
Source117: cfsignal.service
markusboehme marked this conversation as resolved.
Show resolved Hide resolved
Source118: generate-network-config.service
Source119: prepare-primary-interface.service
Source120: reboot-if-required.service

# 2xx sources: tmpfilesd configs
Source200: migration-tmpfiles.conf
Expand Down Expand Up @@ -402,7 +403,7 @@ install -d %{buildroot}%{_cross_unitdir}
install -p -m 0644 \
%{S:100} %{S:101} %{S:102} %{S:103} %{S:105} \
%{S:106} %{S:107} %{S:110} %{S:111} %{S:112} \
%{S:113} %{S:114} %{S:118} %{S:119} \
%{S:113} %{S:114} %{S:118} %{S:119} %{S:120} \
%{buildroot}%{_cross_unitdir}

%if %{with nvidia_flavor}
Expand Down Expand Up @@ -563,6 +564,7 @@ install -p -m 0644 %{S:300} %{buildroot}%{_cross_udevrulesdir}/80-ephemeral-stor

%files -n %{_cross_os}prairiedog
%{_cross_bindir}/prairiedog
%{_cross_unitdir}/reboot-if-required.service

%files -n %{_cross_os}certdog
%{_cross_bindir}/certdog
Expand Down
11 changes: 11 additions & 0 deletions packages/os/reboot-if-required.service
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[Unit]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still want this unit to be a oneshot and configured with RemainAfterExit, right? According to the docs, since the unit has ExecStart it won't be considered oneshot by default, and the behavior of a simple service doesn't quite fit for reboot-if-required:

the service manager will consider the unit started immediately after the main service process has been forked off.
...
the service manager will immediately proceed starting follow-up units, right after creating the main service process, and before executing the service's binary. Note that this means systemctl start command lines for simple services will report success even if the service's binary cannot be invoked successfully

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good catch, thanks! Previously, there was a brief window of time in which the boot would proceed, which practically speaking probably wouldn't be problematic, but still is super undesirable. I set Type=oneshot explicitly now, and the debug logs confirm the mechanism is working as intended:

reboot-if-required.service: starting held back, waiting for: configured.target
activate-multi-user.service: starting held back, waiting for: reboot-if-required.service
reboot-if-required.service: About to execute /usr/bin/prairiedog reboot-if-required
reboot-if-required.service: Changed dead -> start
[   13.305648] prairiedog[1163]: 19:35:00 [INFO] Boot settings changed and require a reboot to take effect. Initiating reboot...
Unit reboot.target has alias ctrl-alt-del.target.
reboot.target: Trying to enqueue job reboot.target/start/replace-irreversibly
shutdown.target: Installed new job shutdown.target/start as 472
reboot-if-required.service: Job 425 reboot-if-required.service/start finished, result=canceled
reboot-if-required.service: Installed new job reboot-if-required.service/stop as 510
activate-multi-user.service: Job 424 activate-multi-user.service/start finished, result=canceled
activate-multi-user.service: Installed new job activate-multi-user.service/stop as 452
reboot.target: Installed new job reboot.target/start as 426

Description=Reboot system if needed for configuration changes to take effect
# Block manual interactions for now to prevent unplanned reboots.
RefuseManualStart=true
RefuseManualStop=true
After=configured.target

[Service]
Type=oneshot
ExecStart=/usr/bin/prairiedog reboot-if-required
StandardError=journal+console
4 changes: 2 additions & 2 deletions packages/release/activate-multi-user.service
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[Unit]
Description=Isolates multi-user.target
After=configured.target
Requires=configured.target
After=configured.target reboot-if-required.service
Requires=configured.target reboot-if-required.service

[Service]
Type=oneshot
Expand Down
7 changes: 7 additions & 0 deletions sources/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions sources/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ members = [
# "api/migration/migrations/vX.Y.Z/..."
"api/migration/migrations/v1.10.0/dns-settings",
"api/migration/migrations/v1.10.0/dns-settings-metadata",
"api/migration/migrations/v1.10.0/reboot-to-reconcile-setting",

"bottlerocket-release",

Expand Down
12 changes: 12 additions & 0 deletions sources/api/migration/migrations/v1.10.0/reboot-to-reconcile-setting/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "reboot-to-reconcile-setting"
version = "0.1.0"
edition = "2018"
authors = ["Markus Boehme <[email protected]>"]
license = "Apache-2.0 OR MIT"
publish = false
# Don't rebuild crate just because of changes to README.
exclude = ["README.md"]

[dependencies]
migration-helpers = { path = "../../../migration-helpers", version = "0.1.0"}
21 changes: 21 additions & 0 deletions sources/api/migration/migrations/v1.10.0/reboot-to-reconcile-setting/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![deny(rust_2018_idioms)]

use migration_helpers::common_migrations::AddSettingsMigration;
use migration_helpers::{migrate, Result};
use std::process;

/// We added a new setting for letting the host reboot if boot settings changed,
/// `settings.boot.reboot-to-reconcile`
fn run() -> Result<()> {
migrate(AddSettingsMigration(&["settings.boot.reboot-to-reconcile"]))
}

// Returning a Result from main makes it print a Debug representation of the error, but with Snafu
// we have nice Display representations of the error, so we wrap "main" (run) and print any error.
// https://github.com/shepmaster/snafu/issues/110
fn main() {
if let Err(e) = run() {
eprintln!("{}", e);
process::exit(1);
}
}
Loading