Merge ~twom/launchpad:oci-fix-multi-arch-uploads-vanishing into launchpad:master

[Tom Wardill]

Upload single-arch to manifest digest, rather than final tag

lp: #1929693

Previously, we were uploading each build as they finished to the tag from the push rule, then uploading the multi-arch manifest as a final step.
If there was latency in the builds for different architectures finishing, this caused flapping over whether an image for an architecture that wasn't built yet existed.

Instead, upload the single-arch manifest to it's 'sha256:xxx' location, then refer to it when we updated the multi-arch manifest.

This ensures that an image will always exist for an architecture that has been built.

86d8f8a... by Tom Wardill on 2021-07-08

Test for uploading to a sha tag

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index 98b925e..9672875 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -296,6 +296,8 @@ class OCIRegistryClient:
         # Specifically the Schema 2 manifest.
         digest = None
         data = json.dumps(registry_manifest).encode("UTF-8")
+        if tag is None:
+            tag = "sha256:{}".format(hashlib.sha256(data).hexdigest())
         size = len(data)
         content_type = registry_manifest.get(
             "mediaType",
@@ -323,7 +325,8 @@ class OCIRegistryClient:
 
     @classmethod
     def _upload_to_push_rule(
-            cls, push_rule, build, manifest, digests, preloaded_data, tag):
+            cls, push_rule, build, manifest, digests, preloaded_data,
+            tag=None):
         http_client = RegistryHTTPClient.getInstance(push_rule)
 
         for section in manifest:
@@ -390,13 +393,12 @@ class OCIRegistryClient:
         exceptions = []
         try:
             for push_rule in build.recipe.push_rules:
-                for tag in cls._calculateTags(build.recipe):
-                    try:
-                        cls._upload_to_push_rule(
-                            push_rule, build, manifest, digests,
-                            preloaded_data, tag)
-                    except Exception as e:
-                        exceptions.append(e)
+                try:
+                    cls._upload_to_push_rule(
+                        push_rule, build, manifest, digests,
+                        preloaded_data, tag=None)
+                except Exception as e:
+                    exceptions.append(e)
             if len(exceptions) == 1:
                 raise exceptions[0]
             elif len(exceptions) > 1:
diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
index 286bdce..90f4db4 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -23,7 +23,6 @@ from requests.exceptions import (
     HTTPError,
     )
 import responses
-from tenacity import RetryError
 from testtools.matchers import (
     AfterPreprocessing,
     ContainsDict,
@@ -216,6 +215,9 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
 
         self.client.upload(self.build)
 
+        # We should have uploaded to the digest, not the tag
+        self.assertIn('sha256:', responses.calls[1].request.url)
+        self.assertNotIn('edge', responses.calls[1].request.url)
         request = json.loads(responses.calls[1].request.body.decode("UTF-8"))
 
         self.assertThat(request, MatchesDict({
@@ -1006,6 +1008,91 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
             HTTPError, self.client.uploadManifestList,
             build_request, [self.build])
 
+    @responses.activate
+    def test_multi_arch_manifest_with_existing_architectures(self):
+        """Ensure that an existing arch release does not vanish
+        while waiting for a new upload."""
+        current_manifest = {
+            "schemaVersion": 2,
+            "mediaType": "application/"
+                         "vnd.docker.distribution.manifest.list.v2+json",
+            "manifests": [{
+                "platform": {"os": "linux", "architecture": "386"},
+                "mediaType": "application/"
+                             "vnd.docker.distribution.manifest.v2+json",
+                "digest": "initial-386-digest",
+                "size": 110
+            }, {
+                "platform": {"os": "linux", "architecture": "amd64"},
+                "mediaType": "application/"
+                             "vnd.docker.distribution.manifest.v2+json",
+                "digest": "initial-amd64-digest",
+                "size": 220
+            }]
+        }
+
+        recipe = self.factory.makeOCIRecipe(git_ref=self.git_ref)
+        distroseries = self.factory.makeDistroSeries(
+            distribution=recipe.distribution, status=SeriesStatus.CURRENT)
+        for architecturetag, processor_name in (
+                ("amd64", "amd64"),
+                ("i386", "386"),
+                ):
+            self.factory.makeDistroArchSeries(
+                distroseries=distroseries, architecturetag=architecturetag,
+                processor=getUtility(IProcessorSet).getByName(processor_name))
+        build1 = self.factory.makeOCIRecipeBuild(
+            recipe=recipe, distro_arch_series=distroseries["amd64"])
+        build2 = self.factory.makeOCIRecipeBuild(
+            recipe=recipe, distro_arch_series=distroseries["i386"])
+
+        job = mock.Mock()
+        job.builds = [build1, build2]
+        job.uploaded_manifests = {
+            build1.id: {"digest": "new-build1-digest", "size": 1111},
+            build2.id: {"digest": "new-build2-digest", "size": 2222},
+        }
+        job_source = mock.Mock()
+        job_source.getByOCIRecipeAndID.return_value = job
+        self.useFixture(
+            ZopeUtilityFixture(job_source, IOCIRecipeRequestBuildsJobSource))
+        build_request = OCIRecipeBuildRequest(recipe, -1)
+
+        push_rule = self.factory.makeOCIPushRule(recipe=recipe)
+        responses.add(
+            "GET", "{}/v2/{}/manifests/v1.0-20.04_edge".format(
+                push_rule.registry_url, push_rule.image_name),
+            json=current_manifest,
+            status=200)
+        self.addManifestResponses(push_rule, status_code=201)
+
+        responses.add(
+            "GET", "{}/v2/".format(push_rule.registry_url), status=200)
+        self.addManifestResponses(push_rule, status_code=201)
+
+        self.client.uploadManifestList(build_request, [build1])
+        self.assertEqual(3, len(responses.calls))
+
+        # Check that we have the old manifest for 386,
+        # but the new one for amd64
+        self.assertEqual({
+            "schemaVersion": 2,
+            "mediaType": "application/"
+                         "vnd.docker.distribution.manifest.list.v2+json",
+            "manifests": [
+                {
+                    "platform": {"os": "linux", "architecture": "386"},
+                    "mediaType": "application"
+                                 "/vnd.docker.distribution.manifest.v2+json",
+                    "digest": "initial-386-digest", "size": 110},
+                {
+                    "platform": {"os": "linux", "architecture": "amd64"},
+                    "mediaType": "application"
+                                 "/vnd.docker.distribution.manifest.v2+json",
+                    "digest": "new-build1-digest", "size": 1111
+                }]
+        }, json.loads(responses.calls[2].request.body.decode("UTF-8")))
+
 
 class TestRegistryHTTPClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
                              TestCaseWithFactory):