From 73d3020ecbc0e9e80093c0cc9d082baafaf5f9bf Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Jun 17 2025 21:37:07 +0000 Subject: [PATCH 1/4] containers: refactor manifest creation and publishing Breaking this into two separate functions and hoisting the registry loop up makes it easier to adjust what gets pushed to each repository. This is prep work for pushing nightly build tags to Quay, which supports annotations for garbage collection. Signed-off-by: Jeremy Cline --- diff --git a/fedora-image-uploader/fedora_image_uploader/handler.py b/fedora-image-uploader/fedora_image_uploader/handler.py index db4663e..1de3cc3 100644 --- a/fedora-image-uploader/fedora_image_uploader/handler.py +++ b/fedora-image-uploader/fedora_image_uploader/handler.py @@ -6,6 +6,7 @@ import subprocess import tempfile import time from collections.abc import Iterable +from dataclasses import dataclass from fedfind import exceptions as ff_exceptions from fedfind import helpers as ff_helpers @@ -39,6 +40,45 @@ except ImportError: DOCKER_ARCHES = {"amd64": "x86_64", "arm64": "aarch64", "ppc64le": "ppc64le", "s390x": "s390x"} +@dataclass +class Manifest: + """ + A container manifest, or index or something like that. + + Someone who knows what these are can write better docs. + """ + + index: dict + + def as_json(self) -> bytes: + return json.dumps(self.index, indent=4).encode("utf8") + + def sha256sum(self) -> str: + return hashlib.sha256(self.as_json()).hexdigest() + + def archive_index(self) -> dict: + return { + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.index.v1+json", + "manifests": [ + { + "mediaType": "application/vnd.oci.image.index.v1+json", + "digest": f"sha256:{self.sha256sum()}", + "size": len(self.as_json()), + } + ], + } + + def write_to_dir(self, image_dir: str): + blobdir = os.path.join(image_dir, "blobs/sha256") + os.makedirs(blobdir) + with open(os.path.join(blobdir, self.sha256sum()), "wb") as f: + f.write(self.as_json()) + + with open(os.path.join(image_dir, "index.json"), "w") as f: + json.dump(self.archive_index(), f, indent=4) + + def _run(args: Iterable[str], failok=False) -> subprocess.CompletedProcess: """Run a command and handle errors.""" _log.debug("image_uploader running command %s", " ".join(args)) @@ -173,22 +213,7 @@ class Uploader: if self.container_repos: self.publish_container_manifests(ffrel) - def create_and_publish_container_manifest( - self, repo: str, refs: Iterable[tuple[str, str]], tags: Iterable[str] - ): - """ - Create a container manifest for a given set of refs. - - Mostly copied from - https://pagure.io/koji-flatpak/raw/bb45b055c47d199b21c6b163e33048dda7684289/ - f/koji_flatpak/plugins/flatpak_builder_plugin.py - License: LGPL v2.1 (not plus) - We do this ourselves instead of using podman or buildah because - they do not work in infra openshift containers. See - https://github.com/containers/buildah/issues/5750 for some - background. - """ - + def create_container_manifest(self, refs: Iterable[tuple[str, str]]) -> Manifest: def manifest_descriptor(ref, arch): matched = False for darch, oarch in DOCKER_ARCHES.items(): @@ -218,58 +243,52 @@ class Uploader: }, } - index = { - "schemaVersion": 2, - "mediaType": "application/vnd.oci.image.index.v1+json", - "manifests": [manifest_descriptor(ref, arch) for (ref, arch) in refs], - } - - index_contents = json.dumps(index, indent=4).encode("utf8") - index_contents_digest = hashlib.sha256(index_contents).hexdigest() + return Manifest( + index={ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.index.v1+json", + "manifests": [manifest_descriptor(ref, arch) for (ref, arch) in refs], + } + ) - archive_index = { - "schemaVersion": 2, - "mediaType": "application/vnd.oci.image.index.v1+json", - "manifests": [ - { - "mediaType": "application/vnd.oci.image.index.v1+json", - "digest": f"sha256:{index_contents_digest}", - "size": len(index_contents), - } - ], - } + def publish_container_manifest( + self, registry: dict, repo: str, index: Manifest, tags: Iterable[str] + ): + """ + Create a container manifest for a given set of refs. + Mostly copied from + https://pagure.io/koji-flatpak/raw/bb45b055c47d199b21c6b163e33048dda7684289/ + f/koji_flatpak/plugins/flatpak_builder_plugin.py + License: LGPL v2.1 (not plus) + We do this ourselves instead of using podman or buildah because + they do not work in infra openshift containers. See + https://github.com/containers/buildah/issues/5750 for some + background. + """ with tempfile.TemporaryDirectory() as workdir: image_dir = os.path.join(workdir, "index_image") - blobdir = os.path.join(image_dir, "blobs/sha256") - os.makedirs(blobdir) - with open(os.path.join(blobdir, index_contents_digest), "wb") as f: - f.write(index_contents) - - with open(os.path.join(image_dir, "index.json"), "w") as f: - json.dump(archive_index, f, indent=4) - - for registry in self.conf["container"]["registries"]: - prefix = registry["credential_prefix"] - registry = registry["url"] - try: - cert_dir = os.environ[f"{prefix}CERT_DIR"] - except KeyError: - cert_dir = None - # something like "docker://registry.fedoraproject.org/fedora:40" - for target in [f"docker://{registry}/{repo}:{tag}" for tag in tags]: - pushargs = [ - "skopeo", - "copy", - ] - if cert_dir: - pushargs.append(f"--dest-cert-dir={cert_dir}") - pushargs += [ - f"oci:{image_dir}", - target, - "--multi-arch=index-only", - ] - _run(pushargs) + index.write_to_dir(image_dir) + prefix = registry["credential_prefix"] + registry = registry["url"] + try: + cert_dir = os.environ[f"{prefix}CERT_DIR"] + except KeyError: + cert_dir = None + # something like "docker://registry.fedoraproject.org/fedora:40" + for target in [f"docker://{registry}/{repo}:{tag}" for tag in tags]: + pushargs = [ + "skopeo", + "copy", + ] + if cert_dir: + pushargs.append(f"--dest-cert-dir={cert_dir}") + pushargs += [ + f"oci:{image_dir}", + target, + "--multi-arch=index-only", + ] + _run(pushargs) def publish_container_manifests(self, ffrel: ff_release.Release): """ @@ -317,7 +336,10 @@ class Uploader: # the next repo continue refs = [(f"{firstregref}-{arch}", arch) for arch in builtarches] - self.create_and_publish_container_manifest(repo, refs, tags) + index = self.create_container_manifest(refs) + + for registry in self.conf["container"]["registries"]: + self.publish_container_manifest(registry, repo, index, tags) message = ContainerPublishedV1( topic=".".join( From 233c7a91728b6617bb3ab1d7535e0fb74ab8a352 Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Jun 18 2025 17:10:37 +0000 Subject: [PATCH 2/4] containers: push nightly tags to quay.io with expiry annotations This adjusts the tags pushed to quay.io to include nightly tags set to expire when the image reaches EOL. Signed-off-by: Jeremy Cline --- diff --git a/fedora-image-uploader/fedora_image_uploader/handler.py b/fedora-image-uploader/fedora_image_uploader/handler.py index 1de3cc3..6372449 100644 --- a/fedora-image-uploader/fedora_image_uploader/handler.py +++ b/fedora-image-uploader/fedora_image_uploader/handler.py @@ -1,3 +1,4 @@ +import datetime import hashlib import json import logging @@ -7,6 +8,7 @@ import tempfile import time from collections.abc import Iterable from dataclasses import dataclass +from typing import Optional from fedfind import exceptions as ff_exceptions from fedfind import helpers as ff_helpers @@ -16,7 +18,13 @@ from fedora_messaging import config from fedora_messaging import exceptions as fm_exceptions from fedora_messaging import message as fm_message -from .utils import download_image, fallible_publish, get_milestone +from .utils import ( + download_image, + fallible_publish, + get_eol, + get_milestone, + parse_release, +) _log = logging.getLogger(__name__) @@ -50,6 +58,15 @@ class Manifest: index: dict + def annotations(self, annotations: Optional[dict[str, str]]): + if annotations is None: + try: + del self.index["annotations"] + except KeyError: + pass + else: + self.index["annotations"] = annotations + def as_json(self) -> bytes: return json.dumps(self.index, indent=4).encode("utf8") @@ -302,10 +319,19 @@ class Uploader: """ # Determine the manifest names to push; we always push the release number, # and if it's rawhide or the latest stable release, it has an alias as well. - tags = [str(ffrel.relnum)] + # For quay.io, we also push a nightly tag with garbage collection annotations. + _, y, z = parse_release(ffrel) + nightly_tag = f"{str(ffrel.relnum)}.{y}.{z}" + eol = get_eol(ffrel) + if eol is None: + quay_annotations = {"quay.expires-after": "90d"} + else: + delta_to_eol = eol - datetime.datetime.today() + quay_annotations = {"quay.expires-after": f"{delta_to_eol.days}d"} + tags = [str(ffrel.relnum), nightly_tag] if ffrel.release.lower() == "eln": - # per sgallagh ELN wants only a 'latest' tag, nothing else - tags = ["latest"] + # per sgallagh ELN wants only a 'latest' and nightly tags, nothing else + tags = ["latest", nightly_tag] elif ffrel.release.lower() == "rawhide": tags.append("rawhide") elif ffrel.relnum == ff_helpers.get_current_release(branched=True) + 1: @@ -339,7 +365,15 @@ class Uploader: index = self.create_container_manifest(refs) for registry in self.conf["container"]["registries"]: - self.publish_container_manifest(registry, repo, index, tags) + push_tags = tags.copy() + # Don't push nightly tags to registry.fedoraproject.org as we have no + # cleanup mechanism for them. + if "fedoraproject.org" in registry["url"]: + push_tags = [t for t in push_tags if t != nightly_tag] + index.annotations(None) + if "quay.io" in registry["url"]: + index.annotations(quay_annotations) + self.publish_container_manifest(registry, repo, index, push_tags) message = ContainerPublishedV1( topic=".".join( diff --git a/fedora-image-uploader/tests/test_handler.py b/fedora-image-uploader/tests/test_handler.py index 1b914ac..2e86bc5 100644 --- a/fedora-image-uploader/tests/test_handler.py +++ b/fedora-image-uploader/tests/test_handler.py @@ -11,7 +11,7 @@ from fedora_messaging import testing as fm_testing from freezegun import freeze_time from fedora_image_uploader import Uploader -from fedora_image_uploader.handler import _run +from fedora_image_uploader.handler import Manifest, _run @pytest.mark.vcr @@ -46,6 +46,7 @@ from fedora_image_uploader.handler import _run lambda b, c, decompress: f"/test/{os.path.basename(b['path'].removesuffix('.xz'))}", ) @mock.patch("fedora_image_uploader.handler.Uploader._missing_manifest_arches", return_value=set()) +@mock.patch("fedora_image_uploader.handler.get_eol", return_value=None) @mock.patch("fedora_image_uploader.handler.Uploader.registry_login") @mock.patch("subprocess.run") @pytest.mark.parametrize( @@ -69,6 +70,7 @@ from fedora_image_uploader.handler import _run "fedora-sericea": ["aarch64", "x86_64"], "fedora-silverblue": ["aarch64", "x86_64"], }, + ["43", "43.20250526.0", "rawhide"], # expected tags ), ( "messages/rc_compose.json", @@ -86,6 +88,7 @@ from fedora_image_uploader.handler import _run "fedora-sericea": ["aarch64", "x86_64"], "fedora-silverblue": ["aarch64", "x86_64"], }, + ["42", "42.1.1", "latest"], # expected tags ), ( "messages/eln_compose.json", @@ -97,6 +100,7 @@ from fedora_image_uploader.handler import _run { "eln": ["aarch64", "ppc64le", "s390x", "x86_64"], }, + ["latest", "11.20250527.0"], ), ( "messages/iot_compose.json", @@ -109,18 +113,26 @@ from fedora_image_uploader.handler import _run "fedora-iot": ["aarch64", "x86_64"], "fedora-bootc": ["aarch64", "ppc64le", "s390x", "x86_64"], }, + ["43", "43.20250527.0", "rawhide"], # expected tags ), ], ) def test_containers( - mock_subrun, _mock_login, mock_missing, fixtures_dir, caplog, compose, consistent_tempdir + mock_subrun, + _mock_login, + mock_get_eol, + mock_missing, + fixtures_dir, + caplog, + compose, + consistent_tempdir, ): mock_subrun.return_value.returncode = 0 # this is for the `skopeo inspect --raw` command we use to inspect # the single-image manifests we just pushed when creating the # multi-arch manifest mock_subrun.return_value.stdout = '{"somejson": "something"}' - message_file, cid, fnver, relnum, alias, milestone, expected_images = compose + message_file, cid, fnver, relnum, alias, milestone, expected_images, exptags = compose # mapping of reponames to expected image filename base strings repotoid = { "eln": "Fedora-ELN-Container-Base-Generic", @@ -139,14 +151,8 @@ def test_containers( msg = message.load_message(json.load(fd)) consumer = Uploader() - if milestone == "eln": - # we don't do a relnum tag for ELN, so the first ref has the - # alias, not the relnum - exptags = [alias] - maintag = alias - else: - exptags = [relnum, alias] - maintag = relnum + maintag = exptags[0] + nightlytag = exptags[1] expected_messages = [ ContainerPublishedV1( topic=(f"fedora_image_uploader.published.v1.container.{milestone}.{repo}"), @@ -203,7 +209,13 @@ def test_containers( # find the expected skopeo calls for manifest publish, we cannot # match these precisely as they have randomly-generated temporary # directory names in them - for exptag in exptags: + # + # The tag filtering is a gross hack, sorry. + if registry == "registry.fedoraproject.org": + tags = [t for t in exptags if t != nightlytag] + else: + tags = exptags + for exptag in tags: mancall = [ call for call in runcalls @@ -220,20 +232,34 @@ def test_containers( indexfname = os.path.join(manifestpath, "index.json") with open(indexfname, "r", encoding="utf-8") as indexfh: index = json.load(indexfh) - # as our mock _run always returns the same fake manifest, the - # expected contents of the index depends only on the number of - # images (arches). we can only see the JSON for the *last* repo - # processed at this point + blobfname = os.path.join( + manifestpath, "blobs/sha256/", index["manifests"][0]["digest"].split(":")[-1] + ) + with open(blobfname, "r", encoding="utf-8") as blobf: + blob = json.load(blobf) + # our mock _run always returns the same fake manifest, but annotations are pushed to + # the quay.io registry and not to registry.fedoraproject.org. + # We can only see the JSON for the *last* repo processed at this point so handle both + # cases. arches = list(expected_images.items())[-1][1] - if len(arches) == 3: + if len(arches) == 3 and "annotations" not in blob: + expdigest = "sha256:83f8fcc80dcc6661fad15a22e52db8c758b3be02b774584f26e0000207bf95ca" + explen = 1085 + if len(arches) == 3 and "annotations" in blob: expdigest = "sha256:83f8fcc80dcc6661fad15a22e52db8c758b3be02b774584f26e0000207bf95ca" explen = 1085 - elif len(arches) == 4: + elif len(arches) == 4 and "annotations" not in blob: expdigest = "sha256:da3ea76329f68466e4ebb8e64ed26af1f4df902faee63aed88ff39f40d20fe03" explen = 1409 - elif len(arches) == 2: + elif len(arches) == 4 and "annotations" in blob: + expdigest = "sha256:ef9795771ddd3acd948b67a9b666fc71afcfa36d34f59cbddb6e3d9f88d84a25" + explen = 1473 + elif len(arches) == 2 and "annotations" not in blob: expdigest = "sha256:7e1f3e15a3c41961e308487c9cccf09b40cb4743fa1f134ac9620b019dd392af" explen = 759 + elif len(arches) == 2 and "annotations" in blob: + expdigest = "sha256:f2d41c88156c6307afc98607a8bab3bbb7e1c7811907e80dff883eaed399045c" + explen = 823 else: expdigest = f"FIXME PLEASE UPDATE FOR {str(len(arches))} ARCHES" explen = 0 @@ -447,3 +473,35 @@ def test_run_error(caplog): with freeze_time("2024-05-28", auto_tick_seconds=10000): _run(("ls",)) assert caplog.records[-1].getMessage() == "Command: ls timed out after two hours" + + +def test_manifest_annotations(): + """Test annotations can be added or removed from an index""" + manifest = Manifest(index={"not": "real"}) + + # Setting it to none even when there are no annotations should be fine + manifest.annotations(None) + + manifest.annotations({"a": "note"}) + assert "annotations" in manifest.index + assert manifest.index["annotations"] == {"a": "note"} + + manifest.annotations(None) + assert "annotations" not in manifest.index + + +def test_manifest_write_to_dir(): + manifest = Manifest(index={"not": "real"}) + + with tempfile.TemporaryDirectory() as testdir: + manifest.write_to_dir(testdir) + expected_manifest_path = os.path.join(testdir, f"blobs/sha256/{manifest.sha256sum()}") + expected_index_path = os.path.join(testdir, "index.json") + + assert os.path.isfile(expected_manifest_path) + with open(expected_index_path) as f: + assert f.read() == manifest.as_json() + + assert os.path.isfile(expected_index_path) + with open(expected_index_path) as f: + assert f.read() == manifest.archive_index() From cbfbd518b01494f223b81253f467e2d5918fcc8c Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Jul 15 2025 20:15:47 +0000 Subject: [PATCH 3/4] Tweak container manifest name and docblock Suggested-by: Adam Williamson Signed-off-by: Jeremy Cline --- diff --git a/fedora-image-uploader/fedora_image_uploader/handler.py b/fedora-image-uploader/fedora_image_uploader/handler.py index 6372449..8c6d69b 100644 --- a/fedora-image-uploader/fedora_image_uploader/handler.py +++ b/fedora-image-uploader/fedora_image_uploader/handler.py @@ -49,11 +49,14 @@ DOCKER_ARCHES = {"amd64": "x86_64", "arm64": "aarch64", "ppc64le": "ppc64le", "s @dataclass -class Manifest: +class FatManifest: """ - A container manifest, or index or something like that. + A "fat" container manifest. - Someone who knows what these are can write better docs. + It contains an "image index" pointing to multiple individual image + manifests, one per arch, so forming a multiple-architecture manifest. + + See https://specs.opencontainers.org/image-spec/manifest/. """ index: dict @@ -230,7 +233,7 @@ class Uploader: if self.container_repos: self.publish_container_manifests(ffrel) - def create_container_manifest(self, refs: Iterable[tuple[str, str]]) -> Manifest: + def create_container_manifest(self, refs: Iterable[tuple[str, str]]) -> FatManifest: def manifest_descriptor(ref, arch): matched = False for darch, oarch in DOCKER_ARCHES.items(): @@ -260,7 +263,7 @@ class Uploader: }, } - return Manifest( + return FatManifest( index={ "schemaVersion": 2, "mediaType": "application/vnd.oci.image.index.v1+json", @@ -269,7 +272,7 @@ class Uploader: ) def publish_container_manifest( - self, registry: dict, repo: str, index: Manifest, tags: Iterable[str] + self, registry: dict, repo: str, index: FatManifest, tags: Iterable[str] ): """ Create a container manifest for a given set of refs. diff --git a/fedora-image-uploader/tests/test_handler.py b/fedora-image-uploader/tests/test_handler.py index 2e86bc5..f61f54b 100644 --- a/fedora-image-uploader/tests/test_handler.py +++ b/fedora-image-uploader/tests/test_handler.py @@ -11,7 +11,7 @@ from fedora_messaging import testing as fm_testing from freezegun import freeze_time from fedora_image_uploader import Uploader -from fedora_image_uploader.handler import Manifest, _run +from fedora_image_uploader.handler import FatManifest, _run @pytest.mark.vcr @@ -477,7 +477,7 @@ def test_run_error(caplog): def test_manifest_annotations(): """Test annotations can be added or removed from an index""" - manifest = Manifest(index={"not": "real"}) + manifest = FatManifest(index={"not": "real"}) # Setting it to none even when there are no annotations should be fine manifest.annotations(None) @@ -491,7 +491,7 @@ def test_manifest_annotations(): def test_manifest_write_to_dir(): - manifest = Manifest(index={"not": "real"}) + manifest = FatManifest(index={"not": "real"}) with tempfile.TemporaryDirectory() as testdir: manifest.write_to_dir(testdir) From 189b5ebc9b8190e29c12b166a479ffaeb5295c94 Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Jul 16 2025 13:46:55 +0000 Subject: [PATCH 4/4] Rename FatManifest::archive_index to image_index Signed-off-by: Jeremy Cline --- diff --git a/fedora-image-uploader/fedora_image_uploader/handler.py b/fedora-image-uploader/fedora_image_uploader/handler.py index 8c6d69b..3027a67 100644 --- a/fedora-image-uploader/fedora_image_uploader/handler.py +++ b/fedora-image-uploader/fedora_image_uploader/handler.py @@ -76,7 +76,7 @@ class FatManifest: def sha256sum(self) -> str: return hashlib.sha256(self.as_json()).hexdigest() - def archive_index(self) -> dict: + def image_index(self) -> dict: return { "schemaVersion": 2, "mediaType": "application/vnd.oci.image.index.v1+json", @@ -96,7 +96,7 @@ class FatManifest: f.write(self.as_json()) with open(os.path.join(image_dir, "index.json"), "w") as f: - json.dump(self.archive_index(), f, indent=4) + json.dump(self.image_index(), f, indent=4) def _run(args: Iterable[str], failok=False) -> subprocess.CompletedProcess: diff --git a/fedora-image-uploader/tests/test_handler.py b/fedora-image-uploader/tests/test_handler.py index f61f54b..774aa11 100644 --- a/fedora-image-uploader/tests/test_handler.py +++ b/fedora-image-uploader/tests/test_handler.py @@ -504,4 +504,4 @@ def test_manifest_write_to_dir(): assert os.path.isfile(expected_index_path) with open(expected_index_path) as f: - assert f.read() == manifest.archive_index() + assert f.read() == manifest.image_index()