update_agent: support rebase to OCI pullspec #1241

jbtrystram · 2024-12-04T14:04:10Z

Add a configuration knob in update to optionnaly use OCI pullspec instead of ostree checksums.

This will default to false.
Requires coreos/fedora-coreos-cincinnati#99 and coreos/rpm-ostree#5120

travier

Very quick look but looks good 👍🏻

src/rpm_ostree/cli_deploy.rs

tests/fixtures/00-config-sample.toml

jlebon

Let's cross-link to coreos/fedora-coreos-tracker#1823 in the commit message for the larger context.

tests/fixtures/00-config-sample.toml

src/cincinnati/mod.rs

src/rpm_ostree/cli_deploy.rs

Parse rpm-ostree status to detect if the current booted deployment is an OCI image. If so, query the OCI graph for cincinnati and rebase to the correct OCI image. Requires coreos/fedora-coreos-cincinnati#99 and coreos/rpm-ostree#5120 Part of: coreos/fedora-coreos-tracker#1823

…ee-remote-image to verify fedora ostree signatures

jbtrystram

I updated this to use ostree-remote-image so we validate the ostree signatures, as suggested in coreos/fedora-coreos-tracker#1823 (comment)

I think this is ready for review now. I will clean up and sqash commits once the design is agreed

src/rpm_ostree/cli_status.rs

src/rpm_ostree/cli_deploy.rs

jbtrystram · 2025-01-13T13:47:10Z

src/rpm_ostree/cli_status.rs

    pub fn base_revision(&self) -> String {
-        self.base_checksum
+        self.container_image_reference
            .clone()
+            .map(|pullspec| {
+                pullspec
+                    .strip_prefix("ostree-remote-image:fedora:docker://")
+                    .map(|s| s.to_string())
+            })
+            .flatten()
+            .or(self.base_checksum.clone())


Maybe it would make more sense to add an Option<String> to the Deployement struct to be able to have both the pullspec and the base commit ?
However the cincinnati graph does not contains checksum in the OCI case so we can't compare ostree commits when doing rebase later anyway.

haven't had the need for it in the Zincati context, except maybe if we want Zincati to do OCI -> OSTree ? (in that case, after calling rpm-ostree deploy zincati will fail as the checksum won't match

jbtrystram · 2025-01-13T14:36:54Z

OK, had some more discussion with Jlebon and travier today and there are some more things to flesh out

src/rpm_ostree/cli_status.rs

src/rpm_ostree/mod.rs

jbtrystram · 2025-01-15T17:07:39Z

Allright, I think this is ready for review now. I highlighted a couple of things I am unsure of.
I will squash the commits after addressing review comments

Switch boot images to use OCI for updates. This is a step towards bootable containers support and bootc rebase. See https://fedoraproject.org/wiki/Changes/CoreOSOstree2OCIUpdates Requires coreos/zincati#1241 See coreos/fedora-coreos-tracker#1823

jlebon · 2025-01-27T15:38:08Z

src/cincinnati/client.rs

-            Some(params) => params,
-            None => HashMap::new(),
-        };
+        let query_params = self.query_params.unwrap_or_default();


Minor: clippy fix I'm guessing? This could be its own separate commit.

jlebon · 2025-01-27T15:39:35Z

src/cincinnati/mod.rs

+            log::warn!(
+                "booted deployment {} not found in the update graph",
+                &booted_depl.checksum
+            );


Minor: this could be its own separate commit.

src/rpm_ostree/cli_deploy.rs

src/rpm_ostree/cli_status.rs

src/cincinnati/mod.rs

src/rpm_ostree/actor.rs

tests/fixtures/rpm-ostree-oci-status.json

jlebon · 2025-01-28T21:56:47Z

src/rpm_ostree/cli_finalize.rs

+    // get the latest commit but that would be racy, so let's finalize the latest
+    // commit.
+    if release.is_oci {
+        cmd.arg("--allow-missing-checksum")


I think probably the cleaner thing is to have rpm-ostree accept a digested pullspec the way it accepts commit checksums and it can verify that it matches the digested pullspec that was staged. Then we can drop this.

src/rpm_ostree/mod.rs

jlebon · 2025-01-28T22:02:54Z

src/update_agent/actor.rs

-            bail!(
+            log::error!(
                "expected pending deployment '{}', but found '{}' instead",
                release.version,
                pending.version
            );
+            return Ok(false);
        }
        if pending.checksum != release.checksum {
-            bail!(
+            log::error!(
                "detected checksum mismatch for pending deployment '{}', got unexpected value '{}'",
                release.version,
                release.checksum,
            );
+            return Ok(false);


Hmm, why do we need this?

The rest of the function was built that way, and the bail error text was swallowed so I just aligned all the checks to use the same syntax

This changes the semantics of the function. I think those differences might be on purpose.

In the OSTree case, if the checksum doesn't match, then something went really wrong because we expicitly deployed by checksum. Whereas the stream check below this line is done here because it can't be done before deploying; it's part of the commit metadata. (Though I guess we could pull it before deploying.)

We can't exactly transpose this to the OCI case, because there while we do deploy by digest, that digest doesn't actually mean much within the object store. That said, here I think checksum is actually the pullspec in the OCI path as this PR is written (right?), so we do also expect it to be always true. It's basically checking that the custom origin URL we've set has correctly been set. I think then we do still want to keep this a hard error?

For the version check, I think the change to not make it a hard error makes sense.

jbtrystram · 2025-02-05T15:52:36Z

Allright, I reworked this PR a bunch following the review comments, that ended up being pretty substantial changes.

Overview :

A local deployment is now unserialized with an enum containing either a Checksum or a Pullspec. Both are simply strings, but I use that to match my way around, rather than a is_oci boolean.
custom-origin fields are now purely cosmetic : Zincati will cue from the container-image-reference.
Now properly parses the OCI manifest output from rpm-ostree --json and get the fedora-coreos.stream key from that. This is working around rpm-ostree status --json does not properly serialize ostree.manifest for OCI deployments rpm-ostree#5196
imported a couple of new crates : oci-spec for the above issue and ostree-rs to properly parse ostree image references and containers pull specs. I removed the hacky split(':') and split(":sha256")` in favor of that.

Testing

Here are my notes to test this out it case someone wants to experiment:

Start a VM with Zincati disabled : cosa run --qemu-image fedora-coreos-41.20241215.2.0-qemu.x86_64.qcow2 --add-ignition noautoupdate

Rebase to an OCI image in graph (recent enough to get rpm-ostree-2024.9) :

    sudo rpm-ostree rebase ostree-remote-image:fedora:registry:quay.io/fedora/fedora-coreos@sha256:74448159fae255797012a12eaf9089ea3c1018ffb0b3e76e03483cba59b50bb2 \
     --custom-origin-url=quay.io/fedora/fedora-coreos@sha256:74448159fae255797012a12eaf9089ea3c1018ffb0b3e76e03483cba59b50bb2 \
     --custom-origin-description="Fedora CoreOS testing stream"
     sudo reboot

Now import Zincati build with this PR then:

    sudo rm /etc/zincati/config.d/90-disable-auto-updates.toml
    sudo systemctl restart zincati

Zincati should now pick up an OCI update through the OCI graph

Getting zincati custom build in the COSA VM

While we can make a custom FCOS build with an override to get our Zincati build inside, it will be erased by the content of the OCI image with a rebase, so here is how I did, after building zincati and copy the binary into $COSA_DIR/tmp

rpm-ostree usroverlay
cp /mnt/workdir-tmp/zincati /usr/libexec/zincati
cp /mnt/workdir-tmp/zincati.rules /usr/share/polkit-1/rules.d/zincati.rules

# restart polkit
systemctl restart polkit.service

# re enable updates
rm /etc/zincati/config.d/90-disable-auto-updates.toml

# restart zincati
systemctl restart zincati.service

# Show the update
journalctl --no-pager -o cat -u zincati --follow

In the meantime, I'll squash this and try to make commits that make sense :)

jbtrystram force-pushed the oci_scheme branch 2 times, most recently from f0713e1 to 1ca8a87 Compare December 4, 2024 14:18

jbtrystram changed the title ~~update_agent: support rebaseing to OCI pullspec~~ update_agent: support rebase to OCI pullspec Dec 4, 2024

travier reviewed Dec 4, 2024

View reviewed changes

cgwalters reviewed Dec 4, 2024

View reviewed changes

src/rpm_ostree/cli_deploy.rs Outdated Show resolved Hide resolved

dustymabe reviewed Dec 9, 2024

View reviewed changes

tests/fixtures/00-config-sample.toml Outdated Show resolved Hide resolved

jlebon reviewed Dec 10, 2024

View reviewed changes

tests/fixtures/00-config-sample.toml Outdated Show resolved Hide resolved

src/cincinnati/mod.rs Outdated Show resolved Hide resolved

src/cincinnati/mod.rs Outdated Show resolved Hide resolved

src/rpm_ostree/cli_deploy.rs Outdated Show resolved Hide resolved

jbtrystram force-pushed the oci_scheme branch 3 times, most recently from 4b9c09b to 11aa880 Compare December 16, 2024 10:45

jbtrystram force-pushed the oci_scheme branch from 11aa880 to 7000352 Compare December 16, 2024 13:51

jbtrystram added 5 commits December 19, 2024 11:41

wip oci status

7c601c7

WIP: pull the stream id from the booted coreos version

f10a80c

finalize: skip the checksum when updating to an OCI release

c8fc51c

nest is_oci to Release struct to pass less things around and use ostr…

646a958

…ee-remote-image to verify fedora ostree signatures

cleanup some log statements

8d45c47

jbtrystram commented Jan 13, 2025

View reviewed changes

jbtrystram marked this pull request as draft January 13, 2025 14:36

jbtrystram added 2 commits January 14, 2025 10:06

use custom origin to get the ostree image spec and stream

9d96a80

update tests

e3f99a4

jbtrystram commented Jan 15, 2025

View reviewed changes

src/rpm_ostree/cli_status.rs Outdated Show resolved Hide resolved

jbtrystram commented Jan 15, 2025

View reviewed changes

src/rpm_ostree/mod.rs Outdated Show resolved Hide resolved

refactor some logic and cleanup

5dd3969

jbtrystram requested review from jlebon and cgwalters January 15, 2025 17:07

improve unit tests

20f1692

jbtrystram marked this pull request as ready for review January 15, 2025 17:20

fix failpoints test

5e7e494

jbtrystram mentioned this pull request Jan 16, 2025

Move from OSTree to OCI for updates coreos/fedora-coreos-tracker#1823

Open

jbtrystram force-pushed the oci_scheme branch from 90c5b25 to 450550b Compare January 23, 2025 09:59

jbtrystram mentioned this pull request Jan 23, 2025

[testing] deploy container images by default coreos/fedora-coreos-config#3333

Open

cgwalters mentioned this pull request Jan 23, 2025

parse OCI deployment status into ImageManifest coreos/rpm-ostree#5208

Closed

jbtrystram force-pushed the oci_scheme branch from 450550b to add9bf7 Compare January 24, 2025 09:36

CI have different clippy flags

c5e0459

jbtrystram force-pushed the oci_scheme branch from add9bf7 to c5e0459 Compare January 24, 2025 10:59

jlebon reviewed Jan 28, 2025

View reviewed changes

jbtrystram mentioned this pull request Jan 30, 2025

rust/rpmostree-client: parse OCI deployment manifest coreos/rpm-ostree#5266

Open

address review

c2dce91

properly parse ostree imgref

d3a9096

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update_agent: support rebase to OCI pullspec #1241

update_agent: support rebase to OCI pullspec #1241

jbtrystram commented Dec 4, 2024

travier left a comment

jlebon left a comment

jbtrystram left a comment

jbtrystram Jan 13, 2025

jbtrystram Feb 5, 2025 •

edited

Loading

jbtrystram commented Jan 13, 2025

jbtrystram commented Jan 15, 2025 •

edited

Loading

jlebon Jan 27, 2025

jlebon Jan 27, 2025

jlebon Jan 28, 2025

jlebon Jan 28, 2025

jbtrystram Jan 29, 2025

jlebon Jan 29, 2025

jbtrystram commented Feb 5, 2025

update_agent: support rebase to OCI pullspec #1241

Are you sure you want to change the base?

update_agent: support rebase to OCI pullspec #1241

Conversation

jbtrystram commented Dec 4, 2024

travier left a comment

Choose a reason for hiding this comment

jlebon left a comment

Choose a reason for hiding this comment

jbtrystram left a comment

Choose a reason for hiding this comment

jbtrystram Jan 13, 2025

Choose a reason for hiding this comment

jbtrystram Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

jbtrystram commented Jan 13, 2025

jbtrystram commented Jan 15, 2025 • edited Loading

jlebon Jan 27, 2025

Choose a reason for hiding this comment

jlebon Jan 27, 2025

Choose a reason for hiding this comment

jlebon Jan 28, 2025

Choose a reason for hiding this comment

jlebon Jan 28, 2025

Choose a reason for hiding this comment

jbtrystram Jan 29, 2025

Choose a reason for hiding this comment

jlebon Jan 29, 2025

Choose a reason for hiding this comment

jbtrystram commented Feb 5, 2025

Overview :

Testing

Getting zincati custom build in the COSA VM

jbtrystram Feb 5, 2025 •

edited

Loading

jbtrystram commented Jan 15, 2025 •

edited

Loading