Merge ~twom/launchpad:oci-layer-tar-sizes into launchpad:master

Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: a8ad1da5d41b5288f0d635bbc670a7f3d6746f1b
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~twom/launchpad:oci-layer-tar-sizes
Merge into: launchpad:master
Diff against target: 122 lines (+20/-14)
2 files modified
lib/lp/oci/model/ociregistryclient.py (+12/-4)
lib/lp/oci/tests/test_ociregistryclient.py (+8/-10)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+391926@code.launchpad.net

Commit message

Use layer tar sizes in registry upload

Description of the change

The registry requires the size of the layer.tar, not the size of the .gzipped version that we have in the librarian.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM

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/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index 0176a89..7561267 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -148,12 +148,13 @@ class OCIRegistryClient:
148 continue148 continue
149 fileobj = un_zipped.extractfile(tarinfo)149 fileobj = un_zipped.extractfile(tarinfo)
150 cls._upload(digest, push_rule, fileobj, http_client)150 cls._upload(digest, push_rule, fileobj, http_client)
151 return tarinfo.size
151 finally:152 finally:
152 lfa.close()153 lfa.close()
153154
154 @classmethod155 @classmethod
155 def _build_registry_manifest(cls, digests, config, config_json,156 def _build_registry_manifest(cls, digests, config, config_json,
156 config_sha, preloaded_data):157 config_sha, preloaded_data, layer_sizes):
157 """Create an image manifest for the uploading image.158 """Create an image manifest for the uploading image.
158159
159 This involves nearly everything as digests and lengths are required.160 This involves nearly everything as digests and lengths are required.
@@ -163,6 +164,7 @@ class OCIRegistryClient:
163 :param config: The contents of the manifest config file as a dict.164 :param config: The contents of the manifest config file as a dict.
164 :param config_json: The config file as a JSON string.165 :param config_json: The config file as a JSON string.
165 :param config_sha: The sha256sum of the config JSON string.166 :param config_sha: The sha256sum of the config JSON string.
167 :param layer_sizes: Dict of layer digests and their size in bytes.
166 """168 """
167 # Create the initial manifest data with empty layer information169 # Create the initial manifest data with empty layer information
168 manifest = {170 manifest = {
@@ -181,7 +183,10 @@ class OCIRegistryClient:
181 manifest["layers"].append({183 manifest["layers"].append({
182 "mediaType":184 "mediaType":
183 "application/vnd.docker.image.rootfs.diff.tar.gzip",185 "application/vnd.docker.image.rootfs.diff.tar.gzip",
184 "size": preloaded_data[layer].content.filesize,186 # This should be the size of the `layer.tar` that we extracted
187 # from the OCI image at build time. It is not the size of the
188 # gzipped version that we have in the librarian.
189 "size": layer_sizes[layer],
185 "digest": layer})190 "digest": layer})
186 return manifest191 return manifest
187192
@@ -238,12 +243,14 @@ class OCIRegistryClient:
238 file_data = preloaded_data[section["Config"]]243 file_data = preloaded_data[section["Config"]]
239 config = file_data["config_file"]244 config = file_data["config_file"]
240 # Upload the layers involved245 # Upload the layers involved
246 layer_sizes = {}
241 for diff_id in config["rootfs"]["diff_ids"]:247 for diff_id in config["rootfs"]["diff_ids"]:
242 cls._upload_layer(248 layer_size = cls._upload_layer(
243 diff_id,249 diff_id,
244 push_rule,250 push_rule,
245 file_data[diff_id],251 file_data[diff_id],
246 http_client)252 http_client)
253 layer_sizes[diff_id] = layer_size
247 # The config file is required in different forms, so we can254 # The config file is required in different forms, so we can
248 # calculate the sha, work these out and upload255 # calculate the sha, work these out and upload
249 config_json = json.dumps(config).encode("UTF-8")256 config_json = json.dumps(config).encode("UTF-8")
@@ -258,7 +265,8 @@ class OCIRegistryClient:
258 # and associated configs265 # and associated configs
259 registry_manifest = cls._build_registry_manifest(266 registry_manifest = cls._build_registry_manifest(
260 digests, config, config_json, config_sha,267 digests, config, config_json, config_sha,
261 preloaded_data[section["Config"]])268 preloaded_data[section["Config"]],
269 layer_sizes)
262270
263 # Upload the registry manifest271 # Upload the registry manifest
264 try:272 try:
diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
index 7459a43..3c5aa8c 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -148,7 +148,8 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
148 self.useFixture(MockPatch(148 self.useFixture(MockPatch(
149 "lp.oci.model.ociregistryclient.OCIRegistryClient._upload"))149 "lp.oci.model.ociregistryclient.OCIRegistryClient._upload"))
150 self.useFixture(MockPatch(150 self.useFixture(MockPatch(
151 "lp.oci.model.ociregistryclient.OCIRegistryClient._upload_layer"))151 "lp.oci.model.ociregistryclient.OCIRegistryClient._upload_layer",
152 return_value=999))
152153
153 push_rule = self.build.recipe.push_rules[0]154 push_rule = self.build.recipe.push_rules[0]
154 responses.add("GET", "%s/v2/" % push_rule.registry_url, status=200)155 responses.add("GET", "%s/v2/" % push_rule.registry_url, status=200)
@@ -169,14 +170,12 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
169 "mediaType": Equals(170 "mediaType": Equals(
170 "application/vnd.docker.image.rootfs.diff.tar.gzip"),171 "application/vnd.docker.image.rootfs.diff.tar.gzip"),
171 "digest": Equals("diff_id_1"),172 "digest": Equals("diff_id_1"),
172 "size": Equals(173 "size": Equals(999)}),
173 self.layer_files[0].library_file.content.filesize)}),
174 MatchesDict({174 MatchesDict({
175 "mediaType": Equals(175 "mediaType": Equals(
176 "application/vnd.docker.image.rootfs.diff.tar.gzip"),176 "application/vnd.docker.image.rootfs.diff.tar.gzip"),
177 "digest": Equals("diff_id_2"),177 "digest": Equals("diff_id_2"),
178 "size": Equals(178 "size": Equals(999)})
179 self.layer_files[1].library_file.content.filesize)})
180 ]),179 ]),
181 "schemaVersion": Equals(2),180 "schemaVersion": Equals(2),
182 "config": MatchesDict({181 "config": MatchesDict({
@@ -295,21 +294,20 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
295 self.config,294 self.config,
296 json.dumps(self.config),295 json.dumps(self.config),
297 "config-sha",296 "config-sha",
298 preloaded_data["config_file_1.json"])297 preloaded_data["config_file_1.json"],
298 {"diff_id_1": 999, "diff_id_2": 9001})
299 self.assertThat(manifest, MatchesDict({299 self.assertThat(manifest, MatchesDict({
300 "layers": MatchesListwise([300 "layers": MatchesListwise([
301 MatchesDict({301 MatchesDict({
302 "mediaType": Equals(302 "mediaType": Equals(
303 "application/vnd.docker.image.rootfs.diff.tar.gzip"),303 "application/vnd.docker.image.rootfs.diff.tar.gzip"),
304 "digest": Equals("diff_id_1"),304 "digest": Equals("diff_id_1"),
305 "size": Equals(305 "size": Equals(999)}),
306 self.layer_files[0].library_file.content.filesize)}),
307 MatchesDict({306 MatchesDict({
308 "mediaType": Equals(307 "mediaType": Equals(
309 "application/vnd.docker.image.rootfs.diff.tar.gzip"),308 "application/vnd.docker.image.rootfs.diff.tar.gzip"),
310 "digest": Equals("diff_id_2"),309 "digest": Equals("diff_id_2"),
311 "size": Equals(310 "size": Equals(9001)})
312 self.layer_files[1].library_file.content.filesize)})
313 ]),311 ]),
314 "schemaVersion": Equals(2),312 "schemaVersion": Equals(2),
315 "config": MatchesDict({313 "config": MatchesDict({

Subscribers

People subscribed via source and target branches

to status/vote changes: