Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add new HostConfig field, Mounts. #22373

Merged
merged 2 commits into from
Sep 13, 2016

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Apr 27, 2016

- What I did
Add capability to /containers/create API to specify mounts in a more granular and safer way

Mounts allows users to specify in a much safer way the volumes they
want to use in the container.

- How I did it
Add new field Mounts to HostConfig as well as new MountConfig

This replaces Binds and Volumes, which both still exist, but
Mounts and Binds/Volumes are exclusive.
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.

- How to verify it
Example usage of Mounts:

$ curl -XPOST localhost:2375/containers/create -d '{
  "Image": "alpine:latest",
  "HostConfig": {
    "Mounts": [{
      "Type": "Volume",
      "Destination": "/foo"
      },{
      "Type": "bind",
      "Source": "/var/run/docker.sock",
      "Destination": "/var/run/docker.sock",
      },{
      "Type": "volume",
      "Name": "important_data",
      "Target": "/var/data",
      "Mode": "ro"
      "Volume": {
    "Driver": "awesomeStorage",
    "DriverOpts": { "Size": "10G" }
    }
      }]
    }
}'

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.

- Description for the changelog

Add capability to /containers/create API to specify mounts in a more granular and safer way

- A picture of a cute animal (not mandatory but encouraged)

@cpuguy83 cpuguy83 force-pushed the add_mounts_api_on_create branch from b8223b6 to ba06fec Compare April 27, 2016 19:51
@icecrime icecrime added status/failing-ci Indicates that the PR in its current state fails the test suite and removed platform/windows labels Apr 27, 2016
@cpuguy83
Copy link
Member Author

Have some cleanup on windows and probably some reconciliation on validations between the string spec (-v /foo:/bar:baz) and the new MountConfig style... they should be the same, but there is duplicated logic that is likely best not to duplicate.

@cpuguy83 cpuguy83 changed the title Add new HostConfig field, Mounts. [WIP] Add new HostConfig field, Mounts. Apr 27, 2016
- **Mounts** - List of mount configurations to create the container with. This replaces `Binds` and `Volumes`. If `Mounts` is specified along with either `Binds` or `Volumes`, the request will be rejected. Fields for a mount configuration:
+ **Type** - String specifying the type of mount, e.g. `hostbind`, `ephemeral`, or `persistent` (required)
+ **Source** - String specifying the host path to use for the mount. Path must exist. (required, `hostbind` only)
+ **Destination** - String specifying the container destination path for the mount (required)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Target, only because it is shorter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I used "Destination" is because we already use this term in container.MountPoints

@stevvooe
Copy link
Contributor

@cpuguy83 Overall, design LGTM. I have a few naming concerns, but those are easy to work out.

I would like to see a mapping for the current "volume syntax" to these types, as part of this effort. It should be straightforward to work out.

@cpuguy83 cpuguy83 force-pushed the add_mounts_api_on_create branch from ba06fec to 44e31b8 Compare April 28, 2016 21:07
@cpuguy83 cpuguy83 changed the title [WIP] Add new HostConfig field, Mounts. Add new HostConfig field, Mounts. Apr 28, 2016
@cpuguy83
Copy link
Member Author

Ok, this is updated, more tests, reconciled some of the validation/parsing between the string spec parser and the new config type... though I did not address @stevvooe's concerns yet

@cpuguy83 cpuguy83 force-pushed the add_mounts_api_on_create branch 8 times, most recently from 4851f95 to fc1538a Compare May 2, 2016 19:20
@cpuguy83 cpuguy83 force-pushed the add_mounts_api_on_create branch 4 times, most recently from 0b8f1b5 to 241412f Compare May 3, 2016 16:07
`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>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the add_mounts_api_on_create branch from d182d62 to 29b1c1d Compare September 13, 2016 13:55
@thaJeztah
Copy link
Member

LGTM!

@cpuguy83 cpuguy83 merged commit b4c1645 into moby:master Sep 13, 2016
@cpuguy83 cpuguy83 deleted the add_mounts_api_on_create branch September 13, 2016 18:39
@cpuguy83
Copy link
Member Author

🎉

@AkihiroSuda
Copy link
Member

Is there a PR or a plan to support this new Mount in Swarmkit?

I'd like to use it in #26331
(CC @justincormack )

@justincormack
Copy link
Contributor

@AkihiroSuda it is used in swarmkit, it is not yet supported on docker run cli, but with this PR it is in API.

@cpuguy83
Copy link
Member Author

Well, swarmkit is still using the Binds API.

@justincormack
Copy link
Contributor

oh, sorry.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Sep 22, 2016

Thank you, and while looking into the implementation, I got surprised that Swarmkit defines MountTypeTmpfs but the engine API not.
Is there plan to migrate tmpfs to the new mount api as well?
If so, can I open PR?

https://github.com/docker/docker/blob/master/api/types/mount/mount.go#L7-L10
https://github.com/docker/swarmkit/blob/master/api/types.proto#L140-L142

@cpuguy83
Copy link
Member Author

@AkihiroSuda Yes, tmpfs needs to be implemented as well, thanks!

@AkihiroSuda
Copy link
Member

Implemented MountTypeTmpfs in #26837
Please look into this? @cpuguy83

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.