Skip to content

Conversation

@bshephar
Copy link
Contributor

@bshephar bshephar commented Sep 6, 2025

This change adds a function that writes the /run/reboot-required file.
This function is called from any deployment operation with the relevant
context about the image that has been staged.

This expands on the functionality requested in: #1574
Related to: #1583

@bootc-bot bootc-bot bot requested a review from cgwalters September 6, 2025 08:40
// Unconditionally create or update /run/reboot-required to signal a reboot is needed.
// This is monitored by kured (Kubernetes Reboot Daemon).
let reboot_message = format!("bootc: Reboot required for image: {}", &spec.image.image);
write_reboot_required(&image.manifest_digest.as_ref(), bootc_action.as_str())?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change from &spec.image.image to the manifest_digest is intentional to conform with the standard established by:

"Rolling back to image: {}",
rollback_image.manifest_digest

Which results in:

❯ journalctl --no-pager | grep -i rolling
Sep 06 16:46:43 fedora .tmpuGbC3K[51776]: Rolling back to image: sha256:465668e4bea0d739e2a0553ca826ce2c34f398293b64b3ad3a440c412fd22c93

This is technically more informative than the image name, particularly when tags are used instead of the digest. So the result of this change in the reboot-required file is:

❯ cat /run/reboot-required
bootc rollback: Reboot required for image: sha256:465668e4bea0d739e2a0553ca826ce2c34f398293b64b3ad3a440c412fd22c93

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any specification for this file? Everything I can find shows tooling just checking for its presence. If it can contain freeform text, why not have multiple lines?

And if we can have multiple lines we can just render it like:

bootc: Reboot required for:
  <current human readable status for a deployment>

right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is a spec for this file tbh. I could only find a bunch of mailing list discussions, but nothing that appeared to establish a standard per se. It seems like this file is often created via touch /run/reboot-required in a package script, rather than applications doing it.

Maybe we're best just keeping the single line.

@bshephar
Copy link
Contributor Author

bshephar commented Sep 6, 2025

Outcome of this change:

❯ sudo target/release/bootc upgrade
[sudo] password for bshephar:
layers already present: 67; layers needed: 1 (712.6 MB)
Fetched layers: 679.62 MiB in 37 seconds (18.54 MiB/s)
  Deploying: done (5 seconds)
Pruned images: 0 (layers: 0, objsize: 95.3 MB)
Queued for next boot: ghcr.io/bshephar/fedora-asahi-bootc:42
  Version: 42.20250904.0
  Digest: sha256:3e65df95b8bafaceb17a9eca6418ef63c3ce19c2c0aa30f0cbcebad1a050fb00
Total new layers: 68    Size: 3.2 GB
Removed layers:   1     Size: 695.2 MB
Added layers:     1     Size: 712.6 MB

reboot-required file:

❯ cat /run/reboot-required
bootc deploy: Reboot required for image: sha256:3e65df95b8bafaceb17a9eca6418ef63c3ce19c2c0aa30f0cbcebad1a050fb00%

And for rollback:

❯ sudo target/release/bootc rollback
notice: Reverting queued rollback state
Next boot: current deployment
❯ cat /run/reboot-required
bootc rollback: Reboot required for image: sha256:465668e4bea0d739e2a0553ca826ce2c34f398293b64b3ad3a440c412fd22c93%

@bshephar
Copy link
Contributor Author

bshephar commented Sep 8, 2025

hmm, I'll have to figure out if this is somehow related to my change:

2025-09-08T04:50:08.2405963Z $ sudo podman run --rm --privileged --pid=host -v /ostree:/ostree -v /boot:/boot localhost/bootc-integration bootc state wipe-ostree
2025-09-08T04:50:08.2407096Z running 3 tests
2025-09-08T04:50:11.6137513Z $ sudo /bin/sh -c "rm -rf /ostree/deploy/*"
2025-09-08T04:50:11.6258369Z $ sudo /bin/sh -c "rm -rf /ostree/"
2025-09-08T04:51:17.3610367Z $ sudo podman run --rm --privileged --pid=host -v /ostree:/ostree -v /boot:/boot localhost/bootc-integration bootc state wipe-ostree
2025-09-08T04:51:17.3611400Z test default behavior ... FAILED
2025-09-08T04:51:19.1094703Z $ sudo /bin/sh -c "rm -rf /ostree/deploy/*"
2025-09-08T04:51:19.1196153Z $ sudo /bin/sh -c "rm -rf /ostree/"
2025-09-08T04:51:22.7715845Z $ sudo podman run --rm --privileged --pid=host -v /ostree:/ostree -v /boot:/boot localhost/bootc-integration bootc state wipe-ostree
2025-09-08T04:51:22.7716975Z test disk space check ... ok
2025-09-08T04:51:23.1276294Z $ sudo /bin/sh -c "rm -rf /ostree/deploy/*"
2025-09-08T04:51:23.1401534Z $ sudo /bin/sh -c "rm -rf /ostree/"
2025-09-08T04:52:28.1369561Z test image pull check ... ok
2025-09-08T04:52:28.1371331Z 
2025-09-08T04:52:28.1371683Z failures:
2025-09-08T04:52:28.1371969Z 
2025-09-08T04:52:28.1372143Z ---- default behavior ----
2025-09-08T04:52:28.1373267Z Timeout Error: Expected "Operation complete, rebooting in 10 seconds. Press Ctrl-C to cancel reboot, or press enter to continue immediately." but got "`\r``\n`
2025-09-08T04:52:28.1375065Z layers already present: 0; layers needed: 70 (2.1Â GB)`\r``\n`
2025-09-08T04:52:28.1375788Z Deploying container image...done (19 seconds)`\r``\n`
2025-09-08T04:52:28.1376417Z Injected: etc/tmpfiles.d/bootc-root-ssh.conf`\r``\n`
2025-09-08T04:52:28.1376885Z Installing bootloader via bootupd`\r``\n`
2025-09-08T04:52:28.1377231Z " (after waiting 60000 ms)
2025-09-08T04:52:28.1377374Z 
2025-09-08T04:52:28.1377378Z 
2025-09-08T04:52:28.1377530Z failures:
2025-09-08T04:52:28.1377797Z     default behavior
2025-09-08T04:52:28.1377935Z 
2025-09-08T04:52:28.1378200Z test result: FAILED. 2 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 139.90s

Leaving it in draft until I have time to look into that

// Unconditionally create or update /run/reboot-required to signal a reboot is needed.
// This is monitored by kured (Kubernetes Reboot Daemon).
let reboot_message = format!("bootc: Reboot required for image: {}", &spec.image.image);
write_reboot_required(&image.manifest_digest.as_ref(), bootc_action.as_str())?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any specification for this file? Everything I can find shows tooling just checking for its presence. If it can contain freeform text, why not have multiple lines?

And if we can have multiple lines we can just render it like:

bootc: Reboot required for:
  <current human readable status for a deployment>

right?

@cgwalters
Copy link
Collaborator

Oops thought my last comment got lost, but it didn't...

@cgwalters
Copy link
Collaborator

hmm, I'll have to figure out if this is somehow related to my change:

It's a preexisting flake, I'll file an issue. I restarted the test and it worked.

This change adds a function that writes the /run/reboot-required file.
This function is called from any deployment operation with the relevant
context about the image that has been staged.

This expands on the functionality requested in: bootc-dev#1574
Related to: bootc-dev#1583

Signed-off-by: Brendan Shephard <[email protected]>
@bshephar bshephar marked this pull request as ready for review September 8, 2025 22:54
@bootc-bot bootc-bot bot requested a review from cgwalters September 8, 2025 22:54
@bshephar bshephar changed the title Add function to write reboot-required file with context [Feature] Add function to write reboot-required Sep 8, 2025
@cgwalters cgwalters merged commit 77c607a into bootc-dev:main Sep 9, 2025
17 of 28 checks passed
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.

2 participants