diff --git a/cli/contributing/mirror/providers.py b/cli/contributing/mirror/providers.py index 7eed037c3d..6e1a4246d0 100644 --- a/cli/contributing/mirror/providers.py +++ b/cli/contributing/mirror/providers.py @@ -5,6 +5,7 @@ from abc import ABC, abstractmethod from typing import TYPE_CHECKING from utils.docker.mirror import mirror_image_base +from utils.docker.registry import manifest_digest if TYPE_CHECKING: import argparse @@ -70,6 +71,24 @@ class GHCRProvider(RegistryProvider): ) return r.returncode == 0 + def is_stale(self, image: ImageRef) -> bool: + """Return True if the mirrored tag no longer matches its upstream tag. + + Args: + image: the upstream image whose mirror copy is compared. + + Returns: + ``True`` only when both registries answer with a digest and the + digests differ, i.e. upstream re-pushed the tag after it was + mirrored. An unanswered side counts as current, so an upstream + outage or a tag deleted upstream never forces a copy. + """ + upstream = manifest_digest(f"{image.registry}/{image.name}", image.version) + if upstream is None: + return False + mirrored = manifest_digest(self.image_base(image), image.version) + return mirrored is not None and mirrored != upstream + def _run_copy(self, *, src: str, dest: str, extra: list[str] | None = None) -> None: cmd = [ "skopeo", diff --git a/cli/contributing/mirror/sync/__main__.py b/cli/contributing/mirror/sync/__main__.py index d453732b46..538d834e6a 100644 --- a/cli/contributing/mirror/sync/__main__.py +++ b/cli/contributing/mirror/sync/__main__.py @@ -95,10 +95,26 @@ def main() -> int: try: if args.only_missing and provider.tag_exists(img): + if not provider.is_stale(img): + print( + f"[mirror] {label}: destination exists, skipping ({dest})", + flush=True, + ) + continue print( - f"[mirror] {label}: destination exists, skipping ({dest})", + f"[mirror] {label}: upstream re-pushed the tag, refreshing " + f"{src} -> {dest}", flush=True, ) + try: + provider.mirror(img) + except subprocess.CalledProcessError as e: + print( + f"[mirror] {label}: refresh FAILED (exit={e.returncode}), " + "keeping the existing copy", + file=sys.stderr, + flush=True, + ) continue print(f"[mirror] {label}: {src} -> {dest}", flush=True) diff --git a/docs/contributing/artefact/image/mirror.md b/docs/contributing/artefact/image/mirror.md index 36ffe55616..85963d16c7 100644 --- a/docs/contributing/artefact/image/mirror.md +++ b/docs/contributing/artefact/image/mirror.md @@ -55,7 +55,7 @@ This file is consumed by the inventory creator to substitute mirror URLs into ho ### Sync 🔄 -`cli/contributing/mirror/sync/__main__.py` copies each image from the upstream source to the GHCR mirror destination using `skopeo copy`. Supports `--only-missing` to skip already-mirrored images and `--images-per-hour` for throttling. +`cli/contributing/mirror/sync/__main__.py` copies each image from the upstream source to the GHCR mirror destination using `skopeo copy`. Supports `--only-missing` to skip already-mirrored images and `--images-per-hour` for throttling. Under `--only-missing` an existing tag is still re-copied when its manifest digest differs from the upstream tag's, read with HEAD requests on both registries; a failed re-copy keeps the existing copy and does not fail the sync. ### Wait Script ⏳ diff --git a/tests/unit/python/cli/contributing/mirror/test_providers.py b/tests/unit/python/cli/contributing/mirror/test_providers.py index ed759f9be8..5f2afcd098 100644 --- a/tests/unit/python/cli/contributing/mirror/test_providers.py +++ b/tests/unit/python/cli/contributing/mirror/test_providers.py @@ -1,6 +1,7 @@ from __future__ import annotations import unittest +from unittest.mock import patch from cli.contributing.mirror.model import ImageRef from cli.contributing.mirror.providers import GHCRProvider @@ -60,5 +61,51 @@ class TestGHCRProviderImageBase(unittest.TestCase): self.assertEqual(provider.repository, "myrepo") +class TestGHCRProviderIsStale(unittest.TestCase): + UPSTREAM = "docker.io/roundcube/roundcubemail" + MIRROR = "ghcr.io/acme/myrepo/mirror/docker.io/roundcube/roundcubemail" + + def setUp(self) -> None: + self.image = ImageRef( + role="web-app-stalwart", + service="webmail", + name="roundcube/roundcubemail", + version="1.7.5-apache", + source="docker.io/roundcube/roundcubemail:1.7.5-apache", + registry="docker.io", + ) + self.provider = GHCRProvider("acme", "myrepo") + + def _is_stale(self, digests: dict) -> bool: + def fake(image: str, reference: str) -> str | None: + self.assertEqual(reference, "1.7.5-apache") + return digests[image] + + with patch( + "cli.contributing.mirror.providers.manifest_digest", side_effect=fake + ): + return self.provider.is_stale(self.image) + + def test_upstream_repushed_tag_makes_the_copy_stale(self) -> None: + self.assertTrue( + self._is_stale({self.UPSTREAM: "sha256:new", self.MIRROR: "sha256:old"}) + ) + + def test_matching_digests_are_current(self) -> None: + self.assertFalse( + self._is_stale({self.UPSTREAM: "sha256:same", self.MIRROR: "sha256:same"}) + ) + + def test_indeterminate_upstream_counts_as_current(self) -> None: + self.assertFalse( + self._is_stale({self.UPSTREAM: None, self.MIRROR: "sha256:old"}) + ) + + def test_indeterminate_mirror_counts_as_current(self) -> None: + self.assertFalse( + self._is_stale({self.UPSTREAM: "sha256:new", self.MIRROR: None}) + ) + + if __name__ == "__main__": # pragma: no cover unittest.main() diff --git a/tests/unit/python/cli/contributing/mirror/test_sync.py b/tests/unit/python/cli/contributing/mirror/test_sync.py index 2f8860eb69..5bf496a5d9 100644 --- a/tests/unit/python/cli/contributing/mirror/test_sync.py +++ b/tests/unit/python/cli/contributing/mirror/test_sync.py @@ -1,5 +1,6 @@ from __future__ import annotations +import subprocess import unittest from unittest.mock import patch @@ -8,20 +9,20 @@ from cli.contributing.mirror.model import ImageRef class TestMirrorSync(unittest.TestCase): - def test_only_missing_skips_existing_destination(self) -> None: - image = ImageRef( - role="web-app-nextcloud", - service="app", - name="nextcloud", - version="31.0.0", - source="docker.io/library/nextcloud:31.0.0", - registry="docker.io", - ) + IMAGE = ImageRef( + role="web-app-nextcloud", + service="app", + name="nextcloud", + version="31.0.0", + source="docker.io/library/nextcloud:31.0.0", + registry="docker.io", + ) + def _run_only_missing(self, *, stale: bool, mirror_error=None): with ( patch( "cli.contributing.mirror.sync.__main__.iter_role_images", - return_value=[image], + return_value=[self.IMAGE], ), patch( "cli.contributing.mirror.sync.__main__.iter_runner_images", @@ -35,7 +36,12 @@ class TestMirrorSync(unittest.TestCase): patch.object( sync_main.GHCRProvider, "tag_exists", return_value=True ) as mock_tag_exists, - patch.object(sync_main.GHCRProvider, "mirror") as mock_mirror, + patch.object( + sync_main.GHCRProvider, "is_stale", return_value=stale + ) as mock_is_stale, + patch.object( + sync_main.GHCRProvider, "mirror", side_effect=mirror_error + ) as mock_mirror, patch( "sys.argv", [ @@ -49,11 +55,30 @@ class TestMirrorSync(unittest.TestCase): ), ): result = sync_main.main() + mock_tag_exists.assert_called_once_with(self.IMAGE) + mock_is_stale.assert_called_once_with(self.IMAGE) + return result, mock_mirror + + def test_only_missing_skips_current_destination(self) -> None: + result, mock_mirror = self._run_only_missing(stale=False) self.assertEqual(result, 0) - mock_tag_exists.assert_called_once_with(image) mock_mirror.assert_not_called() + def test_only_missing_refreshes_a_stale_destination(self) -> None: + result, mock_mirror = self._run_only_missing(stale=True) + + self.assertEqual(result, 0) + mock_mirror.assert_called_once_with(self.IMAGE) + + def test_failed_refresh_keeps_the_existing_copy_without_failing(self) -> None: + error = subprocess.CalledProcessError(1, ["skopeo", "copy"]) + + result, mock_mirror = self._run_only_missing(stale=True, mirror_error=error) + + self.assertEqual(result, 0) + mock_mirror.assert_called_once_with(self.IMAGE) + def test_runner_images_are_mirrored_with_the_role_images(self) -> None: image = ImageRef( role="runner", diff --git a/tests/unit/python/utils/docker/test_registry.py b/tests/unit/python/utils/docker/test_registry.py index 022f31ff1e..fe91eac26e 100644 --- a/tests/unit/python/utils/docker/test_registry.py +++ b/tests/unit/python/utils/docker/test_registry.py @@ -94,6 +94,46 @@ class TestManifestExists(_IsolatedProbeCache): self.assertEqual(url, "https://quay.io/v2/keycloak/keycloak/manifests/26.6.3") +class TestManifestDigest(unittest.TestCase): + def test_returns_the_content_digest_of_a_head_request(self) -> None: + with mock.patch.object( + registry.urllib.request, + "urlopen", + return_value=_Resp( + status=200, headers={"Docker-Content-Digest": "sha256:abc"} + ), + ) as opened: + digest = registry.manifest_digest("roundcube/roundcubemail", "1.7.5-apache") + self.assertEqual(digest, "sha256:abc") + request = opened.call_args.args[0] + self.assertEqual(request.get_method(), "HEAD") + self.assertIn( + "application/vnd.oci.image.index.v1+json", request.headers["Accept"] + ) + self.assertEqual( + request.full_url, + "https://registry-1.docker.io/v2/roundcube/roundcubemail/manifests/1.7.5-apache", + ) + + def test_missing_tag_has_no_digest(self) -> None: + with mock.patch.object( + registry.urllib.request, "urlopen", side_effect=_http_error(404) + ): + self.assertIsNone(registry.manifest_digest("postgres", "999")) + + def test_rate_limit_has_no_digest(self) -> None: + with mock.patch.object( + registry.urllib.request, "urlopen", side_effect=_http_error(429) + ): + self.assertIsNone(registry.manifest_digest("postgres", "16")) + + def test_answer_without_digest_header_has_no_digest(self) -> None: + with mock.patch.object( + registry.urllib.request, "urlopen", return_value=_Resp(status=200) + ): + self.assertIsNone(registry.manifest_digest("postgres", "16")) + + class TestFetchRegistryTags(unittest.TestCase): def test_anonymous_tags(self) -> None: with mock.patch.object( diff --git a/utils/docker/registry.py b/utils/docker/registry.py index 2e3615c029..7878ce1537 100644 --- a/utils/docker/registry.py +++ b/utils/docker/registry.py @@ -419,6 +419,29 @@ def manifest_platforms_probed( return None, "none" +def manifest_digest(image: str, reference: str) -> str | None: + """Return the ``Docker-Content-Digest`` the registry serves for a reference. + + Args: + image: image name, with or without registry host. + reference: tag or digest. + + Returns: + The digest of the index or manifest behind ``image:reference``, read + with HEAD because Docker Hub counts every manifest GET against its pull + allowance. ``None`` for a missing tag and on any indeterminate outcome. + """ + resolved = _resolve(image) + if resolved is None: + return None + host, repo = resolved + url = f"https://{host}/v2/{quote(repo, safe='/')}/manifests/{quote(reference, safe='')}" + result = _request(url, repo, method="HEAD", accept=_MANIFEST_ACCEPT) + if result is None or result[0] != 200: + return None + return result[1].get("Docker-Content-Digest") or None + + def manifest_exists(image: str, reference: str) -> bool | None: """Whether ``image:reference`` resolves to a manifest.