launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25074
[Merge] ~pappacena/launchpad:continue-oci-push-on-failure into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:continue-oci-push-on-failure into launchpad:master.
Commit message:
When an OCI recipe has multiple push rules, continue to the next push rule in case one of them fails to upload.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1888578 in Launchpad itself: "OCI build fails all push rules after the first failure"
https://bugs.launchpad.net/launchpad/+bug/1888578
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/387888
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:continue-oci-push-on-failure into launchpad:master.
diff --git a/lib/lp/oci/interfaces/ociregistryclient.py b/lib/lp/oci/interfaces/ociregistryclient.py
index 260f75b..184a93f 100644
--- a/lib/lp/oci/interfaces/ociregistryclient.py
+++ b/lib/lp/oci/interfaces/ociregistryclient.py
@@ -10,6 +10,7 @@ __all__ = [
'BlobUploadFailed',
'IOCIRegistryClient',
'ManifestUploadFailed',
+ 'MultipleOCIRegistryError',
'OCIRegistryError',
]
@@ -24,6 +25,19 @@ class OCIRegistryError(Exception):
self.errors = errors
+class MultipleOCIRegistryError(OCIRegistryError):
+ def __init__(self, exceptions):
+ self.exceptions = exceptions
+
+ def __str__(self):
+ return " / ".join(str(i) for i in self.exceptions)
+
+ @property
+ def errors(self):
+ return [i.errors for i in self.exceptions
+ if isinstance(i, OCIRegistryError)]
+
+
class BlobUploadFailed(OCIRegistryError):
pass
diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index 2ba6da1..0176a89 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -42,6 +42,7 @@ from zope.interface import implementer
from lp.oci.interfaces.ociregistryclient import (
BlobUploadFailed,
IOCIRegistryClient,
+ MultipleOCIRegistryError,
ManifestUploadFailed,
)
from lp.services.timeout import urlfetch
@@ -227,6 +228,59 @@ class OCIRegistryClient:
return "{}".format("edge")
@classmethod
+ def _upload_to_push_rule(
+ cls, push_rule, build, manifest, digests, preloaded_data):
+ http_client = RegistryHTTPClient.getInstance(push_rule)
+
+ for section in manifest:
+ # Work out names and tags
+ tag = cls._calculateTag(build, push_rule)
+ file_data = preloaded_data[section["Config"]]
+ config = file_data["config_file"]
+ # Upload the layers involved
+ for diff_id in config["rootfs"]["diff_ids"]:
+ cls._upload_layer(
+ diff_id,
+ push_rule,
+ file_data[diff_id],
+ http_client)
+ # The config file is required in different forms, so we can
+ # calculate the sha, work these out and upload
+ config_json = json.dumps(config).encode("UTF-8")
+ config_sha = hashlib.sha256(config_json).hexdigest()
+ cls._upload(
+ "sha256:{}".format(config_sha),
+ push_rule,
+ BytesIO(config_json),
+ http_client)
+
+ # Build the registry manifest from the image manifest
+ # and associated configs
+ registry_manifest = cls._build_registry_manifest(
+ digests, config, config_json, config_sha,
+ preloaded_data[section["Config"]])
+
+ # Upload the registry manifest
+ try:
+ manifest_response = http_client.requestPath(
+ "/manifests/{}".format(tag),
+ json=registry_manifest,
+ headers={
+ "Content-Type":
+ "application/"
+ "vnd.docker.distribution.manifest.v2+json"
+ },
+ method="PUT")
+ except HTTPError as http_error:
+ manifest_response = http_error.response
+ if manifest_response.status_code != 201:
+ raise cls._makeRegistryError(
+ ManifestUploadFailed,
+ "Failed to upload manifest for {} ({}) in {}".format(
+ build.recipe.name, push_rule.registry_url, build.id),
+ manifest_response)
+
+ @classmethod
def upload(cls, build):
"""Upload the artifacts from an OCIRecipeBuild to a registry.
@@ -244,56 +298,17 @@ class OCIRegistryClient:
# Preload the requested files
preloaded_data = cls._preloadFiles(build, manifest, digests)
+ exceptions = []
for push_rule in build.recipe.push_rules:
- http_client = RegistryHTTPClient.getInstance(push_rule)
-
- for section in manifest:
- # Work out names and tags
- tag = cls._calculateTag(build, push_rule)
- file_data = preloaded_data[section["Config"]]
- config = file_data["config_file"]
- # Upload the layers involved
- for diff_id in config["rootfs"]["diff_ids"]:
- cls._upload_layer(
- diff_id,
- push_rule,
- file_data[diff_id],
- http_client)
- # The config file is required in different forms, so we can
- # calculate the sha, work these out and upload
- config_json = json.dumps(config).encode("UTF-8")
- config_sha = hashlib.sha256(config_json).hexdigest()
- cls._upload(
- "sha256:{}".format(config_sha),
- push_rule,
- BytesIO(config_json),
- http_client)
-
- # Build the registry manifest from the image manifest
- # and associated configs
- registry_manifest = cls._build_registry_manifest(
- digests, config, config_json, config_sha,
- preloaded_data[section["Config"]])
-
- # Upload the registry manifest
- try:
- manifest_response = http_client.requestPath(
- "/manifests/{}".format(tag),
- json=registry_manifest,
- headers={
- "Content-Type":
- "application/"
- "vnd.docker.distribution.manifest.v2+json"
- },
- method="PUT")
- except HTTPError as http_error:
- manifest_response = http_error.response
- if manifest_response.status_code != 201:
- raise cls._makeRegistryError(
- ManifestUploadFailed,
- "Failed to upload manifest for {} in {}".format(
- build.recipe.name, build.id),
- manifest_response)
+ try:
+ cls._upload_to_push_rule(
+ push_rule, build, manifest, digests, preloaded_data)
+ except Exception as e:
+ exceptions.append(e)
+ if len(exceptions) == 1:
+ raise exceptions[0]
+ elif len(exceptions) > 1:
+ raise MultipleOCIRegistryError(exceptions)
class OCIRegistryAuthenticationError(Exception):
diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
index 2edec41..7459a43 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -38,6 +38,7 @@ from zope.security.proxy import removeSecurityProxy
from lp.oci.interfaces.ociregistryclient import (
BlobUploadFailed,
ManifestUploadFailed,
+ MultipleOCIRegistryError,
)
from lp.oci.model.ociregistryclient import (
BearerTokenRegistryClient,
@@ -216,6 +217,59 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
self.assertEqual(
http_client.credentials, ('test-username', 'test-password'))
+ @responses.activate
+ def test_upload_skip_failed_push_rule(self):
+ self._makeFiles()
+ upload_fixture = self.useFixture(MockPatch(
+ "lp.oci.model.ociregistryclient.OCIRegistryClient._upload"))
+ self.useFixture(MockPatch(
+ "lp.oci.model.ociregistryclient.OCIRegistryClient._upload_layer"))
+
+ push_rules = [
+ self.push_rule,
+ self.factory.makeOCIPushRule(recipe=self.build.recipe),
+ self.factory.makeOCIPushRule(recipe=self.build.recipe)]
+ # Set the first 2 rules to fail with 400 at the PUT operation.
+ for i, push_rule in enumerate(push_rules):
+ push_rule.registry_credentials.setCredentials({
+ "username": "test-username-%s" % i,
+ "password": "test-password-%s" % i
+ })
+ responses.add(
+ "GET", "%s/v2/" % push_rule.registry_url, status=200)
+
+ manifests_url = "{}/v2/{}/manifests/edge".format(
+ push_rule.registry_credentials.url, push_rule.image_name)
+ status = 400 if i < 2 else 201
+ responses.add("PUT", manifests_url, status=status)
+
+ error = self.assertRaises(
+ MultipleOCIRegistryError, self.client.upload, self.build)
+
+ # Check that it tried to call the upload for each one of the push rules
+ self.assertEqual(3, upload_fixture.mock.call_count)
+ used_credentials = {
+ args[0][-1].credentials
+ for args in upload_fixture.mock.call_args_list}
+ self.assertSetEqual(
+ {('test-username-0', 'test-password-0'),
+ ('test-username-1', 'test-password-1'),
+ ('test-username-2', 'test-password-2')},
+ used_credentials)
+
+ # Check that we received back an exception of the correct type.
+ self.assertIsInstance(error, MultipleOCIRegistryError)
+ self.assertEqual(2, len(error.errors))
+ self.assertEqual(2, len(error.exceptions))
+
+ expected_error_msg = (
+ "Failed to upload manifest for {recipe} ({url1}) in {build} / "
+ "Failed to upload manifest for {recipe} ({url2}) in {build}"
+ ).format(
+ recipe=self.build.recipe.name, build=self.build.id,
+ url1=push_rules[0].registry_url, url2=push_rules[1].registry_url)
+ self.assertEqual(expected_error_msg, str(error))
+
def test_preloadFiles(self):
self._makeFiles()
files = self.client._preloadFiles(
@@ -433,6 +487,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
responses.add(
"PUT", manifests_url, status=400, json={"errors": put_errors})
+ expected_msg = "Failed to upload manifest for {} ({}) in {}".format(
+ self.build.recipe.name, self.push_rule.registry_url, self.build.id)
self.assertThat(
partial(self.client.upload, self.build),
Raises(MatchesException(
@@ -440,9 +496,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
MatchesAll(
AfterPreprocessing(
str,
- Equals(
- "Failed to upload manifest for {} in {}".format(
- self.build.recipe.name, self.build.id))),
+ Equals(expected_msg)),
MatchesStructure.byEquality(errors=put_errors)))))
@responses.activate
@@ -462,6 +516,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
push_rule.image_name)
responses.add("PUT", manifests_url, status=200)
+ expected_msg = "Failed to upload manifest for {} ({}) in {}".format(
+ self.build.recipe.name, self.push_rule.registry_url, self.build.id)
self.assertThat(
partial(self.client.upload, self.build),
Raises(MatchesException(
@@ -469,9 +525,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
MatchesAll(
AfterPreprocessing(
str,
- Equals(
- "Failed to upload manifest for {} in {}".format(
- self.build.recipe.name, self.build.id))),
+ Equals(expected_msg)),
MatchesStructure(errors=Is(None))))))
