From 3b27d14a2ecdfbe8f6923620af3c463d65ce3d48 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Jun 06 2024 19:18:29 +0000
Subject: [PATCH 1/2] Add `failok` arg for _run, use it for buildah rmi commands


buildah rmi fails if the image/manifest doesn't already exist.
This is fine, so we need a mode to say it's fine.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

---

diff --git a/fedora_image_uploader/handler.py b/fedora_image_uploader/handler.py
index defa8aa..f1f9c16 100644
--- a/fedora_image_uploader/handler.py
+++ b/fedora_image_uploader/handler.py
@@ -26,7 +26,7 @@ from . import PLAYBOOKS
 _log = logging.getLogger(__name__)
 
 
-def _run(args: Iterable[str]):
+def _run(args: Iterable[str], failok=False):
     """Run a command and handle errors."""
     _log.debug("image_uploader running command %s", " ".join(args))
     try:
@@ -37,7 +37,7 @@ def _run(args: Iterable[str]):
     except OSError as err:
         _log.error("Command: %s caused error %s", " ".join(args), err)
         raise fm_exceptions.Nack()
-    if ret.returncode:
+    if ret.returncode and not failok:
         _log.error("Command: %s returned %d", " ".join(args), ret.returncode)
         _log.error("stdout: %s", ret.stdout)
         _log.error("stderr: %s", ret.stderr)
@@ -104,7 +104,7 @@ class Uploader:
                         targets.append(f"{registry}/{repo}:latest")
                     for target in targets:
                         # wipe the manifest if it exists already
-                        _run(("buildah", "rmi", target))
+                        _run(("buildah", "rmi", target), failok=True)
                         # create the manifest with all arches
                         createargs = ["buildah", "manifest", "create", target]
                         createargs.extend(
diff --git a/tests/test_handler.py b/tests/test_handler.py
index 63638a0..0e995e3 100644
--- a/tests/test_handler.py
+++ b/tests/test_handler.py
@@ -506,6 +506,8 @@ def test_run_error(caplog):
         caplog.records[-1].getMessage()
         == "stderr: ls: cannot access '/some/where/that/doesnt/exist': No such file or directory\n"
     )
+    # this should *not* raise anything
+    _run(("ls", "/some/where/that/doesnt/exist"), failok=True)
     assert caplog.records[-2].getMessage() == "stdout: "
     assert caplog.records[-3].getMessage() == "Command: ls /some/where/that/doesnt/exist returned 2"
     with pytest.raises(exceptions.Nack):

From 55f622fbe8120fd843ca282329e9a7b9c7d8e821 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Jun 06 2024 19:20:29 +0000
Subject: [PATCH 2/2] Move container manifest publishing into its own method


This is just a bit cleaner.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

---

diff --git a/fedora_image_uploader/handler.py b/fedora_image_uploader/handler.py
index f1f9c16..9117f09 100644
--- a/fedora_image_uploader/handler.py
+++ b/fedora_image_uploader/handler.py
@@ -90,39 +90,50 @@ class Uploader:
             raise
 
         if self.container_repos:
-            # manifest stuff
-            for repo in self.container_repos:
-                for registry in self.conf["container"]["registries"]:
-                    # something like "registry.fedoraproject.org/fedora:40"
-                    regname = f"{registry}/{repo}:{str(ffrel.relnum)}"
-                    targets = [regname]
-                    # we also create aliased manifests for rawhide and
-                    # latest stable
-                    if ffrel.release.lower() == "rawhide":
-                        targets.append(f"{registry}/{repo}:rawhide")
-                    elif ffrel.relnum == ff_helpers.get_current_release(branched=False):
-                        targets.append(f"{registry}/{repo}:latest")
-                    for target in targets:
-                        # wipe the manifest if it exists already
-                        _run(("buildah", "rmi", target), failok=True)
-                        # create the manifest with all arches
-                        createargs = ["buildah", "manifest", "create", target]
-                        createargs.extend(
-                            # it's intentional that this is regname not target
-                            f"{regname}-{arch}"
-                            for arch in self.container_repos[repo]
-                        )
-                        _run(createargs)
-                        # push it
-                        pushargs = (
-                            "buildah",
-                            "manifest",
-                            "push",
-                            target,
-                            f"docker://{target}",
-                            "--all",
-                        )
-                        _run(pushargs)
+            self.publish_container_manifests(ffrel)
+
+    def publish_container_manifests(self, ffrel: ff_release.Release):
+        """
+        Generate and publish multi-arch container manifests. These are
+        the target most real-world consumers see - the thing they hit
+        when they ask for 'fedora:40', for instance. The manifest
+        points to the correct image for each of the arches we built an
+        image for, so you can ask for 'fedora:40' from any supported
+        arch and you'll get the correct image. This is how commands,
+        Containerfiles and so on can be arch-independent.
+        """
+        for repo in self.container_repos:
+            for registry in self.conf["container"]["registries"]:
+                # something like "registry.fedoraproject.org/fedora:40"
+                regname = f"{registry}/{repo}:{str(ffrel.relnum)}"
+                targets = [regname]
+                # we also create aliased manifests for rawhide and
+                # latest stable
+                if ffrel.release.lower() == "rawhide":
+                    targets.append(f"{registry}/{repo}:rawhide")
+                elif ffrel.relnum == ff_helpers.get_current_release(branched=False):
+                    targets.append(f"{registry}/{repo}:latest")
+                for target in targets:
+                    # wipe the manifest if it exists already
+                    _run(("buildah", "rmi", target), failok=True)
+                    # create the manifest with all arches
+                    createargs = ["buildah", "manifest", "create", target]
+                    createargs.extend(
+                        # it's intentional that this is regname not target
+                        f"{regname}-{arch}"
+                        for arch in self.container_repos[repo]
+                    )
+                    _run(createargs)
+                    # push it
+                    pushargs = (
+                        "buildah",
+                        "manifest",
+                        "push",
+                        target,
+                        f"docker://{target}",
+                        "--all",
+                    )
+                    _run(pushargs)
 
     def download_image(self, image: dict, dest_dir: str, decompress=False) -> str:
         """