diff --git a/update-engine/src/buffer.rs b/update-engine/src/buffer.rs index 2426814444..a9e04c1d12 100644 --- a/update-engine/src/buffer.rs +++ b/update-engine/src/buffer.rs @@ -7,6 +7,7 @@ use std::{ collections::{HashMap, VecDeque}, fmt, + sync::Arc, time::Duration, }; @@ -554,10 +555,14 @@ impl EventStore { info: CompletionInfo, root_event_index: RootEventIndex, ) { + let info = Arc::new(info); if let Some(value) = self.map.get_mut(&root_key) { // Completion status only applies to the root key. Nodes reachable // from this node are still marked as complete, but without status. - value.mark_completed(Some(info), root_event_index); + value.mark_completed( + CompletionReason::StepCompleted(info.clone()), + root_event_index, + ); } // Mark anything reachable from this node as completed. @@ -567,7 +572,13 @@ impl EventStore { if let EventTreeNode::Step(key) = key { if key != root_key { if let Some(value) = self.map.get_mut(&key) { - value.mark_completed(None, root_event_index); + value.mark_completed( + CompletionReason::ParentCompleted { + parent_step: root_key, + parent_info: info.clone(), + }, + root_event_index, + ); } } } @@ -580,9 +591,13 @@ impl EventStore { info: CompletionInfo, root_event_index: RootEventIndex, ) { + let info = Arc::new(info); if let Some(value) = self.map.get_mut(&root_key) { // Completion status only applies to the root key. - value.mark_completed(Some(info), root_event_index); + value.mark_completed( + CompletionReason::StepCompleted(info.clone()), + root_event_index, + ); } let mut dfs = DfsPostOrder::new( @@ -593,7 +608,27 @@ impl EventStore { if let EventTreeNode::Step(key) = key { if key != root_key { if let Some(value) = self.map.get_mut(&key) { - value.mark_completed(None, root_event_index); + // There's two kinds of nodes reachable from + // EventTreeNode::Root that could be marked as + // completed: subsequent steps within the same + // execution, and steps in child executions. + if key.execution_id == root_key.execution_id { + value.mark_completed( + CompletionReason::SubsequentStarted { + later_step: root_key, + root_total_elapsed: info.root_total_elapsed, + }, + root_event_index, + ); + } else { + value.mark_completed( + CompletionReason::ParentCompleted { + parent_step: root_key, + parent_info: info.clone(), + }, + root_event_index, + ); + } } } } @@ -606,7 +641,8 @@ impl EventStore { info: FailureInfo, root_event_index: RootEventIndex, ) { - self.mark_step_failed_impl(root_key, root_event_index, |value, kind| { + let info = Arc::new(info); + self.mark_step_failed_impl(root_key, |value, kind| { match kind { MarkStepFailedImplKind::Root => { value.mark_failed( @@ -616,11 +652,14 @@ impl EventStore { } MarkStepFailedImplKind::Descendant => { value.mark_failed( - FailureReason::ParentFailed { parent_step: root_key }, + FailureReason::ParentFailed { + parent_step: root_key, + parent_info: info.clone(), + }, root_event_index, ); } - MarkStepFailedImplKind::Future => { + MarkStepFailedImplKind::Subsequent => { value.mark_will_not_be_run( WillNotBeRunReason::PreviousStepFailed { step: root_key, @@ -628,6 +667,15 @@ impl EventStore { root_event_index, ); } + MarkStepFailedImplKind::PreviousCompleted => { + value.mark_completed( + CompletionReason::SubsequentStarted { + later_step: root_key, + root_total_elapsed: info.root_total_elapsed, + }, + root_event_index, + ); + } }; }) } @@ -638,42 +686,48 @@ impl EventStore { info: AbortInfo, root_event_index: RootEventIndex, ) { - self.mark_step_failed_impl( - root_key, - root_event_index, - |value, kind| { - match kind { - MarkStepFailedImplKind::Root => { - value.mark_aborted( - AbortReason::StepAborted(info.clone()), - root_event_index, - ); - } - MarkStepFailedImplKind::Descendant => { - value.mark_aborted( - AbortReason::ParentAborted { - parent_step: root_key, - }, - root_event_index, - ); - } - MarkStepFailedImplKind::Future => { - value.mark_will_not_be_run( - WillNotBeRunReason::PreviousStepAborted { - step: root_key, - }, - root_event_index, - ); - } - }; - }, - ); + let info = Arc::new(info); + self.mark_step_failed_impl(root_key, |value, kind| { + match kind { + MarkStepFailedImplKind::Root => { + value.mark_aborted( + AbortReason::StepAborted(info.clone()), + root_event_index, + ); + } + MarkStepFailedImplKind::Descendant => { + value.mark_aborted( + AbortReason::ParentAborted { + parent_step: root_key, + parent_info: info.clone(), + }, + root_event_index, + ); + } + MarkStepFailedImplKind::Subsequent => { + value.mark_will_not_be_run( + WillNotBeRunReason::PreviousStepAborted { + step: root_key, + }, + root_event_index, + ); + } + MarkStepFailedImplKind::PreviousCompleted => { + value.mark_completed( + CompletionReason::SubsequentStarted { + later_step: root_key, + root_total_elapsed: info.root_total_elapsed, + }, + root_event_index, + ); + } + }; + }); } fn mark_step_failed_impl( &mut self, root_key: StepKey, - root_event_index: RootEventIndex, mut cb: impl FnMut(&mut EventBufferStepData, MarkStepFailedImplKind), ) { if let Some(value) = self.map.get_mut(&root_key) { @@ -686,7 +740,7 @@ impl EventStore { for index in 0..root_key.index { let key = StepKey { execution_id: root_key.execution_id, index }; if let Some(value) = self.map.get_mut(&key) { - value.mark_completed(None, root_event_index); + (cb)(value, MarkStepFailedImplKind::PreviousCompleted); } } @@ -713,7 +767,7 @@ impl EventStore { while let Some(key) = dfs.next(&self.event_tree) { if let EventTreeNode::Step(key) = key { if let Some(value) = self.map.get_mut(&key) { - (cb)(value, MarkStepFailedImplKind::Future); + (cb)(value, MarkStepFailedImplKind::Subsequent); } } } @@ -723,7 +777,8 @@ impl EventStore { enum MarkStepFailedImplKind { Root, Descendant, - Future, + Subsequent, + PreviousCompleted, } /// Actions taken by a recursion step. @@ -965,12 +1020,12 @@ impl EventBufferStepData { fn mark_completed( &mut self, - status: Option, + reason: CompletionReason, root_event_index: RootEventIndex, ) { match self.step_status { StepStatus::NotStarted | StepStatus::Running { .. } => { - self.step_status = StepStatus::Completed { info: status }; + self.step_status = StepStatus::Completed { reason }; self.update_root_event_index(root_event_index); } StepStatus::Completed { .. } @@ -1011,7 +1066,7 @@ impl EventBufferStepData { match &mut self.step_status { StepStatus::NotStarted => { match reason { - AbortReason::ParentAborted { parent_step } => { + AbortReason::ParentAborted { parent_step, .. } => { // A parent was aborted and this step hasn't been // started. self.step_status = StepStatus::WillNotBeRun { @@ -1116,10 +1171,8 @@ pub enum StepStatus { /// The step has completed execution. Completed { - /// Completion information. - /// - /// This might be unavailable in some cases. - info: Option, + /// The reason for completion. + reason: CompletionReason, }, /// The step has failed. @@ -1179,6 +1232,43 @@ impl StepStatus { } } +#[derive(Clone, Debug)] +pub enum CompletionReason { + /// This step completed. + StepCompleted(Arc), + /// A later step within the same execution was started and we don't have + /// information regarding this step. + SubsequentStarted { + /// The later step that was started. + later_step: StepKey, + + /// The root total elapsed time at the moment the later step was started. + root_total_elapsed: Duration, + }, + /// A parent step within the same execution completed and we don't have + /// information regarding this step. + ParentCompleted { + /// The parent step that completed. + parent_step: StepKey, + + /// Completion info associated with the parent step. + parent_info: Arc, + }, +} + +impl CompletionReason { + /// Returns the [`CompletionInfo`] for this step, if this is the + /// [`Self::StepCompleted`] variant. + pub fn step_completed_info(&self) -> Option<&Arc> { + match self { + Self::StepCompleted(info) => Some(info), + Self::SubsequentStarted { .. } | Self::ParentCompleted { .. } => { + None + } + } + } +} + #[derive(Clone, Debug)] pub struct CompletionInfo { pub attempt: usize, @@ -1192,17 +1282,21 @@ pub struct CompletionInfo { #[derive(Clone, Debug)] pub enum FailureReason { /// This step failed. - StepFailed(FailureInfo), + StepFailed(Arc), /// A parent step failed. ParentFailed { /// The parent step that failed. parent_step: StepKey, + + /// Failure info associated with the parent step. + parent_info: Arc, }, } impl FailureReason { - /// Returns the [`FailureInfo`] if present. - pub fn info(&self) -> Option<&FailureInfo> { + /// Returns the [`FailureInfo`] for this step, if this is the + /// [`Self::StepFailed`] variant. + pub fn step_failed_info(&self) -> Option<&Arc> { match self { Self::StepFailed(info) => Some(info), Self::ParentFailed { .. } => None, @@ -1224,17 +1318,21 @@ pub struct FailureInfo { #[derive(Clone, Debug)] pub enum AbortReason { /// This step was aborted. - StepAborted(AbortInfo), + StepAborted(Arc), /// A parent step was aborted. ParentAborted { /// The parent step key that was aborted. parent_step: StepKey, + + /// Abort info associated with the parent step. + parent_info: Arc, }, } impl AbortReason { - /// Returns the [`AbortInfo`] if present. - pub fn info(&self) -> Option<&AbortInfo> { + /// Returns the [`AbortInfo`] for this step, if this is the + /// [`Self::StepAborted`] variant. + pub fn step_aborted_info(&self) -> Option<&Arc> { match self { Self::StepAborted(info) => Some(info), Self::ParentAborted { .. } => None, @@ -1311,14 +1409,15 @@ impl ExecutionSummary { StepStatus::Running { .. } => { execution_status = ExecutionStatus::Running { step_key }; } - StepStatus::Completed { info } => { - let (root_total_elapsed, leaf_total_elapsed) = match info { - Some(info) => ( - Some(info.root_total_elapsed), - Some(info.leaf_total_elapsed), - ), - None => (None, None), - }; + StepStatus::Completed { reason } => { + let (root_total_elapsed, leaf_total_elapsed) = + match reason.step_completed_info() { + Some(info) => ( + Some(info.root_total_elapsed), + Some(info.leaf_total_elapsed), + ), + None => (None, None), + }; let terminal_status = ExecutionTerminalInfo { kind: TerminalKind::Completed, @@ -1331,7 +1430,7 @@ impl ExecutionSummary { } StepStatus::Failed { reason } => { let (root_total_elapsed, leaf_total_elapsed) = - match reason.info() { + match reason.step_failed_info() { Some(info) => ( Some(info.root_total_elapsed), Some(info.leaf_total_elapsed), @@ -1350,7 +1449,7 @@ impl ExecutionSummary { } StepStatus::Aborted { reason, .. } => { let (root_total_elapsed, leaf_total_elapsed) = - match reason.info() { + match reason.step_aborted_info() { Some(info) => ( Some(info.root_total_elapsed), Some(info.leaf_total_elapsed), diff --git a/wicket/src/ui/panes/update.rs b/wicket/src/ui/panes/update.rs index 2819b3ddda..d76c6c3b49 100644 --- a/wicket/src/ui/panes/update.rs +++ b/wicket/src/ui/panes/update.rs @@ -29,8 +29,8 @@ use ratatui::widgets::{ use slog::{info, o, Logger}; use tui_tree_widget::{Tree, TreeItem, TreeState}; use update_engine::{ - AbortReason, ExecutionStatus, FailureReason, StepKey, TerminalKind, - WillNotBeRunReason, + AbortReason, CompletionReason, ExecutionStatus, FailureReason, StepKey, + TerminalKind, WillNotBeRunReason, }; use wicket_common::update_events::{ EventBuffer, EventReport, ProgressEvent, StepOutcome, StepStatus, @@ -282,7 +282,9 @@ impl UpdatePane { // TODO: show previous attempts } - StepStatus::Completed { info: Some(info) } => { + StepStatus::Completed { + reason: CompletionReason::StepCompleted(info), + } => { let mut spans = vec![Span::styled("Status: ", style::selected())]; @@ -333,9 +335,9 @@ impl UpdatePane { push_text_lines(&message, prefix, &mut body.lines); } } - StepStatus::Completed { info: None } => { - // No information is available, so all we can do is say that - // this step is completed. + StepStatus::Completed { reason: _ } => { + // No information about this step is available, so all we can do + // is say that this step is completed. body.lines.push(Line::from(vec![ Span::styled("Status: ", style::selected()), Span::styled("Completed", style::successful_update_bold()), @@ -383,7 +385,7 @@ impl UpdatePane { } } StepStatus::Failed { - reason: FailureReason::ParentFailed { parent_step }, + reason: FailureReason::ParentFailed { parent_step, .. }, } => { let mut spans = vec![ Span::styled("Status: ", style::selected()), @@ -442,7 +444,7 @@ impl UpdatePane { } } StepStatus::Aborted { - reason: AbortReason::ParentAborted { parent_step }, + reason: AbortReason::ParentAborted { parent_step, .. }, last_progress, } => { let mut spans = vec![ @@ -2017,24 +2019,25 @@ impl ComponentUpdateListState { } style::selected() } - StepStatus::Completed { info } => { - let (character, style) = if let Some(info) = info { - match info.outcome { - StepOutcome::Success { .. } => { - ('✔', style::successful_update()) - } - StepOutcome::Warning { .. } => { - ('⚠', style::warning_update()) - } - StepOutcome::Skipped { .. } => { - ('*', style::successful_update()) + StepStatus::Completed { reason } => { + let (character, style) = + if let Some(info) = reason.step_completed_info() { + match info.outcome { + StepOutcome::Success { .. } => { + ('✔', style::successful_update()) + } + StepOutcome::Warning { .. } => { + ('⚠', style::warning_update()) + } + StepOutcome::Skipped { .. } => { + ('*', style::successful_update()) + } } - } - } else { - // No information available for this step -- just mark - // it successful. - ('✔', style::successful_update()) - }; + } else { + // No information available for this step -- just mark + // it successful. + ('✔', style::successful_update()) + }; item_spans.push(Span::styled( format!("{:>5} ", character), style,