From 716e06190eb181f341a38482d74c142507f2f41b Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Jun 06 2024 20:02:02 +0000 Subject: containers: only create one manifest per repo, clarify the name After @jcline and I played around with this stuff a bit, it seems clear that: * We don't need to create a separate manifest for each 'target' * We can give the local copy of the manifest a disposable name At present we give the local manifests we create the exact name that would usually refer to the *remote* manifest they are ultimately published as. That is, we were doing something like: buildah manifest create quay.io/fedora/fedora:41 ... which is really kind of confusing, because usually doing e.g.: buildah manifest inspect quay.io/fedora/fedora:41 would show you the contents of the manifest *from quay.io*, but if you create a local manifest with that exact same name, that one will take precedence. It's a confusing mechanism and we don't *need* to name the manifests we're creating in this confusing way, so let's not, let's just give them names that are clearly local (inspired by podman), clearly temporary, and clearly tied to this tool (fiu). We were also creating a separate manifest per "target" - the various names on the various registries that we ultimately publish the manifest as - because this is what the bash script this code replaces did. But some testing indicates this really is not necessary. The manifests just refer to the images by their sha256sums, which of course are always the same. For e.g. if you do: buildah manifest create test1 quay.io/fedora/fedora:41-x86_64 buildah manifest create test2 registry.fedoraproject.org/fedora:41-x86_64 buildah manifest create test3 oci-archive:/path/to/source/image ...where /path/to/source/image is the image that we published to those two registries under the name `fedora:41-x86_64` via `skopeo copy`, those three manifests will all be exactly identical. So we can just create the manifest *once* per repo, and publish that same manifest to the different tags on the different repositories. Signed-off-by: Adam Williamson --- diff --git a/fedora_image_uploader/handler.py b/fedora_image_uploader/handler.py index 9117f09..15e106d 100644 --- a/fedora_image_uploader/handler.py +++ b/fedora_image_uploader/handler.py @@ -102,11 +102,23 @@ class Uploader: 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 repo, arch in self.container_repos.items(): + # our local name for the manifest we're creating + manref = f"localhost/fiu-temp-{repo}-{str(ffrel.relnum)}" + # we don't have the image files any more, so we'll just + # find them on the first registry + firstreg = self.conf["container"]["registries"][0] + imgrefbase = f"{firstreg}/{repo}:{str(ffrel.relnum)}" + # wipe the manifest if it exists already + _run(("buildah", "rmi", manref), failok=True) + # create the manifest with all arches + createargs = ["buildah", "manifest", "create", manref] + createargs.extend(f"{imgrefbase}-{arch}" for arch in self.container_repos[repo]) + _run(createargs) for registry in self.conf["container"]["registries"]: # something like "registry.fedoraproject.org/fedora:40" - regname = f"{registry}/{repo}:{str(ffrel.relnum)}" - targets = [regname] + mainref = f"{registry}/{repo}:{str(ffrel.relnum)}" + targets = [mainref] # we also create aliased manifests for rawhide and # latest stable if ffrel.release.lower() == "rawhide": @@ -114,26 +126,18 @@ class Uploader: 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 + # push the manifest to this target pushargs = ( "buildah", "manifest", "push", - target, + manref, f"docker://{target}", "--all", ) _run(pushargs) + # wipe the local copy + _run(("buildah", "rmi", manref), failok=True) def download_image(self, image: dict, dest_dir: str, decompress=False) -> str: """ diff --git a/tests/test_handler.py b/tests/test_handler.py index 0e995e3..56616f6 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -139,8 +139,21 @@ def test_containers(mock_subrun, fixtures_dir, compose): # them all, and assert it's empty at the end calls = [" ".join(call.args[0]) for call in mock_subrun.call_args_list] - for registry in ["registry.fedoraproject.org", "quay.io/fedora"]: - for exprepo, arches in expected_images.items(): + for exprepo, arches in expected_images.items(): + # find the manifest creation calls (these are just per repo, + # and get images from the first configured registry) + expmanref = f"localhost/fiu-temp-{exprepo}-{relnum}" + expimgbase = f"registry.fedoraproject.org/{exprepo}:{relnum}" + expcalls = ( + f"buildah rmi {expmanref}", + f"buildah manifest create {expmanref} " + + " ".join(f"{expimgbase}-{arch}" for arch in arches), + f"buildah rmi {expmanref}", + ) + for call in expcalls: + assert call in calls + calls.remove(call) + for registry in ["registry.fedoraproject.org", "quay.io/fedora"]: # find the expected calls to skopeo copy for arch in arches: ident = repotoid[exprepo] @@ -156,16 +169,10 @@ def test_containers(mock_subrun, fixtures_dir, compose): calls.remove(expcall) repopath = f"{registry}/{exprepo}:{relnum}" aliaspath = f"{registry}/{exprepo}:{alias}" - # find the expected calls to buildah + # find the expected calls to buildah for manifest publish expcalls = ( - f"buildah rmi {repopath}", - f"buildah rmi {aliaspath}", - f"buildah manifest create {repopath} " - + " ".join(f"{repopath}-{arch}" for arch in arches), - f"buildah manifest create {aliaspath} " - + " ".join(f"{repopath}-{arch}" for arch in arches), - f"buildah manifest push {repopath} docker://{repopath} --all", - f"buildah manifest push {aliaspath} docker://{aliaspath} --all", + f"buildah manifest push {expmanref} docker://{repopath} --all", + f"buildah manifest push {expmanref} docker://{aliaspath} --all", ) for call in expcalls: assert call in calls