Skip to content

Commit

Permalink
[update-engine] move terminal ExecutionStatus into its own type (#4364)
Browse files Browse the repository at this point in the history 
I found myself repeating the completed/failed/aborted branches very
often, and making this its own type simplifies that quite nicely.

Also track the total elapsed time, and show it in the wicket UI.

Note the "after 2.93ms" in this screenshot:

![image](https://github.com/oxidecomputer/omicron/assets/180618/dd202abc-41f9-4722-a4ac-1ac99e0f9c8a)

Depends on #4361.
  • Loading branch information
@sunshowers
sunshowers authored Oct 26, 2023
1 parent 6017d62 commit c1a7d09
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 80 deletions.
135 changes: 104 additions & 31 deletions update-engine/src/buffer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1307,14 +1307,61 @@ impl ExecutionSummary {
StepStatus::Running { .. } => {
execution_status = ExecutionStatus::Running { step_key };
}
StepStatus::Completed { .. } => {
execution_status = ExecutionStatus::Completed { 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),
};

let terminal_status = ExecutionTerminalInfo {
kind: TerminalKind::Completed,
root_total_elapsed,
leaf_total_elapsed,
step_key,
};
execution_status =
ExecutionStatus::Terminal(terminal_status);
}
StepStatus::Failed { .. } => {
execution_status = ExecutionStatus::Failed { step_key };
StepStatus::Failed { reason } => {
let (root_total_elapsed, leaf_total_elapsed) =
match reason.info() {
Some(info) => (
Some(info.root_total_elapsed),
Some(info.leaf_total_elapsed),
),
None => (None, None),
};

let terminal_status = ExecutionTerminalInfo {
kind: TerminalKind::Failed,
root_total_elapsed,
leaf_total_elapsed,
step_key,
};
execution_status =
ExecutionStatus::Terminal(terminal_status);
}
StepStatus::Aborted { .. } => {
execution_status = ExecutionStatus::Aborted { step_key };
StepStatus::Aborted { reason, .. } => {
let (root_total_elapsed, leaf_total_elapsed) =
match reason.info() {
Some(info) => (
Some(info.root_total_elapsed),
Some(info.leaf_total_elapsed),
),
None => (None, None),
};

let terminal_status = ExecutionTerminalInfo {
kind: TerminalKind::Aborted,
root_total_elapsed,
leaf_total_elapsed,
step_key,
};
execution_status =
ExecutionStatus::Terminal(terminal_status);
}
StepStatus::WillNotBeRun { .. } => {
// Ignore steps that will not be run -- a prior step failed.
Expand Down Expand Up @@ -1352,7 +1399,7 @@ impl StepSortKey {
/// Status about a single execution ID.
///
/// Part of [`ExecutionSummary`].
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum ExecutionStatus {
/// This execution has not been started yet.
NotStarted,
Expand All @@ -1365,27 +1412,50 @@ pub enum ExecutionStatus {
step_key: StepKey,
},

/// This execution completed running.
Completed {
/// The last step that completed.
step_key: StepKey,
},
/// Execution has finished.
Terminal(ExecutionTerminalInfo),
}

/// This execution failed.
Failed {
/// The step key that failed.
///
/// Use [`EventBuffer::get`] to get more information about this step.
step_key: StepKey,
},
/// Terminal status about a single execution ID.
///
/// Part of [`ExecutionStatus`].
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct ExecutionTerminalInfo {
/// The way in which this execution reached a terminal state.
pub kind: TerminalKind,

/// Total elapsed time (root) for this execution.
///
/// The total elapsed time may not be available if execution was interrupted
/// and we inferred that it was terminated.
pub root_total_elapsed: Option<Duration>,

/// Total elapsed time (leaf) for this execution.
///
/// The total elapsed time may not be available if execution was interrupted
/// and we inferred that it was terminated.
pub leaf_total_elapsed: Option<Duration>,

/// The step key that was running when this execution was terminated.
///
/// * For completed executions, this is the last step that completed.
/// * For failed or aborted executions, this is the step that failed.
/// * For aborted executions, this is the step that was running when the
/// abort happened.
pub step_key: StepKey,
}

/// The way in which an execution was terminated.
///
/// Part of [`ExecutionStatus`].
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum TerminalKind {
/// This execution completed running.
Completed,
/// This execution failed.
Failed,
/// This execution was aborted.
Aborted {
/// The step that was running when the abort happened.
///
/// Use [`EventBuffer::get`] to get more information about this step.
step_key: StepKey,
},
Aborted,
}

/// Keys for the event tree.
Expand Down Expand Up @@ -1971,8 +2041,9 @@ mod tests {
if is_last_event {
ensure!(
matches!(
summary[&root_execution_id].execution_status,
ExecutionStatus::Completed { .. },
&summary[&root_execution_id].execution_status,
ExecutionStatus::Terminal(info)
if info.kind == TerminalKind::Completed
),
"this is the last event so ExecutionStatus must be completed"
);
Expand All @@ -1987,8 +2058,9 @@ mod tests {
.expect("this is the first nested engine");
ensure!(
matches!(
nested_summary.execution_status,
ExecutionStatus::Failed { .. },
&nested_summary.execution_status,
ExecutionStatus::Terminal(info)
if info.kind == TerminalKind::Failed
),
"for this engine, the ExecutionStatus must be failed"
);
Expand All @@ -1998,8 +2070,9 @@ mod tests {
.expect("this is the second nested engine");
ensure!(
matches!(
nested_summary.execution_status,
ExecutionStatus::Completed { .. },
&nested_summary.execution_status,
ExecutionStatus::Terminal(info)
if info.kind == TerminalKind::Completed
),
"for this engine, the ExecutionStatus must be succeeded"
);
Expand Down
111 changes: 62 additions & 49 deletions wicket/src/ui/panes/update.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ use ratatui::widgets::{
use slog::{info, o, Logger};
use tui_tree_widget::{Tree, TreeItem, TreeState};
use update_engine::{
AbortReason, ExecutionStatus, FailureReason, StepKey, WillNotBeRunReason,
AbortReason, ExecutionStatus, FailureReason, StepKey, TerminalKind,
WillNotBeRunReason,
};
use wicket_common::update_events::{
EventBuffer, EventReport, ProgressEvent, StepOutcome, StepStatus,
Expand Down Expand Up @@ -1009,9 +1010,7 @@ impl UpdatePane {
}

ExecutionStatus::NotStarted
| ExecutionStatus::Completed { .. }
| ExecutionStatus::Failed { .. }
| ExecutionStatus::Aborted { .. } => None,
| ExecutionStatus::Terminal(_) => None,
}
} else {
None
Expand Down Expand Up @@ -1041,9 +1040,7 @@ impl UpdatePane {
associated with it",
);
match summary.execution_status {
ExecutionStatus::Completed { .. }
| ExecutionStatus::Failed { .. }
| ExecutionStatus::Aborted { .. } => {
ExecutionStatus::Terminal(_) => {
// If execution has reached a terminal
// state, we can clear it.
self.popup =
Expand Down Expand Up @@ -1888,7 +1885,7 @@ impl ComponentUpdateListState {
"root execution ID should have a summary associated with it",
);

match summary.execution_status {
match &summary.execution_status {
ExecutionStatus::NotStarted => {
status_text.push(Span::styled(
"Update not started",
Expand All @@ -1913,47 +1910,63 @@ impl ComponentUpdateListState {
));
Some(ComponentUpdateShowHelp::Running)
}
ExecutionStatus::Completed { .. } => {
status_text
.push(Span::styled("Update ", style::plain_text()));
status_text.push(Span::styled(
"completed",
style::successful_update_bold(),
));
Some(ComponentUpdateShowHelp::Completed)
}
ExecutionStatus::Failed { step_key } => {
status_text
.push(Span::styled("Update ", style::plain_text()));
status_text.push(Span::styled(
"failed",
style::failed_update_bold(),
));
status_text.push(Span::styled(
format!(
" at step {}/{}",
step_key.index + 1,
summary.total_steps,
),
style::plain_text(),
));
Some(ComponentUpdateShowHelp::Completed)
}
ExecutionStatus::Aborted { step_key } => {
status_text
.push(Span::styled("Update ", style::plain_text()));
status_text.push(Span::styled(
"aborted",
style::failed_update_bold(),
));
status_text.push(Span::styled(
format!(
" at step {}/{}",
step_key.index + 1,
summary.total_steps,
),
style::plain_text(),
));
ExecutionStatus::Terminal(info) => {
match info.kind {
TerminalKind::Completed => {
status_text.push(Span::styled(
"Update ",
style::plain_text(),
));
status_text.push(Span::styled(
"completed",
style::successful_update_bold(),
));
}
TerminalKind::Failed => {
status_text.push(Span::styled(
"Update ",
style::plain_text(),
));
status_text.push(Span::styled(
"failed",
style::failed_update_bold(),
));
status_text.push(Span::styled(
format!(
" at step {}/{}",
info.step_key.index + 1,
summary.total_steps,
),
style::plain_text(),
));
}
TerminalKind::Aborted => {
status_text.push(Span::styled(
"Update ",
style::plain_text(),
));
status_text.push(Span::styled(
"aborted",
style::failed_update_bold(),
));
status_text.push(Span::styled(
format!(
" at step {}/{}",
info.step_key.index + 1,
summary.total_steps,
),
style::plain_text(),
));
}
}

if let Some(total_elapsed) = info.root_total_elapsed {
status_text.push(Span::styled(
format!(" after {:.2?}", total_elapsed),
style::plain_text(),
));
}

Some(ComponentUpdateShowHelp::Completed)
}
}
Expand Down

0 comments on commit c1a7d09

Please sign in to comment.