The LoopkupImage method is only used by the inspect image route and
returns an api/type struct. The depenency to api/types of the
daemon/images package is wrong, the daemon doesn't need to know about
the api types.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Using the swagger.yaml to generate api models will create incompatible field types. Some inconsistencies had already been mentioned at #39131. I've added more fixes from real life experience, some only occurring on Windows.
Closes#39131
Signed-off-by: Tobias Gesellchen <tobias@gesellix.de>
On Linux the daemon was not respecting the HostConfig.ConsoleSize
property and relied on cli initializing the tty size after the container
was created. This caused a delay between container creation and
the tty actually being resized.
This is also a small change to the api description, because
HostConfig.ConsoleSize is no longer Windows-only.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This import was left behind due to some PR's being merged, both
affecting the imports that were used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
Both of these pass the signal to daemon.containerStop(), which already validates
the signal; 2ed904cad7/daemon/stop.go (L48-L52)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Logging boolean was unconditionally set to true and ignored in all locations,
except for enabling the debugging middleware, which was also gated by the active
logrus logging level.
While it could make sense to have a Loglevel option configured on the API server,
we don't have this currently, and to make that actually useful, that config would
need to be tollerated by all locations that produce logs (which isn't the case
either).
Looking at the history of this option; a boolean to disable logging was originally
added in commit c423a790d6, which hard-coded it to
"disabled" in a test, and "enabled" for the API server outside of tests (before
that commit, logging was always enabled).
02ddaad5d9 and 5c42b2b512
changed the hard-coded values to be configurable through a `Logging` env-var (env-
vars were used _internally_ at the time to pass on options), which later became
a configuration struct in a0bf80fe03.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
ContainerConfig is used in multiple locations (for example, both for
Image.Config and Image.ContainerConfig). Unfortunately, swagger does
not allow documenting individual uses if a type is used; for this type,
the content is _optional_ when used as Image.ContainerConfig (which is
set by the classic builder, which does a "commit" of a container, but
not used when building an image with BuildKit).
This patch attempts to address this confusion by documenting that
"it may be empty (or fields not propagated) if it's used for the
Image.ContainerConfig field".
Perhaps alternatives are possible (aliasing the type?) but we can
look at those in a follow-up.
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>
This fixes the "deprecated" comment to have the correct format to be picked
up by editors, and adds `omitempty` labels for KernelMemory and KernelMemoryTCP.
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>
