Containers can have a default stop-signal (`--stop-signal` / `STOPSIGNAL`) and
timeout (`--stop-timeout`). It is currently not possible to update either of
these after the container is created (`docker update` does not allow updating
them), and while either of these can be overridden through some commands, we
currently do not have a command that can override *both*:
command | stop-signal | stop-timeout | notes
----------------|-------------|--------------|----------------------------
docker kill | yes | DNA | only sends a single signal
docker restart | no | yes |
docker stop | no | yes |
As a result, if a user wants to stop a container with a custom signal and
timeout, the only option is to do this manually:
docker kill -s <custom signal> mycontainer
# wait <desired timeout>
# press ^C to cancel the graceful stop
# forcibly kill the container
docker kill mycontainer
This patch adds a new `signal` query parameter to the container "stop" and
"restart" endpoints. This parameter can be added as a new flag on the CLI,
which would allow stopping and restarting with a custom timeout and signal,
for example:
docker stop --signal=SIGWINCH --time=120 mycontainer
docker restart --signal=SIGWINCH --time=120 mycontainer
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Implement a ReadJSON() utility to help reduce some code-duplication,
and to make sure we handle JSON requests consistently (e.g. always
check for the content-type).
Differences compared to current handling:
- prevent possible panic if request.Body is nil ("should never happen")
- always require Content-Type to be "application/json"
- be stricter about additional content after JSON (previously ignored)
- but, allow the body to be empty (an empty body is not invalid);
update TestContainerInvalidJSON accordingly, which was testing the
wrong expectation.
- close body after reading (some code did this)
We should consider to add a "max body size" on this function, similar to
7b9275c0da/api/server/middleware/debug.go (L27-L40)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These were changes I drafted when reviewing 7c731e02a9,
and had these stashed in my local git;
- rename receiver to prevent "unconsistent receiver name" warnings
- make NewRouter() slightly more idiomatic, and wrap the options,
to make them easier to read.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This adds an additional "Swarm" header to the _ping endpoint response,
which allows a client to detect if Swarm is enabled on the daemon, without
having to call additional endpoints.
This change is not versioned in the API, and will be returned irregardless
of the API version that is used. Clients should fall back to using other
endpoints to get this information if the header is not present.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts the changes made in 2a9c987e5a, which
moved the GetHTTPErrorStatusCode() utility to the errdefs package.
While it seemed to make sense at the time to have the errdefs package provide
conversion both from HTTP status codes errdefs and the reverse, a side-effect
of the move was that the errdefs package now had a dependency on various external
modules, to handle conversio of errors coming from those sub-systems, such as;
- github.com/containerd/containerd
- github.com/docker/distribution
- google.golang.org/grpc
This patch moves the conversion from (errdef-) errors to HTTP status-codes to a
api/server/httpstatus package, which is only used by the API server, and should
not be needed by client-code using the errdefs package.
The MakeErrorHandler() utility was moved to the API server itself, as that's the
only place it's used. While the same applies to the GetHTTPErrorStatusCode func,
I opted for keeping that in its own package for a slightly cleaner interface.
Why not move it into the api/server/httputils package?
The api/server/httputils package is also imported in the client package, which
uses the httputils.ParseForm() and httputils.HijackConnection() functions as
part of the TestTLSCloseWriter() test. While this is only used in tests, I
wanted to avoid introducing the indirect depdencencies outside of the api/server
code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
All other endpoints handle this in the API; given that the JSON format for
filters is part of the API, it makes sense to handle it there, and not have
that concept leak into further down the code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the default to the service itself, and produce the correct status code
if an invalid limit was specified. The default is currently set both on the
cli and on the daemon side, and it should be only set on one of them.
There is a slight change in behavior; previously, searching with `--limit=0`
would produce an error, but with this change, it's considered the equivalent
of "no limit set" (and using the default).
We could keep the old behavior by passing a pointer (`nil` means "not set"),
but I left that for a follow-up exercise (we may want to pass an actual
config instead of separate arguments, as well as some other things that need
cleaning up).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Omit `KernelMemory` and `KernelMemoryTCP` fields in `/info` response if they're
not supported, or when using API v1.42 or up.
- Re-enable detection of `KernelMemory` (as it's still needed for older API versions)
- Remove warning about kernel memory TCP in daemon logs (a warning is still returned
by the `/info` endpoint, but we can consider removing that).
- Prevent incorrect "Minimum kernel memory limit allowed" error if the value was
reset because it's not supported by the host.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- remove KernelMemory option from `v1.42` api docs
- remove KernelMemory warning on `/info`
- update changes for `v1.42`
- remove `KernelMemory` field from endpoints docs
Signed-off-by: aiordache <anca.iordache@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The endpoint was silently ignoring invalid values for the "condition" parameter.
This patch now returns a 400 status if an unknown, non-empty "condition" is passed.
With this patch:
curl --unix-socket /var/run/docker.sock -XPOST 'http://localhost/v1.41/containers/foo/wait?condition=foobar'
{"message":"invalid condition: \"foobar\""}
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit 0380fbff37 added the ability to pass a
--platform flag on `docker import` when importing an archive. The intent
of that commit was to allow importing a Linux rootfs on a Windows daemon
(as part of the experimental LCOW feature).
A later commit (337ba71fc1) changed some
of this code to take both OS and Architecture into account (for `docker build`
and `docker pull`), but did not yet update the `docker image import`.
This patch updates the import endpoitn to allow passing both OS and
Architecture. Note that currently only matching OSes are accepted,
and an error will be produced when (e.g.) specifying `linux` on Windows
and vice-versa.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This error is meant to be used in the output stream, and some comments
were added to prevent accidentally using local variables.
Renaming the variable instead to make it less ambiguous.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Let clients choose object types to compute disk usage of.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
The reasoning for this change is to be able to query image shared size without having to rely on the more heavyweight `/system/df` endpoint.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
This makes it easier to add more options to the backend without having to change
the signature.
While we're changing the signature, also adding a context.Context, which is not
currently used, but probably should be at some point.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
Ensure empty `BuildCache` field is represented as empty JSON array(`[]`)
instead of `null` to be consistent with `Images`, `Containers` etc.
Signed-off-by: Roman Volosatovs <roman.volosatovs@docker.com>
The LCOW implementation in dockerd has been deprecated in favor of re-implementation
in containerd (in progress). Microsoft started removing the LCOW V1 code from the
build dependencies we use in Microsoft/opengcs (soon to be part of Microsoft/hcshhim),
which means that we need to start removing this code.
This first step removes the lcow graphdriver, the LCOW initialization code, and
some LCOW-related utilities.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
After moving libnetwork to this repo, we need to update all the import
paths for libnetwork to point to docker/docker/libnetwork instead of
docker/libnetwork.
This change implements that.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This fixes a regression based on expectations of the runtime:
```
docker pull arm32v7/alpine
docker run arm32v7/alpine
```
Without this change, the `docker run` will fail due to platform
matching on non-arm32v7 systems, even though the image could run
(assuming the system is setup correctly).
This also emits a warning to make sure that the user is aware that a
platform that does not match the default platform of the system is being
run, for the cases like:
```
docker pull --platform armhf busybox
docker run busybox
```
Not typically an issue if the requests are done together like that, but
if the image was already there and someone did `docker run` without an
explicit `--platform`, they may very well be expecting to run a native
version of the image instead of the armhf one.
This warning does add some extra noise in the case of platform specific
images being run, such as `arm32v7/alpine`, but this can be supressed by
explicitly setting the platform.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Add Ulimits field to the ContainerSpec API type and wire it to Swarmkit.
This is related to #40639.
Signed-off-by: Albin Kerouanton <albin@akerouanton.name>
This patch adds a new "prune" event type to indicate that pruning of a resource
type completed.
This event-type can be used on systems that want to perform actions after
resources have been cleaned up. For example, Docker Desktop performs an fstrim
after resources are deleted (https://github.com/linuxkit/linuxkit/tree/v0.7/pkg/trim-after-delete).
While the current (remove, destroy) events can provide information on _most_
resources, there is currently no event triggered after the BuildKit build-cache
is cleaned.
Prune events have a `reclaimed` attribute, indicating the amount of space that
was reclaimed (in bytes). The attribute can be used, for example, to use as a
threshold for performing fstrim actions. Reclaimed space for `network` events
will always be 0, but the field is added to be consistent with prune events for
other resources.
To test this patch:
Create some resources:
for i in foo bar baz; do \
docker network create network_$i \
&& docker volume create volume_$i \
&& docker run -d --name container_$i -v volume_$i:/volume busybox sh -c 'truncate -s 5M somefile; truncate -s 5M /volume/file' \
&& docker tag busybox:latest image_$i; \
done;
docker pull alpine
docker pull nginx:alpine
echo -e "FROM busybox\nRUN truncate -s 50M bigfile" | DOCKER_BUILDKIT=1 docker build -
Start listening for "prune" events in another shell:
docker events --filter event=prune
Prune containers, networks, volumes, and build-cache:
docker system prune -af --volumes
See the events that are returned:
docker events --filter event=prune
2020-07-25T12:12:09.268491000Z container prune (reclaimed=15728640)
2020-07-25T12:12:09.447890400Z network prune (reclaimed=0)
2020-07-25T12:12:09.452323000Z volume prune (reclaimed=15728640)
2020-07-25T12:12:09.517236200Z image prune (reclaimed=21568540)
2020-07-25T12:12:09.566662600Z builder prune (reclaimed=52428841)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
After dicussing with maintainers, it was decided putting the burden of
providing the full cap list on the client is not a good design.
Instead we decided to follow along with the container API and use cap
add/drop.
This brings in the changes already merged into swarmkit.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The initial implementation followed the Swarm API, where
PidsLimit is located in ContainerSpec. This is not the
desired place for this property, so moving the field to
TaskTemplate.Resources in our API.
A similar change should be made in the SwarmKit API (likely
keeping the old field for backward compatibility, because
it was merged some releases back)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This prevents projects that import only the api/types package from
also having to use the errdefs package (and because of that, containerd)
as a dependency.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Support for PidsLimit was added to SwarmKit in docker/swarmkit/pull/2415,
but never exposed through the Docker remove API.
This patch exposes the feature in the repote API.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
While the docker cli may be sending a "version" header, this header
is not part of the API, or at least should not determin what API
version is used.
This code was added in c0afd9c873, to
adjust the handling of requests when an older version of the API was
used, but because the code relied on the "version" header set by the
CLI, it didn't work with other clients (e.g. when using cURL to make
an API request).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This enables image lookup when creating a container to fail when the
reference exists but it is for the wrong platform. This prevents trying
to run an image for the wrong platform, as can be the case with, for
example binfmt_misc+qemu.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Metrics collectors generally don't need the daemon to prime the stats
with something to compare since they already have something to compare
with.
Before this change, the API does 2 collection cycles (which takes
roughly 2s) in order to provide comparison for CPU usage over 1s. This
was primarily added so that `docker stats --no-stream` had something to
compare against.
Really the CLI should have just made a 2nd call and done the comparison
itself rather than forcing it on all API consumers.
That ship has long sailed, though.
With this change, clients can set an option to just pull a single stat,
which is *at least* a full second faster:
Old:
```
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=false > /dev/null
2>&1
real0m1.864s
user0m0.005s
sys0m0.007s
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=false > /dev/null
2>&1
real0m1.173s
user0m0.010s
sys0m0.006s
```
New:
```
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=true > /dev/null
2>&1
real0m0.680s
user0m0.008s
sys0m0.004s
time curl --unix-socket
/go/src/github.com/docker/docker/bundles/test-integration-shell/docker.sock
http://./containers/test/stats?stream=false\&one-shot=true > /dev/null
2>&1
real0m0.156s
user0m0.007s
sys0m0.007s
```
This fixes issues with downstreams ability to use the stats API to
collect metrics.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
