This PR is a small gofmt fix of https://goreportcard.com/report/github.com/moby/moby#gofmt
```
gofmt99%
Gofmt formats Go programs. We run gofmt -s on your code, where -s is for the "simplify" command
moby/integration-cli/docker_cli_run_test.go
Line 1: warning: file is not gofmted with -s (gofmt)
```
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Instead of having to go through files or registry values as is currently the
case.
While adding GMSA support to Kubernetes (https://github.com/kubernetes/kubernetes/pull/73726)
I stumbled upon the fact that Docker currently only allows passing Windows
credential specs through files or registry values, forcing the Kubelet
to perform a rather awkward dance of writing-then-deleting to either the
disk or the registry to be able to create a Windows container with cred
specs.
This patch solves this problem by making it possible to directly pass
whole base64-encoded cred specs to the engine's API. I took the opportunity
to slightly refactor the method responsible for Windows cred spec as it
seemed hard to read to me.
Added some unit tests on Windows credential specs handling, as there were
previously none.
Added/amended the relevant integration tests.
I have also tested it manually: given a Windows container using a cred spec
that you would normally start with e.g.
```powershell
docker run --rm --security-opt "credentialspec=file://win.json" mcr.microsoft.com/windows/servercore:ltsc2019 nltest /parentdomain
# output:
# my.ad.domain.com. (1)
# The command completed successfully
```
can now equivalently be started with
```powershell
$rawCredSpec = & cat 'C:\ProgramData\docker\credentialspecs\win.json'
$escaped = $rawCredSpec.Replace('"', '\"')
docker run --rm --security-opt "credentialspec=raw://$escaped" mcr.microsoft.com/windows/servercore:ltsc2019 nltest /parentdomain
# same output!
```
I'll do another PR on Swarmkit after this is merged to allow services to use
the same option.
(It's worth noting that @dperny faced the same problem adding GMSA support
to Swarmkit, to which he came up with an interesting solution - see
https://github.com/moby/moby/pull/38632 - but alas these tricks are not
available to the Kubelet.)
Signed-off-by: Jean Rouge <rougej+github@gmail.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
Also fixes https://github.com/moby/moby/issues/22874
This commit is a pre-requisite to moving moby/moby on Windows to using
Containerd for its runtime.
The reason for this is that the interface between moby and containerd
for the runtime is an OCI spec which must be unambigious.
It is the responsibility of the runtime (runhcs in the case of
containerd on Windows) to ensure that arguments are escaped prior
to calling into HCS and onwards to the Win32 CreateProcess call.
Previously, the builder was always escaping arguments which has
led to several bugs in moby. Because the local runtime in
libcontainerd had context of whether or not arguments were escaped,
it was possible to hack around in daemon/oci_windows.go with
knowledge of the context of the call (from builder or not).
With a remote runtime, this is not possible as there's rightly
no context of the caller passed across in the OCI spec. Put another
way, as I put above, the OCI spec must be unambigious.
The other previous limitation (which leads to various subtle bugs)
is that moby is coded entirely from a Linux-centric point of view.
Unfortunately, Windows != Linux. Windows CreateProcess uses a
command line, not an array of arguments. And it has very specific
rules about how to escape a command line. Some interesting reading
links about this are:
https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/https://stackoverflow.com/questions/31838469/how-do-i-convert-argv-to-lpcommandline-parameter-of-createprocesshttps://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments?view=vs-2017
For this reason, the OCI spec has recently been updated to cater
for more natural syntax by including a CommandLine option in
Process.
What does this commit do?
Primary objective is to ensure that the built OCI spec is unambigious.
It changes the builder so that `ArgsEscaped` as commited in a
layer is only controlled by the use of CMD or ENTRYPOINT.
Subsequently, when calling in to create a container from the builder,
if follows a different path to both `docker run` and `docker create`
using the added `ContainerCreateIgnoreImagesArgsEscaped`. This allows
a RUN from the builder to control how to escape in the OCI spec.
It changes the builder so that when shell form is used for RUN,
CMD or ENTRYPOINT, it builds (for WCOW) a more natural command line
using the original as put by the user in the dockerfile, not
the parsed version as a set of args which loses fidelity.
This command line is put into args[0] and `ArgsEscaped` is set
to true for CMD or ENTRYPOINT. A RUN statement does not commit
`ArgsEscaped` to the commited layer regardless or whether shell
or exec form were used.
There's no need to try to re-build the test images if they already
exist. This change makes basically no difference to the upstream
integration test-suite running, but for users who want to run the
integration-cli suite on a host machine (such as distributions doing
tests) this change allows images to be pre-loaded such that compilers
aren't needed on the test machine.
However, this does remove the accidental re-compilation of nnp-test, as
well as handling errors far more cleanly (previously if an error
occurred during a test build, further tests won't attempt to rebuild
it).
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Move the test case from integration-cli to integration.
The test logic itself has not changed, except these
two things:
* the new test sets default-ipc-mode via command line
rather than via daemon.json (less code);
* the new test uses current API version.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As people are using the UUID in `docker info` that was based on the v1 manifest signing key, replace
with a UUID instead.
Remove deprecated `--disable-legacy-registry` option that was scheduled to be removed in 18.03.
Signed-off-by: Justin Cormack <justin.cormack@docker.com>
- Add support for exact list of capabilities, support only OCI model
- Support OCI model on CapAdd and CapDrop but remain backward compatibility
- Create variable locally instead of declaring it at the top
- Use const for magic "ALL" value
- Rename `cap` variable as it overlaps with `cap()` built-in
- Normalize and validate capabilities before use
- Move validation for conflicting options to validateHostConfig()
- TweakCapabilities: simplify logic to calculate capabilities
Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test sometimes failed because the number of events received did not
match the expected number:
FAIL: docker_cli_events_test.go:316: DockerSuite.TestEventsFilterLabels
docker_cli_events_test.go:334:
c.Assert(len(events), checker.Equals, 3)
... obtained int = 2
... expected int = 3
This patch makes the test more stable, by:
- use a wider range between `--since` and `--until`. These options were set
so that the client detaches after events were received, but the actual
range should not matter. Changing the range will cause more events to be
returned, but we're specifically looking for the container ID's, so this
should not make a difference for the actual test.
- use `docker create` instead of `docker run` for the containers. the
containers don't have to be running to trigger an event; using `create`
speeds up the test.
- check the exit code of the `docker create` to verify the containers were
succesfully created.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
I noticed that this test failed, because the node was in status "pending".
The test checks for the node's status immediately after it was restarted, so
possibly it needs some time to unlock.
14:07:10 FAIL: docker_cli_swarm_test.go:1128: DockerSwarmSuite.TestSwarmLockUnlockCluster
...
14:07:10 docker_cli_swarm_test.go:1168:
14:07:10 checkSwarmLockedToUnlocked(c, d)
14:07:10 docker_cli_swarm_test.go:1017:
14:07:10 c.Assert(getNodeStatus(c, d), checker.Equals, swarm.LocalNodeStateActive)
14:07:10 ... obtained swarm.LocalNodeState = "pending"
14:07:10 ... expected swarm.LocalNodeState = "active"
This patch adds a `waitAndAssert` for the node's status, with a 1 second timeout.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
RHEL/CentOS 3.10 kernels report that kernel-memory accounting is supported,
but it actually does not work.
Runc (when compiled for those kernels) will be compiled without kernel-memory
support, so even though the daemon may be reporting that it's supported,
it actually is not.
This cause tests to fail when testing against a daemon that's using a runc
version without kmem support.
For now, skip these tests based on the kernel version reported by the daemon.
This should fix failures such as:
```
FAIL: /go/src/github.com/docker/docker/integration-cli/docker_cli_run_unix_test.go:499: DockerSuite.TestRunWithKernelMemory
assertion failed:
Command: /usr/bin/docker run --kernel-memory 50M --name test1 busybox cat /sys/fs/cgroup/memory/memory.kmem.limit_in_bytes
ExitCode: 0
Error: <nil>
Stdout: 9223372036854771712
Stderr: WARNING: You specified a kernel memory limit on a kernel older than 4.0. Kernel memory limits are experimental on older kernels, it won't work as expected and can cause your system to be unstable.
Failures:
Expected stdout to contain "52428800"
FAIL: /go/src/github.com/docker/docker/integration-cli/docker_cli_update_unix_test.go:125: DockerSuite.TestUpdateKernelMemory
/go/src/github.com/docker/docker/integration-cli/docker_cli_update_unix_test.go:136:
...open /go/src/github.com/docker/docker/integration-cli/docker_cli_update_unix_test.go: no such file or directory
... obtained string = "9223372036854771712"
... expected string = "104857600"
----------------------------------------------------------------------
FAIL: /go/src/github.com/docker/docker/integration-cli/docker_cli_update_unix_test.go:139: DockerSuite.TestUpdateKernelMemoryUninitialized
/go/src/github.com/docker/docker/integration-cli/docker_cli_update_unix_test.go:149:
...open /go/src/github.com/docker/docker/integration-cli/docker_cli_update_unix_test.go: no such file or directory
... value = nil
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- TestAPISwarmLeaderElection
- TestAPISwarmRaftQuorum
- TestSwarmClusterRotateUnlockKey
because they are known to be flaky.
Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
Using `errors.Errorf()` passes the error with the stack trace for
debugging purposes.
Also using `errdefs.InvalidParameter` for Windows, so that the API
will return a 4xx status, instead of a 5xx, and added tests for
both validations.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Description:
When using local volume option such as size=10G, type=tmpfs, if we provide wrong options, we could create volume successfully.
But when we are ready to use it, it will fail to start container by failing to mount the local volume(invalid option).
We should check the options at when we create it.
Signed-off-by: Wentao Zhang <zhangwentao234@huawei.com>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A lack of check in the test code can lead to a panic due to
`len(ids)` being `0`.
Avoid the panic by adding appropriate checks. Note `Assert()` should be
used rather than `Check()` as if it fails we should not proceed with the
test.
Originally found in https://github.com/moby/moby/pull/38404.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
This is a follow-on from https://github.com/moby/moby/pull/38277
but had to be done in a couple of stages to ensure that CI didn't
break. v1.1 of the busybox image is now based on a CMD of "sh"
rather than using an entrypoint. And it also uses the bin directory
rather than `c:\busybox`. This makes it look a lot closer to the
Linux busybox image, and means that a couple of Windows-isms in
CI tests can be reverted back to be identical to their Linux
equivalents.
When starting docker daemons for swarm testing, we disable iptables
and use lo for communication (in order to avoid network conflicts).
The problem is, these options are lost on restart, that can lead
to any sorts of network conflicts and thus connectivity issues
between swarm nodes.
Fix this. This does not fix issues with swarm test failures, but
it seems they appear are less often after this one.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is repeated 6 times in different tests, with slight
minor variations. Let's factor it out, for clarity.
While at it, simplify the code: instead of more complex
parsing of "docker swarm init|update --autolock" output (1)
and checking if the key is also present in
"docker swarm unlock-key" output (2), get the key
from (2) and check it is present in (1).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 17173efbe0 checkSwarmLockedToUnlocked() no longer
require its third argument, so remove it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
