The `normalizeWorkdir` function has two branches, one that returns a
result of `filepath.Join` which always returns a cleaned path, and
another one where the input string is returned unmodified.
To make these two outputs consistent, also clean the path in the second
branch.
This also makes the cleaning of the container workdir explicit in the
`normalizeWorkdir` function instead of relying on the
`SetupWorkingDirectory` to mutate it.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The NetworkMode "default" is now normalized into the value it
aliases ("bridge" on Linux and "nat" on Windows) by the
ContainerCreate endpoint, the legacy image builder, Swarm's
cluster executor and by the container restore codepath.
builder-next is left untouched as it already uses the normalized
value (ie. bridge).
Going forward, this will make maintenance easier as there's one
less NetworkMode to care about.
Signed-off-by: Albin Kerouanton <albinker@gmail.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>
API v1.20 (Docker Engine v1.11.0) and older allowed a HostConfig to be passed
when starting a container. This feature was deprecated in API v1.21 (Docker
Engine v1.10.0) in 3e7405aea8, and removed in
API v1.23 (Docker Engine v1.12.0) in commit 0a8386c8be.
API v1.23 and older are deprecated, and this patch removes the feature.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously this was done indirectly - the `compare` function didn't
check the `ArgsEscaped`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
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>
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>
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>
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>
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>
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 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>
Now that most uses of reexec have been replaced with non-reexec
solutions, most of the reexec.Init() calls peppered throughout the test
suites are unnecessary. Furthermore, most of the reexec.Init() calls in
test code neglects to check the return value to determine whether to
exit, which would result in the reexec'ed subprocesses proceeding to run
the tests, which would reexec another subprocess which would proceed to
run the tests, recursively. (That would explain why every reexec
callback used to unconditionally call os.Exit() instead of returning...)
Remove unneeded reexec.Init() calls from test and example code which no
longer needs it, and fix the reexec.Init() calls which are not inert to
exit after a reexec callback is invoked.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Trying to remove the "docker.io" domain from locations where it's not relevant.
In these cases, this domain was used as a "random" domain for testing or example
purposes.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This utility wasn't very related to all other utilities in pkg/ioutils.
Moving it to longpath to also make it more clear what it does.
It looks like there's only a single (public) external consumer of this
utility, and only used in a test, and it's not 100% clear if it was
intentional to use our package, of if it was a case of "I actually meant
`io/ioutil.MkdirTemp`" so we could consider skipping the alias.
While moving the package, I also renamed `TempDir` to `MkdirTemp`, which
is the signature it matches in "os" from stdlib.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This patch:
- Deprecates pkg/system.DefaultPathEnv
- Moves the implementation inside oci
- Adds TODOs to align the default in the Builder with the one used elsewhere
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
builder/remotecontext/detect_test.go:64:66: empty-lines: extra empty line at the end of a block (revive)
builder/remotecontext/detect_test.go:78:46: empty-lines: extra empty line at the end of a block (revive)
builder/remotecontext/detect_test.go:91:51: empty-lines: extra empty line at the end of a block (revive)
builder/dockerfile/internals_test.go:95:38: empty-lines: extra empty line at the end of a block (revive)
builder/dockerfile/copy.go:86:112: empty-lines: extra empty line at the end of a block (revive)
builder/dockerfile/dispatchers_test.go:286:39: empty-lines: extra empty line at the start of a block (revive)
builder/dockerfile/builder.go:280:38: empty-lines: extra empty line at the end of a block (revive)
builder/dockerfile/dispatchers.go:66:85: empty-lines: extra empty line at the start of a block (revive)
builder/dockerfile/dispatchers.go:559:85: empty-lines: extra empty line at the start of a block (revive)
builder/builder-next/adapters/localinlinecache/inlinecache.go:26:183: empty-lines: extra empty line at the start of a block (revive)
builder/builder-next/adapters/containerimage/pull.go:441:9: empty-lines: extra empty line at the start of a block (revive)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
rename some variables that collided with imports or (upcoming)
changes, e.g. `ctx`, which is commonly used for `context.Context`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go don't format comments, so committing this as
a separate commit, so that we can already make these changes before
we upgrade to Go 1.19.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
strings.ReplaceAll(s, old, new) is a wrapper function for
strings.Replace(s, old, new, -1). But strings.ReplaceAll is more
readable and removes the hardcoded -1.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This allows the postContainersKill() handler to pass values as-is. As part of
the rewrite, I also moved the daemon.GetContainer(name) call later in the
function, so that we can fail early if an invalid signal is passed, before
doing the (heavier) fetching of the container.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
pkg/urlutil (despite its poorly chosen name) is not really intended as a generic
utility to handle URLs, and should only be used by the builder to handle (remote)
build contexts.
- IsURL() only does a very rudimentary check for http(s):// prefixes, without any
other validation, but due to its name may give incorrect expectations.
- IsGitURL() is written specifically with docker build remote git contexts in
mind, and has handling for backward-compatibility, where strings that are
not URLs, but start with "github.com/" are accepted.
Because of the above, this patch:
- moves the package inside builder/remotecontext, close to where it's intended
to be used (ideally this would be part of build/remotecontext itself, but this
package imports many other dependencies, which would introduce those as extra
dependencies in the CLI).
- deprecates pkg/urlutil, but adds aliases as there are some external consumers.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
