Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cli: add `--mount` to `docker run` #32251

Merged
merged 1 commit into from Apr 5, 2017

Conversation

@AkihiroSuda
Copy link
Member

commented Mar 31, 2017

- What I did

Revert #28838
Update #28527

Plese refer to #28527 (comment) for the discussion so far.

This PR adds --mount to docker run.

The syntax of docker run --mount is identical to docker service create --mount.

Some notes:

  • --volume-driver is ignored for --mounts. A warning will be printed on the client side when specified.
  • --mount still only supports "CSV" form. i.e. --mount type=volume,src=foo,dst=/bar works but --mount foo:/bar does NOT work.
  • user can mix up --mount and -v simultaneously.

- How I did it

Revert #28838 + warning about "--volume-driver is ignored for --mount volumes"

- How to verify it

$ docker run --rm --mount type=bind,src=/host-foo,dst=/foo --mount type=bind,src=/host-bar,dst=/bar -v /host-baz:/baz  busybox ls /foo /bar /baz
/bar:
..
/baz:
..
/foo:
..
$ docker run --volume-driver foo --mount type=volume,dst=/foo -it --rm busybox
WARN[0000] `--volume-driver` is ignored for volumes specified via `--mount`. Use `--mount type=volume,volume-driver
...

- Description for the changelog

cli: add --mount to docker run

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

Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

@dnephin
Copy link
Member

left a comment

LGTM

@stevvooe

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2017

LGTM

@cpuguy83

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2017

Moving to code review.

"--mount", fmt.Sprintf("type=bind,src=%s,target=/bar", mnt2),
},
{
"--volume", fmt.Sprintf("%s:/foo", mnt1),

This comment has been minimized.

Copy link
@aaronlehmann

aaronlehmann Apr 1, 2017

Contributor

mnt1 + ":/foo" instead of Sprintf?

This comment has been minimized.

Copy link
@AkihiroSuda

AkihiroSuda Apr 4, 2017

Author Member

done

"--mount", fmt.Sprintf("type=volume,src=%s,dst=/bar", mnt2),
},
{
"--volume", fmt.Sprintf("%s:/foo", mnt1),

This comment has been minimized.

Copy link
@aaronlehmann

aaronlehmann Apr 1, 2017

Contributor

mnt1 + ":/foo" instead of Sprintf?

This comment has been minimized.

Copy link
@AkihiroSuda

AkihiroSuda Apr 4, 2017

Author Member

done

@aaronlehmann

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2017

LGTM

@vdemeester
Copy link
Member

left a comment

LGTM 🐸

@AkihiroSuda AkihiroSuda force-pushed the AkihiroSuda:run-mount branch from fa1de22 to a45191e Apr 4, 2017

@thaJeztah
Copy link
Member

left a comment

one nit, LGTM otherwise 😅

(sorry, forgot to submit my review)

@@ -61,6 +61,7 @@ docker-run - Run a command in a new container
[**--memory-reservation**[=*MEMORY-RESERVATION*]]
[**--memory-swap**[=*LIMIT*]]
[**--memory-swappiness**[=*MEMORY-SWAPPINESS*]]
[**--mount**[=*MOUNT*]]

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Apr 4, 2017

Member

Can you also add the --mount flag to OPTIONS below?

cli: add `--mount` to `docker run`
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>

@AkihiroSuda AkihiroSuda force-pushed the AkihiroSuda:run-mount branch from a45191e to 77fe35b Apr 5, 2017

@AkihiroSuda

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2017

@thaJeztah done
cc @icecrime @mstanleyjones

@AkihiroSuda AkihiroSuda referenced this pull request Apr 5, 2017
6 of 7 tasks complete
@thaJeztah
Copy link
Member

left a comment

LGTM, thanks!!!

@thaJeztah thaJeztah added this to the 17.05.0 milestone Apr 5, 2017

@vdemeester vdemeester merged commit 945a119 into moby:master Apr 5, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 32545 has succeeded
Details
janky Jenkins build Docker-PRs 41176 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 1336 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 12268 has succeeded
Details
z Jenkins build Docker-PRs-s390x 1168 has succeeded
Details
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
Merge pull request moby#32251 from AkihiroSuda/run-mount
cli: add `--mount` to `docker run`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.