Merge pull request #4598 from cgwalters/compose-if-not-exists

compose-image: Add `--initialize-mode`

Cargo.toml

@@ -50,7 +50,7 @@ camino = "1.1.6"
 cap-std-ext = "3.0"
 cap-primitives = "2"
 cap-std = { version = "2", features = ["fs_utf8"] }
-containers-image-proxy = "0.5.1"
+containers-image-proxy = { version = "0.5.1", features = ["proxy_v0_2_4"] }
 # Explicitly force on libc
 rustix = { version = "0.38", features = ["use-libc", "process", "fs"] }
 chrono = { version = "0.4.30", features = ["serde"] }

docs/container.md

@@ -146,15 +146,15 @@ In the future, this command may perform more operations.
 There is now an `rpm-ostree compose image` command which generates a new base image using a treefile:
 
 ```
-$ rpm-ostree compose image --initialize --format=ociarchive workstation-ostree-config/fedora-silverblue.yaml fedora-silverblue.ociarchive
+$ rpm-ostree compose image --initialize-mode=if-not-exists --format=ociarchive workstation-ostree-config/fedora-silverblue.yaml fedora-silverblue.ociarchive
 ```
 
-The `--initialize` command here will create a new image unconditionally.  If not provided,
-the target image must exist, and will be used for change detection.  You can also directly push
-to a registry:
+The `--initialize-mode=if-not-exists` command here is what you almost always want: to create
+the image if it doesn't exist, but to otherwise check for changes.  It isn't the default
+for historical reasons.
 
 ```
-$ rpm-ostree compose image --initialize --format=registry workstation-ostree-config/fedora-silverblue.yaml quay.io/example/exampleos:latest
+$ rpm-ostree compose image  --initialize-mode=if-not-exists --format=registry workstation-ostree-config/fedora-silverblue.yaml quay.io/example/exampleos:latest
 ```
 
 ## Converting OSTree commits to new base images

rust/src/compose.rs

@@ -40,6 +40,36 @@ impl Into<ostree_container::Transport> for OutputFormat {
     }
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum)]
+enum InitializeMode {
+    /// Require the image to already exist.  For backwards compatibility reasons, this is the default.
+    Query,
+    /// Always overwrite the target image, even if it already exists and there were no changes.
+    Always,
+    /// Error out if the target image does not already exist.
+    Never,
+    /// Initialize if the target image does not already exist.
+    IfNotExists,
+}
+
+impl std::fmt::Display for InitializeMode {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let s = match self {
+            InitializeMode::Query => "query",
+            InitializeMode::Always => "always",
+            InitializeMode::Never => "never",
+            InitializeMode::IfNotExists => "if-not-exists",
+        };
+        f.write_str(s)
+    }
+}
+
+impl Default for InitializeMode {
+    fn default() -> Self {
+        Self::Query
+    }
+}
+
 #[derive(Debug, Parser)]
 struct Opt {
     #[clap(long)]
@@ -57,10 +87,14 @@ struct Opt {
     #[clap(value_parser)]
     layer_repo: Option<Utf8PathBuf>,
 
-    #[clap(long, short = 'i')]
+    #[clap(long, short = 'i', conflicts_with = "initialize_mode")]
     /// Do not query previous image in target location; use this for the first build
     initialize: bool,
 
+    /// Control conditions under which the image is written
+    #[clap(long, conflicts_with = "initialize", default_value_t)]
+    initialize_mode: InitializeMode,
+
     #[clap(long, value_enum, default_value_t)]
     format: OutputFormat,
 
@@ -105,17 +139,12 @@ struct ImageMetadata {
 }
 
 /// Fetch the previous metadata from the container image metadata.
-fn fetch_previous_metadata(
+async fn fetch_previous_metadata(
     proxy: &containers_image_proxy::ImageProxy,
-    imgref: &ostree_container::ImageReference,
+    oi: &containers_image_proxy::OpenedImage,
 ) -> Result<ImageMetadata> {
-    let handle = tokio::runtime::Handle::current();
-    let (manifest, _digest, config) = handle.block_on(async {
-        let oi = &proxy.open_image(&imgref.to_string()).await?;
-        let (digest, manifest) = proxy.fetch_manifest(oi).await?;
-        let config = proxy.fetch_config(oi).await?;
-        Ok::<_, anyhow::Error>((manifest, digest, config))
-    })?;
+    let manifest = proxy.fetch_manifest(oi).await?.1;
+    let config = proxy.fetch_config(oi).await?;
     const INPUTHASH_KEY: &str = "rpmostree.inputhash";
     let labels = config
         .config()
@@ -186,9 +215,34 @@ pub(crate) fn compose_image(args: Vec<String>) -> CxxResult<()> {
         transport: opt.format.clone().into(),
         name: opt.output.to_string(),
     };
-    let previous_meta = (!opt.initialize)
-        .then(|| fetch_previous_metadata(&proxy, &target_imgref))
-        .transpose()?;
+    let previous_meta = if opt.initialize || matches!(opt.initialize_mode, InitializeMode::Always) {
+        None
+    } else {
+        assert!(!opt.initialize); // Checked by clap
+        let handle = tokio::runtime::Handle::current();
+        handle.block_on(async {
+            let oi = if matches!(opt.initialize_mode, InitializeMode::Query) {
+                // In the default query mode, we error if the image doesn't exist, so this always
+                // gets mapped to Some().
+                Some(proxy.open_image(&target_imgref.to_string()).await?)
+            } else {
+                // All other cases check the Option.
+                proxy
+                    .open_image_optional(&target_imgref.to_string())
+                    .await?
+            };
+            let meta = match (opt.initialize_mode, oi.as_ref()) {
+                (InitializeMode::Always, _) => unreachable!(), // Handled above
+                (InitializeMode::Query, None) => unreachable!(), // Handled above
+                (InitializeMode::Never, Some(_)) => anyhow::bail!("Target image already exists"),
+                (InitializeMode::IfNotExists | InitializeMode::Never, None) => None,
+                (InitializeMode::IfNotExists | InitializeMode::Query, Some(oi)) => {
+                    Some(fetch_previous_metadata(&proxy, oi).await?)
+                }
+            };
+            anyhow::Ok(meta)
+        })?
+    };
     let mut compose_args_extra = Vec::new();
     if let Some(m) = previous_meta.as_ref() {
         compose_args_extra.extend(["--previous-inputhash", m.inputhash.as_str()]);

tests/compose-image.sh

@@ -44,6 +44,9 @@ repos:
   - fedora  # Intentially using frozen GA repo
 EOF
 cp /etc/yum.repos.d/*.repo .
+if rpm-ostree compose image --cachedir=../cache-container --label=foo=bar --label=baz=blah --initialize-mode=never minimal.yaml minimal.ociarchive 2>/dev/null; then
+  fatal "built an image in --initialize-mode=never"
+fi
 rpm-ostree compose image --cachedir=../cache-container --label=foo=bar --label=baz=blah --initialize minimal.yaml minimal.ociarchive
 skopeo inspect oci-archive:minimal.ociarchive > inspect.json
 test $(jq -r '.Labels["foo"]' < inspect.json) = bar
@@ -72,18 +75,33 @@ repos:
   - fedora  # Intentially using frozen GA repo
 EOF
 cp /etc/yum.repos.d/*.repo .
-rpm-ostree compose image --cachedir=../cache --touch-if-changed=changed.stamp --initialize minimal.yaml minimal.ociarchive
+# Unfortunately, --initialize-mode=if-not-exists is broken with .ociarchive...
+rpm-ostree compose image --cachedir=../cache --touch-if-changed=changed.stamp --initialize-mode=always minimal.yaml minimal.ociarchive
 # TODO actually test this container image
 cd ..
 echo "ok minimal"
 
-# Next, test the full Fedora Silverblue config
+# Next, test the full Fedora Silverblue config, and also using an OCI directory
 test -d workstation-ostree-config || git clone --depth=1 https://pagure.io/workstation-ostree-config --branch "${BRANCH}"
-rpm-ostree compose image --cachedir=cache --touch-if-changed=changed.stamp --initialize workstation-ostree-config/fedora-silverblue.yaml fedora-silverblue.ociarchive
-skopeo inspect oci-archive:fedora-silverblue.ociarchive
+mkdir_oci() {
+  local d
+  d=$1
+  shift
+  mkdir $d
+  echo '{ "imageLayoutVersion": "1.0.0" }' > $d/oci-layout
+  echo '{ "schemaVersion": 2, "mediaType": "application/vnd.oci.image.index.v1+json", "manifests": []}' > $d/index.json
+  mkdir -p $d/blobs/sha256
+}
+destocidir=fedora-silverblue.oci
+rm "${destocidir}" -rf
+mkdir_oci "${destocidir}"
+destimg="${destocidir}:silverblue"
+# Sadly --if-not-exists is broken for oci: too
+rpm-ostree compose image --cachedir=cache --touch-if-changed=changed.stamp --initialize-mode=always --format=oci workstation-ostree-config/fedora-silverblue.yaml "${destimg}"
+skopeo inspect "oci:${destimg}"
 test -f changed.stamp
 rm -f changed.stamp
-rpm-ostree compose image --cachedir=cache --offline --touch-if-changed=changed.stamp workstation-ostree-config/fedora-silverblue.yaml fedora-silverblue.ociarchive | tee out.txt
+rpm-ostree compose image --cachedir=cache --offline --touch-if-changed=changed.stamp --initialize-mode=if-not-exists --format=oci workstation-ostree-config/fedora-silverblue.yaml "${destimg}"| tee out.txt
 test '!' -f changed.stamp
 assert_file_has_content_literal out.txt 'No apparent changes since previous commit'