launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:oci-fix-multi-arch-uploads-vanishing into launchpad:master.

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

lp: #1929693

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/405376

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.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-fix-multi-arch-uploads-vanishing into launchpad:master.

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..88daf9e 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,
@@ -1006,6 +1005,71 @@ 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))
+
 
 class TestRegistryHTTPClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
                              TestCaseWithFactory):