When go-1.11beta1 is used for building, the following error is
reported:
> 14:56:20 daemon\graphdriver\lcow\lcow.go:236: Debugf format %s reads
> arg #2, but call has 1 arg
While fixing this, let's also fix a few other things in this
very function (startServiceVMIfNotRunning):
1. Do not use fmt.Printf when not required.
2. Use `title` whenever possible.
3. Don't add `id` to messages as `title` already has it.
4. Remove duplicated colons.
5. Try to unify style of messages.
6. s/startservicevmifnotrunning/startServiceVMIfNotRunning/
...
In general, logging/debugging here is a mess and requires much more
love than I can give it at the moment. Areas for improvement:
1. Add a global var logger = logrus.WithField("storage-driver", "lcow")
and use it everywhere else in the code.
2. Use logger.WithField("id", id) whenever possible (same for "context"
and other similar fields).
3. Revise all the errors returned to be uniform.
4. Make use of errors.Wrap[f] whenever possible.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: b7a95a3ce4c731c0fca204435be758ea89d6050f
Component: engine
There are two build errors when using go-1.11beta1:
> daemon/logger/loggerutils/logfile.go:367: Warningf format %q arg f.Name is a func value, not called
> daemon/logger/loggerutils/logfile.go:564: Debug call has possible formatting directive %v
In the first place, the file name is actually not required as error
message already includes it.
While at it, fix a couple of other places for more correct messages, and
make sure to not add a file name if an error already has it.
Fixes: f69f09f44c
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 09ad434f10cff48741322854a3003686b28295b5
Component: engine
Fix the following go-1.11beta1 build error:
> daemon/graphdriver/aufs/aufs.go:376: Wrapf format %s reads arg #1, but call has 0 args
While at it, change '%s' to %q.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 2e30e9e6db42043cb2bd67d25a7152488c834f9f
Component: engine
Go 1.11beta1 (rightfully) complains:
> 15:38:37 daemon/cluster/controllers/plugin/controller.go:183:
> Entry.Debugf format %#T has unrecognized flag #
This debug print was added by commit 72c3bcf2a5.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: a9a136572dc2a2c2ec5da320c4d0a32b5a15c550
Component: engine
In particular, these two:
> daemon/daemon_unix.go:1129: Wrapf format %v reads arg #1, but call has 0 args
> daemon/kill.go:111: Warn call has possible formatting directive %s
and a few more.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 3737194b9f2875c10f3f2117c1816054ba0ff262
Component: engine
Commit c0bc14e8 wrapped the return value of nw.Delete() with some extra
information. However, this breaks the code in
containerAdaptor.removeNetworks() which ignores certain specific
libnetwork error return codes. Said codes actually don't represent
errors, but just regular conditions to be expected in normal operation.
The removeNetworks() call checked for these errors by type assertions
which the errors.Wrap(err...) breaks.
This has a cascading effect, because controller.Remove() invokes
containerAdaptor.removeNetworks() and if the latter returns an error,
then Remove() fails to remove the container itself. This is not
necessarily catastrophic since the container reaper apparently will
purge the container later, but it is clearly not the behavior we want.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
Upstream-commit: 6225d1f15c5fd916c3e0ef3afe022f6cc14ac696
Component: engine
This patch is required for the updated version of libnetwork and entails
two minor changes.
First, it uses the new libnetwork.NetworkDeleteOptionRemoveLB option to
the network.Delete() method to automatically remove the load balancing
endpoint for ingress networks. This allows removal of the
deleteLoadBalancerSandbox() function whose functionality is now within
libnetwork.
The second change is to allocate a load balancer endpoint IP address for
all overlay networks rather than just "ingress" and windows overlay
networks. Swarmkit is already performing this allocation, but moby was
not making use of these IP addresses for Linux overlay networks (except
ingress). The current version of libnetwork makes use of these IP
addresses by creating a load balancing sandbox and endpoint similar to
ingress's for all overlay network and putting all load balancing state
for a given node in that sandbox only. This reduces the amount of linux
kernel state required per node.
In the prior scheme, libnetwork would program each container's network
namespace with every piece of load balancing state for every other
container that shared *any* network with the first container. This
meant that the amount of kernel state on a given node scaled with the
square of the number of services in the cluster and with the square of
the number of containers per service. With the new scheme, kernel state
at each node scales linearly with the number of services and the number
of containers per service. This also reduces the number of system calls
required to add or remove tasks and containers. Previously the number
of system calls required grew linearly with the number of other
tasks that shared a network with the container. Now the number of
system calls grows linearly only with the number of networks that the
task/container is attached to. This results in a significant
performance improvement when adding and removing services to a cluster
that already heavily loaded.
The primary disadvantage to this scheme is that it requires the
allocation of an additional IP address per node per subnet for every
node in the cluster that has a task on the given subnet. However, as
mentioned, swarmkit is already allocating these IP addresses for every
node and they are going unused. Future swarmkit modifications should be
examined to only allocate said IP addresses when nodes actually require
them.
Signed-off-by: Chris Telfer <ctelfer@docker.com>
Upstream-commit: 8e0f6bc90351525f3e52f3bc357378fcccccdd27
Component: engine
When using the mounts API, bind mounts are not supposed to be
automatically created.
Before this patch there is a race condition between valiating that a
bind path exists and then actually setting up the bind mount where the
bind path may exist during validation but was removed during mountpooint
setup.
This adds a field to the mountpoint struct to ensure that binds created
over the mounts API are not accidentally created.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 1caeb79963d3c9f770b23be2f12c584adf49538d
Component: engine
This stuff doesn't belong here and is causing imports of libnetwork into
the router, which is not what we want.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: c0bc14e8dd5a31f5edc804e9a1347bb3eb44483e
Component: engine
Signed-off-by: John Howard <jhoward@microsoft.com>
Addresses https://github.com/moby/moby/pull/35089#issuecomment-367802698.
This change enables the daemon to automatically select an image under LCOW
that can be used if the API doesn't specify an explicit platform.
For example:
FROM supertest2014/nyan
ADD Dockerfile /
And docker build . will download the linux image (not a multi-manifest image)
And similarly docker pull ubuntu will match linux/amd64
Upstream-commit: 35193c0e7dc301e1d2f6ea96e0ce34ffd2d4b88d
Component: engine
The archive changes function is not implemented correctly
to handle opaque directories.
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Upstream-commit: ac5d363e67e01c769b4d9e20ed76a278434ee4de
Component: engine
It's already supported by `swarmkit`, and act the same as
`HostConfig.Init` on container creation.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Upstream-commit: e401b88e59e098745744917c555d549f08353e6d
Component: engine
This adds MaskedPaths and ReadOnlyPaths options to HostConfig for containers so
that a user can override the default values.
When the value sent through the API is nil the default is used.
Otherwise the default is overridden.
Adds integration tests for MaskedPaths and ReadonlyPaths.
Signed-off-by: Jess Frazelle <acidburn@microsoft.com>
Upstream-commit: 3694c1e34e40fa2e255a97b5541645cec9c8d1d5
Component: engine
With a full attach, each attach was leaking 4 goroutines.
This updates attach to use errgroup instead of the hodge-podge of
waitgroups and channels.
In addition, the detach event was never being sent.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 0f5147701775a6c5d4980a7b7c0ed2e830688034
Component: engine
This makes it a bit simpler to remove this interface for v2 plugins
and not break external projects (libnetwork and swarmkit).
Note that before we remove the `Client()` interface from `CompatPlugin`
libnetwork and swarmkit must be updated to explicitly check for the v1
client interface as is done int his PR.
This is just a minor tweak that I realized is needed after trying to
implement the needed changes on libnetwork.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 7c77df8acc597cd4f540d873de5fe53a3d414ba9
Component: engine
This cleans up some of the package API's used for interacting with
volumes, and simplifies management.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: e4b6adc88e967971de634596654d9bc33e7bd7e0
Component: engine
If "ps" fails, in many cases it prints a meaningful error message
which a user can benefit from. Let's use it.
While at it, let's use errdefs.System to classify the error,
as well as errors.Wrap.
Before:
> $ docker top $CT <any bad ps options>
> Error response from daemon: Error running ps: exit status 1
After:
> $ docker top $CT auxm
> Error response from daemon: ps: error: thread display conflicts with forest display
or
> $ docker top $CT saur
> Error response from daemon: ps: error: conflicting format options
or, if there's no meaningful error on stderr, same as before:
> $ docker top $CT 1234
> Error response from daemon: ps: exit status 1
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: a41328d5704b8d1adbcd099fb4bb0697060df806
Component: engine
Current ContainerTop (a.k.a. docker top) implementation uses "ps"
to get the info about *all* running processes, then parses it, then
filters the results to only contain PIDs used by the container.
Collecting data only to throw most of it away is inefficient,
especially on a system running many containers (or processes).
For example, "docker top" on a container with a single process
can take up to 0.5 seconds to execute (on a mostly idle system)
which is noticeably slow.
Since the containers PIDs are known beforehand, let's use ps's
"-q" option to provide it with a list of PIDs we want info about.
The problem with this approach is, some ps options can't be used
with "-q" (the only one I'm aware of is "f" ("forest view") but
there might be more). As the list of such options is not known,
in case ps fails, it is executed again without "q" (retaining
the old behavior).
Next, the data produced by "ps" is filtered in the same way as before.
The difference here is, in case "-q" worked, the list is much shorter.
I ran some benchmarks on my laptop, with about 8000 "sleep" processes
running to amplify the savings.
The improvement in "docker top" execution times is 5x to 10x (roughly
0.05s vs 0.5s).
The improvement in ContainerTop() execution time is up to 100x
(roughly 3ms vs 300ms).
I haven't measured the memory or the CPU time savings, guess those
are not that critical.
NOTE that busybox ps does not implement -q so the fallback is always
used, but AFAIK it is not usable anyway and Docker expects a normal
ps to be on the system (say the list of fields produced by
"busybox ps -ef" differs from normal "ps -ef" etc.).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: a076badb8b33f1ecdc5d46f0a3701f10c0579f73
Component: engine
remove unnescessary import aliases, brackets, and so on.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: f23c00d8701e4bd0f2372a586dacbf66a26f9a51
Component: engine
It does not make sense to keep looking for PID once
we found it, so let's give it a break.
The side effect of this patch is, if there's more than one column
titled "PID", the last (rightmost) column was used before, and now
the first (leftmost) column is used. Should make no practical
difference whatsoever.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: 654a7625fc9f0b7b04da0e0e4d151af04a65cc7f
Component: engine
