When this was called concurrently from the moby image
exporter there could be a data race where a layer was
written to the refs map when it was already there.
In that case the reference count got mixed up and on
release only one of these layers was actually released.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 37545cc644)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Archives being unpacked by Dockerfiles may have been created on other
OSes with different conventions and semantics for xattrs, making them
impossible to apply when extracting. Restore the old best-effort xattr
behaviour users have come to depend on in the classic builder.
The (archive.Archiver).UntarPath function does not allow the options
passed to Untar to be customized. It also happens to be a trivial
wrapper around the Untar function. Inline the function body and add the
option.
Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 5bcd2f6860)
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Previously this was done indirectly - the `compare` function didn't
check the `ArgsEscaped`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 96d461d27e)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Make sure the cache candidate platform matches the requested.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 877ebbe038)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The names of extended attributes are not completely freeform. Attributes
are namespaced, and the kernel enforces (among other things) that only
attributes whose names are prefixed with a valid namespace are
permitted. The name of the attribute therefore needs to be known in
order to diagnose issues with lsetxattr. Include the name of the
extended attribute in the errors returned from the Lsetxattr and
Lgetxattr so users and us can more easily troubleshoot xattr-related
issues. Include the name in a separate rich-error field to provide code
handling the error enough information to determine whether or not the
failure can be ignored.
Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 43bf65c174)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Escape period (.) so regular expression does not match any character before "git".
Signed-off-by: David Dooling <david.dooling@docker.com>
(cherry picked from commit 768146b1b0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
That means 'null', not that we can call builder-next on Windows. If and
when we do get builder-next going, this will need to be solved properly
in some way.
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The only use is in `builder/builder-next/adapters/snapshot.EnsureLayer()`,
which always calls the function with an _empty_ `oldTarDataPath`;
7082aecd54/builder/builder-next/adapters/snapshot/layer.go (L81)
When called with an empty `oldTarDataPath`, this function was an alias for
`checksumForGraphIDNoTarsplit`, so let's make it that.
Note that this code was added in 500e77bad0, as
part of the migration from "v1" images to "v2" (content-addressable) images.
Given that the remaining code lives in a "migration" file, possibly more code
can be removed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `ExecBackend.ContainerKill()` function was called before removing a build-
container.
This function is backed by `daemon.ContainerKill()` which, if no signal is passed,
performed a `daemon.Kill()`, using `SIGKILL` as signal. However, the
`ExecBackend.ContainerRm()` (backed by `daemonContainerRm()`), which is called
after this, is executed with the `ForceRemove` option set, which calls
`daemon.cleanupContainer()` with `ForceRemove` set, which also results in
`daemon.Kill()` being called:
1a0c15abbb/daemon/delete.go (L84-L95)
This makes the `ExecBackend.ContainerKill()` redundant, so removing this from
the interface.
While looking at this code, one (possible) race-condition was found in
`daemon.cleanupContainer()`, where `daemon.Kill()` could return a `errdefs.Conflict`
if the container was already stopped. An extra check was added for this case to
prevent `daemon.cleanupContainer()` from terminating early.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prevent cleanup from terminating early when failing to remove a container;
- continue trying to remove remaining containers
- ignore errors due to containers that were not found
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was just a very thin wrapper for backend.ContainerRm(), and the
error it returned was not handled, so moving this code inline.
Moving it inline also allows differentiating the error message to
distinguish the "removing all intermediate containers" from "removing container"
(when cancelling a build).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This repository is not yet a module (i.e., does not have a `go.mod`). This
is not problematic when building the code in GOPATH or "vendor" mode, but
when using the code as a module-dependency (in module-mode), different semantics
are applied since Go1.21, which switches Go _language versions_ on a per-module,
per-package, or even per-file base.
A condensed summary of that logic [is as follows][1]:
- For modules that have a go.mod containing a go version directive; that
version is considered a minimum _required_ version (starting with the
go1.19.13 and go1.20.8 patch releases: before those, it was only a
recommendation).
- For dependencies that don't have a go.mod (not a module), go language
version go1.16 is assumed.
- Likewise, for modules that have a go.mod, but the file does not have a
go version directive, go language version go1.16 is assumed.
- If a go.work file is present, but does not have a go version directive,
language version go1.17 is assumed.
When switching language versions, Go _downgrades_ the language version,
which means that language features (such as generics, and `any`) are not
available, and compilation fails. For example:
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
Note that these fallbacks are per-module, per-package, and can even be
per-file, so _(indirect) dependencies_ can still use modern language
features, as long as their respective go.mod has a version specified.
Unfortunately, these failures do not occur when building locally (using
vendor / GOPATH mode), but will affect consumers of the module.
Obviously, this situation is not ideal, and the ultimate solution is to
move to go modules (add a go.mod), but this comes with a non-insignificant
risk in other areas (due to our complex dependency tree).
We can revert to using go1.16 language features only, but this may be
limiting, and may still be problematic when (e.g.) matching signatures
of dependencies.
There is an escape hatch: adding a `//go:build` directive to files that
make use of go language features. From the [go toolchain docs][2]:
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a `go.mod` file with a go line such as `go 1.21` or `go 1.21.3`.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding `//go:build go1.22` to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This patch adds `//go:build` directives to those files using recent additions
to the language. It's currently using go1.19 as version to match the version
in our "vendor.mod", but we can consider being more permissive ("any" requires
go1.18 or up), or more "optimistic" (force go1.21, which is the version we
currently use to build).
For completeness sake, note that any file _without_ a `//go:build` directive
will continue to use go1.16 language version when used as a module.
[1]: 58c28ba286/src/cmd/go/internal/gover/version.go (L9-L56)
[2]: https://go.dev/doc/toolchain
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `ContainerCreateConfig` and `ContainerRmConfig` structs are used for
options to be passed to the backend, and are not used in client code.
Thess struct currently is intended for internal use only (for example, the
`AdjustCPUShares` is an internal implementation details to adjust the container's
config when older API versions are used).
Somewhat ironically, the signature of the Backend has a nicer UX than that
of the client's `ContainerCreate` signature (which expects all options to
be passed as separate arguments), so we may want to update that signature
to be closer to what the backend is using, but that can be left as a future
exercise.
This patch moves the `ContainerCreateConfig` and `ContainerRmConfig` structs
to the backend package to prevent it being imported in the client, and to make
it more clear that this is part of internal APIs, and not public-facing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use a strong type for the DNS IP-addresses so that we can use flags.IPSliceVar,
instead of implementing our own option-type and validation.
Behavior should be the same, although error-messages have slightly changed:
Before this patch:
dockerd --dns 1.1.1.1oooo --validate
Status: invalid argument "1.1.1.1oooo" for "--dns" flag: 1.1.1.1oooo is not an ip address
See 'dockerd --help'., Code: 125
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1"]}
dockerd --dns 2.2.2.2 --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1])
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1oooo"]}
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: 1.1.1.1ooooo is not an ip address
With this patch:
dockerd --dns 1.1.1.1oooo --validate
Status: invalid argument "1.1.1.1oooo" for "--dns" flag: invalid string being converted to IP address: 1.1.1.1oooo
See 'dockerd --help'., Code: 125
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1"]}
dockerd --dns 2.2.2.2 --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives are specified both as a flag and in the configuration file: dns: (from flag: [2.2.2.2], from file: [1.1.1.1])
cat /etc/docker/daemon.json
{"dns": ["1.1.1.1oooo"]}
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: invalid IP address: 1.1.1.1oooo
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The build target is not quoted and it makes it difficult for some
persons to see what the problem is.
By quoting it we emphasize that the target name is variable.
Signed-off-by: Frank Villaro-Dixon <frank.villarodixon@merkle.com>
These platforms are filled by default from containerd
introspection API and may not be normalized. Initializing
wrong platform in here results in incorrect platform
for BUILDPLATFORM and TARGETPLATFORM build-args for
Dockerfile frontend (and probably other side effects).
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
The github.com/opencontainers/runc/libcontainer/user package was moved
to a separate module. While there's still uses of the old module in
our code-base, runc itself is migrating to the new module, and deprecated
the old package (for runc 1.2).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
update buildkit to the latest code in the v0.12 branch:
full diff: f94ed7cec3...6560bb937e
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
The github.com/containerd/containerd/log package was moved to a separate
module, which will also be used by upcoming (patch) releases of containerd.
This patch moves our own uses of the package to use the new module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Now that this is a generic, we can define a struct type at the package
level, and remove the casting logic necessary when we had to use
interface{}.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
SourcePolicy was accounted for in 330cf7ae7d
TODO: replace applySourcePolicies with BuildKit's implementation, which
is currently unexported.
Co-authored-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
With BuildKit 0.12, some existing types are now required to be wrapped
by new types:
* containerd's LeaseManager and ContentStore have to be a
(namespace-aware) BuildKit type since f044e0a946
* BuildKit's solver.CacheManager is used instead of
bboltstorage.CacheKeyStorage since 2b30693409
* The MaxAge config field is a bkconfig.Duration since e06c96274f
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
The following changes were required:
* integration/build: progressui's signature changed in 6b8fbed01e
* builder-next: flightcontrol.Group has become a generic type in 8ffc03b8f0
* builder-next/executor: add github.com/moby/buildkit/executor/resources types, necessitated by 6e87e4b455
* builder-next: stub util/network/Namespace.Sample(), necessitated by 963f16179f
Co-authored-by: CrazyMax <crazy-max@users.noreply.github.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
The DeepEqual ignore required in the daemon tests is a bit ugly, but it
works given the new protoc output.
We also have to ignore lints related to schema1 deprecations; these do
not apply as we must continue to support this schema version.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
The current executor is only tested on Linux, so let's be honest about
that. Stubbing this correctly helps avoid incorrectly trying to call
into Linux-only code in e.g. libnetwork.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Implement a function that returns an error to replace existing uses of
the IsOSSupported utility, where callers had to produce the error after
checking.
The IsOSSupported function was used in combination with images, so implementing
a utility in "image" to prevent having to import pkg/system (which contains many
unrelated functions)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This uses otel standard environment variables to configure tracing in
the daemon.
It also adds support for propagating trace contexts in the client and
reading those from the API server.
See
https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/
for details on otel environment variables.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The BuildKit dockerignore package was integrated in the patternmatcher
repository / module. This patch updates our uses of the BuildKit package
with its new location.
A small local change was made to keep the format of the existing error message,
because the "ignorefile" package is slightly more agnostic in that respect
and doesn't include ".dockerignore" in the error message.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is something that stood out to me: removing the intermediate
container is part of a build step, but unlike the other output from
the build, wasn't indented (and prefixed with `--->`) to be shown
as part of the build.
This patch adds the `--->` prefix, to make it clearer what step the
removal was part of.
While at it, I also updated the message itself: this output is printed
_after_ the intermediate container has been removed, so we may as well
make it match reality, so I changed "removing" to "removed".
Before:
echo -e 'FROM busybox\nRUN echo hello > /dev/null\nRUN echo world > /dev/null\n' | DOCKER_BUILDKIT=0 docker build --no-cache -
Sending build context to Docker daemon 2.048kB
Step 1/3 : FROM busybox
---> a416a98b71e2
Step 2/3 : RUN echo hello > /dev/null
---> Running in a1a65b9365ac
Removing intermediate container a1a65b9365ac
---> 8c6b57ebebdd
Step 3/3 : RUN echo world > /dev/null
---> Running in 9fa977b763a5
Removing intermediate container 9fa977b763a5
---> 795c1f2fc7b9
Successfully built 795c1f2fc7b9
After:
echo -e 'FROM busybox\nRUN echo hello > /dev/null\nRUN echo world > /dev/null\n' | DOCKER_BUILDKIT=0 docker build --no-cache -
Sending build context to Docker daemon 2.048kB
Step 1/3 : FROM busybox
---> fc9db2894f4e
Step 2/3 : RUN echo hello > /dev/null
---> Running in 38d7c34c2178
---> Removed intermediate container 38d7c34c2178
---> 7c0dbc45111d
Step 3/3 : RUN echo world > /dev/null
---> Running in 629620285d4c
---> Removed intermediate container 629620285d4c
---> b92f70f2e57d
Successfully built b92f70f2e57d
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Golang map iteration order is not guaranteed, so in some cases the built slice has it's output of order as well. This means that testing for exact warning messages in docker build output would result in random test failures, making it more annoying for end-users to test against this functionality.
Signed-off-by: Jose Diaz-Gonzalez <email@josediazgonzalez.com>
Use string-literal for reduce escaped quotes, which makes for easier grepping.
While at it, also changed http -> https to keep some linters at bay.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We missed a case when parsing extra hosts from the dockerfile
frontend so the build fails.
To handle this case we need to set a dedicated worker label
that contains the host gateway IP so clients like Buildx
can just set the proper host:ip when parsing extra hosts
that contain the special string "host-gateway".
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
