Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable selecting compression on registry/localdir cache export #2685

Merged
merged 1 commit into from
Mar 4, 2022
Merged

Enable selecting compression on registry/localdir cache export #2685

merged 1 commit into from
Mar 4, 2022

Conversation

ktock
Copy link
Collaborator

@ktock ktock commented Feb 28, 2022

Following up #2648 (comment)

Currently, cache exporter doesn't have options about compression types selection but always tries to select gzip compression.
This commit allows user to specify compression type of the exporting cache.
This commit adds options about compression type to registry and local cache exporter.

# cat <<EOF > /tmp/ctx/Dockerfile
FROM ghcr.io/stargz-containers/ubuntu:20.04-org
RUN echo hello > /hello
EOF
# buildctl build --progress=plain --frontend=dockerfile.v0 --local context=/tmp/ctx --local dockerfile=/tmp/ctx \
               --export-cache type=registry,ref=registry2-buildkit:5000/cache/test:1,compression=zstd,force-compression=true,oci-mediatypes=true
# crane manifest registry2-buildkit:5000/cache/test:1 | jq
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+zstd",
      "digest": "sha256:20dfff5ee21a9e68dccc178472b8e49e1dfdb06eae1aa0fd671f9fb29f83410d",
      "size": 172,
      "annotations": {
        "containerd.io/uncompressed": "sha256:2f140462f3bcf8cf3752461e27dfd4b3531f266fa10cda716166bd3a78a19103"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+zstd",
      "digest": "sha256:260c0424f21bddb6a869aa6f51594db5beb919ca89e67e47504dcae13a822f5a",
      "size": 105,
      "annotations": {
        "containerd.io/uncompressed": "sha256:34f12a64ae8118df5b8831d6bb94b1549bebc5cb717d24a7cb1156a0cd4a5cd8"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+zstd",
      "digest": "sha256:64a09378aec39ffad8ff1ce64f55d7eace718b19c6fcc4783dc8f18a4ca1999d",
      "size": 824,
      "annotations": {
        "containerd.io/uncompressed": "sha256:63c99163f47292f80f9d24c5b475751dbad6dc795596e935c5c7f1c73dc08107"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+zstd",
      "digest": "sha256:fc5c587e079836a11470497a584831d733d6766e6285fae67854e17486c3fa5a",
      "size": 24234821,
      "annotations": {
        "containerd.io/uncompressed": "sha256:ccdbb80308cc5ef43b605ac28fac29c6a597f89f5a169bbedbb8dec29c987439"
      }
    },
    {
      "mediaType": "application/vnd.buildkit.cacheconfig.v0",
      "digest": "sha256:9632ea5574578ec567c437363383a9a7311a9ed44158adc6962793f435a2b7ed",
      "size": 748
    }
  ]
}

@ktock ktock marked this pull request as ready for review February 28, 2022 03:26
AkihiroSuda
AkihiroSuda reviewed Mar 2, 2022
View reviewed changes
README.md
@@ -360,6 +360,9 @@ buildctl build ... \
* `mode=max`: export all the layers of all intermediate steps.
* `ref=docker.io/user/image:tag`: reference
* `oci-mediatypes=true|false`: whether to use OCI mediatypes in exported manifests. Since BuildKit `v0.8` defaults to true.
* `compression=[uncompressed,gzip,estargz,zstd]`: choose compression type for layers newly created and cached, gzip is default value. estargz and zstd should be used with `oci-mediatypes=true`.
* `compression-level=[value]`: compression level for gzip, estargz (0-9) and zstd (0-22)
* `force-compression=true`: forcibly apply `compression` option to all layers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compression-force

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review.
This follows the naming convention of --output option which have force-compression.
Let's rename them to compression-force (and deprecate force-compression) in a separated PR.

@tonistiigi tonistiigi added this to the v0.10.0 milestone Mar 4, 2022
@tonistiigi
Copy link
Member

@ktock I can't get the estargz import to work for the local cache. It always seems to pull the full blobs and not lazy mount. Same for gha.

@ktock
Copy link
Collaborator Author

ktock commented Mar 4, 2022

@tonistiigi Do you mean that --import-cache type=local|gha doesn' perform lazy mounts?

This is because stargz-snapshotter (as of now) doesn't support local tarball and gha as the mount source.
So, as of now, type=registry is only possible to perform lazy mount of estargz (available since #2648).

I'll work on modifying stargz-snapshotter to support it.

tonistiigi
tonistiigi approved these changes Mar 4, 2022
View reviewed changes
solver/llbsolver/solver.go Outdated
@@ -278,11 +278,18 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
}
ctx = withDescHandlerCacheOpts(ctx, workerRef.ImmutableRef)

// Configure compression if exporters supports it.
compressionConfig := compression.New(compression.Default)
if c, ok := e.(remotecache.ExporterConfig); ok {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just make it a required part of the interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review. Fixed this.

@ktock ktock mentioned this pull request Mar 4, 2022
Open
@tonistiigi tonistiigi merged commit 0a7d890 into moby:master Mar 4, 2022
tonistiigi
tonistiigi reviewed Mar 4, 2022
View reviewed changes
client/client_test.go
testBasicCacheImportExport(t, sb, []CacheOptionsEntry{im}, []CacheOptionsEntry{ex})
}

func testUncompressedRegistryCacheImportExport(t *testing.T, sb integration.Sandbox) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktock This new test failed in the merge commit https://github.com/moby/buildkit/runs/5425504420?check_suite_focus=true

@ktock ktock deleted the remotecachecompression branch March 5, 2022 01:16
@ktock ktock mentioned this pull request Mar 9, 2022
Merged
@ktock
Copy link
Collaborator Author

ktock commented Mar 9, 2022

Thank you for the comment. #2712 will fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonistiigi tonistiigi tonistiigi approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.10.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ktock @tonistiigi @AkihiroSuda