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

support --mount type=bind,bind-nonrecursive,... #1430

Merged
merged 1 commit into from
Jan 10, 2019

Conversation

AkihiroSuda
Copy link
Collaborator

@AkihiroSuda AkihiroSuda commented Oct 10, 2018

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

- What I did

Add --mount type=bind,bind-nonrecursive...

- How I did it

By using API v1.40 (unmerged PR: moby/moby#38003)

- How to verify it

$ mountpoint /run
/run is a mountpoint
$ docker run --rm --mount type=bind,src=/,target=/host,readonly busybox touch /host/run/compromise_success
$ docker run --rm --mount type=bind,src=/,target=/host,readonly,bind-nonrecursive busybox touch /host/run/compromise_fail
touch: /host/run/compromise_fail: Read-only file system
$ sudo ls /run/compromise_*
/run/compromise_success

- Description for the changelog

support --mount type=bind,bind-nonrecursive,...

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

https://upload.wikimedia.org/wikipedia/commons/2/28/Kaiserpinguine_mit_Jungen.jpg
penguins

@AkihiroSuda AkihiroSuda changed the title [DNM/dirtyvendor] support --mount type=bind,bind-norecursive,... [moby#38003] support --mount type=bind,bind-norecursive,... Oct 10, 2018
@AkihiroSuda AkihiroSuda force-pushed the non-recursive-bind branch 2 times, most recently from 4eb3d9a to 7faef7a Compare October 10, 2018 17:05
@codecov-io
Copy link

codecov-io commented Oct 10, 2018

Codecov Report

Merging #1430 into master will decrease coverage by 0.06%.
The diff coverage is 8%.

@@            Coverage Diff             @@
##           master    #1430      +/-   ##
==========================================
- Coverage   55.38%   55.32%   -0.07%     
==========================================
  Files         283      283              
  Lines       19305    19330      +25     
==========================================
+ Hits        10692    10694       +2     
- Misses       7919     7939      +20     
- Partials      694      697       +3

@AkihiroSuda AkihiroSuda changed the title [moby#38003] support --mount type=bind,bind-norecursive,... [moby#38003] support --mount type=bind,bind-nonrecursive,... Oct 31, 2018
@AkihiroSuda
Copy link
Collaborator Author

rebased

@AkihiroSuda
Copy link
Collaborator Author

Test failure seems unrelated

--- FAIL: TestActivateBadLicense (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x6985d2]

goroutine 9 [running]:
testing.tRunner.func1(0xc00011c900)
	/usr/local/go/src/testing/testing.go:792 +0x387
panic(0xd55bc0, 0x1610670)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
net/http.(*Client).deadline(0x0, 0xc0000c6080, 0xc000124300, 0xc000034b40)
	/usr/local/go/src/net/http/client.go:187 +0x22
net/http.(*Client).do(0x0, 0xc00011ce00, 0x0, 0x0, 0x0)
	/usr/local/go/src/net/http/client.go:527 +0xab
net/http.(*Client).Do(0x0, 0xc00011ce00, 0x0, 0x0, 0xc000305020)
	/usr/local/go/src/net/http/client.go:509 +0x35
github.com/docker/cli/vendor/github.com/docker/docker/client.(*Client).doRequest(0xc000124100, 0xf852c0, 0xc000086010, 0xc00011cd00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/vendor/github.com/docker/docker/client/request.go:132 +0x13b
github.com/docker/cli/vendor/github.com/docker/docker/client.(*Client).sendRequest(0xc000124100, 0xf852c0, 0xc000086010, 0xe7089f, 0x3, 0xe71b0e, 0x5, 0xc0000ac5d0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/vendor/github.com/docker/docker/client/request.go:121 +0x153
github.com/docker/cli/vendor/github.com/docker/docker/client.(*Client).get(0xc000124100, 0xf852c0, 0xc000086010, 0xe71b0e, 0x5, 0xc0000ac5d0, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/vendor/github.com/docker/docker/client/request.go:36 +0xe3
github.com/docker/cli/vendor/github.com/docker/docker/client.(*Client).Info(0xc000124100, 0xf852c0, 0xc000086010, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/vendor/github.com/docker/docker/client/info.go:15 +0x144
github.com/docker/cli/cli/command.ElectAuthServer(0xf852c0, 0xc000086010, 0xf90000, 0xc000344640, 0xc0000ad3e8, 0x40bb6f)
	/go/src/github.com/docker/cli/cli/command/registry.go:30 +0xbd
github.com/docker/cli/cli/command.ResolveAuthConfig(0xf852c0, 0xc000086010, 0xf90000, 0xc000344640, 0xc00002e600, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/cli/command/registry.go:75 +0x192
github.com/docker/cli/cli/command/engine.authResolver.func1(0xf852c0, 0xc000086010, 0xc00002e600, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/cli/command/engine/auth.go:32 +0xa1
github.com/docker/cli/cli/trust.GetImageReferencesAndAuth(0xf852c0, 0xc000086010, 0x0, 0x0, 0xc00000ae20, 0xc000308d80, 0x16, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/cli/trust/trust.go:326 +0x140
github.com/docker/cli/cli/command/engine.getRegistryAuth(0xf90000, 0xc000344640, 0xe82bee, 0x16, 0xc000096980, 0x0, 0x0)
	/go/src/github.com/docker/cli/cli/command/engine/auth.go:23 +0x193
github.com/docker/cli/cli/command/engine.runActivate(0xf90000, 0xc000344640, 0xe78d56, 0xb, 0x0, 0x0, 0xe82bee, 0x16, 0x0, 0x0, ...)
	/go/src/github.com/docker/cli/cli/command/engine/activate.go:81 +0x14f
github.com/docker/cli/cli/command/engine.newActivateCommand.func1(0xc0000e0f00, 0xc00000ad60, 0x0, 0x2, 0x0, 0x0)
	/go/src/github.com/docker/cli/cli/command/engine/activate.go:52 +0x7e
github.com/docker/cli/vendor/github.com/spf13/cobra.(*Command).execute(0xc0000e0f00, 0xc00002e040, 0x2, 0x2, 0xc0000e0f00, 0xc00002e040)
	/go/src/github.com/docker/cli/vendor/github.com/spf13/cobra/command.go:762 +0x473
github.com/docker/cli/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0xc0000e0f00, 0xc00011ca00, 0xc0000e0f00, 0xc00011fb20)
	/go/src/github.com/docker/cli/vendor/github.com/spf13/cobra/command.go:852 +0x2fd
github.com/docker/cli/vendor/github.com/spf13/cobra.(*Command).Execute(0xc0000e0f00, 0xe74d07, 0x7)
	/go/src/github.com/docker/cli/vendor/github.com/spf13/cobra/command.go:800 +0x2b
github.com/docker/cli/cli/command/engine.TestActivateBadLicense(0xc00011c900)
	/go/src/github.com/docker/cli/cli/command/engine/activate_test.go:47 +0xd6
testing.tRunner(0xc00011c900, 0xeba620)
	/usr/local/go/src/testing/testing.go:827 +0xbf
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:878 +0x353
FAIL	github.com/docker/cli/cli/command/engine	0.019s
make: *** [Makefile:22: test-coverage] Error 1

https://circleci.com/gh/docker/cli/18777?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@AkihiroSuda
Copy link
Collaborator Author

The CI failure above seems related to moby/moby@3e5b9cb

@cpuguy83 PTAL?

@AkihiroSuda AkihiroSuda changed the title [moby#38003] support --mount type=bind,bind-nonrecursive,... support --mount type=bind,bind-nonrecursive,... Nov 7, 2018
@vdemeester
Copy link
Collaborator

Should be fixed #1504

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@AkihiroSuda
Copy link
Collaborator Author

rebased

@AkihiroSuda AkihiroSuda changed the title support --mount type=bind,bind-nonrecursive,... [WIP:doc is missing]support --mount type=bind,bind-nonrecursive,... Nov 8, 2018
@AkihiroSuda
Copy link
Collaborator Author

Marked the PR as WIP, as this PR lacks doc

@AkihiroSuda AkihiroSuda changed the title [WIP:doc is missing]support --mount type=bind,bind-nonrecursive,... support --mount type=bind,bind-nonrecursive,... Nov 9, 2018
@AkihiroSuda
Copy link
Collaborator Author

added docs.

@thaJeztah PTAL?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Thanks for the docs! Found some small issues, but looks good otherwise

opts/mount.go Show resolved Hide resolved
docs/reference/commandline/service_create.md Show resolved Hide resolved
docs/reference/commandline/service_create.md Outdated Show resolved Hide resolved
@AkihiroSuda
Copy link
Collaborator Author

updated

man/docker-run.1.md Outdated Show resolved Hide resolved
@AkihiroSuda
Copy link
Collaborator Author

ping @thaJeztah @cpuguy83

Copy link
Collaborator

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda
Copy link
Collaborator Author

ping @thaJeztah ^^

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@AkihiroSuda
Copy link
Collaborator Author

@thaJeztah PTAL?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Last few docs comments, but LGTM after those are done 😅 (sorry for the delay)

docs/reference/commandline/service_create.md Outdated Show resolved Hide resolved
docs/reference/commandline/service_create.md Outdated Show resolved Hide resolved
docs/reference/commandline/service_create.md Outdated Show resolved Hide resolved
man/docker-run.1.md Outdated Show resolved Hide resolved
man/docker-run.1.md Outdated Show resolved Hide resolved
docs/reference/commandline/service_create.md Outdated Show resolved Hide resolved
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
@AkihiroSuda
Copy link
Collaborator Author

updated

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

are follow-up changes needed to support this in the compose file format?

@thaJeztah thaJeztah merged commit f5280d6 into docker:master Jan 10, 2019
@GordonTheTurtle GordonTheTurtle added this to the 19.03.0 milestone Jan 10, 2019
@AkihiroSuda
Copy link
Collaborator Author

cc @shin-

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.

6 participants