Before this change, volume management was relying on the fact that
everything the plugin mounts is visible on the host within the plugin's
rootfs. In practice this caused some issues with mount leaks, so we
changed the behavior such that mounts are not visible on the plugin's
rootfs, but available outside of it, which breaks volume management.
To fix the issue, allow the plugin to scope the path correctly rather
than assuming that everything is visible in `p.Rootfs`.
In practice this is just scoping the `PropagatedMount` paths to the
correct host path.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 0e5eaf8ee32662182147f5f62c1bfebef66f5c47
Component: engine
This protects the daemon from volume plugins that are slow or
deadlocked.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: b15f8d2d4f054a87052a7065c50441f7e8479fa9
Component: engine
Both lcow_parser.go and linux_parser.go are duplicating the error:
"invalid specification: destination can't be '/'"
This commit creates a new error called "ErrVolumeTargetIsRoot"
that is used by both linux_parser and lcow_parser and remove
the duplication in the code.
Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>
Upstream-commit: 62143af5437a29d4b95f971d1905cfef763b0847
Component: engine
Instead of having to create a bunch of custom error types that are doing
nothing but wrapping another error in sub-packages, use a common helper
to create errors of the requested type.
e.g. instead of re-implementing this over and over:
```go
type notFoundError struct {
cause error
}
func(e notFoundError) Error() string {
return e.cause.Error()
}
func(e notFoundError) NotFound() {}
func(e notFoundError) Cause() error {
return e.cause
}
```
Packages can instead just do:
```
errdefs.NotFound(err)
```
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 87a12421a94faac294079bebc97c8abb4180dde5
Component: engine
Validation of Mounts was only performed on container _creation_, not on
container _start_. As a result, if the host-path no longer existed
when the container was started, a directory was created in the given
location.
This is the wrong behavior, because when using the `Mounts` API, host paths
should never be created, and an error should be produced instead.
This patch adds a validation step on container start, and produces an
error if the host path is not found.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 7cb96ba308dc53824d2203fd343a4a297d17976e
Component: engine
This subtle bug keeps lurking in because error checking for `Mkdir()`
and `MkdirAll()` is slightly different wrt to `EEXIST`/`IsExist`:
- for `Mkdir()`, `IsExist` error should (usually) be ignored
(unless you want to make sure directory was not there before)
as it means "the destination directory was already there"
- for `MkdirAll()`, `IsExist` error should NEVER be ignored.
Mostly, this commit just removes ignoring the IsExist error, as it
should not be ignored.
Also, there are a couple of cases then IsExist is handled as
"directory already exist" which is wrong. As a result, some code
that never worked as intended is now removed.
NOTE that `idtools.MkdirAndChown()` behaves like `os.MkdirAll()`
rather than `os.Mkdir()` -- so its description is amended accordingly,
and its usage is handled as such (i.e. IsExist error is not ignored).
For more details, a quote from my runc commit 6f82d4b (July 2015):
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.
Quoting MkdirAll documentation:
> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.
This means two things:
1. If a directory to be created already exists, no error is
returned.
2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.
The above is a theory, based on quoted documentation and my UNIX
knowledge.
3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in #2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.
Because of #1, IsExist check after MkdirAll is not needed.
Because of #2 and #3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.
Note this error is all over the tree, I guess due to copy-paste,
or trying to follow the same usage pattern as for Mkdir(),
or some not quite correct examples on the Internet.
[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: 516010e92d56cfcd6d1e343bdc02b6f04bc43039
Component: engine
Before this, if a volume exists in a driver but not in the local cache,
the store would just return a bare volume. This means that if a user
supplied options or labels, they will not get stored.
Instead only return early if we have the volume stored locally. Note
this could still have an issue with labels/opts passed in by the user
differing from what is stored, however this isn't really a new problem.
This fixes a problem where if there is a shared storage backend between
two docker nodes, a create on one node will have labels stored and a
create on the other node will not.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 4d8598ad0506b29c12632c1b8ed92eb58fc2f0e2
Component: engine
For obvious reasons that it is not really supported now.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Upstream-commit: 5a9b5f10cf967f31f0856871ad08f9a0286b4a46
Component: engine
In some circumstances we were not properly releasing plugin references,
leading to failures in removing a plugin with no way to recover other
than restarting the daemon.
1. If volume create fails (in the driver)
2. If a driver validation fails (should be rare)
3. If trying to get a plugin that does not match the passed in capability
Ideally the test for 1 and 2 would just be a unit test, however the
plugin interfaces are too complicated as `plugingetter` relies on
github.com/pkg/plugin/Client (a concrete type), which will require
spinning up services from within the unit test... it just wouldn't be a
unit test at this point.
I attempted to refactor this a bit, but since both libnetwork and
swarmkit are reliant on `plugingetter` as well, this would not work.
This really requires a re-write of the lower-level plugin management to
decouple these pieces.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 3816b514387efd24394f0b8e61d55502aa6ac9ac
Component: engine
When using a volume via the `Binds` API, a shared selinux label is
automatically set.
The `Mounts` API is not setting this, which makes volumes specified via
the mounts API useless when selinux is enabled.
This fix adopts the same selinux label for volumes on the mounts API as on
binds.
Note in the case of both the `Binds` API and the `Mounts` API, the
selinux label is only applied when the volume driver is the `local`
driver.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 5bbf5cc671ec8007bf8e0416799fff01d6a79b7e
Component: engine
Use strongly typed errors to set HTTP status codes.
Error interfaces are defined in the api/errors package and errors
returned from controllers are checked against these interfaces.
Errors can be wraeped in a pkg/errors.Causer, as long as somewhere in the
line of causes one of the interfaces is implemented. The special error
interfaces take precedence over Causer, meaning if both Causer and one
of the new error interfaces are implemented, the Causer is not
traversed.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: ebcb7d6b406fe50ea9a237c73004d75884184c33
Component: engine
Current insider builds of Windows have support for mounting individual
named pipe servers from the host to the guest. This allows, for example,
exposing the docker engine's named pipe to a container.
This change allows the user to request such a mount via the normal bind
mount syntax in the CLI:
docker run -v \\.\pipe\docker_engine:\\.\pipe\docker_engine <args>
Signed-off-by: John Starks <jostarks@microsoft.com>
Upstream-commit: 54354db850664783918a1fc9d208bcfcf47c28e2
Component: engine
Don't create source directory while the daemon is being shutdown, fix#30348
Upstream-commit: cd2255a296acf8408d2afb65b897560479f1ecd3
Component: engine
If a container mount the socket the daemon is listening on into
container while the daemon is being shutdown, the socket will
not exist on the host, then daemon will assume it's a directory
and create it on the host, this will cause the daemon can't start
next time.
fix issue https://github.com/moby/moby/issues/30348
To reproduce this issue, you can add following code
```
--- a/daemon/oci_linux.go
+++ b/daemon/oci_linux.go
@@ -8,6 +8,7 @@ import (
"sort"
"strconv"
"strings"
+ "time"
"github.com/Sirupsen/logrus"
"github.com/docker/docker/container"
@@ -666,7 +667,8 @@ func (daemon *Daemon) createSpec(c *container.Container) (*libcontainerd.Spec, e
if err := daemon.setupIpcDirs(c); err != nil {
return nil, err
}
-
+ fmt.Printf("===please stop the daemon===\n")
+ time.Sleep(time.Second * 2)
ms, err := daemon.setupMounts(c)
if err != nil {
return nil, err
```
step1 run a container which has `--restart always` and `-v /var/run/docker.sock:/sock`
```
$ docker run -ti --restart always -v /var/run/docker.sock:/sock busybox
/ #
```
step2 exit the the container
```
/ # exit
```
and kill the daemon when you see
```
===please stop the daemon===
```
in the daemon log
The daemon can't restart again and fail with `can't create unix socket /var/run/docker.sock: is a directory`.
Signed-off-by: Lei Jitang <leijitang@huawei.com>
Upstream-commit: 7318eba5b2f8bb4b867ca943c3229260ca98a3bc
Component: engine
Closes#32663 by adding CreatedAt field when volume is created.
Displaying CreatedAt value when volume is inspected
Adding tests to verfiy the new field is correctly populated
Signed-off-by: Marianna <mtesselh@gmail.com>
Moving CreatedAt tests from the CLI
Moving the tests added for the newly added CreatedAt field for Volume, from CLI to API tests
Signed-off-by: Marianna <mtesselh@gmail.com>
Upstream-commit: a46f757c4043031379362c5d6b3bad7562ab9fed
Component: engine
This makes sure that multiple users of MountPoint pointer can
mount/unmount without affecting each other.
Before this PR, if you run a container (stay running), then do `docker
cp`, when the `docker cp` is done the MountPoint is mutated such that
when the container stops the volume driver will not get an Unmount
request. Effectively there would be two mounts with only one unmount.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: df0d317a64e4a74433359e826bc1d606e050a5ed
Component: engine
When there is an error unmounting a local volume, it is still possible
to call `Remove()` on the volume causing removal of the mounted
resources which is generally not desirable.
This ensures that resources are unmounted before attempting removal.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: db3576f8a08ca70287bd3fdf9b21e162537f9d3a
Component: engine
Until and unless user has specified a propagation property for volume, they
should default to "rprivate" and it should be passed to runc.
We can't make it conditional on HasPropagation(). GetPropagation() returns
default of rprivate if noting was passed in by user.
If we don't pass "rprivate" to runc, then bind mount could be shared even
if user did not ask for it. For example, mount two volumes in a container.
One is "shared" while other's propagation is not specified by caller. If
both volume has same source mount point of "shared", then second volume
will also be shared inside container (instead of being private).
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Upstream-commit: af8a1430f1c1a9d4c45c7d722b90c19094171651
Component: engine
This adds 'consistency' mode flags to the mount command line argument.
Initially, the valid 'consistency' flags are 'consistent', 'cached',
'delegated', and 'default'.
Signed-off-by: David Sheets <dsheets@docker.com>
Signed-off-by: Jeremy Yallop <yallop@docker.com>
Upstream-commit: f13297c0beaf4fcc6742a9f3c047cbfeef955ac1
Component: engine
