Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
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

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
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,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
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
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
iddqd.workspace = true
id-map.workspace = true
internal-dns-resolver.workspace = true
Expand Down
58 changes: 56 additions & 2 deletions nexus/mgs-updates/src/common_sp_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ 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
Expand Down Expand Up @@ -319,12 +319,66 @@ pub enum PrecheckError {
WrongInactiveVersion { expected: ExpectedVersion, found: FoundVersion },
}

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

#[error("transient error: {message:?}")]
TransientError { message: String },

#[error("fatal error: {error:?}")]
FatalError { error: String },
}

impl PostUpdateError {
pub fn is_fatal(&self) -> bool {
match self {
PostUpdateError::GatewayClientError(error) => {
!matches!(error, gateway_client::Error::CommunicationError(_))
}
PostUpdateError::TransientError { .. } => false,
PostUpdateError::FatalError { .. } => true,
}
}
}

#[derive(Debug, Clone)]
pub enum FoundVersion {
MissingVersion,
Version(String),
}

impl FoundVersion {
pub fn matches(
self,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - this should take &self. We can .clone() ourselves in the (rare) error case; better than forcing callers to always clone to call us.

expected: &ExpectedVersion,
) -> Result<(), PrecheckError> {
match (expected, &self) {
// expected garbage, found garbage
(ExpectedVersion::NoValidVersion, FoundVersion::MissingVersion) => {
()
}
// expected a specific version and found it
(
ExpectedVersion::Version(artifact_version),
FoundVersion::Version(found_version),
) if artifact_version.to_string() == *found_version => (),
// anything else is a mismatch
(ExpectedVersion::NoValidVersion, FoundVersion::Version(_))
| (ExpectedVersion::Version(_), FoundVersion::MissingVersion)
| (ExpectedVersion::Version(_), FoundVersion::Version(_)) => {
return Err(PrecheckError::WrongInactiveVersion {
expected: expected.clone(),
found: self,
});
}
};

Ok(())
}
}

pub(crate) fn error_means_caboose_is_invalid(
error: &GatewayClientError,
) -> bool {
Expand Down
Loading
Loading