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

make configurable boot-assessment checks #2018

Merged
merged 4 commits into from
Mar 22, 2024

Conversation

davidcassany
Copy link
Contributor

@davidcassany davidcassany requested a review from a team as a code owner March 20, 2024 08:30
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 69.56522% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 72.36%. Comparing base (c0f39f2) to head (3fc254c).
Report is 1 commits behind head on main.

Files Patch % Lines
pkg/action/install.go 62.50% 2 Missing and 1 partial ⚠️
pkg/action/reset.go 25.00% 2 Missing and 1 partial ⚠️
pkg/features/features.go 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2018      +/-   ##
==========================================
+ Coverage   72.34%   72.36%   +0.01%     
==========================================
  Files          76       76              
  Lines        8992     9009      +17     
==========================================
+ Hits         6505     6519      +14     
- Misses       1944     1946       +2     
- Partials      543      544       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

frelon
frelon previously approved these changes Mar 20, 2024
Copy link
Contributor

@frelon frelon left a comment

Choose a reason for hiding this comment

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

Nice improvement to the boot-assessment!

Copy link
Contributor Author

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

If no checkers are provided boot-assessment behaves as it used to do, with the only real difference that the boot-assessment is verified as passed in a slightly later phase than it used to do (boot yip's stage).

pkg/config/config.go Outdated Show resolved Hide resolved

StartLimitAction=reboot
StartLimitIntervalSec=300
StartLimitBurst=5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there are more than 5 failures within a time frame of 5min this will trigger a reboot.

#!/bin/bash

declare checkResultsPath="/run/elemental/boot-assessment"
declare checkersPath="/usr/libexec/elemental-checker"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any script/executable included in this folder will be executed with check argument. If it returns no error marks the script as passed (so it won't be executed again in any eventual retry), if it returns an error raises the error flag and continues with the rest of the checks. In case any of the executed scripts fails this script runs an exit 1 which causes the service to fail and restart again after 30seconds.

@davidcassany davidcassany marked this pull request as draft March 20, 2024 08:42
@davidcassany davidcassany force-pushed the boot_assessment branch 5 times, most recently from 9797424 to 2128723 Compare March 21, 2024 15:04
# sourced in /grubcustom/custom.cfg file, which is only meant to source additional files.
# So far there is no regexp module in most grub2 efi signed images, hence it is not possible to
# to iterate over the configuration files in /grubcustom folder to load them all. Instead the
# pattern is to manually append additional sourced files in /grubcustom/custom.cfg.
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 could not manage to find a proper way of doing it. I did not want to include this in grub.cfg because this is coupled with the boot-assessment feature and grub is provided by another independent feature. I like the idea of keeping the option of installing without any kind of boot-assessment logic.

My desired setup was defining a /grubcustom folder for which the main grub.cfg would source all the existing configuration files, this way we could easily include additional custom changes besides the bootassessment.cfg. However I could not find a way to iterate over files found in a path, this could be possible by using the regexp grub2 module, but this does not come included signed efi images by default.

@davidcassany davidcassany requested a review from frelon March 21, 2024 15:13
@davidcassany davidcassany marked this pull request as ready for review March 21, 2024 15:14
@davidcassany
Copy link
Contributor Author

The fallback test new logic tests the following: upgrades to a broken system:

  1. Upgrades to a broken system, the new OS can't boot which causes a reboot to passive
  2. Rebooting to passive succeeds, then from there it upgrades to a system including a checker that always fails. So after actually rebooting to active will reboot again after 5 failed attempts to pass the checker.
  3. The reboot sequence will then attempt to boot to the broken system, which will fail again. Then the boot-assessment logic in grub will select the next system, which the snapshot installed during installation.
  4. The former installed image will successfully boot and detect it booted into passive.

@davidcassany davidcassany force-pushed the boot_assessment branch 3 times, most recently from 3fc254c to bdc94a5 Compare March 22, 2024 08:08
Copy link
Contributor

@frelon frelon left a comment

Choose a reason for hiding this comment

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

Awesome, great job!

Signed-off-by: David Cassany <dcassany@suse.com>
Signed-off-by: David Cassany <dcassany@suse.com>
Signed-off-by: David Cassany <dcassany@suse.com>
This commit refactors the boot-assessment logic to run checks
with a customizable service and to store grub variables in
already existing files in EFI partition and OEM.

Also the EFI partition is made accessible in after-*-chroot
hooks. This makes easier to write and manage files in EFI
partition if needed (e.g. grub_oem_env variable file), without
having to relay on mounts or remounts.

Signed-off-by: David Cassany <dcassany@suse.com>
@davidcassany davidcassany enabled auto-merge (squash) March 22, 2024 13:50
@davidcassany davidcassany merged commit 80bd8d8 into rancher:main Mar 22, 2024
17 checks passed
@davidcassany davidcassany deleted the boot_assessment branch March 22, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants