remotecache/s3: honor compression attributes - #7121
Open
ernetas wants to merge 2 commits into
Open
Conversation
mc stores its aliases in a single configuration folder that defaults to $HOME/.mc. NewMinioServer registers an alias there for every server it starts, so two servers running at the same time race each other while rewriting that file: one invocation can drop the alias another one just registered, and a subsequent mc call then either fails outright or resolves the alias to the wrong server. In the latter case the bucket gets created on a different server than the one the test handed to buildkitd, which later fails the export with NoSuchBucket. Only a single test used a minio server so far, so the aliases never overlapped in practice. Point the mc invocations of every server at their own folder through MC_CONFIG_DIR so that additional parallel tests keep working. Setting it in the environment rather than passing --config-dir makes it impossible for an invocation to miss it, which matters because mc treats "<alias>/<bucket>" of an unknown alias as a local path and silently succeeds. As the folder is a t.TempDir(), the alias no longer needs a random name or an explicit cleanup. Signed-off-by: Ernestas Lukoševičius <ernetas@gmail.com>
ernetas
force-pushed
the
s3-cache-compression
branch
from
September 5, 2026 13:04
95a899d to
73389f4
Compare
The s3 cache exporter returned a hardcoded compression.New(compression. Default) from Config(), so compression, compression-level and force-compression passed to --export-cache type=s3 were silently ignored and every layer was always exported as gzip. That value was never a deliberate choice for this backend. e3f6d7b added Config() to the Exporter interface to make the compression selectable, and wired compression.ParseAttributes into the registry and local backends while leaving a placeholder returning the default in gha and inline. The s3 backend was added two months later in 09c5a7c and copied that placeholder instead of the registry implementation. Nothing in the s3 cache format is tied to gzip. Finalize already records the real media type of every layer in the cache manifest and the importer reads it back, so blobs of any supported compression round-trip. Parse the attributes in ResolveCacheExporterFunc and return them from Config(), the same way the registry and local backends do. Fixes moby#3071 Signed-off-by: Ernestas Lukoševičius <ernetas@gmail.com>
ernetas
force-pushed
the
s3-cache-compression
branch
from
September 5, 2026 15:53
73389f4 to
5bbc929
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
--export-cache type=s3silently ignorescompression,compression-levelandforce-compression: the exporter returns a hardcodedcompression.New(compression.Default)fromConfig(), so every layer is exported as gzip.buildctl build ... \ --export-cache type=s3,region=eu-west-1,bucket=my_bucket,compression=zstd,force-compression=true # layers are still application/vnd.oci.image.layer.v1.tar+gzipThat value was never deliberate. e3f6d7b added
Config()to theExporterinterface and wiredcompression.ParseAttributesintoregistryandlocal, leaving a placeholder returning the default inghaandinline; the s3 backend landed two months later in 09c5a7c and, having to satisfy the now-mandatory method, copied that placeholder rather than the registry implementation. Nothing in the s3 format is tied to gzip —Finalizealready records the real media type of every layer and the importer reads it back, so blobs of any supported compression round-trip.Changes
remotecache/s3: honor compression attributes— parse the attributes inResolveCacheExporterFuncand return them fromConfig(), the same wayregistryandlocaldo. Tests: a table test over the parsing (each valid value, and the error each invalid one produces), plus two integration tests that export, re-import, and assert the cache manifest records the requested media type and that the uploaded blobs really are in that format:testZstdS3CacheImportExportalso exportsllb.Image("busybox").Run(...)under a second manifest name. That build is what pinsforce-compression— the first only creates layers born in the requested compression, while this one has to convert the gzip base image layers.testUncompressedS3CacheImportExportcovers the other end of the range, where the blob digest equals the diffID and the media type carries no compression suffix.test: isolate mc config dir for minio servers— a prerequisite.mckeeps its aliases in a single folder defaulting to$HOME/.mcandNewMinioServerregisters an alias there per server, so concurrent servers race while rewriting it:mc mbintermittently created the bucket on another test's server and the export then failed withNoSuchBucket. Each server now gets its own folder throughMC_CONFIG_DIR, set in the environment rather than passed as--config-dirso that no invocation can miss it —mctreats<alias>/<bucket>of an unknown alias as a local path and silently succeeds.README documents the three attributes, with a caveat that
estargzlayers are recorded as plain gzip in the s3 manifest and so cannot be lazily pulled from s3:LayerAnnotationshas nowhere to keep their eStargz annotations.Testing
Both new integration tests fail without the
Config()change, e.g.:Notes
compression,compression-levelorforce-compressionvalue ontype=s3now fails when the exporter is resolved — before any build work, and regardless ofignore-error— instead of being silently ignored. This matchesregistryandlocal.azblobandghastill carry the same placeholder; theghaone is noted in Support ZSTD compression in s3 remote cache #3071. Left out here to keep the change focused.Fixes #3071