This should eliminate a bunch of new (go-1.11 related) validation
errors telling that the code is not formatted with `gofmt -s`.
No functional change, just whitespace (i.e.
`git show --ignore-space-change` shows nothing).
Patch generated with:
> git ls-files | grep -v ^vendor/ | grep .go$ | xargs gofmt -s -w
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
This is a fix for a few related scenarios where it's impossible to remove layers or containers
until the host is rebooted. Generally (or at least easiest to repro) through a forced daemon kill
while a container is running.
Possibly slightly worse than that, as following a host reboot, the scratch layer would possibly be leaked and
left on disk under the dataroot\windowsfilter directory after the container is removed.
One such example of a failure:
1. run a long running container with the --rm flag
docker run --rm -d --name test microsoft/windowsservercore powershell sleep 30
2. Force kill the daemon not allowing it to cleanup. Simulates a crash or a host power-cycle.
3. (re-)Start daemon
4. docker ps -a
PS C:\control> docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
7aff773d782b malloc "powershell start-sl…" 11 seconds ago Removal In Progress malloc
5. Try to remove
PS C:\control> docker rm 7aff
Error response from daemon: container 7aff773d782bbf35d95095369ffcb170b7b8f0e6f8f65d5aff42abf61234855d: driver "windowsfilter" failed to remove root filesystem: rename C:\control\windowsfilter\7aff773d782bbf35d95095369ffcb170b7b8f0e6f8f65d5aff42abf61234855d C:\control\windowsfilter\7aff773d782bbf35d95095369ffcb170b7b8f0e6f8f65d5aff42abf61234855d-removing: Access is denied.
PS C:\control>
Step 5 fails.
This should test that
- all the messages produced are delivered (i.e. not lost)
- followLogs() exits
Loosely based on the test having the same name by Brian Goff, see
https://gist.github.com/cpuguy83/e538793de18c762608358ee0eaddc197
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When daemon.ContainerLogs() is called with options.follow=true
(as in "docker logs --follow"), the "loggerutils.followLogs()"
function never returns (even then the logs consumer is gone).
As a result, all the resources associated with it (including
an opened file descriptor for the log file being read, two FDs
for a pipe, and two FDs for inotify watch) are never released.
If this is repeated (such as by running "docker logs --follow"
and pressing Ctrl-C a few times), this results in DoS caused by
either hitting the limit of inotify watches, or the limit of
opened files. The only cure is daemon restart.
Apparently, what happens is:
1. logs producer (a container) is gone, calling (*LogWatcher).Close()
for all its readers (daemon/logger/jsonfilelog/jsonfilelog.go:175).
2. WatchClose() is properly handled by a dedicated goroutine in
followLogs(), cancelling the context.
3. Upon receiving the ctx.Done(), the code in followLogs()
(daemon/logger/loggerutils/logfile.go#L626-L638) keeps to
send messages _synchronously_ (which is OK for now).
4. Logs consumer is gone (Ctrl-C is pressed on a terminal running
"docker logs --follow"). Method (*LogWatcher).Close() is properly
called (see daemon/logs.go:114). Since it was called before and
due to to once.Do(), nothing happens (which is kinda good, as
otherwise it will panic on closing a closed channel).
5. A goroutine (see item 3 above) keeps sending log messages
synchronously to the logWatcher.Msg channel. Since the
channel reader is gone, the channel send operation blocks forever,
and resource cleanup set up in defer statements at the beginning
of followLogs() never happens.
Alas, the fix is somewhat complicated:
1. Distinguish between close from logs producer and logs consumer.
To that effect,
- yet another channel is added to LogWatcher();
- {Watch,}Close() are renamed to {Watch,}ProducerGone();
- {Watch,}ConsumerGone() are added;
*NOTE* that ProducerGone()/WatchProducerGone() pair is ONLY needed
in order to stop ConsumerLogs(follow=true) when a container is stopped;
otherwise we're not interested in it. In other words, we're only
using it in followLogs().
2. Code that was doing (logWatcher*).Close() is modified to either call
ProducerGone() or ConsumerGone(), depending on the context.
3. Code that was waiting for WatchClose() is modified to wait for
either ConsumerGone() or ProducerGone(), or both, depending on the
context.
4. followLogs() are modified accordingly:
- context cancellation is happening on WatchProducerGone(),
and once it's received the FileWatcher is closed and waitRead()
returns errDone on EOF (i.e. log rotation handling logic is disabled);
- due to this, code that was writing synchronously to logWatcher.Msg
can be and is removed as the code above it handles this case;
- function returns once ConsumerGone is received, freeing all the
resources -- this is the bugfix itself.
While at it,
1. Let's also remove the ctx usage to simplify the code a bit.
It was introduced by commit a69a59ffc7 ("Decouple removing the
fileWatcher from reading") in order to fix a bug. The bug was actually
a deadlock in fsnotify, and the fix was just a workaround. Since then
the fsnofify bug has been fixed, and a new fsnotify was vendored in.
For more details, please see
https://github.com/moby/moby/pull/27782#issuecomment-416794490
2. Since `(*filePoller).Close()` is fixed to remove all the files
being watched, there is no need to explicitly call
fileWatcher.Remove(name) anymore, so get rid of the extra code.
Should fix https://github.com/moby/moby/issues/37391
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This test case checks that followLogs() exits once the reader is gone.
Currently it does not (i.e. this test is supposed to fail) due to #37391.
[kolyshkin@: test case Brian Goff, changelog and all bugs are by me]
Source: https://gist.github.com/cpuguy83/e538793de18c762608358ee0eaddc197
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This code has many return statements, for some of them the
"end logs" or "end stream" message was not printed, giving
the impression that this "for" loop never ended.
Make sure that "begin logs" is to be followed by "end logs".
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case a volume is specified via Mounts API, and SELinux is enabled,
the following error happens on container start:
> $ docker volume create testvol
> $ docker run --rm --mount source=testvol,target=/tmp busybox true
> docker: Error response from daemon: error setting label on mount
> source '': no such file or directory.
The functionality to relabel the source of a local mount specified via
Mounts API was introduced in commit 5bbf5cc and later broken by commit
e4b6adc, which removed setting mp.Source field.
With the current data structures, the host dir is already available in
v.Mountpoint, so let's just use it.
Fixes: e4b6adc
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 5c8da2e967 updated the filtering behavior
to match container-names without having to specify the leading slash.
This change caused a regression in situations where a regex was provided as
filter, using an explicit leading slash (`--filter name=^/mycontainername`).
This fix changes the filters to match containers both with, and without the
leading slash, effectively making the leading slash optional when filtering.
With this fix, filters with and without a leading slash produce the same result:
$ docker ps --filter name=^a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 2 minutes ago Up 2 minutes a2
56e53770e316 busybox "sh" 2 minutes ago Up 2 minutes a1
$ docker ps --filter name=^/a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 2 minutes ago Up 2 minutes a2
56e53770e316 busybox "sh" 3 minutes ago Up 3 minutes a1
$ docker ps --filter name=^b
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
b69003b6a6fe busybox "sh" About a minute ago Up About a minute b1
$ docker ps --filter name=^/b
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
b69003b6a6fe busybox "sh" 56 seconds ago Up 54 seconds b1
$ docker ps --filter name=/a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 3 minutes ago Up 3 minutes a2
56e53770e316 busybox "sh" 4 minutes ago Up 4 minutes a1
$ docker ps --filter name=a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 3 minutes ago Up 3 minutes a2
56e53770e316 busybox "sh" 4 minutes ago Up 4 minutes a1
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In case a user wants to have a child reaper inside a container
(i.e. run "docker --init") AND a bind-mounted /dev, the following
error occurs:
> docker run -d -v /dev:/dev --init busybox top
> 088c96808c683077f04c4cc2711fddefe1f5970afc085d59e0baae779745a7cf
> docker: Error response from daemon: OCI runtime create failed: container_linux.go:296: starting container process caused "exec: "/dev/init": stat /dev/init: no such file or directory": unknown.
This happens because if a user-suppled /dev is provided, all the
built-in /dev/xxx mounts are filtered out.
To solve, let's move in-container init to /sbin, as the chance that
/sbin will be bind-mounted to a container is smaller than that for /dev.
While at it, let's give it more unique name (docker-init).
NOTE it still won't work for the case of bind-mounted /sbin.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The remote API allows full privilege escalation and is equivalent to
having root access on the host. Because of this, the API should never
be accessible through an insecure connection (TCP without TLS, or TCP
without TLS verification).
Although a warning is already logged on startup if the daemon uses an
insecure configuration, this warning is not very visible (unless someone
decides to read the logs).
This patch attempts to make insecure configuration more visible by sending
back warnings through the API (which will be printed when using `docker info`).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When requesting information about the daemon's configuration through the `/info`
endpoint, missing features (or non-recommended settings) may have to be presented
to the user.
Detecting these situations, and printing warnings currently is handled by the
cli, which results in some complications:
- duplicated effort: each client has to re-implement detection and warnings.
- it's not possible to generate warnings for reasons outside of the information
returned in the `/info` response.
- cli-side detection has to be updated for new conditions. This means that an
older cli connecting to a new daemon may not print all warnings (due to
it not detecting the new conditions)
- some warnings (in particular, warnings about storage-drivers) depend on
driver-status (`DriverStatus`) information. The format of the information
returned in this field is not part of the API specification and can change
over time, resulting in cli-side detection no longer being functional.
This patch adds a new `Warnings` field to the `/info` response. This field is
to return warnings to be presented by the user.
Existing warnings that are currently handled by the CLI are copied to the daemon
as part of this patch; This change is backward-compatible with existing
clients; old client can continue to use the client-side warnings, whereas new
clients can skip client-side detection, and print warnings that are returned by
the daemon.
Example response with this patch applied;
```bash
curl --unix-socket /var/run/docker.sock http://localhost/info | jq .Warnings
```
```json
[
"WARNING: bridge-nf-call-iptables is disabled",
"WARNING: bridge-nf-call-ip6tables is disabled"
]
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Blocks the execution of tasks during the Prepare phase until there
exists an IP address for every overlay network in use by the task. This
prevents a task from starting before the NetworkAttachment containing
the IP address has been sent down to the node.
Includes a basic test for the correct use case.
Signed-off-by: Drew Erny <drew.erny@docker.com>
This feature allows user to specify list of subnets for global
default address pool. User can configure subnet list using
'swarm init' command. Daemon passes the information to swarmkit.
We validate the information in swarmkit, then store it in cluster
object. when IPAM init is called, we pass subnet list to IPAM driver.
Signed-off-by: selansen <elango.siva@docker.com>
* Expose license status in Info
This wires up a new field in the Info payload that exposes the license.
For moby this is hardcoded to always report a community edition.
Downstream enterprise dockerd will have additional licensing logic wired
into this function to report details about the current license status.
Signed-off-by: Daniel Hiltgen <daniel.hiltgen@docker.com>
* Code review comments
Signed-off-by: Daniel Hiltgen <daniel.hiltgen@docker.com>
* Add windows autogen support
Signed-off-by: Daniel Hiltgen <daniel.hiltgen@docker.com>
This driver uses protobuf to store log messages and has better defaults
for log file handling (e.g. compression and file rotation enabled by
default).
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
