Images created with buildx sometimes have layers that are incorrect/zero bytes #2198
Comments
|
I've been trying to track down what is going on from the logs and I believe that I have captured a build where it has problems with debug logging: It seems like every build I do, I get the "reference for unknown type: application/vnd.buildkit.cacheconfig.v0" warning and occasionally "invalid image config with unaccounted layers" but I am not sure that specifically has anything to do with the problem I am seeing. Looking at the above log output, I am not sure I see any smoking gun either so I am not exactly sure what's happening. I do know that the image What it should be: |
|
Do you have a runnable reproducer. I see you have a repo with code but don't see the exact order of commands that would reproduce this. |
|
Thanks for the reply. I was spending this morning trying to get a way to reproduce this and have not yet figured out how to do so. It doesn't seem to have anything to do with specific commands or files in a build as I've seen it happen with different builds of two completely different applications and different layers. I've seen it happen with both RUN and COPY layers. I'll keep trying to see what I might be able to do to reproduce this. I was also curious if there was anything else I should be doing to try to capture the right information. I've added |
|
My hunch is that this has something to do with |
|
There are no special "cache debug" options atm. without code modification. |
|
Interesting. Well thanks for the insights into a possibly why it could be happening. I hadn't tried to look further into a previous build's output where it would have pushed the cache and inspected the specific image cache that is getting pushed to my registry for anything that sticks out. Unfortunately I am not doing so well at reproducing the issue. I've tried all sort of various combinations of existing builder, existing builder with builder cache cleared and creating builders from scratch and looping through on builds. I think I've probably built this image about 1000 times 😄 There doesn't seem to be any real reason or trigger as to why it happens as far as I can tell. If you'd rather I close this, that's fine. I'll just keep looking to see if there is anything I can do to try to figure out what is going on and how I can trigger it. |
|
@tonistiigi - any chance you know approximately when that sort of behavior might have been introduced? I was chatting with a former coworker and he had been using buildkit |
|
Sounds really like the problem I described in #1980 (comment) that commit dda009a broke it, also needed to downgrade to 0.7 I would really appreciate if the behavior is changed in the next Bugfix release, as it is critical if those images were tried to deployed on prod. We wrote test scripts to prevent this. |
|
@Patrick-Remy We need a reproducer before anything can be concluded from this. If you have one please post it. |
|
I did already in my issue: https://github.com/Patrick-Remy/buildkit-missing-layer-repro |
|
@Patrick-Remy Tried to run in both with v0.8.1 and v0.8.3 but doesn't look like it is doing anything weird for me. Logs https://gist.github.com/tonistiigi/f135e38d25d0d204985d9659f19888ed |
|
I just could simplify the setup massively, based on @mbentley's provided repo. I updated my repo https://github.com/Patrick-Remy/buildkit-missing-layer-repro. The |
|
Nice, thanks @Patrick-Remy! I've been able to use your repro myself on Docker for Mac and it takes anywhere from 10-30 seconds fairly reliably to get it to reproduce the issue. Just to add a quick note, I did test a number of versions with your script to reproduce it ( |
|
I can confirm that I seem to be able to repro with the latest version. |
Yes, as I wrote in the README I already debugged it to be caused by this commit (I built versions myself for commits between 0.8.0 and 0.7.2): dda009a
That's great! I look forward that you will find the possible bug in the implementation ;) |
|
I made one PR regarding the repro. #2261 It fixes the repro but obviously is not the main issue here. This has left me quite puzzled how changing trivial things in the repro changes the behavior. The 1 and 2 caches contain identical files but it does not work if 2 does not run. Even if I clear the local state and now 2 should have no effect it still changes behavior. I'll continue to look into this but we have hit the deadline with the release and need to move on there. |
|
Did you have any time to investigate this another time? We are still freezed to v0.7.2 due to this bug. |
jgiannuzzi
added a commit
to jgiannuzzi/buildkit
that referenced
this issue
Sep 16, 2021
jgiannuzzi
added a commit
to jgiannuzzi/buildkit
that referenced
this issue
Sep 16, 2021
|
I have investigated this issue further and made another repro, based on @Patrick-Remy's excellent work. It does not need to import 2 identical caches to make the issue appear, and has an even shorter I summarised my understanding of the issue in the repro README (mind you, I have never looked at the Why is this broken?Let's take a look at the FROM alpine:latest
# create a layer (empty or not)
RUN echo 1
# create a layer that also depends on the context
COPY repro.txt /
# create an empty layer
RUN echo 2When importing the cache of a run that has empty layers removed, some vertexes will point to the same result, e.g. In If the 'wrong' item gets used ( Those will eventually be saved in the During a second run with the same cache, but this time with a partially populated During the result filtering, results originating from both caches could be walked, and the result for Unfortunately, |
jgiannuzzi
added a commit
to jgiannuzzi/buildkit
that referenced
this issue
Sep 21, 2021
jgiannuzzi
added a commit
to jgiannuzzi/buildkit
that referenced
this issue
Sep 21, 2021
Signed-off-by: Jonathan Giannuzzi <[email protected]>
|
I've had the chance to do a couple re-builds of all 90 of my images that I push to Docker Hub using buildkit from your PR @jgiannuzzi and while it is a small sample size of two full runs, I am not seeing any zero bytes layers being detected 🤞 |
tonistiigi
pushed a commit
to tonistiigi/buildkit
that referenced
this issue
Oct 1, 2021
Signed-off-by: Jonathan Giannuzzi <[email protected]> (cherry picked from commit 2c540bd)
alexcb
added a commit
to earthly/buildkit-old-fork
that referenced
this issue
Oct 26, 2021
* dockerfile: fix git version detection Signed-off-by: Tonis Tiigi <[email protected]> * Add support for heredocs with ONBUILD Signed-off-by: Justin Chadwell <[email protected]> * dockerfile: use none differ for dockerfile/dockerignore This avoids wrong metadata matches on small files Signed-off-by: Tonis Tiigi <[email protected]> * progressui: print logs for failed step as summary in plain mode Signed-off-by: Tonis Tiigi <[email protected]> * grpcerrors: avoid rpc error wrapping in error messages Signed-off-by: Tonis Tiigi <[email protected]> * exec: improve error message on exec errors Signed-off-by: Tonis Tiigi <[email protected]> * Improve heredoc parsing to allow more generic words Previously, heredoc names were restricted to simple alphanumeric strings. However, heredocs should support much more complex use-cases, including quoting anywhere, as well as allowing special symbols like `.` for easily expressing file extensions. This patch adds support for these more complex cases, by using the shell lexer to parse each heredoc name. Additionally, we include improvements to the lexer to optionally preserve escape tokens to avoid problems when lexing words that have already been lexed before. Signed-off-by: Justin Chadwell <[email protected]> * Improve progress and history messages for heredoc-related commands Signed-off-by: Justin Chadwell <[email protected]> * Remove unneeded Finalize method from ImmutableRef. Finalize was only used outside the cache package in one place, which called it with the commit arg set to false. The code path followed when commit==false turned out to essentially be a no-op because it set "retain cache" to true if it was already set to true. It was thus safe to remove the only external call to it and remove it from the interface. This should be helpful for future efforts to simplify the equal{Mutable,Immutable} fields in cacheRecord, which exist due to the "lazy commit" feature that Finalize is tied into. Signed-off-by: Erik Sipsma <[email protected]> * Fix ref leak if fileop ref fails to mount. Signed-off-by: Erik Sipsma <[email protected]> * add error suggest pkg Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: suggest mistyped flag names Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: provide suggestions for mount options Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: add tests for error suggestions Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: remove unnecessary error wrappings Signed-off-by: Tonis Tiigi <[email protected]> * enable riscv64 build Signed-off-by: Tonis Tiigi <[email protected]> * Update QEMU emulators Signed-off-by: CrazyMax <[email protected]> * dockerfile: move run network to stable channel Signed-off-by: Tonis Tiigi <[email protected]> * Automatically detect default git branch Instead of just assuming that the default branch is master, use ls-remote to find out. Also removed tests that didn't specifiy a branch but required authentication, because those will fail now that the repo is actually checked. Signed-off-by: Levi Harrison <[email protected]> * Moved getDefaultBranch function to gitsource It is my suspecion that the tests were failing on previous commits because of the lack of authentication and other stuff like that available in gitidentifier as compared to gitsource Signed-off-by: Levi Harrison <[email protected]> * Fix tests Unfortunately, further test cases will have to be removed because gitindentifier will now leave the branch blank instead of filling it in Signed-off-by: Levi Harrison <[email protected]> * git: fix default branch detection Signed-off-by: Tonis Tiigi <[email protected]> * Enable to forcefully specify compression type Signed-off-by: ktock <[email protected]> * Add full timestamp to logs Signed-off-by: Yamazaki Masashi <[email protected]> * Remove meaningless encode Signed-off-by: Yamazaki Masashi <[email protected]> * Ignore missing providers for blobs w/ same chainid. GetByBlob checks to see if there are any other blobs with the same (uncompressed) ChainID and, if so, reuses their unpacked snapshot if it exists. The problem is if this code finds a match, it was trying to get the matching record, but couldn't do so when the match is lazy because the caller doesn't necessarily have descriptor handlers setup for it. This commit changes the behavior to just ignore any match with the same ChainID that's also lazy as they just aren't usable for the snapshot-reuse optimization. Signed-off-by: Erik Sipsma <[email protected]> * authprovider: handle eaccess on storing token seeds Signed-off-by: Tonis Tiigi <[email protected]> * log with traceID and spanID Signed-off-by: Morlay <[email protected]> * github: update CI buildkit to v0.9.0-rc1 Signed-off-by: Tonis Tiigi <[email protected]> * initial version of github cache Signed-off-by: Tonis Tiigi <[email protected]> * vendor: add goactionscache Signed-off-by: Tonis Tiigi <[email protected]> * caps: add cap for gha cache backend Signed-off-by: Tonis Tiigi <[email protected]> * remove tracetransform package Signed-off-by: Tonis Tiigi <[email protected]> * resolver: increase default idle conns reuse The current default were even lower than stdlib defaults. Signed-off-by: Tonis Tiigi <[email protected]> * refactor to use util/bklog instead of using logurs directly Signed-off-by: Morlay <[email protected]> * GitHub Actions cache docs Signed-off-by: CrazyMax <[email protected]> * Skips getting UID/GUID if passwd/group file is not found When running a WORKDIR instruction, buildkit will create that folder and chown it to the currently set user. For this, it will try to read the /etc/passwd file to get the proper UID, and if that user is not found in the file, the root user will be considered as the owner. However, Windows image do not have that file, which will result in an error while building the image. We can consider not finding the /etc/passwd file as the same as not finding the user in the file, which would solve this issue. Signed-off-by: Claudiu Belu <[email protected]> * add per domain semaphore to limit concurrent connections This is a safer alternative until we figure out why http.Transport based limiting fails. Some connections like cache export/import do not have a domain key atm and these connections use global pool. Signed-off-by: Tonis Tiigi <[email protected]> * update to github.com/containerd/containerd v1.5.3 Signed-off-by: coryb <[email protected]> * vendor: update go-actions-cache with custom client support Signed-off-by: Tonis Tiigi <[email protected]> * tracing: update to otelhttp roundtripper Signed-off-by: Tonis Tiigi <[email protected]> * Enhance test matrix Signed-off-by: CrazyMax <[email protected]> * fix dropped pull progress output due to canceled context fixes moby#2248 Signed-off-by: coryb <[email protected]> * Add span for layer export This can be a significant amount of time that isn't currently accounted for in traces. Signed-off-by: Aaron Lehmann <[email protected]> * new implementation for limiting tcp connections The previous implementation had many issues. Eg. on fetch, even if the data already existed and no remote connections were needed the request would still be waiting in the queue. Or if two fetches of same blob happened together they would take up two places in queue although there was only one remote request. Signed-off-by: Tonis Tiigi <[email protected]> * limited: allow extra high-priority connection for json requests Signed-off-by: Tonis Tiigi <[email protected]> * ensure wrappers support seeking to continue partial downloads Signed-off-by: Tonis Tiigi <[email protected]> * contentutil: change offset to int64 to simplify Signed-off-by: Tonis Tiigi <[email protected]> * Exporter config digest typo Signed-off-by: CrazyMax <[email protected]> * daemonless: wait for daemon to finish before exit Signed-off-by: Tonis Tiigi <[email protected]> * github: update CI buildkit to v0.9.0 Signed-off-by: Tonis Tiigi <[email protected]> * add docs for new config options Signed-off-by: Tonis Tiigi <[email protected]> * add ktock and crazy-max to maintainers Signed-off-by: Tonis Tiigi <[email protected]> * Update Dockerfile references to use 1.3 Signed-off-by: Tonis Tiigi <[email protected]> * docs: update images-readme to v0.9 Signed-off-by: Tonis Tiigi <[email protected]> * Bump to codecov/codecov-action v2 Signed-off-by: CrazyMax <[email protected]> * build(deps): bump github.com/containerd/containerd from 1.5.3 to 1.5.4 Bumps [github.com/containerd/containerd](https://github.com/containerd/containerd) from 1.5.3 to 1.5.4. - [Release notes](https://github.com/containerd/containerd/releases) - [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md) - [Commits](containerd/containerd@v1.5.3...v1.5.4) --- updated-dependencies: - dependency-name: github.com/containerd/containerd dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> * util/tracing: remove incorrect import enforcing comment This import comment caused compilation of buildx to fail if `GO111MODULE` was set to `off`: Without `GO111MODULE` set (but with `-mod=vendor`: echo $GO111MODULE export PKG=github.com/docker/buildx export LDFLAGS="-X ${PKG}/version.Version=$(git describe --match 'v[0-9]*' --always --tags) -X ${PKG}/version.Revision=$(git rev-parse HEAD) -X ${PKG}/version.Package=${PKG}" GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx bin/docker-buildx version github.com/docker/buildx v0.6.0 d9ee3b134cbc2d09513fa7fee4176a3919e05887 When setting `GO111MODULE=off`, it fails on the incorrect import path in the vendored file (looks like GO111MODULE=on ignores import-path comments?): export GO111MODULE=off root@5a55ec1c1eed:/go/src/github.com/docker/buildx# GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx vendor/github.com/moby/buildkit/client/client.go:20:2: code in directory /go/src/github.com/docker/buildx/vendor/github.com/moby/buildkit/util/tracing/otlptracegrpc expects import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/connection/connection.go:33:2: found import comments "go.opentelemetry.io/otel/exporters/otlp/internal/otlpconfig" (options.go) and "go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig" (optiontypes.go) in /go/src/github.com/docker/buildx/vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig Signed-off-by: Sebastiaan van Stijn <[email protected]> * Fix protoc link Signed-off-by: CrazyMax <[email protected]> * Allow ExitError type to be transmitted over GRPC This will allow clients to retrieve exit error codes returned during a solve without parsing the error messages. Signed-off-by: Aaron Lehmann <[email protected]> * Update to github.com/opencontainers/runc v1.0.1 Signed-off-by: CrazyMax <[email protected]> * Split cache options doc for each exporter Signed-off-by: CrazyMax <[email protected]> * Set default socket permissions to 660 The systemd default is 666, it seems. Signed-off-by: Anders F Björklund <[email protected]> * fix SecurityMode being dropped on gateway container Start Signed-off-by: Cory Bennett <[email protected]> * bump containerd from 1.5.4 to 1.5.5 Signed-off-by: CrazyMax <[email protected]> * go.mod: golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c In preparation of replacing the deprecated github.com/docker/docker/pkg/signal, which uses this version (updating it separately for easier review). Signed-off-by: Sebastiaan van Stijn <[email protected]> * replace use of deprecated github.com/docker/docker/pkg/signal This package was moved to a separate module in github.com/moby/sys/signal Signed-off-by: Sebastiaan van Stijn <[email protected]> * Additional tests and cleanup for cache/contenthash This adds a little extra testing around ** patterns, and adds a (currently skipped) test for copying directories under symlinks (moby#2300). It removes an extra call to `filepath.FromSlash` in `shouldIncludePath` and an unused argument to that function. Signed-off-by: Aaron Lehmann <[email protected]> * all: remove duplicate imports Signed-off-by: Koichi Shiraishi <[email protected]> * all: unify the specs-go package import alias to ocispecs ocispecs means "O"pen "C"ontainer "I"nitiative image-spec/"specs"-go/v1 opencontainers /image-spec/specs-go/v1 Signed-off-by: Koichi Shiraishi <[email protected]> * hack/dockerfiles: upgrade golangci-lint version to v1.41.1 Signed-off-by: Koichi Shiraishi <[email protected]> * golangci-lint: enable importas and add settings for specs-go package Signed-off-by: Koichi Shiraishi <[email protected]> * all: unify the go-digest package import alias to digest Signed-off-by: Koichi Shiraishi <[email protected]> * golangci-lint: add go-digest importas setting Signed-off-by: Koichi Shiraishi <[email protected]> * Fix IncludePattern/ExcludePattern matching The transformation to rootedPatterns seems very wrong and inconsistent with what the copy logic did. Change it to match the copy logic, and add more testing. Signed-off-by: Aaron Lehmann <[email protected]> * dockerfile: fix parsing required key without value Signed-off-by: Tonis Tiigi <[email protected]> * generated files: use "go install" to install binaries Now that this repository moved to go1.16, we can use 'go install' to install these binaries. Signed-off-by: Sebastiaan van Stijn <[email protected]> * util/stack: update protoc options to work with newer versions Generating the util/stack protos failed when updating protoc-gen-go to v1.5.2; it looks like this is the only proto that's not generated using protoc-gen-gogo): util/stack/generate.go protoc-gen-go: unable to determine Go import path for "stack.proto" Please specify either: • a "go_package" option in the .proto source file, or • a "M" argument on the command line. See https://developers.google.com/protocol-buffers/docs/reference/go-generated#package for more information. --go_out: protoc-gen-go: Plugin failed with status code 1. util/stack/generate.go:3: running "protoc": exit status 1 Newer protobuf versions expect a go package to be set. Other .proto files in this repository use the bare package name, but with protoc-gen-go v1.5.2, this produces an error (package names must at least have a "/"). In addition to including the option to the .proto file also changes the generated result (`options go_package "<package name>"`). Using the `-go_opt=M<package name>` option on the other hand, didn't change the result (while still on protoc-gen-go v1.3.5), so I used that option instead. protoc-gen-go v1.5.2 also changed the behavior where the generated file is stored, seemingly relative to the `../../vendor` path specified. This coud be fixed either by setting `--go_out=../../`, which was a bit counter-intuitive, or setting the `--go_opt=paths=source_relative` option. The latter also prevented v1.5.2 from storing the file in `utils/stack/github.com/moby/buildkit/utils/stack/` (sigh). Signed-off-by: Sebastiaan van Stijn <[email protected]> * add missing ExtraHosts to gateway exec Also adding tests for ExtraHosts and NetMode via gateway exec Signed-off-by: Cory Bennett <[email protected]> * add gateway.exec.extrahosts capability Signed-off-by: Cory Bennett <[email protected]> * cache: Fix flightcontrol use in computeBlobChain. Previously, the flightcontrol group was being given a key just set to the ref's ID, which meant that concurrent calls using different values of compressionType, createIfNeeded and forceCompression would incorrectly be de-duplicated. The change here splits up the flightcontrol group into a few separate calls and ensures that all the correct input variables are put into the flightcontrol keys. Signed-off-by: Erik Sipsma <[email protected]> * solver: include cachemap index in flightcontrol. Signed-off-by: Erik Sipsma <[email protected]> * pull: use resolvemode in flightcontrol key. Signed-off-by: Erik Sipsma <[email protected]> * util: remove outdated flightcontrol test assertion. The test was making an assertion that is no longer expected to always be true after moby#2195, which purposely made flightcontrol less deterministic. This lead to occasional failures. Signed-off-by: Erik Sipsma <[email protected]> * update go to 1.17 Signed-off-by: Tonis Tiigi <[email protected]> * gomod: update to go1.17 Signed-off-by: Tonis Tiigi <[email protected]> * Follow links in includedPaths to resolve incorrect caching when source path is behind symlink As discussed in moby#2300, includedPaths does not resolve symlinks when looking up the source path in the prefix tree. If the user requests a path that involves symlinks (for example, /a/foo when a symlink /a -> /b exists), includedPaths will not find it, and will expect nothing to be copied. This does not match the actual copy behavior implemented in fsutil, which will follow symlinks in prefix components of a given path, so it can end up caching an empty result even though the copy will produce a non-empty result, which is quite bad. To fix this, use getFollowLinks to resolve the path before walking it. In the wildcard case, this is done to the non-wildcard prefix of the path (if any), which matches the behavior in fsutil. Fixes the repro case here: https://gist.github.com/aaronlehmann/64054c9a2cff0d27e200cc107bba3d69 Fixes moby#2300 Signed-off-by: Aaron Lehmann <[email protected]> * cmd/buildkitd: replace BurntSushi/toml with pelletier/go-toml The BurntSushi/toml project has been deprecated, and the ecosystem is converging on using pelletier/go-toml as the "canonical" replacement. Signed-off-by: Sebastiaan van Stijn <[email protected]> * control: fix 64bit alignment for buildcount Signed-off-by: Tonis Tiigi <[email protected]> * Use fixed fileutils matching functions This is important for two reasons: 1) Keeps caching logic consistent with recent fsutil changes to use these functions (also vendored here). 2) Allows us to move forward with removal of the original buggy Matches implementation in moby/moby. Signed-off-by: Aaron Lehmann <[email protected]> * Add `estargz` compression type Signed-off-by: Kohei Tokunaga <[email protected]> * Refactor cache metadata interface. There are a few goals with this refactor: 1. Remove external access to fields that no longer make sense and/or won't make sense soon due to other potential changes. For example, there can now be multiple blobs associated with a ref (for different compression types), so the fact that you could access the "Blob" field from the Info method on Ref incorrectly implied there was just a single blob for the ref. This is on top of the fact that there is no need for external access to blob digests. 2. Centralize use of cache metadata inside the cache package. Previously, many parts of the code outside the cache package could obtain the bolt storage item for any ref and read/write it directly. This made it hard to understand what fields are used and when. Now, the Metadata method has been removed from the Ref interface and replaced with getters+setters for metadata fields we want to expose outside the package, which makes it much easier to track and understand. Similar changes have been made to the metadata search interface. 3. Use a consistent getter+setter interface for metadata, replacing the mix of interfaces like Metadata(), Size(), Info() and other inconsistencies. Signed-off-by: Erik Sipsma <[email protected]> * Use containerd/pkg/seccomp.IsEnabled() This replaces the local SeccompSupported() utility for the implementation in containerd, which performs the same check. Signed-off-by: Sebastiaan van Stijn <[email protected]> * Compute diff from the upper dir of overlayfs-based snapshotter Signed-off-by: Kohei Tokunaga <[email protected]> * go.mod: github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 full diff: moby/term@bea5bbe...3f7ff69 updates Azure/go-ansiterm to fix integer overflow on arm Signed-off-by: Sebastiaan van Stijn <[email protected]> * go.mod: split the indirect packages After go1.17, all indirect packages are listed in the go.mod file. In addition, has been introduced the ability to list indirect packages separately. Split the indirect packages to make the dependency packages clearer. Signed-off-by: Koichi Shiraishi <[email protected]> * exporter: support creating blobs with zstd compression Signed-off-by: Tonis Tiigi <[email protected]> * update getremote test for zstd Estargz support has been removed from this test as implementation does not guarantee digest stability and only reason it passed were the exceptions in the test via variant map that ignored cases where timing resulted the digest to go wrong. This needs to be addressed in the follow up if we want to keep estargz support. Signed-off-by: Tonis Tiigi <[email protected]> * Add test case for symlink which is not final path component before wildcard Signed-off-by: Aaron Lehmann <[email protected]> * hack: allow mounting in workdir in shell Signed-off-by: Tonis Tiigi <[email protected]> * Handle the case of multiple path component symlinks (including last component) in wildcard prefix Signed-off-by: Aaron Lehmann <[email protected]> * Use getFollowLinksWalked Signed-off-by: Aaron Lehmann <[email protected]> * bklog: only log tracing ids when span exporter not nil Signed-off-by: Morlay <[email protected]> * Refactor url redacting util Signed-off-by: CrazyMax <[email protected]> * Clean up old TODOs Signed-off-by: Tonis Tiigi <[email protected]> * Move config parsing to a dedicated pkg Signed-off-by: CrazyMax <[email protected]> * Generate and embed build sources Signed-off-by: CrazyMax <[email protected]> * resolver: use different mutext for handlers and hosts hosts mutex is called on initialization, meaning `GetResolver` might block if it is in the middle of auth exchange. This is currently bad in the case where Job initialization needs to register a name before timeout is reached. Signed-off-by: Tonis Tiigi <[email protected]> * resolver: make sure authorizer is not overwritten on other resolvers Authorizer stores the current session.Group so if it is overwritten for another resolver it means that session might have been dropped and authentication will fail. Signed-off-by: Tonis Tiigi <[email protected]> * solver: increase timeout for job registration Signed-off-by: Tonis Tiigi <[email protected]> * go.mod: sort and move self-managed indirect dependencies to first block Signed-off-by: Koichi Shiraishi <[email protected]> * Fix issues moby#1980 and moby#2198 Signed-off-by: Jonathan Giannuzzi <[email protected]> * Add BUILDKIT_SANDBOX_HOSTNAME build-arg Signed-off-by: CrazyMax <[email protected]> * Fix estargz compression loses the original tar metadata Currently, eStargz compression doesn't preserve the original tar metadata (header bytes and their order). This causes failure of `TestGetRemote` because an uncompressed blob converted from a gzip blob provides different digset against the one converted from eStargz blob even if their original tar (computed by differ) are the same. This commit solves this issue by fixing eStargz to preserve original tar's metadata that is modified by eStargz. Signed-off-by: Kohei Tokunaga <[email protected]> * Enhance ANSI color for progress ui Signed-off-by: CrazyMax <[email protected]> * Move resolver config to a dedicated package Signed-off-by: CrazyMax <[email protected]> * Standard user umask for git process Signed-off-by: CrazyMax <[email protected]> * make sure ci runs on version branches Signed-off-by: Tonis Tiigi <[email protected]> * return an error instead of panicking when failing to get edge Signed-off-by: Maxime Lagresle <[email protected]> * Add support for shm size Signed-off-by: CrazyMax <[email protected]> * gha: handle already exist error on save Signed-off-by: Tonis Tiigi <[email protected]> * don't cast Value when pipe is errored Signed-off-by: Maxime Lagresle <[email protected]> * gha: handle missing blob gracefully FromRemote now calls CheckDescriptor to validate if the blob still exists. Otherwise cache loading fallback does not get triggered because cache is actually lazily pulled in only on exporting phase. Signed-off-by: Tonis Tiigi <[email protected]> * solver: make sure previous error gets reset This happens for example when cache loading fails but then fallback step execution succeeds. Signed-off-by: Tonis Tiigi <[email protected]> * vendor: update go-actions-cache to 4d48f2ff Signed-off-by: Tonis Tiigi <[email protected]> * Differ: write diff to the content store over bufio writer Signed-off-by: Kohei Tokunaga <[email protected]> * Do not enable overlayfs differ for fuse-overlayfs-snapshotter Signed-off-by: Kohei Tokunaga <[email protected]> * Converter: make sure uncompressed digest annotation is set Signed-off-by: Kohei Tokunaga <[email protected]> * Use gha cache on CI Signed-off-by: CrazyMax <[email protected]> * Creating tcp socket without using go-connections. Signed-off-by: Jacob MacElroy <[email protected]> * limited: fix possible deadlock when pushhandler calls fetcher Signed-off-by: Tonis Tiigi <[email protected]> * README.md: improve "Building multi-platform images" section Signed-off-by: Akihiro Suda <[email protected]> * Add support for ulimit Signed-off-by: CrazyMax <[email protected]> * solver: fix exporters unsafely sharing records Signed-off-by: Tonis Tiigi <[email protected]> * fix: provide only available capabilities to insecure environment The problem this change is trying to fix are the environments where some capabilities are already dropped, so they can't be granted to the job with `--security=insecure`. I know that probably fixed set of capabilities was implemented to provide a stable build environment, but at the same time this breaks environments with reduced capabilities. Signed-off-by: Andrey Smirnov <[email protected]> * client: allow setting custom dialer for session endpoint Signed-off-by: Tonis Tiigi <[email protected]> * add size to tmpfs mounts Signed-off-by: CrazyMax <[email protected]> * deduplicate mounts Signed-off-by: CrazyMax <[email protected]> * use bytes as given size for tmpfs mount Signed-off-by: CrazyMax <[email protected]> * use `opts.MemBytes` for tmpfs size run mount instruction Signed-off-by: CrazyMax <[email protected]> * Re-add Finalize method to ImmutableRef. It turns out that while Buildkit code did not need this method to be public, moby code does still use it, so we have to re-add it after its removal in moby#2216 (commit b85ef15). This commit is not a revert because some of the changes are still desireable, namely the removal of the "commit" parameter which didn't serve any purpose. Signed-off-by: Erik Sipsma <[email protected]> Co-authored-by: Tonis Tiigi <[email protected]> Co-authored-by: Akihiro Suda <[email protected]> Co-authored-by: Justin Chadwell <[email protected]> Co-authored-by: Erik Sipsma <[email protected]> Co-authored-by: Akihiro Suda <[email protected]> Co-authored-by: CrazyMax <[email protected]> Co-authored-by: Levi Harrison <[email protected]> Co-authored-by: ktock <[email protected]> Co-authored-by: masibw <[email protected]> Co-authored-by: Morlay <[email protected]> Co-authored-by: CrazyMax <[email protected]> Co-authored-by: Claudiu Belu <[email protected]> Co-authored-by: coryb <[email protected]> Co-authored-by: Aaron Lehmann <[email protected]> Co-authored-by: Sebastiaan van Stijn <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sebastiaan van Stijn <[email protected]> Co-authored-by: Anders F Björklund <[email protected]> Co-authored-by: CrazyMax <[email protected]> Co-authored-by: Koichi Shiraishi <[email protected]> Co-authored-by: Jonathan Giannuzzi <[email protected]> Co-authored-by: Maxime Lagresle <[email protected]> Co-authored-by: Jacob MacElroy <[email protected]> Co-authored-by: Andrey Smirnov <[email protected]>
alexcb
added a commit
to earthly/buildkit-old-fork
that referenced
this issue
Oct 28, 2021
* integration: add common context base to all integration tests Signed-off-by: Tonis Tiigi <[email protected]> * vendor: update opentelemetry to 1.0.0-rc Signed-off-by: Tonis Tiigi <[email protected]> * add current tracing context detection and exec propagation Signed-off-by: Tonis Tiigi <[email protected]> (cherry picked from commit bc9a83144c83e9fd78007b7bfe92e8082c59d40e) * add transform package to convert from otlp Signed-off-by: Tonis Tiigi <[email protected]> * tracing: add delegated exporter Signed-off-by: Tonis Tiigi <[email protected]> * otlgrpc: provide a way to get otlp client from grpc conn Hopefully this can be removed with a future upstream change that could make this configurable. The package also needs internal dependency that is copied in. Signed-off-by: Tonis Tiigi <[email protected]> * enable collecting traces via control api Signed-off-by: Tonis Tiigi <[email protected]> * support collecting traces from llb.Exec Signed-off-by: Tonis Tiigi <[email protected]> * client: pass delegated exporter as parameter Avoid client package having dependency on global detect package. Signed-off-by: Tonis Tiigi <[email protected]> * update runc binary to v1.0.0 GA Signed-off-by: Akihiro Suda <[email protected]> * handle unconfigured spans without errors Signed-off-by: Tonis Tiigi <[email protected]> * llb: add constraints to vertex and validate Signed-off-by: Tonis Tiigi <[email protected]> * llb: add constraints to async llb Signed-off-by: Tonis Tiigi <[email protected]> * llb: ensure meta resolver uses platform form constraints Signed-off-by: Tonis Tiigi <[email protected]> * flightcontrol: reduce contention between goroutines Signed-off-by: Tonis Tiigi <[email protected]> * Avoid nil pointer dereference when copying from image with no layers Fix this panic when copying from an image with no layers: ``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x50 pc=0xdd8c17] goroutine 326 [running]: github.com/moby/buildkit/cache/contenthash.(*cacheManager).Checksum(0xc0005ec030, 0x1682c00, 0xc000842140, 0x0, 0x0, 0xc0005d4023, 0x1, 0x0, 0x0, 0x0, ...) /src/cache/contenthash/checksum.go:95 +0x37 github.com/moby/buildkit/cache/contenthash.Checksum(0x1682c00, 0xc000842140, 0x0, 0x0, 0xc0005d4023, 0x1, 0x0, 0x0, 0x0, 0x0, ...) /src/cache/contenthash/checksum.go:59 +0xd5 github.com/moby/buildkit/solver/llbsolver.NewContentHashFunc.func1.1(0x0, 0x4425d6) /src/solver/llbsolver/result.go:59 +0x20a golang.org/x/sync/errgroup.(*Group).Go.func1(0xc00056a360, 0xc000594510) /src/vendor/golang.org/x/sync/errgroup/errgroup.go:57 +0x59 created by golang.org/x/sync/errgroup.(*Group).Go /src/vendor/golang.org/x/sync/errgroup/errgroup.go:54 +0x66 ``` When the path is "/", we allow it because it's a noop. Based on moby#2185 Signed-off-by: Aaron Lehmann <[email protected]> * Add test for copying from scratch Signed-off-by: Aaron Lehmann <[email protected]> * Check that scratch is mounted as empty dir Signed-off-by: Aaron Lehmann <[email protected]> * Make error message consistent when layer is empty Signed-off-by: Aaron Lehmann <[email protected]> * Test with tonistiigi/test:nolayers as well Signed-off-by: Aaron Lehmann <[email protected]> * ensure containerd io is complete and closed before returning Signed-off-by: Cory Bennett <[email protected]> * [moby#2112] progress.Controller should own the progress.Writer to prevent leaks Signed-off-by: Cory Bennett <[email protected]> * [moby#2112] progress.FromContext returns a writer factory this allows progress.Controller to manage the writer lifecycle Signed-off-by: Cory Bennett <[email protected]> * contenthash: use SeekLowerBound to seek radix tree Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: fix git version detection Signed-off-by: Tonis Tiigi <[email protected]> * Add support for heredocs with ONBUILD Signed-off-by: Justin Chadwell <[email protected]> * dockerfile: use none differ for dockerfile/dockerignore This avoids wrong metadata matches on small files Signed-off-by: Tonis Tiigi <[email protected]> * progressui: print logs for failed step as summary in plain mode Signed-off-by: Tonis Tiigi <[email protected]> * grpcerrors: avoid rpc error wrapping in error messages Signed-off-by: Tonis Tiigi <[email protected]> * exec: improve error message on exec errors Signed-off-by: Tonis Tiigi <[email protected]> * Improve heredoc parsing to allow more generic words Previously, heredoc names were restricted to simple alphanumeric strings. However, heredocs should support much more complex use-cases, including quoting anywhere, as well as allowing special symbols like `.` for easily expressing file extensions. This patch adds support for these more complex cases, by using the shell lexer to parse each heredoc name. Additionally, we include improvements to the lexer to optionally preserve escape tokens to avoid problems when lexing words that have already been lexed before. Signed-off-by: Justin Chadwell <[email protected]> * Improve progress and history messages for heredoc-related commands Signed-off-by: Justin Chadwell <[email protected]> * Remove unneeded Finalize method from ImmutableRef. Finalize was only used outside the cache package in one place, which called it with the commit arg set to false. The code path followed when commit==false turned out to essentially be a no-op because it set "retain cache" to true if it was already set to true. It was thus safe to remove the only external call to it and remove it from the interface. This should be helpful for future efforts to simplify the equal{Mutable,Immutable} fields in cacheRecord, which exist due to the "lazy commit" feature that Finalize is tied into. Signed-off-by: Erik Sipsma <[email protected]> * Fix ref leak if fileop ref fails to mount. Signed-off-by: Erik Sipsma <[email protected]> * add error suggest pkg Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: suggest mistyped flag names Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: provide suggestions for mount options Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: add tests for error suggestions Signed-off-by: Tonis Tiigi <[email protected]> * dockerfile: remove unnecessary error wrappings Signed-off-by: Tonis Tiigi <[email protected]> * enable riscv64 build Signed-off-by: Tonis Tiigi <[email protected]> * Update QEMU emulators Signed-off-by: CrazyMax <[email protected]> * dockerfile: move run network to stable channel Signed-off-by: Tonis Tiigi <[email protected]> * Automatically detect default git branch Instead of just assuming that the default branch is master, use ls-remote to find out. Also removed tests that didn't specifiy a branch but required authentication, because those will fail now that the repo is actually checked. Signed-off-by: Levi Harrison <[email protected]> * Moved getDefaultBranch function to gitsource It is my suspecion that the tests were failing on previous commits because of the lack of authentication and other stuff like that available in gitidentifier as compared to gitsource Signed-off-by: Levi Harrison <[email protected]> * Fix tests Unfortunately, further test cases will have to be removed because gitindentifier will now leave the branch blank instead of filling it in Signed-off-by: Levi Harrison <[email protected]> * git: fix default branch detection Signed-off-by: Tonis Tiigi <[email protected]> * Enable to forcefully specify compression type Signed-off-by: ktock <[email protected]> * Add full timestamp to logs Signed-off-by: Yamazaki Masashi <[email protected]> * Remove meaningless encode Signed-off-by: Yamazaki Masashi <[email protected]> * Ignore missing providers for blobs w/ same chainid. GetByBlob checks to see if there are any other blobs with the same (uncompressed) ChainID and, if so, reuses their unpacked snapshot if it exists. The problem is if this code finds a match, it was trying to get the matching record, but couldn't do so when the match is lazy because the caller doesn't necessarily have descriptor handlers setup for it. This commit changes the behavior to just ignore any match with the same ChainID that's also lazy as they just aren't usable for the snapshot-reuse optimization. Signed-off-by: Erik Sipsma <[email protected]> * authprovider: handle eaccess on storing token seeds Signed-off-by: Tonis Tiigi <[email protected]> * log with traceID and spanID Signed-off-by: Morlay <[email protected]> * github: update CI buildkit to v0.9.0-rc1 Signed-off-by: Tonis Tiigi <[email protected]> * initial version of github cache Signed-off-by: Tonis Tiigi <[email protected]> * vendor: add goactionscache Signed-off-by: Tonis Tiigi <[email protected]> * caps: add cap for gha cache backend Signed-off-by: Tonis Tiigi <[email protected]> * remove tracetransform package Signed-off-by: Tonis Tiigi <[email protected]> * resolver: increase default idle conns reuse The current default were even lower than stdlib defaults. Signed-off-by: Tonis Tiigi <[email protected]> * refactor to use util/bklog instead of using logurs directly Signed-off-by: Morlay <[email protected]> * GitHub Actions cache docs Signed-off-by: CrazyMax <[email protected]> * Skips getting UID/GUID if passwd/group file is not found When running a WORKDIR instruction, buildkit will create that folder and chown it to the currently set user. For this, it will try to read the /etc/passwd file to get the proper UID, and if that user is not found in the file, the root user will be considered as the owner. However, Windows image do not have that file, which will result in an error while building the image. We can consider not finding the /etc/passwd file as the same as not finding the user in the file, which would solve this issue. Signed-off-by: Claudiu Belu <[email protected]> * add per domain semaphore to limit concurrent connections This is a safer alternative until we figure out why http.Transport based limiting fails. Some connections like cache export/import do not have a domain key atm and these connections use global pool. Signed-off-by: Tonis Tiigi <[email protected]> * update to github.com/containerd/containerd v1.5.3 Signed-off-by: coryb <[email protected]> * vendor: update go-actions-cache with custom client support Signed-off-by: Tonis Tiigi <[email protected]> * tracing: update to otelhttp roundtripper Signed-off-by: Tonis Tiigi <[email protected]> * Enhance test matrix Signed-off-by: CrazyMax <[email protected]> * fix dropped pull progress output due to canceled context fixes moby#2248 Signed-off-by: coryb <[email protected]> * Add span for layer export This can be a significant amount of time that isn't currently accounted for in traces. Signed-off-by: Aaron Lehmann <[email protected]> * new implementation for limiting tcp connections The previous implementation had many issues. Eg. on fetch, even if the data already existed and no remote connections were needed the request would still be waiting in the queue. Or if two fetches of same blob happened together they would take up two places in queue although there was only one remote request. Signed-off-by: Tonis Tiigi <[email protected]> * limited: allow extra high-priority connection for json requests Signed-off-by: Tonis Tiigi <[email protected]> * ensure wrappers support seeking to continue partial downloads Signed-off-by: Tonis Tiigi <[email protected]> * contentutil: change offset to int64 to simplify Signed-off-by: Tonis Tiigi <[email protected]> * Exporter config digest typo Signed-off-by: CrazyMax <[email protected]> * daemonless: wait for daemon to finish before exit Signed-off-by: Tonis Tiigi <[email protected]> * github: update CI buildkit to v0.9.0 Signed-off-by: Tonis Tiigi <[email protected]> * add docs for new config options Signed-off-by: Tonis Tiigi <[email protected]> * add ktock and crazy-max to maintainers Signed-off-by: Tonis Tiigi <[email protected]> * Update Dockerfile references to use 1.3 Signed-off-by: Tonis Tiigi <[email protected]> * docs: update images-readme to v0.9 Signed-off-by: Tonis Tiigi <[email protected]> * Bump to codecov/codecov-action v2 Signed-off-by: CrazyMax <[email protected]> * build(deps): bump github.com/containerd/containerd from 1.5.3 to 1.5.4 Bumps [github.com/containerd/containerd](https://github.com/containerd/containerd) from 1.5.3 to 1.5.4. - [Release notes](https://github.com/containerd/containerd/releases) - [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md) - [Commits](containerd/containerd@v1.5.3...v1.5.4) --- updated-dependencies: - dependency-name: github.com/containerd/containerd dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> * util/tracing: remove incorrect import enforcing comment This import comment caused compilation of buildx to fail if `GO111MODULE` was set to `off`: Without `GO111MODULE` set (but with `-mod=vendor`: echo $GO111MODULE export PKG=github.com/docker/buildx export LDFLAGS="-X ${PKG}/version.Version=$(git describe --match 'v[0-9]*' --always --tags) -X ${PKG}/version.Revision=$(git rev-parse HEAD) -X ${PKG}/version.Package=${PKG}" GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx bin/docker-buildx version github.com/docker/buildx v0.6.0 d9ee3b134cbc2d09513fa7fee4176a3919e05887 When setting `GO111MODULE=off`, it fails on the incorrect import path in the vendored file (looks like GO111MODULE=on ignores import-path comments?): export GO111MODULE=off root@5a55ec1c1eed:/go/src/github.com/docker/buildx# GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx vendor/github.com/moby/buildkit/client/client.go:20:2: code in directory /go/src/github.com/docker/buildx/vendor/github.com/moby/buildkit/util/tracing/otlptracegrpc expects import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/connection/connection.go:33:2: found import comments "go.opentelemetry.io/otel/exporters/otlp/internal/otlpconfig" (options.go) and "go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig" (optiontypes.go) in /go/src/github.com/docker/buildx/vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig Signed-off-by: Sebastiaan van Stijn <[email protected]> * Fix protoc link Signed-off-by: CrazyMax <[email protected]> * Allow ExitError type to be transmitted over GRPC This will allow clients to retrieve exit error codes returned during a solve without parsing the error messages. Signed-off-by: Aaron Lehmann <[email protected]> * Update to github.com/opencontainers/runc v1.0.1 Signed-off-by: CrazyMax <[email protected]> * Split cache options doc for each exporter Signed-off-by: CrazyMax <[email protected]> * Set default socket permissions to 660 The systemd default is 666, it seems. Signed-off-by: Anders F Björklund <[email protected]> * fix SecurityMode being dropped on gateway container Start Signed-off-by: Cory Bennett <[email protected]> * bump containerd from 1.5.4 to 1.5.5 Signed-off-by: CrazyMax <[email protected]> * go.mod: golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c In preparation of replacing the deprecated github.com/docker/docker/pkg/signal, which uses this version (updating it separately for easier review). Signed-off-by: Sebastiaan van Stijn <[email protected]> * replace use of deprecated github.com/docker/docker/pkg/signal This package was moved to a separate module in github.com/moby/sys/signal Signed-off-by: Sebastiaan van Stijn <[email protected]> * Additional tests and cleanup for cache/contenthash This adds a little extra testing around ** patterns, and adds a (currently skipped) test for copying directories under symlinks (moby#2300). It removes an extra call to `filepath.FromSlash` in `shouldIncludePath` and an unused argument to that function. Signed-off-by: Aaron Lehmann <[email protected]> * all: remove duplicate imports Signed-off-by: Koichi Shiraishi <[email protected]> * all: unify the specs-go package import alias to ocispecs ocispecs means "O"pen "C"ontainer "I"nitiative image-spec/"specs"-go/v1 opencontainers /image-spec/specs-go/v1 Signed-off-by: Koichi Shiraishi <[email protected]> * hack/dockerfiles: upgrade golangci-lint version to v1.41.1 Signed-off-by: Koichi Shiraishi <[email protected]> * golangci-lint: enable importas and add settings for specs-go package Signed-off-by: Koichi Shiraishi <[email protected]> * all: unify the go-digest package import alias to digest Signed-off-by: Koichi Shiraishi <[email protected]> * golangci-lint: add go-digest importas setting Signed-off-by: Koichi Shiraishi <[email protected]> * Fix IncludePattern/ExcludePattern matching The transformation to rootedPatterns seems very wrong and inconsistent with what the copy logic did. Change it to match the copy logic, and add more testing. Signed-off-by: Aaron Lehmann <[email protected]> * dockerfile: fix parsing required key without value Signed-off-by: Tonis Tiigi <[email protected]> * generated files: use "go install" to install binaries Now that this repository moved to go1.16, we can use 'go install' to install these binaries. Signed-off-by: Sebastiaan van Stijn <[email protected]> * util/stack: update protoc options to work with newer versions Generating the util/stack protos failed when updating protoc-gen-go to v1.5.2; it looks like this is the only proto that's not generated using protoc-gen-gogo): util/stack/generate.go protoc-gen-go: unable to determine Go import path for "stack.proto" Please specify either: • a "go_package" option in the .proto source file, or • a "M" argument on the command line. See https://developers.google.com/protocol-buffers/docs/reference/go-generated#package for more information. --go_out: protoc-gen-go: Plugin failed with status code 1. util/stack/generate.go:3: running "protoc": exit status 1 Newer protobuf versions expect a go package to be set. Other .proto files in this repository use the bare package name, but with protoc-gen-go v1.5.2, this produces an error (package names must at least have a "/"). In addition to including the option to the .proto file also changes the generated result (`options go_package "<package name>"`). Using the `-go_opt=M<package name>` option on the other hand, didn't change the result (while still on protoc-gen-go v1.3.5), so I used that option instead. protoc-gen-go v1.5.2 also changed the behavior where the generated file is stored, seemingly relative to the `../../vendor` path specified. This coud be fixed either by setting `--go_out=../../`, which was a bit counter-intuitive, or setting the `--go_opt=paths=source_relative` option. The latter also prevented v1.5.2 from storing the file in `utils/stack/github.com/moby/buildkit/utils/stack/` (sigh). Signed-off-by: Sebastiaan van Stijn <[email protected]> * add missing ExtraHosts to gateway exec Also adding tests for ExtraHosts and NetMode via gateway exec Signed-off-by: Cory Bennett <[email protected]> * add gateway.exec.extrahosts capability Signed-off-by: Cory Bennett <[email protected]> * cache: Fix flightcontrol use in computeBlobChain. Previously, the flightcontrol group was being given a key just set to the ref's ID, which meant that concurrent calls using different values of compressionType, createIfNeeded and forceCompression would incorrectly be de-duplicated. The change here splits up the flightcontrol group into a few separate calls and ensures that all the correct input variables are put into the flightcontrol keys. Signed-off-by: Erik Sipsma <[email protected]> * solver: include cachemap index in flightcontrol. Signed-off-by: Erik Sipsma <[email protected]> * pull: use resolvemode in flightcontrol key. Signed-off-by: Erik Sipsma <[email protected]> * util: remove outdated flightcontrol test assertion. The test was making an assertion that is no longer expected to always be true after moby#2195, which purposely made flightcontrol less deterministic. This lead to occasional failures. Signed-off-by: Erik Sipsma <[email protected]> * update go to 1.17 Signed-off-by: Tonis Tiigi <[email protected]> * gomod: update to go1.17 Signed-off-by: Tonis Tiigi <[email protected]> * Follow links in includedPaths to resolve incorrect caching when source path is behind symlink As discussed in moby#2300, includedPaths does not resolve symlinks when looking up the source path in the prefix tree. If the user requests a path that involves symlinks (for example, /a/foo when a symlink /a -> /b exists), includedPaths will not find it, and will expect nothing to be copied. This does not match the actual copy behavior implemented in fsutil, which will follow symlinks in prefix components of a given path, so it can end up caching an empty result even though the copy will produce a non-empty result, which is quite bad. To fix this, use getFollowLinks to resolve the path before walking it. In the wildcard case, this is done to the non-wildcard prefix of the path (if any), which matches the behavior in fsutil. Fixes the repro case here: https://gist.github.com/aaronlehmann/64054c9a2cff0d27e200cc107bba3d69 Fixes moby#2300 Signed-off-by: Aaron Lehmann <[email protected]> * cmd/buildkitd: replace BurntSushi/toml with pelletier/go-toml The BurntSushi/toml project has been deprecated, and the ecosystem is converging on using pelletier/go-toml as the "canonical" replacement. Signed-off-by: Sebastiaan van Stijn <[email protected]> * control: fix 64bit alignment for buildcount Signed-off-by: Tonis Tiigi <[email protected]> * Use fixed fileutils matching functions This is important for two reasons: 1) Keeps caching logic consistent with recent fsutil changes to use these functions (also vendored here). 2) Allows us to move forward with removal of the original buggy Matches implementation in moby/moby. Signed-off-by: Aaron Lehmann <[email protected]> * Add `estargz` compression type Signed-off-by: Kohei Tokunaga <[email protected]> * Refactor cache metadata interface. There are a few goals with this refactor: 1. Remove external access to fields that no longer make sense and/or won't make sense soon due to other potential changes. For example, there can now be multiple blobs associated with a ref (for different compression types), so the fact that you could access the "Blob" field from the Info method on Ref incorrectly implied there was just a single blob for the ref. This is on top of the fact that there is no need for external access to blob digests. 2. Centralize use of cache metadata inside the cache package. Previously, many parts of the code outside the cache package could obtain the bolt storage item for any ref and read/write it directly. This made it hard to understand what fields are used and when. Now, the Metadata method has been removed from the Ref interface and replaced with getters+setters for metadata fields we want to expose outside the package, which makes it much easier to track and understand. Similar changes have been made to the metadata search interface. 3. Use a consistent getter+setter interface for metadata, replacing the mix of interfaces like Metadata(), Size(), Info() and other inconsistencies. Signed-off-by: Erik Sipsma <[email protected]> * Use containerd/pkg/seccomp.IsEnabled() This replaces the local SeccompSupported() utility for the implementation in containerd, which performs the same check. Signed-off-by: Sebastiaan van Stijn <[email protected]> * Compute diff from the upper dir of overlayfs-based snapshotter Signed-off-by: Kohei Tokunaga <[email protected]> * go.mod: github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 full diff: moby/term@bea5bbe...3f7ff69 updates Azure/go-ansiterm to fix integer overflow on arm Signed-off-by: Sebastiaan van Stijn <[email protected]> * go.mod: split the indirect packages After go1.17, all indirect packages are listed in the go.mod file. In addition, has been introduced the ability to list indirect packages separately. Split the indirect packages to make the dependency packages clearer. Signed-off-by: Koichi Shiraishi <[email protected]> * exporter: support creating blobs with zstd compression Signed-off-by: Tonis Tiigi <[email protected]> * update getremote test for zstd Estargz support has been removed from this test as implementation does not guarantee digest stability and only reason it passed were the exceptions in the test via variant map that ignored cases where timing resulted the digest to go wrong. This needs to be addressed in the follow up if we want to keep estargz support. Signed-off-by: Tonis Tiigi <[email protected]> * Add test case for symlink which is not final path component before wildcard Signed-off-by: Aaron Lehmann <[email protected]> * hack: allow mounting in workdir in shell Signed-off-by: Tonis Tiigi <[email protected]> * Handle the case of multiple path component symlinks (including last component) in wildcard prefix Signed-off-by: Aaron Lehmann <[email protected]> * Use getFollowLinksWalked Signed-off-by: Aaron Lehmann <[email protected]> * bklog: only log tracing ids when span exporter not nil Signed-off-by: Morlay <[email protected]> * Refactor url redacting util Signed-off-by: CrazyMax <[email protected]> * Clean up old TODOs Signed-off-by: Tonis Tiigi <[email protected]> * Move config parsing to a dedicated pkg Signed-off-by: CrazyMax <[email protected]> * Generate and embed build sources Signed-off-by: CrazyMax <[email protected]> * resolver: use different mutext for handlers and hosts hosts mutex is called on initialization, meaning `GetResolver` might block if it is in the middle of auth exchange. This is currently bad in the case where Job initialization needs to register a name before timeout is reached. Signed-off-by: Tonis Tiigi <[email protected]> * resolver: make sure authorizer is not overwritten on other resolvers Authorizer stores the current session.Group so if it is overwritten for another resolver it means that session might have been dropped and authentication will fail. Signed-off-by: Tonis Tiigi <[email protected]> * solver: increase timeout for job registration Signed-off-by: Tonis Tiigi <[email protected]> * go.mod: sort and move self-managed indirect dependencies to first block Signed-off-by: Koichi Shiraishi <[email protected]> * Fix issues moby#1980 and moby#2198 Signed-off-by: Jonathan Giannuzzi <[email protected]> * Add BUILDKIT_SANDBOX_HOSTNAME build-arg Signed-off-by: CrazyMax <[email protected]> * Fix estargz compression loses the original tar metadata Currently, eStargz compression doesn't preserve the original tar metadata (header bytes and their order). This causes failure of `TestGetRemote` because an uncompressed blob converted from a gzip blob provides different digset against the one converted from eStargz blob even if their original tar (computed by differ) are the same. This commit solves this issue by fixing eStargz to preserve original tar's metadata that is modified by eStargz. Signed-off-by: Kohei Tokunaga <[email protected]> * Enhance ANSI color for progress ui Signed-off-by: CrazyMax <[email protected]> * Move resolver config to a dedicated package Signed-off-by: CrazyMax <[email protected]> * Standard user umask for git process Signed-off-by: CrazyMax <[email protected]> * make sure ci runs on version branches Signed-off-by: Tonis Tiigi <[email protected]> * return an error instead of panicking when failing to get edge Signed-off-by: Maxime Lagresle <[email protected]> * Add support for shm size Signed-off-by: CrazyMax <[email protected]> * don't cast Value when pipe is errored Signed-off-by: Maxime Lagresle <[email protected]> * Apply Earthly changes to newer buildkit version This commit squashes previous work done in the earthly-main branch 199ad6a into a single commit which is rebased against moby/master branch d429b0b Co-authored-by: Tõnis Tiigi <[email protected]> Co-authored-by: Akihiro Suda <[email protected]> Co-authored-by: Akihiro Suda <[email protected]> Co-authored-by: Aaron Lehmann <[email protected]> Co-authored-by: Cory Bennett <[email protected]> Co-authored-by: Justin Chadwell <[email protected]> Co-authored-by: Erik Sipsma <[email protected]> Co-authored-by: CrazyMax <[email protected]> Co-authored-by: Levi Harrison <[email protected]> Co-authored-by: ktock <[email protected]> Co-authored-by: masibw <[email protected]> Co-authored-by: Morlay <[email protected]> Co-authored-by: CrazyMax <[email protected]> Co-authored-by: Claudiu Belu <[email protected]> Co-authored-by: Sebastiaan van Stijn <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sebastiaan van Stijn <[email protected]> Co-authored-by: Anders F Björklund <[email protected]> Co-authored-by: CrazyMax <[email protected]> Co-authored-by: Koichi Shiraishi <[email protected]> Co-authored-by: Jonathan Giannuzzi <[email protected]> Co-authored-by: Maxime Lagresle <[email protected]>
sarahhodne
pushed a commit
to sarahhodne/buildkit
that referenced
this issue
Aug 22, 2022
Signed-off-by: Jonathan Giannuzzi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Cross posting from docker/buildx#637 as it's probably more relevant directly to buildkit.
I've been building images with buildx primary for the past couple of weeks and I have seen some very odd behavior where the image manifest appears to be written incorrectly. So far, this only appears to happen when I am re-running a build where at least a portion of the image is cached. Here is an example scenario but it doesn't just happen for this one project. Another user reported the same problem on another image with a build that is done using the exact same template as reported here. Here is the GitHub repo for the app I am showing the logs for below.
docker version:Here are the
buildxsettings:Creating the builder:
For build
buildkit.toml, it just includes values to use my registry cache/mirror to avoid rate limits of Docker Hub:Here is my
docker buildx ls&docker buildx inspectoutput:Image used by the buildx container:
I have also tried using from master just in case there was something that has been fixed since then and I have seen the same issue:
I am seeing some messages in the buildx builder container and I am not sure what the errors exactly mean but they don't seem to specifically indicate that something is wrong as they happen when an image was built fine:
Here is an example of a build command where I am using the same basic structure across many projects:
My registry is just a simple v2 open source registry with valid SSL certs for https.
An example error that you might see is something like this:
Rebuilding the image appears to re-write the manifest and it works fine. Here are some outputs from a few commands showing the issue on these two images:
Broken:
mbentley/airsonic@sha256:e78d073b03e7802217825e7763d33ebf5f07ed4d2adc0bd94d4345854a15d4c3Working:
mbentley/airsonic@sha256:29aa0bb26757325405da60b4aa7d92fa3cf0fb489982ed4c19bc3580ad94ada1docker historyof broken dh_output.txt:docker historyof working dh_output2.txt:In case it is helpful, here are the
docker inspectoutputs:Broken
Working
I am using Jenkins to perform my builds. I see nothing to indicate why this is failing:
Broken build:
Working build:
The text was updated successfully, but these errors were encountered: