When the start interval is 0 we should treat that as unset.
This is especially important for older API versions where we reset the
value to 0.
Instead of using the default probe value we should be using the
configured `interval` value (which may be a default as well) which gives
us back the old behavior before support for start interval was added.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
When starting a daemon in debug mode (such as used in CI), many log-messages
are printed during startup. As a result, the log message indicating whether
graph-drivers or snapshotters are used may appear far separate from the
informational log about the daemon (and selected storage-driver).
The existing log-driver also unconditionally uses the legacy "graph-driver"
terminology, instead of the more generic "storage-driver".
This patch changes the log message shown during startup to use the generic
"graph-driver" as field, and adds a new field that indicates wheter we're
using snapshotters or graph-drivers.
Given that snapshotters will be the default at some point, an alternative
could be to include the _type_ of driver used, for example;
`io.containerd.snapshotter.v1`, which may continue to be relevant after
snapshotters become the default, and at which point (potentially) the
type of snapshotter becomes more relevant.
Before this change:
TEST_INTEGRATION_USE_SNAPSHOTTER=1 DOCKER_GRAPHDRIVER=overlayfs dockerd
...
INFO[2023-10-31T09:12:33.586269801Z] Starting daemon with containerd snapshotter integration enabled
INFO[2023-10-31T09:12:33.586322176Z] Loading containers: start.
INFO[2023-10-31T09:12:33.640514759Z] Loading containers: done.
INFO[2023-10-31T09:12:33.646498134Z] Docker daemon commit=dcf7287d647bcb515015e389df46ccf1e09855b7 graphdriver=overlayfs version=dev
INFO[2023-10-31T09:12:33.646706551Z] Daemon has completed initialization
INFO[2023-10-31T09:12:33.658840592Z] API listen on /var/run/docker.sock
With this change;
TEST_INTEGRATION_USE_SNAPSHOTTER=1 DOCKER_GRAPHDRIVER=overlayfs dockerd
...
INFO[2023-10-31T08:41:38.841155928Z] Starting daemon with containerd snapshotter integration enabled
INFO[2023-10-31T08:41:38.841207512Z] Loading containers: start.
INFO[2023-10-31T08:41:38.902461053Z] Loading containers: done.
INFO[2023-10-31T08:41:38.910535137Z] Docker daemon commit=dcf7287d647bcb515015e389df46ccf1e09855b7 containerd-snapshotter=true storage-driver=overlayfs version=dev
INFO[2023-10-31T08:41:38.910936803Z] Daemon has completed initialization
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In case of `docker push -a`, we need to return an error if there is no
image for the given repository.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Don't wrap the `no basic auth credentials` error from containerd and
return it as-is.
The error will look like:
```
failed to resolve reference "docker.io/library/aodkoakds:latest": pull access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed
```
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Kept `coci` import alias since we use it elsewhere,
maybe to prevent confusion with our own `oci` package.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Having a sandbox/container-wide MacAddress field makes little sense
since a container can be connected to multiple networks at the same
time. This field is an artefact of old times where a container could be
connected to a single network only.
As we now have a way to specify per-endpoint mac address, this field is
now deprecated.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Prior to this commit, only container.Config had a MacAddress field and
it's used only for the first network the container connects to. It's a
relic of old times where custom networks were not supported.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
The same error is already returned by `(*Daemon).containerCreate()` but
since this function is also called by the cluster executor, the error
has to be duplicated.
Doing that allows to remove a nil check on container config in
`postContainersCreate`.
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
containerd's `WithUser` function now resets this property, starting with
[3eda46af12b1deedab3d0802adb2e81cb3521950][1] (v1.7.0-beta.4), so we no
longer need this function.
[1]: 3eda46af12
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
commit def549c8f6 passed through the context
to the daemon.ContainerStart function. As a result, restarting containers
no longer is an atomic operation, because a context cancellation could
interrupt the restart (between "stopping" and "(re)starting"), resulting
in the container being stopped, but not restarted.
Restarting a container, or more factually; making a successful request on
the `/containers/{id]/restart` endpoint, should be an atomic operation.
This patch uses a context.WithoutCancel for restart requests.
It's worth noting that daemon.containerStop already uses context.WithoutCancel,
so in that function, we'll be wrapping the context twice, but this should
likely not cause issues (just redundant for this code-path).
Before this patch, starting a container that bind-mounts the docker socket,
then restarting itself from within the container would cancel the restart
operation. The container would be stopped, but not started after that:
docker run -dit --name myself -v /var/run/docker.sock:/var/run/docker.sock docker:cli sh
docker exec myself sh -c 'docker restart myself'
docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
3a2a741c65ff docker:cli "docker-entrypoint.s…" 26 seconds ago Exited (128) 7 seconds ago myself
With this patch: the stop still cancels the exec, but does not cancel the
restart operation, and the container is started again:
docker run -dit --name myself -v /var/run/docker.sock:/var/run/docker.sock docker:cli sh
docker exec myself sh -c 'docker restart myself'
docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
4393a01f7c75 docker:cli "docker-entrypoint.s…" About a minute ago Up 4 seconds myself
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use context.WithoutCancel so that both the containerStop and
container.Wait can share the same parent context. This context is still
a "TODO", but can be wired up in future.
It's worth noting that daemon.containerStop already uses context.WithoutCancel,
so in that function, we'll be wrapping the context twice, but this should
likely not cause issues (just redundant for this code-path).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Follow-up to fc94ed0a86. Now that
f6e44bc0e8 added the compatcontext
package, we can start using context.WithoutCancel.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In the tagged case the error message when the image/tag is not found
should be "tag does not exist: ref"
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
When choosing the next image, don't reject images without the classic
builder parent label. The intention was to *prefer* images them instead
of making that a condition.
This fixes the ID not being filled for parent images that weren't built
with the classic builder.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The `Tags` slice of each history entry was filled with tags of parent
image. Change it to correctly assign the current image tags.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Check for accurate values that may contain content sizes unknown to the
usage test in the calculation. Avoid asserting using deep equals when
only the expected value range is known to the test.
Signed-off-by: Derek McGowan <derek@mcg.dev>
After a successful push, all pushed blobs should have a
distribution.source label pointing to the new registry.
Before this commit, the label was only appended to the top-level blob
(manifest or manifest list). Adjust this to also do that recursively to
its children.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Use the distribution code to query the remote repository for tags and
pull them sequentially just like the non-c8d pull.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When the default bridge is disabled by setting dockerd's `--bridge=none`
option, the daemon still creates a sandbox for containers with no
network attachment specified. In that case `NetworkDisabled` will be set
to true.
However, currently the `releaseNetwork` call will early return if
NetworkDisabled is true. Thus, these sandboxes won't be deleted until
the daemon is restarted. If a high number of such containers are
created, the daemon would then take few minutes to start.
See https://github.com/moby/moby/issues/42461.
Signed-off-by: payall4u <payall4u@qq.com>
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
This is a follow-up to 2216d3ca8d, which
implemented the StartInterval for health-checks, but did not add validation
for the minimum accepted interval;
> The time to wait between checks in nanoseconds during the start period.
> It should be 0 or at least 1000000 (1 ms). 0 means inherit.
This patch adds validation for the minimum accepted interval (1ms).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
