Close the returned event channel when the forwarding goroutine exits, allowing
consumers to detect that the event stream has stopped.
Also handle the backend subscription channel being closed. Previously, reads
from a closed channel returned nil repeatedly, causing the goroutine to log
unexpected messages and continue looping.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
backend.SubscribeToEvents is always called without since/until filters,
which means that we never receive historic events, only live events.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A task whose image cannot be pulled is rejected outright when the registry
answers with an "unauthorized" error, even when the image is already present
on the node. A service created without registry credentials carries no auth in
its spec and cannot have its digest resolved, so its tasks always attempt an
unauthenticated pull; on nodes that already hold the image those tasks are
rejected instead of started.
Tolerating a failed pull when the image is available locally is long-standing
behaviour, described in the comment above the check, and what the executor did
for every pull error before a7cf7eac0a ("fix: propagate registry auth error in
swarm image pull").
Keep reporting the pull failure, as it is far more useful than the "No such
image" the container create would otherwise produce, but only when the image
really is unavailable. Whether a task can run should not depend on how the
registry error happened to be classified, and that classification is not
consistent: the same registry 401 surfaces as ErrUnauthenticated from the
graphdriver pull path, as ErrNotFound from the containerd image store (via
docker.ErrInvalidAuthorization), and as ErrUnknown when the failure happens
while fetching a token, since the error is then a *url.Error that
translatePullError does not classify. The check therefore only fires for some
combinations of image store and registry.
The test needs DOCKER_SERVICE_PREFER_OFFLINE_IMAGE=0 on the daemon under test:
internal/testutil/daemon sets it to 1, which makes the executor skip the pull
altogether.
Signed-off-by: Sopho Merkviladze <smerkviladze@mirantis.com>
Drop cancellation at the kill boundary because killing starts
irreversible task and cleanup work that must finish after request
cancellation.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When a worker pull fails with unauthorized, the error was being
swallowed and replaced with misleading 'No such image' message.
Fix error propagation so the actual cause is reported.
Signed-off-by: Md_Mushfiqur Rahim <20mahin20201@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move parsing the wire format to the router instead of in the daemon, so
that a strong type can be used internally.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There also appeared to be duplication between daemon.getInspectData,
and the containerRouter.postContainersCreate methods, as both were
back-filling the field.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Most of the code in the filters package relates to the unmarshaling,
validation and application of filters from client requests. None of this
is necessary or particularly useful for Go SDK users. Move the full-fat
filters package into daemon/internal and switch all the daemon code to
import that package so we are free to iterate upon the code without
worrying about source-code interface compatibility.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The PullImage method for the ImageService used positional arguments for its
options, which made it more difficult to introduce new options. This patch
introduces a `PullOptions` struct to specify the options. As part of these
changes, the `platform` option was already adjusted to accept a slice of
platforms, which currently is not supported, but may be in the near future.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the option-types to the client and in some cases create a
copy for the backend. These types are used to construct query-
args, and not marshaled to JSON, and can be replaced with functional
options in the client.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Follow-up to 494677f93f, which added
the aliases, but did not yet replace our own use of the nat types.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The "backend" types in API were designed to decouple the API server
implementation from the daemon, or other parts of the code that
back the API server. This would allow the daemon to evolve (e.g.
functionality moved to different subsystems) without that impacting
the API server's implementation.
Now that the API server is no longer part of the API package (module),
there is no benefit to having it in the API module. The API server
may evolve (and require changes in the backend), which has no direct
relation with the API module (types, responses); the backend definition
is, however, coupled to the API server implementation.
It's worth noting that, while "technically" possible to use the API
server package, and implement an alternative backend implementation,
this has never been a prime objective. The backend definition was
never considered "stable", and we don't expect external users to
(attempt) to use it as such.
This patch moves the backend types to the daemon/server package,
so that they can evolve with the daemon and API server implementation
without that impacting the API module (which we intend to be stable,
following SemVer).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The stringid package is used in many places; while it's trivial
to implement a similar utility, let's just provide it as a utility
package in the client, removing the daemon-specific logic.
For integration tests, I opted to use the implementation in the
client, as those should not ideally not make assumptions about
the daemon implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These comments were added to enforce using the correct import path for
our packages ("github.com/docker/docker", not "github.com/moby/moby").
However, when working in go module mode (not GOPATH / vendor), they have
no effect, so their impact is limited.
Remove these imports in preparation of migrating our code to become an
actual go module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Logic was added to the Swarm executor in commit 0d9b0ed678
to clean up managed networks whenever the node's load-balancer IP
address is removed or changed in order to free up the address in the
case where the container fails to start entirely. Unfortunately, due to
an oversight the function returns early if the Swarm is lacking
an ingress network. Remove the early return so that load-balancer IP
addresses for all the other networks are freed as appropriate,
irrespective of whether an ingress network exists in the Swarm.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Paweł Gronowski