Skip to content

feat(status): display pinned deployments #1285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(status): display pinned deployments #1285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 44 additions & 0 deletions lib/src/fixtures/spec-booted-pinned.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
apiVersion: org.containers.bootc/v1alpha1
kind: BootcHost
metadata:
name: host
spec:
image:
image: quay.io/centos-bootc/centos-bootc:stream9
transport: registry
bootOrder: default
status:
staged: null
booted:
image:
image:
image: quay.io/centos-bootc/centos-bootc:stream9
transport: registry
architecture: arm64
version: stream9.20240807.0
timestamp: null
imageDigest: sha256:47e5ed613a970b6574bfa954ab25bb6e85656552899aa518b5961d9645102b38
cachedUpdate: null
incompatible: false
pinned: true
ostree:
checksum: 439f6bd2e2361bee292c1f31840d798c5ac5ba76483b8021dc9f7b0164ac0f48
deploySerial: 0
pinned:
- image:
image:
image: quay.io/centos-bootc/centos-bootc:stream9
transport: registry
version: stream9.20240807.0
timestamp: null
imageDigest: sha256:47e5ed613a970b6574bfa954ab25bb6e85656552899aa518b5961d9645102b37
architecture: arm64
cachedUpdate: null
incompatible: false
pinned: true
ostree:
checksum: 99b2cc3b6edce9ebaef6a6076effa5ee3e1dcff3523016ffc94a1b27c6c67e12
deploySerial: 0
rollback: null
rollbackQueued: false
type: bootcHost
19 changes: 19 additions & 0 deletions lib/src/spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,26 @@ pub struct HostStatus {
/// Set to true if the rollback entry is queued for the next boot.
#[serde(default)]
pub rollback_queued: bool,
/// All pinned images
pub pinned: Option<Vec<BootEntry>>,

/// The detected type of system
#[serde(rename = "type")]
pub ty: Option<HostType>,
}

impl HostStatus {
/// Check if the slot is pinned
pub fn is_pinned(&self, slot: Slot) -> bool {
match slot {
Slot::Staged => self.staged.as_ref().map_or(false, |s| s.pinned),
Slot::Booted => self.booted.as_ref().map_or(false, |s| s.pinned),
Slot::Rollback => self.rollback.as_ref().map_or(false, |s| s.pinned),
Slot::Pinned => true,
}
}
}

impl Host {
/// Create a new host
pub fn new(spec: HostSpec) -> Self {
Expand Down Expand Up @@ -196,6 +210,11 @@ impl Host {
self.status.staged = None;
self.status.booted = None;
}
Slot::Pinned => {
self.status.staged = None;
self.status.booted = None;
self.status.rollback = None;
}
}
}
}
Expand Down
86 changes: 79 additions & 7 deletions lib/src/status.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ pub(crate) fn get_status(
.map(|d| boot_entry_from_deployment(sysroot, d))
.transpose()
.context("Rollback deployment")?;
let pinned = deployments
.other
.iter()
.filter_map(|d| {
d.is_pinned()
.then(|| boot_entry_from_deployment(sysroot, d).ok())
})
.collect::<Option<Vec<_>>>()
.filter(|v| !v.is_empty());
Comment on lines +256 to +264
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it feels duplicative to include the "primary" outputs (staged/booted/rollback) additionally in the pinned set.

WDYT about instead operating on the other deployment list?

Though that basically scope creeps into generally showing information about those arbitrary other deployments which we don't do now either.

Copy link
Contributor Author

@p5 p5 Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it feels duplicative to include the "primary" outputs (staged/booted/rollback) additionally in the pinned set.

In this case, the primary outputs are not duplicated into the "pinned" object. They are already filtered out before reaching this point (deployments.other excludes staged/booted/rollback).

I'm happy with this approach though, and instead of specifically storing pinned deployments inside the HostStatus, storing all other deployments as a BootEntry array.

let spec = staged
.as_ref()
.or(booted.as_ref())
Expand Down Expand Up @@ -280,6 +289,7 @@ pub(crate) fn get_status(
booted,
rollback,
rollback_queued,
pinned,
ty,
};
Ok((deployments, host))
Expand Down Expand Up @@ -336,6 +346,7 @@ pub enum Slot {
Staged,
Booted,
Rollback,
Pinned,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we follow the above suggestion I think instead of having a Slot for this we'd just display the other deployments unconditionally.

Copy link
Contributor Author

@p5 p5 Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What do you consider to be a "Slot"?
If we were operating on "Other" deployments, would these be considered "Other" slots? Or would these be deployments without a Slot?

My view as an outsider is of a "Slot" basically being a type of boot entry, where any image you're able to boot into should have a Slot, but I appreciate this probably isn't an accurate description once knowing the internals.

}

impl std::fmt::Display for Slot {
Expand All @@ -344,6 +355,7 @@ impl std::fmt::Display for Slot {
Slot::Staged => "staged",
Slot::Booted => "booted",
Slot::Rollback => "rollback",
Slot::Pinned => "pinned",
};
f.write_str(s)
}
Expand All @@ -359,10 +371,11 @@ fn write_row_name(mut out: impl Write, s: &str, prefix_len: usize) -> Result<()>
}

/// Write the data for a container image based status.
fn human_render_imagestatus(
fn human_render_image_deployment(
Comment on lines -362 to +374
Copy link
Contributor Author

@p5 p5 Apr 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I feel this function may need a larger rework than this PR can handle.

● Booted image: ghcr.io/rsturla/eternal-linux/lumina:42-nvidia-open
        Digest: sha256:b1874fbb5f442657f3cc07a547c54292ca3f24fe828ab4f3132df4c9f23f1e3e (amd64)
       Version: 250425 (2025-04-25T01:46:32Z)

  Rollback image: ghcr.io/rsturla/eternal-linux/lumina:42-nvidia-open
          Digest: sha256:b1874fbb5f442657f3cc07a547c54292ca3f24fe828ab4f3132df4c9f23f1e3e (amd64)
         Version: 250425 (2025-04-25T01:46:32Z)

Rather than each block above being an "image", I feel it makes more sense being a "deployment" since it contains more information than just the image. For example, the index, pinned state, /usr overlay state etc. #1258 also updates this function to include HostStatus in order to access the /usr overlay state.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than each block above being an "image", I feel it makes more sense being a "deployment" since it contains more information than just the image.

Right, we can change the function to take a BootEntry instead right? Actually I went ahead and tried that, can you review #1291 and then we'll rebase on that?

mut out: impl Write,
slot: Slot,
image: &crate::spec::ImageStatus,
host_status: &crate::spec::HostStatus,
) -> Result<()> {
let transport = &image.image.transport;
let imagename = &image.image.image;
Expand All @@ -377,6 +390,7 @@ fn human_render_imagestatus(
Slot::Staged => " Staged image".into(),
Slot::Booted => format!("{} Booted image", crate::glyph::Glyph::BlackCircle),
Slot::Rollback => " Rollback image".into(),
Slot::Pinned => " Pinned image".into(),
};
let prefix_len = prefix.chars().count();
writeln!(out, "{prefix}: {imageref}")?;
Expand Down Expand Up @@ -407,20 +421,37 @@ fn human_render_imagestatus(
writeln!(out, "{timestamp}")?;
}

if host_status.is_pinned(slot) {
write_row_name(&mut out, "Pinned", prefix_len)?;
writeln!(out, "yes")?;
}

Ok(())
}

fn human_render_ostree(mut out: impl Write, slot: Slot, ostree_commit: &str) -> Result<()> {
fn human_render_ostree_deployment(
mut out: impl Write,
slot: Slot,
ostree_commit: &str,
host_status: &crate::spec::HostStatus,
) -> Result<()> {
// TODO consider rendering more ostree stuff here like rpm-ostree status does
let prefix = match slot {
Slot::Staged => " Staged ostree".into(),
Slot::Booted => format!("{} Booted ostree", crate::glyph::Glyph::BlackCircle),
Slot::Rollback => " Rollback ostree".into(),
Slot::Pinned => " Pinned ostree".into(),
};
let prefix_len = prefix.len();
writeln!(out, "{prefix}")?;
write_row_name(&mut out, "Commit", prefix_len)?;
writeln!(out, "{ostree_commit}")?;

if host_status.is_pinned(slot) {
write_row_name(&mut out, "Pinned", prefix_len)?;
writeln!(out, "yes")?;
}

Ok(())
}

Expand All @@ -438,14 +469,36 @@ fn human_readable_output_booted(mut out: impl Write, host: &Host) -> Result<()>
writeln!(out)?;
}
if let Some(image) = &host_status.image {
human_render_imagestatus(&mut out, slot_name, image)?;
human_render_image_deployment(&mut out, slot_name, image, &host.status)?;
} else if let Some(ostree) = host_status.ostree.as_ref() {
human_render_ostree(&mut out, slot_name, &ostree.checksum)?;
human_render_ostree_deployment(
&mut out,
slot_name,
&ostree.checksum,
&host.status,
)?;
} else {
writeln!(out, "Current {slot_name} state is unknown")?;
}
}
}

if let Some(pinned) = &host.status.pinned {
writeln!(out)?;
for entry in pinned {
if let Some(image) = &entry.image {
human_render_image_deployment(&mut out, Slot::Pinned, image, &host.status)?;
} else if let Some(ostree) = entry.ostree.as_ref() {
human_render_ostree_deployment(
&mut out,
Slot::Pinned,
&ostree.checksum,
&host.status,
)?;
}
}
}

Ok(())
}

Expand Down Expand Up @@ -480,7 +533,7 @@ mod tests {
Staged image: quay.io/example/someimage:latest
Digest: sha256:16dc2b6256b4ff0d2ec18d2dbfb06d117904010c8cf9732cdb022818cf7a7566 (arm64)
Version: nightly (2023-10-14T19:22:15Z)

● Booted image: quay.io/example/someimage:latest
Digest: sha256:736b359467c9437c1ac915acaae952aad854e07eb4a16a94999a48af08c83c34 (arm64)
Version: nightly (2023-09-30T19:22:16Z)
Expand All @@ -498,7 +551,7 @@ mod tests {
let expected = indoc::indoc! { r"
Staged ostree
Commit: 1c24260fdd1be20f72a4a97a75c582834ee3431fbb0fa8e4f482bb219d633a45

● Booted ostree
Commit: f9fa3a553ceaaaf30cf85bfe7eed46a822f7b8fd7e14c1e3389cbc3f6d27f791
"};
Expand All @@ -514,7 +567,7 @@ mod tests {
Staged image: quay.io/centos-bootc/centos-bootc:stream9
Digest: sha256:47e5ed613a970b6574bfa954ab25bb6e85656552899aa518b5961d9645102b38 (s390x)
Version: stream9.20240807.0

● Booted ostree
Commit: f9fa3a553ceaaaf30cf85bfe7eed46a822f7b8fd7e14c1e3389cbc3f6d27f791
"};
Expand All @@ -534,6 +587,25 @@ mod tests {
similar_asserts::assert_eq!(w, expected);
}

#[test]
fn test_human_readable_booted_pinned_spec() {
// booted image, no staged/rollback
let w = human_status_from_spec_fixture(include_str!("fixtures/spec-booted-pinned.yaml"))
.expect("No spec found");
let expected = indoc::indoc! { r"
● Booted image: quay.io/centos-bootc/centos-bootc:stream9
Digest: sha256:47e5ed613a970b6574bfa954ab25bb6e85656552899aa518b5961d9645102b38 (arm64)
Version: stream9.20240807.0
Pinned: yes

Pinned image: quay.io/centos-bootc/centos-bootc:stream9
Digest: sha256:47e5ed613a970b6574bfa954ab25bb6e85656552899aa518b5961d9645102b37 (arm64)
Version: stream9.20240807.0
Pinned: yes
"};
similar_asserts::assert_eq!(w, expected);
}

#[test]
fn test_human_readable_staged_rollback_spec() {
// staged/rollback image, no booted
Expand Down
Loading