Skip to content

Commit

Permalink
Add multiline diffs to blueprint display output (oxidecomputer#5785)
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrewjstone
andrewjstone authored May 16, 2024
1 parent 5d5aab5 commit 116590c
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 66 deletions.
42 changes: 27 additions & 15 deletions nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,33 @@ to: blueprint 1ac2d88f-27dd-4506-8585-6b2be832528e


omicron zones generation 2 -> 3:
-----------------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
-----------------------------------------------------------------------------------------------------
* crucible 1e1ed0cc-1adc-410f-943a-d1a3107de619 in service -> expunged fd00:1122:3344:103::27
* crucible 2307bbed-02ba-493b-89e3-46585c74c8fc in service -> expunged fd00:1122:3344:103::28
* crucible 4e36b7ef-5684-4304-b7c3-3c31aaf83d4f in service -> expunged fd00:1122:3344:103::23
* crucible 603e629d-2599-400e-b879-4134d4cc426e in service -> expunged fd00:1122:3344:103::2c
* crucible 9179d6dc-387d-424e-8d62-ed59b2c728f6 in service -> expunged fd00:1122:3344:103::2a
* crucible c28d7b4b-a259-45ad-945d-f19ca3c6964c in service -> expunged fd00:1122:3344:103::29
* crucible e29998e7-9ed2-46b6-bb70-4118159fe07f in service -> expunged fd00:1122:3344:103::26
* crucible f06e91a1-0c17-4cca-adbc-1c9b67bdb11d in service -> expunged fd00:1122:3344:103::2b
* crucible f11f5c60-1ac7-4630-9a3a-a9bc85c75203 in service -> expunged fd00:1122:3344:103::25
* crucible f231e4eb-3fc9-4964-9d71-2c41644852d9 in service -> expunged fd00:1122:3344:103::24
* internal_ntp c62b87b6-b98d-4d22-ba4f-cee4499e2ba8 in service -> expunged fd00:1122:3344:103::21
* nexus 6a70a233-1900-43c0-9c00-aa9d1f7adfbc in service -> expunged fd00:1122:3344:103::22
-------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
-------------------------------------------------------------------------------------------
* crucible 1e1ed0cc-1adc-410f-943a-d1a3107de619 - in service fd00:1122:3344:103::27
└─ + expunged
* crucible 2307bbed-02ba-493b-89e3-46585c74c8fc - in service fd00:1122:3344:103::28
└─ + expunged
* crucible 4e36b7ef-5684-4304-b7c3-3c31aaf83d4f - in service fd00:1122:3344:103::23
└─ + expunged
* crucible 603e629d-2599-400e-b879-4134d4cc426e - in service fd00:1122:3344:103::2c
└─ + expunged
* crucible 9179d6dc-387d-424e-8d62-ed59b2c728f6 - in service fd00:1122:3344:103::2a
└─ + expunged
* crucible c28d7b4b-a259-45ad-945d-f19ca3c6964c - in service fd00:1122:3344:103::29
└─ + expunged
* crucible e29998e7-9ed2-46b6-bb70-4118159fe07f - in service fd00:1122:3344:103::26
└─ + expunged
* crucible f06e91a1-0c17-4cca-adbc-1c9b67bdb11d - in service fd00:1122:3344:103::2b
└─ + expunged
* crucible f11f5c60-1ac7-4630-9a3a-a9bc85c75203 - in service fd00:1122:3344:103::25
└─ + expunged
* crucible f231e4eb-3fc9-4964-9d71-2c41644852d9 - in service fd00:1122:3344:103::24
└─ + expunged
* internal_ntp c62b87b6-b98d-4d22-ba4f-cee4499e2ba8 - in service fd00:1122:3344:103::21
└─ + expunged
* nexus 6a70a233-1900-43c0-9c00-aa9d1f7adfbc - in service fd00:1122:3344:103::22
└─ + expunged


sled fefcf4cf-f7e7-46b3-b629-058526ce440e:
Expand Down
42 changes: 27 additions & 15 deletions nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,33 @@ to: blueprint 9f71f5d3-a272-4382-9154-6ea2e171a6c6


omicron zones generation 2 -> 3:
-----------------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
-----------------------------------------------------------------------------------------------------
* crucible 094f27af-1acb-4d1e-ba97-1fc1377d4bf2 in service -> expunged fd00:1122:3344:103::2c
* crucible 0dcfdfc5-481e-4153-b97c-11cf02b648ea in service -> expunged fd00:1122:3344:103::25
* crucible 2f5e8010-a94d-43a4-9c5c-3f52832f5f7f in service -> expunged fd00:1122:3344:103::27
* crucible 4a9a0a9d-87f0-4f1d-9181-27f6b435e637 in service -> expunged fd00:1122:3344:103::28
* crucible 56ac1706-9e2a-49ba-bd6f-a99c44cb2ccb in service -> expunged fd00:1122:3344:103::24
* crucible 67622d61-2df4-414d-aa0e-d1277265f405 in service -> expunged fd00:1122:3344:103::23
* crucible b91b271d-8d80-4f49-99a0-34006ae86063 in service -> expunged fd00:1122:3344:103::2a
* crucible d6ee1338-3127-43ec-9aaa-b973ccf05496 in service -> expunged fd00:1122:3344:103::26
* crucible e39d7c9e-182b-48af-af87-58079d723583 in service -> expunged fd00:1122:3344:103::29
* crucible f69f92a1-5007-4bb0-a85b-604dc217154b in service -> expunged fd00:1122:3344:103::2b
* internal_ntp 67d913e0-0005-4599-9b28-0abbf6cc2916 in service -> expunged fd00:1122:3344:103::21
* nexus 2aa0ea4f-3561-4989-a98c-9ab7d9a240fb in service -> expunged fd00:1122:3344:103::22
-------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
-------------------------------------------------------------------------------------------
* crucible 094f27af-1acb-4d1e-ba97-1fc1377d4bf2 - in service fd00:1122:3344:103::2c
└─ + expunged
* crucible 0dcfdfc5-481e-4153-b97c-11cf02b648ea - in service fd00:1122:3344:103::25
└─ + expunged
* crucible 2f5e8010-a94d-43a4-9c5c-3f52832f5f7f - in service fd00:1122:3344:103::27
└─ + expunged
* crucible 4a9a0a9d-87f0-4f1d-9181-27f6b435e637 - in service fd00:1122:3344:103::28
└─ + expunged
* crucible 56ac1706-9e2a-49ba-bd6f-a99c44cb2ccb - in service fd00:1122:3344:103::24
└─ + expunged
* crucible 67622d61-2df4-414d-aa0e-d1277265f405 - in service fd00:1122:3344:103::23
└─ + expunged
* crucible b91b271d-8d80-4f49-99a0-34006ae86063 - in service fd00:1122:3344:103::2a
└─ + expunged
* crucible d6ee1338-3127-43ec-9aaa-b973ccf05496 - in service fd00:1122:3344:103::26
└─ + expunged
* crucible e39d7c9e-182b-48af-af87-58079d723583 - in service fd00:1122:3344:103::29
└─ + expunged
* crucible f69f92a1-5007-4bb0-a85b-604dc217154b - in service fd00:1122:3344:103::2b
└─ + expunged
* internal_ntp 67d913e0-0005-4599-9b28-0abbf6cc2916 - in service fd00:1122:3344:103::21
└─ + expunged
* nexus 2aa0ea4f-3561-4989-a98c-9ab7d9a240fb - in service fd00:1122:3344:103::22
└─ + expunged


sled 68d24ac5-f341-49ea-a92a-0381b52ab387:
Expand Down
27 changes: 14 additions & 13 deletions nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,20 @@ to: blueprint 9f71f5d3-a272-4382-9154-6ea2e171a6c6


omicron zones at generation 2:
--------------------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
--------------------------------------------------------------------------------------------------
crucible 6b53ab2e-d98c-485f-87a3-4d5df595390f in service fd00:1122:3344:105::27
crucible 93b137a1-a1d6-4b5b-b2cb-21a9f11e2883 in service fd00:1122:3344:105::23
crucible 9f0abbad-dbd3-4d43-9675-78092217ffd9 in service fd00:1122:3344:105::25
crucible b0c63f48-01ea-4aae-bb26-fb0dd59d1662 in service fd00:1122:3344:105::28
crucible c406da50-34b9-4bb4-a460-8f49875d2a6a in service fd00:1122:3344:105::24
crucible d660d7ed-28c0-45ae-9ace-dc3ecf7e8786 in service fd00:1122:3344:105::2a
crucible e98cc0de-abf6-4da4-a20d-d05c7a9bb1d7 in service fd00:1122:3344:105::2b
crucible f55e6aaf-e8fc-4913-9e3c-8cd1bd4bdad3 in service fd00:1122:3344:105::29
- crucible 4f1ce8a2-d3a5-4a38-be4c-9817de52db37 in service fd00:1122:3344:105::2c
* crucible 19fbc4f8-a683-4f22-8f5a-e74782b935be in service -> quiesced fd00:1122:3344:105::26
----------------------------------------------------------------------------------------
zone type zone id disposition underlay IP
----------------------------------------------------------------------------------------
crucible 6b53ab2e-d98c-485f-87a3-4d5df595390f in service fd00:1122:3344:105::27
crucible 93b137a1-a1d6-4b5b-b2cb-21a9f11e2883 in service fd00:1122:3344:105::23
crucible 9f0abbad-dbd3-4d43-9675-78092217ffd9 in service fd00:1122:3344:105::25
crucible b0c63f48-01ea-4aae-bb26-fb0dd59d1662 in service fd00:1122:3344:105::28
crucible c406da50-34b9-4bb4-a460-8f49875d2a6a in service fd00:1122:3344:105::24
crucible d660d7ed-28c0-45ae-9ace-dc3ecf7e8786 in service fd00:1122:3344:105::2a
crucible e98cc0de-abf6-4da4-a20d-d05c7a9bb1d7 in service fd00:1122:3344:105::2b
crucible f55e6aaf-e8fc-4913-9e3c-8cd1bd4bdad3 in service fd00:1122:3344:105::29
- crucible 4f1ce8a2-d3a5-4a38-be4c-9817de52db37 in service fd00:1122:3344:105::2c
* crucible 19fbc4f8-a683-4f22-8f5a-e74782b935be - in service fd00:1122:3344:105::26
└─ + quiesced


sled 48d95fef-bc9f-4f50-9a53-1e075836291d:
Expand Down
7 changes: 5 additions & 2 deletions nexus/types/src/deployment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,10 @@ impl BpSledSubtableData for &OmicronPhysicalDisksConfig {
self.disks.iter().map(|d| d.identity.clone()).collect();

sorted_disk_ids.into_iter().map(move |d| {
BpSledSubtableRow::new(state, vec![d.vendor, d.model, d.serial])
BpSledSubtableRow::from_strings(
state,
vec![d.vendor, d.model, d.serial],
)
})
}
}
Expand All @@ -319,7 +322,7 @@ impl BpSledSubtableData for BlueprintOrCollectionZonesConfig {
state: BpDiffState,
) -> impl Iterator<Item = BpSledSubtableRow> {
self.zones().map(move |zone| {
BpSledSubtableRow::new(
BpSledSubtableRow::from_strings(
state,
vec![
zone.kind().to_string(),
Expand Down
26 changes: 13 additions & 13 deletions nexus/types/src/deployment/blueprint_diff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
use super::blueprint_display::{
constants::*, linear_table_modified, linear_table_unchanged, BpDiffState,
BpGeneration, BpOmicronZonesSubtableSchema, BpPhysicalDisksSubtableSchema,
BpSledSubtable, BpSledSubtableData, BpSledSubtableRow, KvListWithHeading,
KvPair,
BpSledSubtable, BpSledSubtableColumn, BpSledSubtableData,
BpSledSubtableRow, KvListWithHeading, KvPair,
};
use super::zone_sort_key;
use omicron_common::api::external::Generation;
Expand Down Expand Up @@ -48,7 +48,7 @@ impl BpSledSubtableData for BpDiffZoneDetails {
state: BpDiffState,
) -> impl Iterator<Item = BpSledSubtableRow> {
self.zones.iter().map(move |zone| {
BpSledSubtableRow::new(
BpSledSubtableRow::from_strings(
state,
vec![
zone.kind().to_string(),
Expand Down Expand Up @@ -149,18 +149,18 @@ impl BpSledSubtableData for BpDiffZonesModified {
state: BpDiffState,
) -> impl Iterator<Item = BpSledSubtableRow> {
self.zones.iter().map(move |zone| {
let disposition = format!(
"{} {ARROW} {}",
zone.prior_disposition,
zone.zone.disposition()
);
BpSledSubtableRow::new(
state,
vec![
zone.zone.kind().to_string(),
zone.zone.id().to_string(),
disposition,
zone.zone.underlay_address().to_string(),
BpSledSubtableColumn::value(zone.zone.kind().to_string()),
BpSledSubtableColumn::value(zone.zone.id().to_string()),
BpSledSubtableColumn::diff(
zone.prior_disposition.to_string(),
zone.zone.disposition().to_string(),
),
BpSledSubtableColumn::value(
zone.zone.underlay_address().to_string(),
),
],
)
})
Expand Down Expand Up @@ -421,7 +421,7 @@ impl BpSledSubtableData for DiffPhysicalDisksDetails {
state: BpDiffState,
) -> impl Iterator<Item = BpSledSubtableRow> {
self.disks.iter().map(move |d| {
BpSledSubtableRow::new(
BpSledSubtableRow::from_strings(
state,
vec![d.vendor.clone(), d.model.clone(), d.serial.clone()],
)
Expand Down
103 changes: 95 additions & 8 deletions nexus/types/src/deployment/blueprint_display.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ pub mod constants {
pub(super) const MODIFIED_PREFIX: char = '*';
pub(super) const UNCHANGED_PREFIX: char = ' ';

#[allow(unused)]
pub(super) const SUB_NOT_LAST: &str = "├─";
pub(super) const SUB_LAST: &str = "└─";

pub const ARROW: &str = "->";
pub const METADATA_HEADING: &str = "METADATA";
pub const CREATED_BY: &str = "created by";
Expand Down Expand Up @@ -97,16 +101,57 @@ impl fmt::Display for BpGeneration {
}
}

pub enum BpSledSubtableColumn {
Value(String),
Diff { before: String, after: String },
}

impl BpSledSubtableColumn {
pub fn value(s: String) -> BpSledSubtableColumn {
BpSledSubtableColumn::Value(s)
}

pub fn diff(before: String, after: String) -> BpSledSubtableColumn {
BpSledSubtableColumn::Diff { before, after }
}

pub fn len(&self) -> usize {
match self {
BpSledSubtableColumn::Value(s) => s.len(),
BpSledSubtableColumn::Diff { before, after } => {
// Add 1 for the added/removed prefix and 1 for a space
//
// This will need to change if we change how we render diffs in
// the `Display` impl for `BpSledSubtable`. However, putting it
// here allows to minimize any extra horizontal spacing in case
// other values for the same column are already longer than the
// the before or after values + 2.
usize::max(before.len(), after.len()) + 2
}
}
}
}

/// A row in a [`BpSledSubtable`]
pub struct BpSledSubtableRow {
state: BpDiffState,
columns: Vec<String>,
columns: Vec<BpSledSubtableColumn>,
}

impl BpSledSubtableRow {
pub fn new(state: BpDiffState, columns: Vec<String>) -> Self {
pub fn new(state: BpDiffState, columns: Vec<BpSledSubtableColumn>) -> Self {
BpSledSubtableRow { state, columns }
}

pub fn from_strings(state: BpDiffState, columns: Vec<String>) -> Self {
BpSledSubtableRow {
state,
columns: columns
.into_iter()
.map(BpSledSubtableColumn::Value)
.collect(),
}
}
}

/// Metadata about all instances of specific type of [`BpSledSubtable`],
Expand Down Expand Up @@ -190,10 +235,10 @@ impl fmt::Display for BpSledSubtable {
for (i, (column, width)) in
self.column_names.iter().zip(&widths).enumerate()
{
if i != 0 {
write!(f, "{:<COLUMN_GAP$}{column:<width$}", "")?;
} else {
if i == 0 {
write!(f, "{column:<width$}")?;
} else {
write!(f, "{:<COLUMN_GAP$}{column:<width$}", "")?;
}
}

Expand All @@ -204,16 +249,58 @@ impl fmt::Display for BpSledSubtable {
for row in &self.rows {
let prefix = row.state.prefix();
write!(f, "{prefix:<SUBTABLE_INDENT$}")?;
let mut multiline_row = false;
for (i, (column, width)) in
row.columns.iter().zip(&widths).enumerate()
{
if i != 0 {
write!(f, "{:<COLUMN_GAP$}{column:<width$}", "")?;
} else {
let (column, needs_multiline) = match column {
BpSledSubtableColumn::Value(s) => (s.clone(), false),
BpSledSubtableColumn::Diff { before, .. } => {
// If we remove the prefix and space, we'll need to also
// modify `BpSledSubtableColumn::len` to reflect this.
(format!("{REMOVED_PREFIX} {before}"), true)
}
};
multiline_row |= needs_multiline;

if i == 0 {
write!(f, "{column:<width$}")?;
} else {
write!(f, "{:<COLUMN_GAP$}{column:<width$}", "")?;
}
}
write!(f, "\n")?;

// Do we need any multiline output?
if multiline_row {
write!(f, "{UNCHANGED_PREFIX:<SUBTABLE_INDENT$}")?;
for (i, (column, width)) in
row.columns.iter().zip(&widths).enumerate()
{
// Write the after columns or nothing
let column = if let BpSledSubtableColumn::Diff {
after,
..
} = column
{
// If we remove the prefix and space, we'll need to also
// modify `BpSledSubtableColumn::len` to reflect this.
format!("{ADDED_PREFIX} {after}")
} else {
"".to_string()
};

if i == 0 {
// First column should never be modifiable
assert!(column.is_empty());
let column = format!(" {SUB_LAST}");
write!(f, "{column:<width$}")?;
} else {
write!(f, "{:<COLUMN_GAP$}{column:<width$}", "")?;
}
}
write!(f, "\n")?;
}
}

Ok(())
Expand Down

0 comments on commit 116590c

Please sign in to comment.