Skip to content

[reconfigurator] Pre-checks and post_update actions for RoT bootloader update #8325

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

Merged
merged 24 commits into from
Jul 9, 2025
Merged

[reconfigurator] Pre-checks and post_update actions for RoT bootloader update #8325

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2baa799
[reconfigurator] Pre-checks and post_update actions for RoT bootloade…
karencfv Jun 12, 2025
8072c7d
building blocks in case I can get PostUpdateError to work
karencfv Jun 12, 2025
2de9298
Introduce PostUpdateError
karencfv Jun 13, 2025
f578e35
implement WaitingForOngoingUpdate variant
karencfv Jun 16, 2025
fa1ba5e
Merge branch 'main' into rot-bootloader-pre-and-post-actions
karencfv Jun 16, 2025
3ced091
Implement a waiting time for an ongoing RoT bootloader update
karencfv Jun 17, 2025
c15ca08
fmt
karencfv Jun 17, 2025
4fb917a
Improved error handling
karencfv Jun 17, 2025
dff0254
Clean up
karencfv Jun 17, 2025
c6be847
Merge branch 'main' into rot-bootloader-pre-and-post-actions
karencfv Jun 18, 2025
25b0f40
add some tests
karencfv Jun 18, 2025
e06418e
address comments
karencfv Jun 26, 2025
b93303f
clean up
karencfv Jun 26, 2025
0fbdcf1
change conditions for determining whether we wait for an ongoing update
karencfv Jun 27, 2025
609122c
Update wait for ongoing update logic
karencfv Jun 27, 2025
0d40a73
Remove ticker
karencfv Jun 27, 2025
92e97e3
clean up
karencfv Jun 27, 2025
69a8a4f
Missed one
karencfv Jun 27, 2025
2fd4e71
merge main
karencfv Jul 8, 2025
99bd5da
fix after merge
karencfv Jul 8, 2025
dc9e787
address comments
karencfv Jul 8, 2025
0e76cce
simplify checks
karencfv Jul 8, 2025
486b54b
address comments
karencfv Jul 8, 2025
fec4545
address comments
karencfv Jul 9, 2025
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
1 change: 1 addition & 0 deletions 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 clients/gateway-client/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ serde.workspace = true
serde_json.workspace = true
schemars.workspace = true
slog.workspace = true
thiserror.workspace = true
uuid.workspace = true
omicron-workspace-hack.workspace = true
2 changes: 1 addition & 1 deletion clients/gateway-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ progenitor::generate_api!(
HostPhase2RecoveryImageId = { derives = [PartialEq, Eq, PartialOrd, Ord] },
ImageVersion = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RotImageDetails = { derives = [PartialEq, Eq, PartialOrd, Ord] },
RotImageError = { derives = [ PartialEq, Eq, PartialOrd, Ord] },
RotImageError = { derives = [ thiserror::Error, PartialEq, Eq, PartialOrd, Ord] },
RotState = { derives = [PartialEq, Eq, PartialOrd, Ord] },
SpComponentCaboose = { derives = [PartialEq, Eq] },
SpIdentifier = { derives = [Copy, PartialEq, Hash, Eq] },
Expand Down
1 change: 1 addition & 0 deletions nexus/mgs-updates/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ chrono.workspace = true
futures.workspace = true
gateway-client.workspace = true
gateway-types.workspace = true
gateway-messages.workspace = true
id-map.workspace = true
internal-dns-resolver.workspace = true
internal-dns-types.workspace = true
Expand Down
16 changes: 15 additions & 1 deletion nexus/mgs-updates/src/common_sp_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use super::MgsClients;
use super::UpdateProgress;
use futures::future::BoxFuture;
use gateway_client::types::RotImageError;
use gateway_client::types::SpType;
use gateway_client::types::SpUpdateStatus;
use gateway_types::rot::RotSlot;
Expand Down Expand Up @@ -267,14 +268,15 @@ pub trait SpComponentUpdateHelper {
log: &'a slog::Logger,
mgs_clients: &'a mut MgsClients,
update: &'a PendingMgsUpdate,
) -> BoxFuture<'a, Result<(), GatewayClientError>>;
) -> BoxFuture<'a, Result<(), PostUpdateError>>;
}

/// Describes the live state of the component before the update begins
#[derive(Debug)]
pub enum PrecheckStatus {
UpdateComplete,
ReadyForUpdate,
WaitingForOngoingRotBootloaderUpdate,
}

#[derive(Debug, Error)]
Expand Down Expand Up @@ -319,6 +321,18 @@ pub enum PrecheckError {
WrongInactiveVersion { expected: ExpectedVersion, found: FoundVersion },
}

#[derive(Debug, thiserror::Error)]
pub enum PostUpdateError {
#[error("communicating with MGS")]
GatewayClientError(#[from] GatewayClientError),

#[error("communicating with RoT: {message:?}")]
RotCommunicationFailed { message: String },

#[error("invalid RoT bootloader image: {error:?}")]
RotBootloaderImageError { error: RotImageError },
Copy link
Collaborator

@davepacheco davepacheco Jun 25, 2025

Choose a reason for hiding this comment

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

I'm curious for @jgallagher's take on this but it would seem nice to me if the generic parts of this package (this file, the driver, and apply_update) didn't know so much about specific devices. This would preclude this type from including more specific typed errors like RotImageError, but I believe the only thing consumers of this error type care about is that the error is fatal to the update attempt.

So I'd consider renaming RotCommunicationFailed to TransientError and RotBootloaderImageError to FatalError. Both would just contain message: String.

Copy link
Contributor Author

@karencfv karencfv Jun 26, 2025

Choose a reason for hiding this comment

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

Yeah, that makes sense. Specifically, RotBootloaderImageError doesn't really mean anything without context. I'll make these more generic

Copy link
Contributor Author

@karencfv karencfv Jun 26, 2025

Choose a reason for hiding this comment

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

Done in e06418e

}

#[derive(Debug)]
pub enum FoundVersion {
MissingVersion,
Expand Down
Loading
Loading