Commit Graph

60 Commits

Author SHA1 Message Date
3f0393a442 Move mount parsing to separate package.
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
2018-04-19 06:35:54 -04:00
be83c11fb0 Add canonical import comment
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Upstream-commit: 4f0d95fa6ee7f865597c03b9e63702cdcb0f7067
Component: engine
2018-02-05 16:51:57 -05:00
bc89af9929 Simplify/fix MkdirAll usage
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
2017-11-27 17:32:12 -08:00
889843574b volume: evaluate symlinks before relabeling mount source
Simple reproducer:

```sh
$ mkdir /var/foo
$ touch /var/foo/test
$ ln -s /var/foo /var/bar
$ docker run -ti -v /var/bar:/var/bar:Z fedora sh
sh-4.3# ls -lZ /var/bar/
ls: cannot open directory '/var/bar/': Permission denied
```

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
Upstream-commit: e0b22c0b9e013527ef121250b51ae780d2d2912d
Component: engine
2017-09-19 10:54:03 +02:00
aeb89eb179 Volume refactoring for LCOW
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
Upstream-commit: e89b6e8c2d2c36c43f22aeaf2a885646c2994051
Component: engine
2017-09-14 12:33:31 -07:00
30f1b651e2 Remove string checking in API error handling
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
2017-08-15 16:01:11 -04:00
fae0c281b6 Windows: Add named pipe mount support
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
2017-08-07 11:34:36 -07:00
643654c2f0 Spelling fixes
* additional
* ambiguous
* anonymous
* anything
* application
* because
* before
* building
* capabilities
* circumstances
* commit
* committer
* compresses
* concatenated
* config
* container
* container's
* current
* definition
* delimiter
* disassociates
* discovery
* distributed
* doesnotexist
* downloads
* duplicates
* either
* enhancing
* enumerate
* escapable
* exactly
* expect
* expectations
* expected
* explicitly
* false
* filesystem
* following
* forbidden
* git with
* healthcheck
* ignore
* independent
* inheritance
* investigating
* irrelevant
* it
* logging
* looking
* membership
* mimic
* minimum
* modify
* mountpoint
* multiline
* notifier
* outputting
* outside
* overridden
* override
* parsable
* plugins
* precedence
* propagation
* provided
* provides
* registries
* repositories
* returning
* settings
* should
* signals
* someone
* something
* specifically
* successfully
* synchronize
* they've
* thinking
* uninitialized
* unintentionally
* unmarshaling
* unnamed
* unreferenced
* verify

Signed-off-by: Josh Soref <jsoref@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 39bcaee47b8a284a46b761afe218ba7deda0d482
Component: engine
2017-07-03 13:13:09 -07:00
8d232fdb68 Refactor MountPoint Setup function in volume.go
Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>
Upstream-commit: fb8b27cd41c92c9a739be70dee11f6e6ffffafdf
Component: engine
2017-06-30 11:09:49 +03:00
5a81510449 Do not error on relabel when relabel not supported
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: ebfdfc5768b74e0a52875cf76a0576bfcd66445c
Component: engine
2017-06-26 17:29:24 -04:00
779caabedf Partial refactor of UID/GID usage to use a unified struct.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Upstream-commit: 09cd96c5ad2de369912cdf708c3c50f41e4586ac
Component: engine
2017-06-07 11:44:33 -04:00
91bac6c3f8 Merge pull request #33330 from coolljt0725/fix_sock_is_dir
Don't create source directory while the daemon is being shutdown, fix #30348
Upstream-commit: cd2255a296acf8408d2afb65b897560479f1ecd3
Component: engine
2017-06-07 12:37:08 +09:00
fe2c9ff86c Don't create source directory while the daemon is being shutdown, fix #30348
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
2017-05-30 22:59:51 -04:00
7acd97d8f9 Merge pull request #33257 from mtesselH/master
Add CreatedAt filed to volume. Display when volume is inspected.
Upstream-commit: 79b19c2e16c43f22187538953f49b2c048b063f8
Component: engine
2017-05-29 10:48:07 +01:00
3d97792442 Add CreatedAt filed to volume. Display when volume is inspected.
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
2017-05-26 11:47:02 -07:00
25ad1db09f Merge pull request #32909 from cpuguy83/32907_volume_unmount_on_cp
Add refcount for MountPoint
Upstream-commit: 09ff5ce29c82b6c97123b24855e1ef11e860b47a
Component: engine
2017-05-09 20:15:41 +02:00
79f9dd11cc Merge pull request #32687 from runcom/oci-selinux
Switch to using opencontainers/selinux for selinux bindings
Upstream-commit: 4219156a6254bdc7d270f06742037df8e2708a9a
Component: engine
2017-04-29 19:05:32 +02:00
ce01ee60c2 Add refcount for MountPoint
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
2017-04-28 16:01:25 -04:00
597451657d Volumes should have default propagation property "rprivate"
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
2017-04-26 16:27:50 -04:00
9432bd8249 Switch to using opencontainers/selinux for selinux bindings
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
Upstream-commit: abbbf914986d6d0ea15923f9a57a99465791bc83
Component: engine
2017-04-24 21:29:47 +02:00
1903b00ea0 We need to fix labels if the user requests on volumes
Currently local volumes and other volumes that support SELinux do
not get labeled correctly.  This patch will allow a user to specify
:Z or :z when  mounting a volume and have it fix the label of the newly
created volume.

Signed-off-by: Dan Walsh <dwalsh@redhat.com>
Upstream-commit: 0c791c8e9f462b0df2c1a76aff45b9033bd962cd
Component: engine
2017-01-10 09:39:31 -05:00
5bd9457743 Correct comment in vol driver interface
Signed-off-by: John Howard <jhoward@microsoft.com>
Upstream-commit: c2246f28f6bd8f4ebc333cfdb8a9e8ab6d1b7845
Component: engine
2017-01-04 12:06:37 -08:00
795fc21c8b Fix a bit typos
Signed-off-by: yuexiao-wang <wang.yuexiao@zte.com.cn>
Upstream-commit: 11454e1c97ed5495e77a06868a0c199d81aa96c3
Component: engine
2016-12-09 03:05:11 +08:00
1689aa276d Fix uneccessary calls to volume.Unmount()
Fixes #22564

When an error occurs on mount, there should not be any call later to
unmount. This can throw off refcounting in the underlying driver
unexpectedly.

Consider these two cases:

```
$ docker run -v foo:/bar busybox true
```

```
$ docker run -v foo:/bar -w /foo busybox true
```

In the first case, if mounting `foo` fails, the volume driver will not
get a call to unmount (this is the incorrect behavior).

In the second case, the volume driver will not get a call to unmount
(correct behavior).

This occurs because in the first case, `/bar` does not exist in the
container, and as such there is no call to `volume.Mount()` during the
`create` phase. It will error out during the `start` phase.

In the second case `/bar` is created before dealing with the volume
because of the `-w`. Because of this, when the volume is being setup
docker will try to copy the image path contents in the volume, in which
case it will attempt to mount the volume and fail. This happens during
the `create` phase. This makes it so the container will not be created
(or at least fully created) and the user gets the error on `create`
instead of `start`. The error handling is different in these two phases.

Changed to only send `unmount` if the volume is mounted.

While investigating the cause of the reported issue I found some odd
behavior in unmount calls so I've cleaned those up a bit here as well.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 9a2d0bc3adc0c21c82cd1974be45ea0449f9f224
Component: engine
2016-11-10 14:04:08 -05:00
ed09e04a52 Merge pull request #27164 from cpuguy83/carry_24205
Fix volume creates blocked by stale cache entries
Upstream-commit: cf55397e130e6ab04ad723ef014cf49a3b62998e
Component: engine
2016-11-03 10:28:13 -07:00
bdd83cf850 Fix volume creates blocked by stale cache entries
When a conflict is found in the volume cache, check with the driver if
that volume still actually exists.
If the volume doesn't exist, purge it from the cache and allow the
create to happen.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 6a0bdffc1adf9225c6cca92f741ccafd2f90a618
Component: engine
2016-11-03 11:56:44 -04:00
ecffd5b73a api: add TypeTmpfs to api/types/mount
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
Upstream-commit: 18768fdc2e76ec6c600c8ab57d2d487ee7877794
Component: engine
2016-10-28 08:38:32 +00:00
6b3066953c Show volume options for docker volume inspect
This fix tries to address the issue raised in 25545 where
volume options at the creation time is not showed up
in `docker volume inspect`.

This fix adds the field `Options` in `Volume` type and
persist the options in volume db so that `volume inspect`
could display the options.

This fix adds a couple of test cases to cover the changes.

This fix fixes 25545.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: 9ce8aac55e6df65bbf49c682374871a94d379bf3
Component: engine
2016-10-20 05:14:27 -07:00
3eaab012dd Fix some places where low-level errors bubbled up
Found a couple of places where pretty low level errors were never being
wrapped with any sort of context.

For example, if you try to create a local volume using some bad mount
options, the kernel will return `invalid argument` when we try to mount
it at container start.
What would happen is a user would `docker run` with this volume and get
an error like `Error response from daemon: invalid argument`.

This uses github.com/pkg/errors to provide some context to the error
message without masking the original error.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 2a5e85e2e871bf037c976471cf8a14da22ddd9a9
Component: engine
2016-09-24 08:01:21 -04:00
06b2219a87 Add new HostConfig field, Mounts.
`Mounts` allows users to specify in a much safer way the volumes they
want to use in the container.
This replaces `Binds` and `Volumes`, which both still exist, but
`Mounts` and `Binds`/`Volumes` are exclussive.
The CLI will continue to use `Binds` and `Volumes` due to concerns with
parsing the volume specs on the client side and cross-platform support
(for now).

The new API follows exactly the services mount API.

Example usage of `Mounts`:

```
$ curl -XPOST localhost:2375/containers/create -d '{
  "Image": "alpine:latest",
  "HostConfig": {
    "Mounts": [{
      "Type": "Volume",
      "Target": "/foo"
      },{
      "Type": "bind",
      "Source": "/var/run/docker.sock",
      "Target": "/var/run/docker.sock",
      },{
      "Type": "volume",
      "Name": "important_data",
      "Target": "/var/data",
      "ReadOnly": true,
      "VolumeOptions": {
	"DriverConfig": {
	  Name: "awesomeStorage",
	  Options: {"size": "10m"},
	  Labels: {"some":"label"}
	}
      }]
    }
}'
```

There are currently 2 types of mounts:

  - **bind**: Paths on the host that get mounted into the
    container. Paths must exist prior to creating the container.
  - **volume**: Volumes that persist after the
    container is removed.

Not all fields are available in each type, and validation is done to
ensure these fields aren't mixed up between types.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: fc7b904dced4d18d49c8a6c47ae3f415d16d0c43
Component: engine
2016-09-13 09:55:35 -04:00
45ddc4bfcb Add engine-api types to docker
This moves the types for the `engine-api` repo to the existing types
package.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Upstream-commit: 91e197d614547f0202e6ae9b8a24d88ee131d950
Component: engine
2016-09-07 11:05:58 -07:00
9a0304155e Make host directory mounts use idtools.MkdirAllNewAs
This makes sure that:
1. Already existing directories are left untouched
2. Newly created directories are chowned to the correct root UID/GID in case of user namespaces
3. All parent directories still get created with host root UID/GID

Fix #21738

Signed-off-by: Antonis Kalipetis <akalipetis@gmail.com>
Upstream-commit: 72d8a77d522896ec73e07f49a1c1bcb44bbf3bbd
Component: engine
2016-09-05 12:46:57 +03:00
3dcc0abf19 revendor engine-api
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 6d98e344c7702a8a713cb9e02a19d83a79d3f930
Component: engine
2016-08-16 14:16:12 -04:00
57c5690493 optimize a print in
Signed-off-by: Yanqiang Miao <miao.yanqiang@zte.com.cn>

update 'optimize-a-print'

Signed-off-by: Yanqiang Miao <miao.yanqiang@zte.com.cn>
Upstream-commit: a503f3ac2fdb5f24b7ca982fc6447423095f229e
Component: engine
2016-07-28 19:20:20 +08:00
acb2cc6fda Change mount-types to lowercase
these values were changed to lowercase in
690cb2d08c,
but not changed accordingly in docker/docker.

this changes the mounttypes to lowercase

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 8f93128cd619e1d11be1bc0ae21f1362b1e3f9ad
Component: engine
2016-07-21 11:25:42 +02:00
38e9398cfc fixes minor typo in comment
Signed-off-by: Robert Terhaar <rterhaar@atlanticdynamic.com>
Upstream-commit: 29fea0fd2e9970eb5df0feb0ebef41c5425295f0
Component: engine
2016-07-01 17:29:08 -04:00
be63983b3a Add Swarm management backend
As described in our ROADMAP.md, introduce new Swarm management API
endpoints relying on swarmkit to deploy services. It currently vendors
docker/engine-api changes.

This PR is fully backward compatible (joining a Swarm is an optional
feature of the Engine, and existing commands are not impacted).

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Victor Vieux <vieux@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
Signed-off-by: Madhu Venugopal <madhu@docker.com>
Upstream-commit: 534a90a99367af6f6bba1ddcc7eb07506e41f774
Component: engine
2016-06-13 22:16:18 -07:00
da4d783e80 Add support for volume scopes
This is similar to network scopes where a volume can either be `local`
or `global`. A `global` volume is one that exists across the entire
cluster where as a `local` volume exists on a single engine.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 2f40b1b281a3be8f34d82a5170988ee46ea1f442
Component: engine
2016-06-05 15:37:15 -04:00
ab8e2e8aee Need to create bind mount volume if it does not exist.
In order to be consistent on creation of volumes for bind mounts
we need to create the source directory if it does not exist and the
user specified he wants it relabeled.

Can not do this lower down the stack, since we are not passing in the
mode fields.

Signed-off-by: Dan Walsh <dwalsh@redhat.com>
Upstream-commit: 322cc99c6962ecb56be3107061eb7f61364d05f8
Component: engine
2016-06-02 07:14:17 -04:00
ecdf06bd1f Enable auto-creation of host-path on Windows
Auto-creation of host-paths has been un-deprecated,
so to have feature-parity between Linux and Windows,
this feature should also be present on Windows.

This enables auto-creation on Windows.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 4e080347af657ca3a0c103c6bc6cd6a8157d20d8
Component: engine
2016-05-07 00:55:16 +02:00
de046a0cf3 When calling volume driver Mount, send opaque ID
This generates an ID string for calls to Mount/Unmount, allowing drivers
to differentiate between two callers of `Mount` and `Unmount`.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 2b6bc294fc7f9e08a9091833b021b7d2a01ad2a6
Component: engine
2016-04-29 09:37:02 -04:00
8cd702acc5 Merge pull request #22065 from thaJeztah/remove-deprecation-message
Remove deprecation warning
Upstream-commit: 4e898ae64be25bc5a86e0e7865a37b9f596e4d17
Component: engine
2016-04-18 15:29:05 -04:00
ed92db4f93 Allow volume drivers to provide a Status field
The `Status` field is a `map[string]interface{}` which allows the driver to pass
back low-level details about the underlying volume.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: 36a1c56cf555f8fe9ceabeebb8fc956e05863fc7
Component: engine
2016-04-15 10:56:38 -04:00
18bcaaee70 Remove deprecation warning
Auto-creation of non-existing host directories
is no longer deprecated (9d5c26bed2ac287542e176d9149250927876e3f5),
so this warning is no longer relevant.

This removes the deprecation warning.

Also removes the "system" package here, because it's only used
on non-Windows, so basically just called os.MkdirAll()

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 1d02ad2a519765179480e0ae113bcf510a2713af
Component: engine
2016-04-15 13:57:19 +02:00
308c81e7d4 Add explicit flags for volume cp/no-cp
This allows a user to specify explicitly to enable
automatic copying of data from the container path to the volume path.
This does not change the default behavior of automatically copying, but
does allow a user to disable it at runtime.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: b0ac69b67ef79c6c937f84bee3df20a1924ad334
Component: engine
2016-03-21 20:38:44 -04:00
16d6520e91 fix some typos.
Signed-off-by: allencloud <allen.sun@daocloud.io>
Upstream-commit: 34b82a69b94ef9c7913e2809ae918e6f4331201e
Component: engine
2016-03-10 10:09:27 +08:00
b2ac99b3fa Remove static errors from errors package.
Moving all strings to the errors package wasn't a good idea after all.

Our custom implementation of Go errors predates everything that's nice
and good about working with errors in Go. Take as an example what we
have to do to get an error message:

```go
func GetErrorMessage(err error) string {
	switch err.(type) {
	case errcode.Error:
		e, _ := err.(errcode.Error)
		return e.Message

	case errcode.ErrorCode:
		ec, _ := err.(errcode.ErrorCode)
		return ec.Message()

	default:
		return err.Error()
	}
}
```

This goes against every good practice for Go development. The language already provides a simple, intuitive and standard way to get error messages, that is calling the `Error()` method from an error. Reinventing the error interface is a mistake.

Our custom implementation also makes very hard to reason about errors, another nice thing about Go. I found several (>10) error declarations that we don't use anywhere. This is a clear sign about how little we know about the errors we return. I also found several error usages where the number of arguments was different than the parameters declared in the error, another clear example of how difficult is to reason about errors.

Moreover, our custom implementation didn't really make easier for people to return custom HTTP status code depending on the errors. Again, it's hard to reason about when to set custom codes and how. Take an example what we have to do to extract the message and status code from an error before returning a response from the API:

```go
	switch err.(type) {
	case errcode.ErrorCode:
		daError, _ := err.(errcode.ErrorCode)
		statusCode = daError.Descriptor().HTTPStatusCode
		errMsg = daError.Message()

	case errcode.Error:
		// For reference, if you're looking for a particular error
		// then you can do something like :
		//   import ( derr "github.com/docker/docker/errors" )
		//   if daError.ErrorCode() == derr.ErrorCodeNoSuchContainer { ... }

		daError, _ := err.(errcode.Error)
		statusCode = daError.ErrorCode().Descriptor().HTTPStatusCode
		errMsg = daError.Message

	default:
		// This part of will be removed once we've
		// converted everything over to use the errcode package

		// FIXME: this is brittle and should not be necessary.
		// If we need to differentiate between different possible error types,
		// we should create appropriate error types with clearly defined meaning
		errStr := strings.ToLower(err.Error())
		for keyword, status := range map[string]int{
			"not found":             http.StatusNotFound,
			"no such":               http.StatusNotFound,
			"bad parameter":         http.StatusBadRequest,
			"conflict":              http.StatusConflict,
			"impossible":            http.StatusNotAcceptable,
			"wrong login/password":  http.StatusUnauthorized,
			"hasn't been activated": http.StatusForbidden,
		} {
			if strings.Contains(errStr, keyword) {
				statusCode = status
				break
			}
		}
	}
```

You can notice two things in that code:

1. We have to explain how errors work, because our implementation goes against how easy to use Go errors are.
2. At no moment we arrived to remove that `switch` statement that was the original reason to use our custom implementation.

This change removes all our status errors from the errors package and puts them back in their specific contexts.
IT puts the messages back with their contexts. That way, we know right away when errors used and how to generate their messages.
It uses custom interfaces to reason about errors. Errors that need to response with a custom status code MUST implementent this simple interface:

```go
type errorWithStatus interface {
	HTTPErrorStatusCode() int
}
```

This interface is very straightforward to implement. It also preserves Go errors real behavior, getting the message is as simple as using the `Error()` method.

I included helper functions to generate errors that use custom status code in `errors/errors.go`.

By doing this, we remove the hard dependency we have eeverywhere to our custom errors package. Yes, you can use it as a helper to generate error, but it's still very easy to generate errors without it.

Please, read this fantastic blog post about errors in Go: http://dave.cheney.net/2014/12/24/inspecting-errors

Signed-off-by: David Calavera <david.calavera@gmail.com>
Upstream-commit: a793564b2591035aec5412fbcbcccf220c773a4c
Component: engine
2016-02-26 15:49:09 -05:00
697a990701 fix common misspell
Signed-off-by: Victor Vieux <vieux@docker.com>
Upstream-commit: 99a396902f0ea9d81ef87a683489b2435408f415
Component: engine
2016-02-11 15:49:36 -08:00
7e07ac8939 On container rm, don't remove named mountpoints
This makes it so when calling `docker run --rm`, or `docker rm -v`, only
volumes specified without a name, e.g. `docker run -v /foo` instead of
`docker run -v awesome:/foo` are removed.

Note that all volumes are named, some are named by the user, some get a
generated name. This is specifically about how the volume was specified
on `run`, assuming that if the user specified it with a name they expect
it to persist after the container is cleaned up.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: dd7d1c8a02d8693aa4f381f82c5bbdcad9a5ff58
Component: engine
2016-01-25 15:51:28 -05:00
db1910a30c Move responsibility of ls/inspect to volume driver
Makes `docker volume ls` and `docker volume inspect` ask the volume
drivers rather than only using what is cached locally.

Previously in order to use a volume from an external driver, one would
either have to use `docker volume create` or have a container that is
already using that volume for it to be visible to the other volume
API's.

For keeping uniqueness of volume names in the daemon, names are bound to
a driver on a first come first serve basis. If two drivers have a volume
with the same name, the first one is chosen, and a warning is logged
about the second one.

Adds 2 new methods to the plugin API, `List` and `Get`.
If a plugin does not implement these endpoints, a user will not be able
to find the specified volumes as well requests go through the drivers.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Upstream-commit: d3eca4451d264aac564594fe46b8c097bd85a5cc
Component: engine
2016-01-05 16:28:38 -05:00