When a container is left running after the daemon exits (e.g. the daemon
is SIGKILL'd or crashes), it should stop any running containers when the
daemon starts back up.
What actually happens is the daemon only sends the container's
configured stop signal and does not check if it has exited.
If the container does not actually exit then it is left running.
This fixes this unexpected behavior by calling the same function to shut
down the container that the daemon shutdown process does.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Before this change, the log decoder function provided by the log driver
to logfile would not be able to re-use buffers, causing undeeded
allocations and memory bloat for dockerd.
This change introduces an interface that allows the log driver to manage
it's memory usge more effectively.
This only affects json-file and local log drivers.
`json-file` still is not great just because of how the json decoder in the
stdlib works.
`local` is significantly improved.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The following fields are unsupported:
* BlkioStats: all fields other than IoServiceBytesRecursive
* CPUStats: CPUUsage.PercpuUsage
* MemoryStats: MaxUsage and Failcnt
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
1. Sscanf is very slow, and we don't use the first two fields -- get rid of it.
2. Since the field we search for is at the end of line and prepended by
a space, we can just use strings.HaveSuffix.
3. Error checking for bufio.Scanner should be done after the Scan()
loop, not inside it.
Fixes: 885b29df09
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Looks like a wrong copy-paste using `RuntimeKernel100ns` twice instead of `RuntimeUser100ns`
Signed-off-by: Vincent Boulineau <vincent.boulineau@datadoghq.com>
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>
Switch to moby/sys/mount and mountinfo. Keep the pkg/mount for potential
outside users.
This commit was generated by the following bash script:
```
set -e -u -o pipefail
for file in $(git grep -l 'docker/docker/pkg/mount"' | grep -v ^pkg/mount); do
sed -i -e 's#/docker/docker/pkg/mount"#/moby/sys/mount"#' \
-e 's#mount\.\(GetMounts\|Mounted\|Info\|[A-Za-z]*Filter\)#mountinfo.\1#g' \
$file
goimports -w $file
done
```
Signed-off-by: Kir Kolyshkin <kolyshkin@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>
Configuration over the API per container is intentionally left out for
the time being, but is supported to configure the default from the
daemon config.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit cbecf48bc352e680a5390a7ca9cff53098cd16d7)
Signed-off-by: Madhu Venugopal <madhu@docker.com>
This supplements any log driver which does not support reads with a
custom read implementation that uses a local file cache.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit d675e2bf2b75865915c7a4552e00802feeb0847f)
Signed-off-by: Madhu Venugopal <madhu@docker.com>
Support cgroup as in Rootless Podman.
Requires cgroup v2 host with crun.
Tested with Ubuntu 19.10 (kernel 5.3, systemd 242), crun v0.12.1.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
The namespaces com.docker.*, io.docker.*, org.dockerproject.*
have been documented to be reserved for Docker's internal use.
Co-Authored-By: Sebastiaan van Stijn <thaJeztah@users.noreply.github.com>
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
Dont assign the --bip value directly to the subnet
for the default bridge. Instead use the network value
from the ParseCIDR output
Addresses: https://github.com/moby/moby/issues/40392
Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
This adds a new `fluentd-request-ack` logging option for the Fluentd
logging driver. If enabled, the server will respond with an acknowledgement.
This option improves the reliability of the message transmission. This
change is not versioned, and affects all API versions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This extracts parsing the driver's configuration to a
function, and uses the same function both when initializing
the driver, and when validating logging options.
Doing so allows validating if the provided options are in
the correct format when calling `ValidateOpts`, instead
of resulting in an error when initializing the logging driver.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`fuse-overlayfs` provides rootless overlayfs functionality without depending
on any kernel patch.
Aside from rootless, `fuse-overlayfs` could be potentially used for eliminating
`chown()` calls that happen in userns-remap mode, because `fuse-overlayfs` also
provides shiftfs functionality.
System requirements:
* fuse-overlayfs needs to be installed. Tested with 0.7.6.
* kernel >= 4.18
Unit test: `go test -exec sudo -v ./daemon/graphdriver/fuse-overlayfs`
The implementation is based on Podman's `overlay` driver which supports
both kernel-mode overlayfs and fuse-overlayfs in the single driver instance:
https://github.com/containers/storage/blob/39a8d5ed/drivers/overlay/overlay.go
However, Moby's implementation aims to decouple `fuse-overlayfs` driver from the
kernel-mode driver (`overlay2`) for simplicity.
Fix#40218
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Before the collection goroutine wakes up every 1 second (as configured).
This sleep interval is in case there are no stats to collect we don't
end up in a tight loop.
Instead use a condition variable to signal that a collection is needed.
This prevents us from waking the goroutine needlessly when there is no
one looking for stats.
For now I've kept the sleep just moved it to the end of the loop, which
gives some space between collections.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This makes sure that things like `--tmpfs` mounts over an anonymous
volume don't create volumes uneccessarily.
One method only checks mountpoints, the other checks both mountpoints
and tmpfs... the usage of these should likely be consolidated.
Ideally, processing for `--tmpfs` mounts would get merged in with the
rest of the mount parsing. I opted not to do that for this change so the
fix is minimal and can potentially be backported with fewer changes of
breaking things.
Merging the mount processing for tmpfs can be handled in a followup.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Docker Desktop (on MAC and Windows hosts) allows containers
running inside a Linux VM to connect to the host using
the host.docker.internal DNS name, which is implemented by
VPNkit (DNS proxy on the host)
This PR allows containers to connect to Linux hosts
by appending a special string "host-gateway" to --add-host
e.g. "--add-host=host.docker.internal:host-gateway" which adds
host.docker.internal DNS entry in /etc/hosts and maps it to host-gateway-ip
This PR also add a daemon flag call host-gateway-ip which defaults to
the default bridge IP
Docker Desktop will need to set this field to the Host Proxy IP
so DNS requests for host.docker.internal can be routed to VPNkit
Addresses: https://github.com/docker/for-linux/issues/264
Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
Adds support for ReplicatedJob and GlobalJob service modes. These modes
allow running service which execute tasks that exit upon success,
instead of daemon-type tasks.
Signed-off-by: Drew Erny <drew.erny@docker.com>
When IPv6 is enabled, make sure fixed-cidr-ipv6 is set
by the user since there is no default IPv6 local subnet
in the IPAM
Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
If TINI_COMMIT isn't set, .go-autogen sets an empty value
as the "expected" commit. Attempting to truncate the value
caused a panic in that situation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
* Requires containerd binaries from containerd/containerd#3799 . Metrics are unimplemented yet.
* Works with crun v0.10.4, but `--security-opt seccomp=unconfined` is needed unless using master version of libseccomp
( containers/crun#156, seccomp/libseccomp#177 )
* Doesn't work with master runc yet
* Resource limitations are unimplemented
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
When a container is started in privileged mode, the device mappings
provided by `--device` flag was ignored. Now the device mappings will
be considered even in privileged mode.
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
The [gelf payload specification](http://docs.graylog.org/en/2.4/pages/gelf.html#gelf-payload-specification)
demands that the field `short_message` *MUST* be set by the client library.
Since docker logging via the gelf driver sends messages line by line, it can happen that messages with an empty
`short_message` are passed on. This causes strict downstream processors (like graylog) to raise an exception.
The logger now skips messages with an empty line.
Resolves: #40232
See also: #37572
Signed-off-by: Jonas Heinrich <Jonas@JonasHeinrich.com>
Now that we do check if overlay is working by performing an actual
overlayfs mount, there's no need in extra checks for the kernel version
or the filesystem type. Actual mount check is sufficient.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Before this commit, overlay check was performed by looking for
`overlay` in /proc/filesystem. This obviously might not work
for rootless Docker (fs is there, but one can't use it as non-root).
This commit changes the check to perform the actual mount, by reusing
the code previously written to check for multiple lower dirs support.
The old check is removed from both drivers, as well as the additional
check for the multiple lower dirs support in overlay2 since it's now
a part of the main check.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This moves supportsMultipleLowerDir() to overlayutils
so it can be used from both overlay and overlay2.
The only changes made were:
* replace logger with logrus
* don't use workDirName mergedDirName constants
* add mnt var to improve readability a bit
This is a preparation for the next commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fix tries to address the issue raised in 39353 where
docker crash when creating namespaces with UID in /etc/subuid and /etc/subgid.
The issue was that, mapping to `/etc/sub[u,g]id` in docker does not
allow numeric ID.
This fix fixes the issue by probing other combinations (uid:groupname, username:gid, uid:gid)
when normal username:groupname fails.
This fix fixes 39353.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
WithBlock makes sure that the following containerd request is reliable.
In one edge case with high load pressure, kernel kills dockerd, containerd
and containerd-shims caused by OOM. When both dockerd and containerd
restart, but containerd will take time to recover all the existing
containers. Before containerd serving, dockerd will failed with gRPC
error. That bad thing is that restore action will still ignore the
any non-NotFound errors and returns running state for
already stopped container. It is unexpected behavior. And
we need to restart dockerd to make sure that anything is OK.
It is painful. Add WithBlock can prevent the edge case. And
n common case, the containerd will be serving in shortly.
It is not harm to add WithBlock for containerd connection.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
The validate step in CI was broken, due to a combination of
086b4541cf, fbdd437d29,
and 85733620eb being merged to master.
```
api/types/filters/parse.go:39:1: exported method `Args.Keys` should have comment or be unexported (golint)
func (args Args) Keys() []string {
^
daemon/config/builder.go:19:6: exported type `BuilderGCFilter` should have comment or be unexported (golint)
type BuilderGCFilter filters.Args
^
daemon/config/builder.go:21:1: exported method `BuilderGCFilter.MarshalJSON` should have comment or be unexported (golint)
func (x *BuilderGCFilter) MarshalJSON() ([]byte, error) {
^
daemon/config/builder.go:35:1: exported method `BuilderGCFilter.UnmarshalJSON` should have comment or be unexported (golint)
func (x *BuilderGCFilter) UnmarshalJSON(data []byte) error {
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As caught by staticcheck (after disabling the default exclusion rules);
Based on the comment, this break was indeed meant to break the
loop and return the error.
```
daemon/graphdriver/aufs/mount.go:54:4: SA4011: ineffective break statement. Did you mean to break out of the outer loop? (staticcheck)
break
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
daemon/container_operations.go:787:2: S1033: unnecessary guard around call to delete (gosimple)
if _, ok := container.NetworkSettings.Networks[n.ID()]; ok {
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
daemon/graphdriver/btrfs/btrfs.go:609:5: SA4003: no value of type uint64 is less than 0 (staticcheck)
if driver.options.size <= 0 {
^
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
daemon/graphdriver/aufs/aufs_test.go:746:8: SA4021: x = append(y) is equivalent to x = y (staticcheck)
ids = append(ids[2:])
^
```
Also pre-allocating the ids slice while we're at it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The t.Log() caused some unneeded noise; changing these
tests to us subtests instead, so that we can track them
more easily.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adds a new ServiceStatus field to the Service object, which includes the
running and desired task counts. This new field is gated behind a
"status" query parameter.
Signed-off-by: Drew Erny <drew.erny@docker.com>
It has been pointed out that sometimes device mapper unit tests
fail with the following diagnostics:
> --- FAIL: TestDevmapperSetup (0.02s)
> graphtest_unix.go:44: graphdriver: loopback attach failed
> graphtest_unix.go:48: loopback attach failed
The root cause is the absence of udev inside the container used
for testing, which causes device nodes (/dev/loop*) to not be
created.
The test suite itself already has a workaround, but it only
creates 8 devices (loop0 till loop7). It might very well be
the case that the first few devices are already used by the
system (on my laptop 15 devices are busy).
The fix is to raise the number of devices being manually created.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If anything marshals the daemon config now or in the future
this commit ensures the correct canonical form for the builder
GC policies' filters.
Signed-off-by: Tibor Vass <tibor@docker.com>
For backwards compatibility, the old incorrect object format for
builder.GC.Rule.Filter still works but is deprecated in favor of array of
strings akin to what needs to be passed on the CLI.
Signed-off-by: Tibor Vass <tibor@docker.com>
This struct now has a properly typed member, so use the properly typed
functions with it.
Also update the vendor directory and hope nothing explodes.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
buildkit supports entitlements like network-host and security-insecure.
this patch aims to make it configurable through daemon.json file.
by default network-host is enabled & secuirty-insecure is disabled.
Signed-off-by: Kunal Kushwaha <kunal.kushwaha@gmail.com>
Previously there was no way for the splunk log driver to work if index
acknowledgment was set on the HEC, and it would in fact fail silently.
This will now allow users to specify if index acknowledgment is set and
will work with that setting.
Signed-off-by: Devon Estes <devon.c.estes@gmail.com>
The `docker/go-connections` package was only used for a quite generic utility.
This patch removes the use of the package by replacing the `GetProxyEnv` utility with
a local function that's based on the one in golang.org/x/net/http/httpproxy:
c21de06aaf/http/httpproxy/proxy.go (L100-L107)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit adds the image variant to the image.(Image) type and
updates related functionality. Images built from another will
inherit the OS, architecture and variant.
Note that if a base image does not specify an architecture, the
local machine's architecture is used for inherited images. On the
other hand, the variant is set equal to the parent image's variant,
even when the parent image's variant is unset.
The legacy builder is also updated to allow the user to specify
a '--platform' argument on the command line when creating an image
FROM scratch. A complete platform specification, including variant,
is supported. The built image will include the variant, as will any
derived images.
Signed-off-by: Chris Price <chris.price@docker.com>
About github.com/opencontainers/runc/libcontainer/user:
According to 195d8d544a
this package has two functions:
- Have a static implementation of user lookup, which is now supported in the
os/user stdlib package with the osusergo build tag, but wasn't at the time.
- Have extra functions that os/user doesn't have, but none of those are used
in homedir.
Since https://github.com/moby/moby/pull/11287, homedir depended directly on
libcontainer's user package for CurrentUser().
This is being replaced with os/user.Current(), because all of our static
binaries are compiled with the osusergo tag, and for dynamic libraries it
is more correct to use libc's implementation than parsing /etc/passwd.
About github.com/docker/docker/pkg/idtools:
Only dependency was from GetStatic() which uses idtools.LookupUID(uid).
The implementation of idtools.LookupUID just calls to
github.com/opencontainers/runc/libcontainer/user.LookupUid or fallbacks
to exec-ing to getent (since https://github.com/moby/moby/pull/27599).
This patch replaces calls to homedir.GetStatic by homedir.Get(), opting out
of supporting nss lookups in static binaries via exec-ing to getent for
the homedir package.
If homedir package users need to support nss lookups, they are advised
to compile dynamically instead.
Signed-off-by: Tibor Vass <tibor@docker.com>
This fixes issues where one goroutine tries to delete or rename a file
while another goroutine has the file open (e.g. a log reader).
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
In case jsonlogfile is used with max-file=1 and max-size set,
the log rotation is not perfomed; instead, the log file is closed
and re-open with O_TRUNC.
This situation is not handled by the log reader in follow mode,
leading to an issue of log reader being stuck forever.
This situation (file close/reopen) could be handled in waitRead(),
but fsnotify library chose to not listen to or deliver this event
(IN_CLOSE_WRITE in inotify lingo).
So, we have to handle this by checking the file size upon receiving
io.EOF from the log reader, and comparing the size with the one received
earlier. In case the new size is less than the old one, the file was
truncated and we need to seek to its beginning.
Fixes#39235.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
TestLogBlocking is intended to test that the Log method blocks by
default. It does this by mocking out the internals of the
awslogs.logStream and replacing one of its internal channels with one
that is controlled by the test. The call to Log occurs inside a
goroutine. Go may or may not schedule the goroutine immediately and the
blocking may or may not be observed outside the goroutine immediately
due to decisions made by the Go runtime. This change adds a small
timeout for test failure so that the Go runtime has the opportunity to
run the goroutine before the test fails.
Signed-off-by: Samuel Karp <skarp@amazon.com>
Moby works perfectly when you are in a situation when one has a good and stable
internet connection. Operating in area's where internet connectivity is likely
to be lost in undetermined intervals, like a satellite connection or 4G/LTE in
rural area's, can become a problem when pulling a new image. When connection is
lost while image layers are being pulled, Moby will try to reconnect up to 5 times.
If this fails, the incompletely downloaded layers are lost will need to be completely
downloaded again during the next pull request. This means that we are using more
data than we might have to.
Pulling a layer multiple times from the start can become costly over a satellite
or 4G/LTE connection. As these techniques (especially 4G) quite common in IoT and
Moby is used to run Azure IoT Edge devices, I would like to add a settable maximum
download attempts. The maximum download attempts is currently set at 5
(distribution/xfer/download.go). I would like to change this constant to a variable
that the user can set. The default will still be 5, so nothing will change from
the current version unless specified when starting the daemon with the added flag
or in the config file.
I added a default value of 5 for DefaultMaxDownloadAttempts and a settable
max-download-attempts in the daemon config file. It is also added to the config
of dockerd so it can be set with a flag when starting the daemon. This value gets
stored in the imageService of the daemon when it is initiated and can be passed
to the NewLayerDownloadManager as a parameter. It will be stored in the
LayerDownloadManager when initiated. This enables us to set the max amount of
retries in makeDownoadFunc equal to the max download attempts.
I also added some tests that are based on maxConcurrentDownloads/maxConcurrentUploads.
You can pull this version and test in a development container. Either create a config
`file /etc/docker/daemon.json` with `{"max-download-attempts"=3}``, or use
`dockerd --max-download-attempts=3 -D &` to start up the dockerd. Start downloading
a container and disconnect from the internet whilst downloading. The result would
be that it stops pulling after three attempts.
Signed-off-by: Lukas Heeren <lukas-heeren@hotmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
daemon/cluster/controllers/plugin/controller.go:37:2: U1000: field `taskID` is unused (unused)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
this looks to be a false positive, but this field is not
used if journald is not supported, which may be the cause
```
daemon/logger/journald/journald.go:21:2: U1000: field `mu` is unused (unused)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
staticcheck go linter says:
> daemon/logger/copier_test.go:451:2: SA2002: the goroutine calls T.Fatal, which must be called in the same goroutine as the test (staticcheck)
What it doesn't say is why. The reason is, t.Fatal() calls t.FailNow(),
which is expected to stop test execution right now. It does so by
calling runtime.Goexit(), which, unless called from a main goroutine,
does not stop test execution.
Anyway, long story short, if we don't care much about stopping the test
case immediately, we can just replace t.Fatalf() with t.Errorf() which
still marks the test case as failed, but won't stop it immediately.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix warnings like this one:
> daemon/logger/jsonfilelog/jsonfilelog_test.go:191:3: SA5001: should check returned error before deferring file.Close() (staticcheck)
> defer file.Close()
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Here, err is never non-nil as it was checked earlier.
Fixes the following linter warning:
> daemon/graphdriver/copy/copy.go:136:10: nilness: impossible condition: nil != nil (govet)
> if err != nil {
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In this code, err is already checked to be nil (or non-nil), so no need
to repeat extra checks.
Fixes the following govet warnings:
> daemon/checkpoint.go:38:12: nilness: tautological condition: nil == nil (govet)
> case err == nil:
> ^
> daemon/checkpoint.go:45:12: nilness: tautological condition: nil == nil (govet)
> case err == nil && stat.IsDir():
> ^
> daemon/checkpoint.go:47:12: nilness: tautological condition: nil == nil (govet)
> case err == nil:
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The last check for err != nil is not needed as err is always non-nil
there. Remove the check.
Also, no need to explicitly define `var err error` here.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Format the source according to latest goimports.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
suppressing the "SA9003: empty branch (staticcheck)" instead of commenting-out
or removing these lines because removing/commenting these lines causes a ripple
effect of changes, and there's still a to-do below.
```
13:06:14 daemon/graphdriver/graphtest/graphbench_unix.go:175:3: SA9003: empty branch (staticcheck)
13:06:14 if applyDiffSize != diffSize {
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
daemon/cluster/nodes.go:69:36: SA4009: argument ctx is overwritten before first use (staticcheck)
13:06:14 return c.lockedManagerAction(func(ctx context.Context, state nodeState) error {
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Trying to link to a non-existing container is not valid, and should return an
"invalid parameter" (400) error. Returning a "not found" error in this situation
would make the client report the container's image could not be found.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test frequently fails on Windows RS1 (mainly), so skipping it
for now on Windows;
```
ok github.com/docker/docker/daemon/logger 0.525s coverage: 43.0% of statements
time="2019-09-09T20:37:35Z" level=info msg="Trying to get region from EC2 Metadata"
time="2019-09-09T20:37:36Z" level=info msg="Log stream already exists" errorCode=ResourceAlreadyExistsException logGroupName= logStreamName= message= origError="<nil>"
--- FAIL: TestLogBlocking (0.02s)
cloudwatchlogs_test.go:313: Expected to be able to read from stream.messages but was unable to
time="2019-09-09T20:37:36Z" level=error msg=Error
time="2019-09-09T20:37:36Z" level=error msg="Failed to put log events" errorCode=InvalidSequenceTokenException logGroupName=groupName logStreamName=streamName message="use token token" origError="<nil>"
time="2019-09-09T20:37:36Z" level=error msg="Failed to put log events" errorCode=DataAlreadyAcceptedException logGroupName=groupName logStreamName=streamName message="use token token" origError="<nil>"
time="2019-09-09T20:37:36Z" level=info msg="Data already accepted, ignoring error" errorCode=DataAlreadyAcceptedException logGroupName=groupName logStreamName=streamName message="use token token"
FAIL
coverage: 78.2% of statements
FAIL github.com/docker/docker/daemon/logger/awslogs 0.630s
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This made my IDE unhappy; `ConfigFilePath` is an exported function, so
it makes sense to use the same signature for both Linux and Windows.
This patch also adds error handling (same as on Linux), even though the
current implementation will never return an error (it's good practice
to handle errors, so I assumed this would be the right approach)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Get rid of too many nested if statements. Remove the redundand check for
err != nil, fixing the following lint issue:
> daemon/logger/awslogs/cloudwatchlogs.go:452:10: nilness: tautological condition: non-nil != nil (govet)
> if err != nil {
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
```
16:04:35 daemon/logger/awslogs/cloudwatchlogs.go:312:25: SA1019: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made. (staticcheck)
16:04:35 return ec2metadata.New(session.New())
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We return immediately after this, so no need to update eventBuffer:
```
16:04:35 daemon/logger/awslogs/cloudwatchlogs.go:554:5: SA4006: this value of `eventBuffer` is never used (staticcheck)
16:04:35 eventBuffer = eventBuffer[:0]
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When mounting overlays which have children, enforce that
the mount is always performed as read only. Newer versions
of the kernel return a device busy error when a lower directory
is in use as an upper directory in another overlay mount.
Adds committed file to indicate when an overlay is being used
as a parent, ensuring it will no longer be mounted with an
upper directory.
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
also renamed the non-windows variant of this file to be
consistent with other files in this package
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
btrfs_noversion was added in d7c37b5a28
for distributions that did not have the `btrfs/version.h` header file.
Seeing how all of the distributions we currently support do have the
`btrfs/version.h` file we should probably just remove this build flag
altogether.
Signed-off-by: Eli Uriegas <eli.uriegas@docker.com>
Reported by govet linter:
> daemon/monitor.go:57:9: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
> ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
> ^
> daemon/monitor.go:128:9: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
> ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
> ^
Fixes: b5f288 ("Handle blocked I/O of exec'd processes")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use "in-place" variables for if statements to limit their scope to
the respectful `if` block.
2. Report the error returned from sd_journal_* by using CErr().
3. Use errors.New() instead of fmt.Errorf().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
From the first glance, `docker logs --tail 0` does not make sense,
as it is supposed to produce no output, but `tail -n 0` from GNU
coreutils is working like that, plus there is even a test case
(`TestLogsTail` in integration-cli/docker_cli_logs_test.go).
Now, something like `docker logs --follow --tail 0` makes total
sense, so let's make it work.
(NOTE if --tail is not used, config.Tail is set to -1)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If we take a long time to process log messages, and during that time
journal file rotation occurs, the journald client library will keep
those rotated files open until sd_journal_process() is called.
By periodically calling sd_journal_process() during the processing
loop we shrink the window of time a client instance has open file
descriptors for rotated (deleted) journal files.
This code is modelled after that of journalctl [1]; the above explanation
as well as the value of 1024 is taken from there.
[v2: fix CErr() argument]
[1] https://github.com/systemd/systemd/blob/dc16327c48d/src/journal/journalctl.c#L2676
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
