- Volume store created dir with wrong permissions
- Local volume driver hardcoded uid/gid 0
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: d15734ec3c10eda667b716f67e18d5d86e708e3e
Component: engine
This is not for the sake of test to run faster of course;
this is to simplify the code as well as have some more
testing for mount.SingleEntryFilter().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: ce468f0ad0d075c5d0c44c78bd61c489e6d7d70c
Component: engine
There is no need to parse mount table and iterate through the list of
mounts, and then call Unmount() which again parses the mount table and
iterates through the list of mounts.
It is totally OK to call Unmount() unconditionally.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: ac39a95ea618601f78662972c35838d928858904
Component: engine
Functions `GetMounts()` and `parseMountTable()` return all the entries
as read and parsed from /proc/self/mountinfo. In many cases the caller
is only interested only one or a few entries, not all of them.
One good example is `Mounted()` function, which looks for a specific
entry only. Another example is `RecursiveUnmount()` which is only
interested in mount under a specific path.
This commit adds `filter` argument to `GetMounts()` to implement
two things:
1. filter out entries a caller is not interested in
2. stop processing if a caller is found what it wanted
`nil` can be passed to get a backward-compatible behavior, i.e. return
all the entries.
A few filters are implemented:
- `PrefixFilter`: filters out all entries not under `prefix`
- `SingleEntryFilter`: looks for a specific entry
Finally, `Mounted()` is modified to use `SingleEntryFilter()`, and
`RecursiveUnmount()` is using `PrefixFilter()`.
Unit tests are added to check filters are working.
[v2: ditch NoFilter, use nil]
[v3: ditch GetMountsFiltered()]
[v4: add unit test for filters]
[v5: switch to gotestyourself]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Upstream-commit: bb934c6aca3e77541dd4fd51b9ab2706294dadda
Component: engine
This moves the platform specific stuff in a separate package and keeps
the `volume` package and the defined interfaces light to import.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 6a70fd222b95643a8a6b88e2634d5f085ae4122a
Component: engine
Instead of using a global store for volume drivers, scope the driver
store to the caller (e.g. the volume store). This makes testing much
simpler.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 977109d808ae94eb3931ae920338b1aa669f627e
Component: engine
Changes Details:
--------------
Fixes: #36395
Refactoring the code to do the following:
1. Add the method `errBindSourceDoesNotExist` inside `validate.go` to be in-line with the rest of error message
2. Utilised the new method inside `linux_parser.go`, `windows_parser.go` and `validate_test.go`
3. Change the format from `bind mount source path: '%s' does not exist` to `bind mount source path does not exist: %s`
4. Reflected the format change into the 2 unit tests, namely: `volume_test.go` and `validate_test.go`
5. Reflected the format change into `docker_api_containers_test.go` integration test
Signed-off-by: Amr Gawish <amr.gawish@gmail.com>
Upstream-commit: df6af282b9048dfedcd7b7a9a89126aca887f4e1
Component: engine
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
