Merge ~pappacena/launchpad:continue-oci-push-on-failure into launchpad:master

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: 47366ec0422076ba909d168f04227a412112fbd9
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:continue-oci-push-on-failure
Merge into: launchpad:master
Diff against target: 281 lines (+138/-55)
3 files modified
lib/lp/oci/interfaces/ociregistryclient.py (+14/-0)
lib/lp/oci/model/ociregistryclient.py (+64/-49)
lib/lp/oci/tests/test_ociregistryclient.py (+60/-6)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+387888@code.launchpad.net

Commit message

When an OCI recipe has multiple push rules, continue to the next push rule in case one of them fails to upload.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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__ = [
10 'BlobUploadFailed',10 'BlobUploadFailed',
11 'IOCIRegistryClient',11 'IOCIRegistryClient',
12 'ManifestUploadFailed',12 'ManifestUploadFailed',
13 'MultipleOCIRegistryError',
13 'OCIRegistryError',14 'OCIRegistryError',
14]15]
1516
@@ -24,6 +25,19 @@ class OCIRegistryError(Exception):
24 self.errors = errors25 self.errors = errors
2526
2627
28class MultipleOCIRegistryError(OCIRegistryError):
29 def __init__(self, exceptions):
30 self.exceptions = exceptions
31
32 def __str__(self):
33 return " / ".join(str(i) for i in self.exceptions)
34
35 @property
36 def errors(self):
37 return [i.errors for i in self.exceptions
38 if isinstance(i, OCIRegistryError)]
39
40
27class BlobUploadFailed(OCIRegistryError):41class BlobUploadFailed(OCIRegistryError):
28 pass42 pass
2943
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
42from lp.oci.interfaces.ociregistryclient import (42from lp.oci.interfaces.ociregistryclient import (
43 BlobUploadFailed,43 BlobUploadFailed,
44 IOCIRegistryClient,44 IOCIRegistryClient,
45 MultipleOCIRegistryError,
45 ManifestUploadFailed,46 ManifestUploadFailed,
46 )47 )
47from lp.services.timeout import urlfetch48from lp.services.timeout import urlfetch
@@ -227,6 +228,59 @@ class OCIRegistryClient:
227 return "{}".format("edge")228 return "{}".format("edge")
228229
229 @classmethod230 @classmethod
231 def _upload_to_push_rule(
232 cls, push_rule, build, manifest, digests, preloaded_data):
233 http_client = RegistryHTTPClient.getInstance(push_rule)
234
235 for section in manifest:
236 # Work out names and tags
237 tag = cls._calculateTag(build, push_rule)
238 file_data = preloaded_data[section["Config"]]
239 config = file_data["config_file"]
240 # Upload the layers involved
241 for diff_id in config["rootfs"]["diff_ids"]:
242 cls._upload_layer(
243 diff_id,
244 push_rule,
245 file_data[diff_id],
246 http_client)
247 # The config file is required in different forms, so we can
248 # calculate the sha, work these out and upload
249 config_json = json.dumps(config).encode("UTF-8")
250 config_sha = hashlib.sha256(config_json).hexdigest()
251 cls._upload(
252 "sha256:{}".format(config_sha),
253 push_rule,
254 BytesIO(config_json),
255 http_client)
256
257 # Build the registry manifest from the image manifest
258 # and associated configs
259 registry_manifest = cls._build_registry_manifest(
260 digests, config, config_json, config_sha,
261 preloaded_data[section["Config"]])
262
263 # Upload the registry manifest
264 try:
265 manifest_response = http_client.requestPath(
266 "/manifests/{}".format(tag),
267 json=registry_manifest,
268 headers={
269 "Content-Type":
270 "application/"
271 "vnd.docker.distribution.manifest.v2+json"
272 },
273 method="PUT")
274 except HTTPError as http_error:
275 manifest_response = http_error.response
276 if manifest_response.status_code != 201:
277 raise cls._makeRegistryError(
278 ManifestUploadFailed,
279 "Failed to upload manifest for {} ({}) in {}".format(
280 build.recipe.name, push_rule.registry_url, build.id),
281 manifest_response)
282
283 @classmethod
230 def upload(cls, build):284 def upload(cls, build):
231 """Upload the artifacts from an OCIRecipeBuild to a registry.285 """Upload the artifacts from an OCIRecipeBuild to a registry.
232286
@@ -244,56 +298,17 @@ class OCIRegistryClient:
244 # Preload the requested files298 # Preload the requested files
245 preloaded_data = cls._preloadFiles(build, manifest, digests)299 preloaded_data = cls._preloadFiles(build, manifest, digests)
246300
301 exceptions = []
247 for push_rule in build.recipe.push_rules:302 for push_rule in build.recipe.push_rules:
248 http_client = RegistryHTTPClient.getInstance(push_rule)303 try:
249304 cls._upload_to_push_rule(
250 for section in manifest:305 push_rule, build, manifest, digests, preloaded_data)
251 # Work out names and tags306 except Exception as e:
252 tag = cls._calculateTag(build, push_rule)307 exceptions.append(e)
253 file_data = preloaded_data[section["Config"]]308 if len(exceptions) == 1:
254 config = file_data["config_file"]309 raise exceptions[0]
255 # Upload the layers involved310 elif len(exceptions) > 1:
256 for diff_id in config["rootfs"]["diff_ids"]:311 raise MultipleOCIRegistryError(exceptions)
257 cls._upload_layer(
258 diff_id,
259 push_rule,
260 file_data[diff_id],
261 http_client)
262 # The config file is required in different forms, so we can
263 # calculate the sha, work these out and upload
264 config_json = json.dumps(config).encode("UTF-8")
265 config_sha = hashlib.sha256(config_json).hexdigest()
266 cls._upload(
267 "sha256:{}".format(config_sha),
268 push_rule,
269 BytesIO(config_json),
270 http_client)
271
272 # Build the registry manifest from the image manifest
273 # and associated configs
274 registry_manifest = cls._build_registry_manifest(
275 digests, config, config_json, config_sha,
276 preloaded_data[section["Config"]])
277
278 # Upload the registry manifest
279 try:
280 manifest_response = http_client.requestPath(
281 "/manifests/{}".format(tag),
282 json=registry_manifest,
283 headers={
284 "Content-Type":
285 "application/"
286 "vnd.docker.distribution.manifest.v2+json"
287 },
288 method="PUT")
289 except HTTPError as http_error:
290 manifest_response = http_error.response
291 if manifest_response.status_code != 201:
292 raise cls._makeRegistryError(
293 ManifestUploadFailed,
294 "Failed to upload manifest for {} in {}".format(
295 build.recipe.name, build.id),
296 manifest_response)
297312
298313
299class OCIRegistryAuthenticationError(Exception):314class 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
38from lp.oci.interfaces.ociregistryclient import (38from lp.oci.interfaces.ociregistryclient import (
39 BlobUploadFailed,39 BlobUploadFailed,
40 ManifestUploadFailed,40 ManifestUploadFailed,
41 MultipleOCIRegistryError,
41 )42 )
42from lp.oci.model.ociregistryclient import (43from lp.oci.model.ociregistryclient import (
43 BearerTokenRegistryClient,44 BearerTokenRegistryClient,
@@ -216,6 +217,59 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
216 self.assertEqual(217 self.assertEqual(
217 http_client.credentials, ('test-username', 'test-password'))218 http_client.credentials, ('test-username', 'test-password'))
218219
220 @responses.activate
221 def test_upload_skip_failed_push_rule(self):
222 self._makeFiles()
223 upload_fixture = self.useFixture(MockPatch(
224 "lp.oci.model.ociregistryclient.OCIRegistryClient._upload"))
225 self.useFixture(MockPatch(
226 "lp.oci.model.ociregistryclient.OCIRegistryClient._upload_layer"))
227
228 push_rules = [
229 self.push_rule,
230 self.factory.makeOCIPushRule(recipe=self.build.recipe),
231 self.factory.makeOCIPushRule(recipe=self.build.recipe)]
232 # Set the first 2 rules to fail with 400 at the PUT operation.
233 for i, push_rule in enumerate(push_rules):
234 push_rule.registry_credentials.setCredentials({
235 "username": "test-username-%s" % i,
236 "password": "test-password-%s" % i
237 })
238 responses.add(
239 "GET", "%s/v2/" % push_rule.registry_url, status=200)
240
241 manifests_url = "{}/v2/{}/manifests/edge".format(
242 push_rule.registry_credentials.url, push_rule.image_name)
243 status = 400 if i < 2 else 201
244 responses.add("PUT", manifests_url, status=status)
245
246 error = self.assertRaises(
247 MultipleOCIRegistryError, self.client.upload, self.build)
248
249 # Check that it tried to call the upload for each one of the push rules
250 self.assertEqual(3, upload_fixture.mock.call_count)
251 used_credentials = {
252 args[0][-1].credentials
253 for args in upload_fixture.mock.call_args_list}
254 self.assertSetEqual(
255 {('test-username-0', 'test-password-0'),
256 ('test-username-1', 'test-password-1'),
257 ('test-username-2', 'test-password-2')},
258 used_credentials)
259
260 # Check that we received back an exception of the correct type.
261 self.assertIsInstance(error, MultipleOCIRegistryError)
262 self.assertEqual(2, len(error.errors))
263 self.assertEqual(2, len(error.exceptions))
264
265 expected_error_msg = (
266 "Failed to upload manifest for {recipe} ({url1}) in {build} / "
267 "Failed to upload manifest for {recipe} ({url2}) in {build}"
268 ).format(
269 recipe=self.build.recipe.name, build=self.build.id,
270 url1=push_rules[0].registry_url, url2=push_rules[1].registry_url)
271 self.assertEqual(expected_error_msg, str(error))
272
219 def test_preloadFiles(self):273 def test_preloadFiles(self):
220 self._makeFiles()274 self._makeFiles()
221 files = self.client._preloadFiles(275 files = self.client._preloadFiles(
@@ -433,6 +487,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
433 responses.add(487 responses.add(
434 "PUT", manifests_url, status=400, json={"errors": put_errors})488 "PUT", manifests_url, status=400, json={"errors": put_errors})
435489
490 expected_msg = "Failed to upload manifest for {} ({}) in {}".format(
491 self.build.recipe.name, self.push_rule.registry_url, self.build.id)
436 self.assertThat(492 self.assertThat(
437 partial(self.client.upload, self.build),493 partial(self.client.upload, self.build),
438 Raises(MatchesException(494 Raises(MatchesException(
@@ -440,9 +496,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
440 MatchesAll(496 MatchesAll(
441 AfterPreprocessing(497 AfterPreprocessing(
442 str,498 str,
443 Equals(499 Equals(expected_msg)),
444 "Failed to upload manifest for {} in {}".format(
445 self.build.recipe.name, self.build.id))),
446 MatchesStructure.byEquality(errors=put_errors)))))500 MatchesStructure.byEquality(errors=put_errors)))))
447501
448 @responses.activate502 @responses.activate
@@ -462,6 +516,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
462 push_rule.image_name)516 push_rule.image_name)
463 responses.add("PUT", manifests_url, status=200)517 responses.add("PUT", manifests_url, status=200)
464518
519 expected_msg = "Failed to upload manifest for {} ({}) in {}".format(
520 self.build.recipe.name, self.push_rule.registry_url, self.build.id)
465 self.assertThat(521 self.assertThat(
466 partial(self.client.upload, self.build),522 partial(self.client.upload, self.build),
467 Raises(MatchesException(523 Raises(MatchesException(
@@ -469,9 +525,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
469 MatchesAll(525 MatchesAll(
470 AfterPreprocessing(526 AfterPreprocessing(
471 str,527 str,
472 Equals(528 Equals(expected_msg)),
473 "Failed to upload manifest for {} in {}".format(
474 self.build.recipe.name, self.build.id))),
475 MatchesStructure(errors=Is(None))))))529 MatchesStructure(errors=Is(None))))))
476530
477531