From 13a278b122b9daef6169cc433f7b002ebdbf32d7 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 26 May 2023 11:38:31 -0400 Subject: [PATCH 1/2] [wicketd] Skip updating the RoT if it already has the target version --- wicket-common/src/update_events.rs | 5 + wicketd/src/update_tracker.rs | 205 ++++++++++++++++++++--------- 2 files changed, 145 insertions(+), 65 deletions(-) diff --git a/wicket-common/src/update_events.rs b/wicket-common/src/update_events.rs index 924ff4cd066..db4179a42db 100644 --- a/wicket-common/src/update_events.rs +++ b/wicket-common/src/update_events.rs @@ -110,6 +110,11 @@ pub enum UpdateTerminalError { #[source] error: anyhow::Error, }, + #[error("getting RoT caboose failed")] + GetRotCabooseFailed { + #[source] + error: gateway_client::Error, + }, #[error("resetting RoT failed")] RotResetFailed { #[source] diff --git a/wicketd/src/update_tracker.rs b/wicketd/src/update_tracker.rs index f4ded083138..6badc54b17a 100644 --- a/wicketd/src/update_tracker.rs +++ b/wicketd/src/update_tracker.rs @@ -565,78 +565,73 @@ impl UpdateDriver { // To update the RoT, we have to know which slot (A or B) it is // currently executing; we must update the _other_ slot. let rot_registrar = engine.for_component(UpdateComponent::Rot); - let rot_firmware_slot_and_artifact = rot_registrar + let rot_interrogation = + rot_registrar + .new_step( + UpdateStepId::InterrogateRot, + "Checking current RoT version and active slot", + |_cx| async move { + update_cx.interrogate_rot(rot_a, rot_b).await + }, + ) + .register(); + + // Send the update to the RoT. + let inner_cx = + SpComponentUpdateContext::new(update_cx, UpdateComponent::Rot); + rot_registrar .new_step( - UpdateStepId::InterrogateRot, - "Interrogating RoT for currently-active slot", - |_cx| async move { - let rot_active_slot = update_cx - .get_component_active_slot( - SpComponent::ROT.const_as_str(), - ) - .await - .map_err(|error| { - UpdateTerminalError::GetRotActiveSlotFailed { - error, - } - })?; + UpdateStepId::SpComponentUpdate, + "Updating RoT", + move |cx| async move { + let rot_interrogation = + rot_interrogation.into_value(cx.token()).await; + + let rot_has_this_version = rot_interrogation + .active_version_matches_artifact_to_apply(); - // Flip these around: if 0 (A) is active, we want to - // update 1 (B), and vice versa. - let (rot_active_slot_char, slot_to_update, artifact) = - match rot_active_slot { - 0 => ('A', 1, rot_b), - 1 => ('B', 0, rot_a), - _ => return Err( - UpdateTerminalError::GetRotActiveSlotFailed { - error: anyhow!( - "unexpected RoT active slot \ - {rot_active_slot}" - ), - }, + // If this RoT already has this version, skip the rest of + // this step, UNLESS we've been told to skip this version + // check. + if rot_has_this_version && !opts.skip_rot_version_check { + return StepSkipped::new( + (), + format!( + "RoT active slot already at version {}", + rot_interrogation.artifact_to_apply.id.version ), - }; + ) + .into(); + } - StepSuccess::new((slot_to_update, artifact)) - .with_message(format!( - "Currently active slot is {rot_active_slot_char}" - )) + cx.with_nested_engine(|engine| { + inner_cx.register_steps( + engine, + rot_interrogation.slot_to_update, + &rot_interrogation.artifact_to_apply, + ); + Ok(()) + }) + .await?; + + // If we updated despite the RoT already having the version + // we updated to, make this step return a warning with that + // message; otherwise, this is a normal success. + if rot_has_this_version { + StepWarning::new( + (), + format!( + "RoT updated despite already having version {}", + rot_interrogation.artifact_to_apply.id.version + ), + ) .into() + } else { + StepSuccess::new(()).into() + } }, ) - .register() - .into_shared(); - - // Send the update to the RoT. - { - let inner_cx = - SpComponentUpdateContext::new(update_cx, UpdateComponent::Rot); - let rot_firmware_slot_and_artifact = - rot_firmware_slot_and_artifact.clone(); - rot_registrar - .new_step( - UpdateStepId::SpComponentUpdate, - "Updating RoT", - move |cx| async move { - let (firmware_slot, artifact) = - rot_firmware_slot_and_artifact - .into_value(cx.token()) - .await; - cx.with_nested_engine(|engine| { - inner_cx.register_steps( - engine, - firmware_slot, - &artifact, - ); - Ok(()) - }) - .await?; - - StepSuccess::new(()).into() - }, - ) - .register(); - } + .register(); let sp_registrar = engine.for_component(UpdateComponent::Sp); @@ -1155,6 +1150,19 @@ impl UpdateDriver { } } +#[derive(Debug)] +struct RotInterrogation { + slot_to_update: u16, + artifact_to_apply: ArtifactIdData, + active_version: Option, +} + +impl RotInterrogation { + fn active_version_matches_artifact_to_apply(&self) -> bool { + Some(&self.artifact_to_apply.id.version) == self.active_version.as_ref() + } +} + struct UpdateContext { update_id: Uuid, sp: SpIdentifier, @@ -1213,6 +1221,73 @@ impl UpdateContext { }) } + async fn interrogate_rot( + &self, + rot_a: ArtifactIdData, + rot_b: ArtifactIdData, + ) -> Result, UpdateTerminalError> { + let rot_active_slot = self + .get_component_active_slot(SpComponent::ROT.const_as_str()) + .await + .map_err(|error| UpdateTerminalError::GetRotActiveSlotFailed { + error, + })?; + + // Flip these around: if 0 (A) is active, we want to + // update 1 (B), and vice versa. + let (active_slot_name, slot_to_update, artifact_to_apply) = + match rot_active_slot { + 0 => ('A', 1, rot_b), + 1 => ('B', 0, rot_a), + _ => { + return Err(UpdateTerminalError::GetRotActiveSlotFailed { + error: anyhow!( + "unexpected RoT active slot {rot_active_slot}" + ), + }) + } + }; + + // Read the caboose of the currently-active slot. + let caboose = self + .mgs_client + .sp_component_caboose_get( + self.sp.type_, + self.sp.slot, + SpComponent::ROT.const_as_str(), + rot_active_slot, + ) + .await + .map_err(|error| UpdateTerminalError::GetRotCabooseFailed { + error, + })? + .into_inner(); + + let message = format!( + "RoT slot {active_slot_name} version {} (git commit {})", + caboose.version.as_deref().unwrap_or("unknown"), + caboose.git_commit + ); + + let make_result = |active_version| RotInterrogation { + slot_to_update, + artifact_to_apply, + active_version, + }; + + match caboose.version.map(|v| v.parse::()) { + Some(Ok(version)) => StepSuccess::new(make_result(Some(version))) + .with_message(message) + .into(), + Some(Err(err)) => StepWarning::new( + make_result(None), + format!("{message} (failed to parse RoT version: {err})"), + ) + .into(), + None => StepWarning::new(make_result(None), message).into(), + } + } + async fn wait_for_first_installinator_progress( &self, cx: &StepContext, From 8940ce9d75f220bba52634d4c65835d285a7b513 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 26 May 2023 14:12:36 -0400 Subject: [PATCH 2/2] Fix wicketd log config --- smf/wicketd/config.toml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/smf/wicketd/config.toml b/smf/wicketd/config.toml index 703e871ba90..804b6a1e07f 100644 --- a/smf/wicketd/config.toml +++ b/smf/wicketd/config.toml @@ -1,11 +1,9 @@ # -# Example wicketd config file +# wicketd config file # [log] -# Show log messages of this level and more severe -level = "debug" -# Example output to a terminal (with colors) -mode = "stderr-terminal" - - +level = "info" +mode = "file" +path = "/dev/stdout" +if_exists = "append"