Skip to content

Add new HostConfig field, Mounts.#22373

Merged
cpuguy83 merged 2 commits intomoby:masterfrom
cpuguy83:add_mounts_api_on_create
Sep 13, 2016
Merged

Add new HostConfig field, Mounts.#22373
cpuguy83 merged 2 commits intomoby:masterfrom
cpuguy83:add_mounts_api_on_create

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
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 <[email protected]>
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.