Encapsulate OCI images across architectures

[p5]

The aim is to get rpm-ostree compose container-encapsulate to work when trying to bundle AARCH64 commits to an OCI while on an AMD64 machine. Possibly by introducing a new --arch flag.

This should resolve the current issue in the Silverblue OCIs built on Fedora's infra where the ARM images contain the AARCH64 contents (i.e. packages), however the image manifest declares the architecture as AMD64. I think this is due to the script being used runs on AMD64 for all architectures.

In theory, the image contents are all built for AARCH64, so given the correct image configuration, it should "just work" ™️ .

I can see that the ImageConfiguration struct from oci-spec-rs has a field to store the architecture, so I believe we just need to feed the correct value through to there.

From my 10 minutes of investigation, I am hoping we may be able to update ostree-ext-rs's Config struct to accept an architecture, extract the architecture from the struct in build_oci and feed it into the imgcfg object.

Once that is updated, we will have ostree-rs-ext exposing the options required for passing in an architecture to the build in the encapsulate function. Now we can update this repo to accept the --arch flag and pass it to the config which ends up being used in the call to ostree-ext-rs's encapsulate function

As mentioned previously, I do not know if this will work as I am intending it to, or if there are any other limitations which may prevent this but I'm hoping someone here does 😄

[cgwalters]

Well this would be trivial if we'd embedded the arch in the commit metadata...it looks like we tried to do that but never did. It's only there as coreos-assembler.basearch for coreos.

From my 10 minutes of investigation, I am hoping we may be able to update ostree-ext-rs's Config struct to accept an

Yes, I think that's what is needed.

---

(In the more medium term of course, a better fix is to switch the order of things; build a container image, and then unpack it into the ostree repo)

[p5]

I would like to have a shot at resolving this (at least the short-term fix).

Would you prefer a flag passed to container-encapsulate which accepts the arch, or do you think extracting the arch from a common label, such as "basearch", would be better?

I completely agree that long-term it would be best to have a "container-first" approach.

[cgwalters]

or do you think extracting the arch from a common label, such as "basearch", would be better?

I don't think we have anything like that in this process at the moment; it was a bad design choice for ostree not to try to standardize the architecture as part of the commit metadata. It's only encoded in the ref names conventionally, and parsing that would be possible but quite ugly.

Would you prefer a flag passed to container-encapsulate which accepts the arch,

I think it has to be this in the short term.

However there is a different fix: patch rpm-ostree to add ostree.architecture unconditionally, then we can teach ostree-ext to honor that in a clean way. That wouldn't fix existing ostree commits, but it would for new ones, and I suspect we could pretty quickly turn that around.

The patch for that on the ostree-ext side should just look like:

diff --git a/lib/src/container/encapsulate.rs b/lib/src/container/encapsulate.rs
index d411f5e1..cb8e9b02 100644
--- a/lib/src/container/encapsulate.rs
+++ b/lib/src/container/encapsulate.rs
@@ -15,6 +15,7 @@ use flate2::Compression;
 use fn_error_context::context;
 use gio::glib;
 use oci_spec::image as oci_image;
+use ocidir::oci_spec::image::Arch;
 use ocidir::{Layer, OciDir};
 use ostree::gio;
 use std::borrow::Cow;
@@ -158,6 +159,18 @@ pub(crate) fn export_chunked(
     Ok(())
 }
 
+fn linux_arch_to_goarch(a: &str) -> Arch {
+    // Copied from https://github.com/containers/oci-spec-rs/blob/71384a59eb7c4fdfbd503b9e1c28ecc663d30cb0/src/image/mod.rs#L435
+    let goarch = match a {
+        "x86_64" => "amd64",
+        "aarch64" => "arm64",
+        o => o,
+    };
+    Arch::from(goarch)
+}
+
 /// Generate an OCI image from a given ostree root
 #[context("Building oci")]
 #[allow(clippy::too_many_arguments)]
@@ -177,6 +190,7 @@ fn build_oci(
         0,
     )
     .unwrap();
+
     let commit_subject = commit_v.child_value(3);
     let commit_subject = commit_subject.str().ok_or_else(|| {
         anyhow::anyhow!(
@@ -186,6 +200,11 @@ fn build_oci(
     })?;
     let commit_meta = &commit_v.child_value(0);
     let commit_meta = glib::VariantDict::new(Some(commit_meta));
+    let mut ostree_arch = commit_meta.lookup::<String>(&ostree::COMMIT_META_KEY_ARCHITECTURE)?;
+    if ostree_arch.is_none() {
+        // Sadly we never standardized this in
+        ostree_arch = commit_meta.lookup::<String>("coreos-assembler.basearch")?;
+    };
 
     let mut ctrcfg = opts.container_config.clone().unwrap_or_default();
     let mut imgcfg = oci_image::ImageConfiguration::default();
@@ -275,7 +294,10 @@ fn build_oci(
     let ctrcfg = writer.write_config(imgcfg)?;
     manifest.set_config(ctrcfg);
     manifest.set_annotations(Some(labels));
-    let platform = oci_image::Platform::default();
+    let mut platform = oci_image::Platform::default();
+    if let Some(arch) = ostree_arch.as_ref() {
+        platform.set_architecture(linux_arch_to_goarch(arch));
+    }
     if let Some(tag) = tag {
         writer.insert_manifest(manifest, Some(tag), platform)?;
     } else {

(untested)

[cgwalters]

Just realized the powerpc handling there is wrong, we can't reference compile-time endianness of course. Edited.

[p5]

Took another look at this, and while I'm not 100% sure where this should fit in, or how to test this without an ARM machine, it somewhat makes sense to do the following on the rpm-ostree side:

diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs
index b43f0fc7..f11d7d82 100644
--- a/rust/src/treefile.rs
+++ b/rust/src/treefile.rs
@@ -132,6 +132,22 @@ fn treefile_parse_stream<R: io::Read>(
             .get_or_insert_with(BTreeMap::new)
             .insert("basearch".into(), VarValue::String(basearch.clone()));
     }
+
+    if let Some(variables) = &mut treefile.base.variables {
+        variables
+            .entry("ostree.architecture".into())
+            .or_insert_with(|| {
+                VarValue::String(
+                    treefile
+                        .base
+                        .basearch
+                        .as_deref()
+                        .unwrap_or(basearch.unwrap())
+                        .into(),
+                )
+            });
+    }
+
     if let Some(ref mut releasever) = treefile.base.releasever {
         treefile
             .base

This sets a variable for ostree.architecture to either the treefile's basearch (if set) or the value of the "basearch", which appears to be computed from the host's machine.

I know it compiles and the variable is being set with the correct KV pair, but not whether this is the right approach

[p5]

Unfortunately the patch to ostree-rs-ext doesn't work as expected.
After hardcoding the arch to aarch64 with the below changes, I still get an amd64 image.

diff --git a/lib/src/container/encapsulate.rs b/lib/src/container/encapsulate.rs
index d411f5e..d60928c 100644
--- a/lib/src/container/encapsulate.rs
+++ b/lib/src/container/encapsulate.rs
@@ -15,6 +15,7 @@ use flate2::Compression;
 use fn_error_context::context;
 use gio::glib;
 use oci_spec::image as oci_image;
+use ocidir::oci_spec::image::Arch;
 use ocidir::{Layer, OciDir};
 use ostree::gio;
 use std::borrow::Cow;
@@ -158,6 +159,16 @@ pub(crate) fn export_chunked(
     Ok(())
 }
 
+fn linux_arch_to_goarch(a: &str) -> Arch {
+    // Copied from https://github.com/containers/oci-spec-rs/blob/71384a59eb7c4fdfbd503b9e1c28ecc663d30cb0/src/image/mod.rs#L435
+    let goarch = match a {
+        "x86_64" => "amd64",
+        "aarch64" => "arm64",
+        o => o,
+    };
+    Arch::from(goarch)
+}
+
 /// Generate an OCI image from a given ostree root
 #[context("Building oci")]
 #[allow(clippy::too_many_arguments)]
@@ -186,6 +197,13 @@ fn build_oci(
     })?;
     let commit_meta = &commit_v.child_value(0);
     let commit_meta = glib::VariantDict::new(Some(commit_meta));
+    let mut ostree_arch = commit_meta.lookup::<String>(&ostree::COMMIT_META_KEY_ARCHITECTURE)?;
+    if ostree_arch.is_none() {
+        // Sadly we never standardized this in
+        ostree_arch = commit_meta.lookup::<String>("coreos-assembler.basearch")?;
+    };
+    // For debugging, force aarch64
+    ostree_arch = Some("aarch64".to_string());
 
     let mut ctrcfg = opts.container_config.clone().unwrap_or_default();
     let mut imgcfg = oci_image::ImageConfiguration::default();
@@ -275,7 +293,11 @@ fn build_oci(
     let ctrcfg = writer.write_config(imgcfg)?;
     manifest.set_config(ctrcfg);
     manifest.set_annotations(Some(labels));
-    let platform = oci_image::Platform::default();
+    let mut platform = oci_image::Platform::default();
+    if let Some(arch) = ostree_arch.as_ref() {
+        platform.set_architecture(linux_arch_to_goarch(arch));
+    }
+    println!("Building image for platform: {:?}", platform);
     if let Some(tag) = tag {
         writer.insert_manifest(manifest, Some(tag), platform)?;
     } else {

I know the arch is getting set correctly since the debug println displays so:

⠁ Generating container image...
Building image for platform: Platform { architecture: ARM64, os: Linux, os_version: None, os_features: None, variant: None, features: None }
Generating container image... done
Pushed digest: sha256:2f7abd48dae71d971cf04d12f571849ed7114f13a82bb910a84addb9884a4c03