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
1diff --git a/lib/lp/oci/interfaces/ociregistryclient.py b/lib/lp/oci/interfaces/ociregistryclient.py
2index 260f75b..184a93f 100644
3--- a/lib/lp/oci/interfaces/ociregistryclient.py
4+++ b/lib/lp/oci/interfaces/ociregistryclient.py
5@@ -10,6 +10,7 @@ __all__ = [
6 'BlobUploadFailed',
7 'IOCIRegistryClient',
8 'ManifestUploadFailed',
9+ 'MultipleOCIRegistryError',
10 'OCIRegistryError',
11 ]
12
13@@ -24,6 +25,19 @@ class OCIRegistryError(Exception):
14 self.errors = errors
15
16
17+class MultipleOCIRegistryError(OCIRegistryError):
18+ def __init__(self, exceptions):
19+ self.exceptions = exceptions
20+
21+ def __str__(self):
22+ return " / ".join(str(i) for i in self.exceptions)
23+
24+ @property
25+ def errors(self):
26+ return [i.errors for i in self.exceptions
27+ if isinstance(i, OCIRegistryError)]
28+
29+
30 class BlobUploadFailed(OCIRegistryError):
31 pass
32
33diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
34index 2ba6da1..0176a89 100644
35--- a/lib/lp/oci/model/ociregistryclient.py
36+++ b/lib/lp/oci/model/ociregistryclient.py
37@@ -42,6 +42,7 @@ from zope.interface import implementer
38 from lp.oci.interfaces.ociregistryclient import (
39 BlobUploadFailed,
40 IOCIRegistryClient,
41+ MultipleOCIRegistryError,
42 ManifestUploadFailed,
43 )
44 from lp.services.timeout import urlfetch
45@@ -227,6 +228,59 @@ class OCIRegistryClient:
46 return "{}".format("edge")
47
48 @classmethod
49+ def _upload_to_push_rule(
50+ cls, push_rule, build, manifest, digests, preloaded_data):
51+ http_client = RegistryHTTPClient.getInstance(push_rule)
52+
53+ for section in manifest:
54+ # Work out names and tags
55+ tag = cls._calculateTag(build, push_rule)
56+ file_data = preloaded_data[section["Config"]]
57+ config = file_data["config_file"]
58+ # Upload the layers involved
59+ for diff_id in config["rootfs"]["diff_ids"]:
60+ cls._upload_layer(
61+ diff_id,
62+ push_rule,
63+ file_data[diff_id],
64+ http_client)
65+ # The config file is required in different forms, so we can
66+ # calculate the sha, work these out and upload
67+ config_json = json.dumps(config).encode("UTF-8")
68+ config_sha = hashlib.sha256(config_json).hexdigest()
69+ cls._upload(
70+ "sha256:{}".format(config_sha),
71+ push_rule,
72+ BytesIO(config_json),
73+ http_client)
74+
75+ # Build the registry manifest from the image manifest
76+ # and associated configs
77+ registry_manifest = cls._build_registry_manifest(
78+ digests, config, config_json, config_sha,
79+ preloaded_data[section["Config"]])
80+
81+ # Upload the registry manifest
82+ try:
83+ manifest_response = http_client.requestPath(
84+ "/manifests/{}".format(tag),
85+ json=registry_manifest,
86+ headers={
87+ "Content-Type":
88+ "application/"
89+ "vnd.docker.distribution.manifest.v2+json"
90+ },
91+ method="PUT")
92+ except HTTPError as http_error:
93+ manifest_response = http_error.response
94+ if manifest_response.status_code != 201:
95+ raise cls._makeRegistryError(
96+ ManifestUploadFailed,
97+ "Failed to upload manifest for {} ({}) in {}".format(
98+ build.recipe.name, push_rule.registry_url, build.id),
99+ manifest_response)
100+
101+ @classmethod
102 def upload(cls, build):
103 """Upload the artifacts from an OCIRecipeBuild to a registry.
104
105@@ -244,56 +298,17 @@ class OCIRegistryClient:
106 # Preload the requested files
107 preloaded_data = cls._preloadFiles(build, manifest, digests)
108
109+ exceptions = []
110 for push_rule in build.recipe.push_rules:
111- http_client = RegistryHTTPClient.getInstance(push_rule)
112-
113- for section in manifest:
114- # Work out names and tags
115- tag = cls._calculateTag(build, push_rule)
116- file_data = preloaded_data[section["Config"]]
117- config = file_data["config_file"]
118- # Upload the layers involved
119- for diff_id in config["rootfs"]["diff_ids"]:
120- cls._upload_layer(
121- diff_id,
122- push_rule,
123- file_data[diff_id],
124- http_client)
125- # The config file is required in different forms, so we can
126- # calculate the sha, work these out and upload
127- config_json = json.dumps(config).encode("UTF-8")
128- config_sha = hashlib.sha256(config_json).hexdigest()
129- cls._upload(
130- "sha256:{}".format(config_sha),
131- push_rule,
132- BytesIO(config_json),
133- http_client)
134-
135- # Build the registry manifest from the image manifest
136- # and associated configs
137- registry_manifest = cls._build_registry_manifest(
138- digests, config, config_json, config_sha,
139- preloaded_data[section["Config"]])
140-
141- # Upload the registry manifest
142- try:
143- manifest_response = http_client.requestPath(
144- "/manifests/{}".format(tag),
145- json=registry_manifest,
146- headers={
147- "Content-Type":
148- "application/"
149- "vnd.docker.distribution.manifest.v2+json"
150- },
151- method="PUT")
152- except HTTPError as http_error:
153- manifest_response = http_error.response
154- if manifest_response.status_code != 201:
155- raise cls._makeRegistryError(
156- ManifestUploadFailed,
157- "Failed to upload manifest for {} in {}".format(
158- build.recipe.name, build.id),
159- manifest_response)
160+ try:
161+ cls._upload_to_push_rule(
162+ push_rule, build, manifest, digests, preloaded_data)
163+ except Exception as e:
164+ exceptions.append(e)
165+ if len(exceptions) == 1:
166+ raise exceptions[0]
167+ elif len(exceptions) > 1:
168+ raise MultipleOCIRegistryError(exceptions)
169
170
171 class OCIRegistryAuthenticationError(Exception):
172diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
173index 2edec41..7459a43 100644
174--- a/lib/lp/oci/tests/test_ociregistryclient.py
175+++ b/lib/lp/oci/tests/test_ociregistryclient.py
176@@ -38,6 +38,7 @@ from zope.security.proxy import removeSecurityProxy
177 from lp.oci.interfaces.ociregistryclient import (
178 BlobUploadFailed,
179 ManifestUploadFailed,
180+ MultipleOCIRegistryError,
181 )
182 from lp.oci.model.ociregistryclient import (
183 BearerTokenRegistryClient,
184@@ -216,6 +217,59 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
185 self.assertEqual(
186 http_client.credentials, ('test-username', 'test-password'))
187
188+ @responses.activate
189+ def test_upload_skip_failed_push_rule(self):
190+ self._makeFiles()
191+ upload_fixture = self.useFixture(MockPatch(
192+ "lp.oci.model.ociregistryclient.OCIRegistryClient._upload"))
193+ self.useFixture(MockPatch(
194+ "lp.oci.model.ociregistryclient.OCIRegistryClient._upload_layer"))
195+
196+ push_rules = [
197+ self.push_rule,
198+ self.factory.makeOCIPushRule(recipe=self.build.recipe),
199+ self.factory.makeOCIPushRule(recipe=self.build.recipe)]
200+ # Set the first 2 rules to fail with 400 at the PUT operation.
201+ for i, push_rule in enumerate(push_rules):
202+ push_rule.registry_credentials.setCredentials({
203+ "username": "test-username-%s" % i,
204+ "password": "test-password-%s" % i
205+ })
206+ responses.add(
207+ "GET", "%s/v2/" % push_rule.registry_url, status=200)
208+
209+ manifests_url = "{}/v2/{}/manifests/edge".format(
210+ push_rule.registry_credentials.url, push_rule.image_name)
211+ status = 400 if i < 2 else 201
212+ responses.add("PUT", manifests_url, status=status)
213+
214+ error = self.assertRaises(
215+ MultipleOCIRegistryError, self.client.upload, self.build)
216+
217+ # Check that it tried to call the upload for each one of the push rules
218+ self.assertEqual(3, upload_fixture.mock.call_count)
219+ used_credentials = {
220+ args[0][-1].credentials
221+ for args in upload_fixture.mock.call_args_list}
222+ self.assertSetEqual(
223+ {('test-username-0', 'test-password-0'),
224+ ('test-username-1', 'test-password-1'),
225+ ('test-username-2', 'test-password-2')},
226+ used_credentials)
227+
228+ # Check that we received back an exception of the correct type.
229+ self.assertIsInstance(error, MultipleOCIRegistryError)
230+ self.assertEqual(2, len(error.errors))
231+ self.assertEqual(2, len(error.exceptions))
232+
233+ expected_error_msg = (
234+ "Failed to upload manifest for {recipe} ({url1}) in {build} / "
235+ "Failed to upload manifest for {recipe} ({url2}) in {build}"
236+ ).format(
237+ recipe=self.build.recipe.name, build=self.build.id,
238+ url1=push_rules[0].registry_url, url2=push_rules[1].registry_url)
239+ self.assertEqual(expected_error_msg, str(error))
240+
241 def test_preloadFiles(self):
242 self._makeFiles()
243 files = self.client._preloadFiles(
244@@ -433,6 +487,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
245 responses.add(
246 "PUT", manifests_url, status=400, json={"errors": put_errors})
247
248+ expected_msg = "Failed to upload manifest for {} ({}) in {}".format(
249+ self.build.recipe.name, self.push_rule.registry_url, self.build.id)
250 self.assertThat(
251 partial(self.client.upload, self.build),
252 Raises(MatchesException(
253@@ -440,9 +496,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
254 MatchesAll(
255 AfterPreprocessing(
256 str,
257- Equals(
258- "Failed to upload manifest for {} in {}".format(
259- self.build.recipe.name, self.build.id))),
260+ Equals(expected_msg)),
261 MatchesStructure.byEquality(errors=put_errors)))))
262
263 @responses.activate
264@@ -462,6 +516,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
265 push_rule.image_name)
266 responses.add("PUT", manifests_url, status=200)
267
268+ expected_msg = "Failed to upload manifest for {} ({}) in {}".format(
269+ self.build.recipe.name, self.push_rule.registry_url, self.build.id)
270 self.assertThat(
271 partial(self.client.upload, self.build),
272 Raises(MatchesException(
273@@ -469,9 +525,7 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
274 MatchesAll(
275 AfterPreprocessing(
276 str,
277- Equals(
278- "Failed to upload manifest for {} in {}".format(
279- self.build.recipe.name, self.build.id))),
280+ Equals(expected_msg)),
281 MatchesStructure(errors=Is(None))))))
282
283