feature: add volumes-from for container #1131

rudyfly · 2018-04-16T06:57:47Z

Ⅰ. Describe what this PR did

Add volumes-from for container, container can use the volumes of another
containers. The format is <containerID>[:mode]

Ⅱ. Does this pull request fix one issue?

fixes #463

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

create old container with volume

# pouch volume create -n test-volume
CreatedAt:    2018-4-16 14:46:18
Driver:       local
Labels:       map[]
Mountpoint:   /mnt/local/test-volume
Name:         test-volume
Scope:
Status:       map[sifter:Default]

# pouch run -d --name volumes-from-test -v test-volume:/mnt registry.hub.docker.com/library/busybox:latest top
6203151a7134ae9305cbeddba90c874d76eff9bf345cc30a9e34277d89a4d7d4

stop old container

# pouch stop volumes-from-test

create new container with old container's volume

# pouch run -d --volumes-from 6203151a7134ae9305cbeddba90c874d76eff9bf345cc30a9e34277d89a4d7d4 registry.hub.docker.com/library/busybox:latest top
6821c74073fb6a2262b1c00aa2ceaabc47463e06f4e6e718e8827889afd83492

Ⅴ. Special notes for reviews

Signed-off-by: Rudy Zhang [email protected]

codecov-io · 2018-04-16T07:17:42Z

Codecov Report

Merging #1131 into master will decrease coverage by 0.24%.
The diff coverage is 54.45%.

@@            Coverage Diff             @@
##           master    #1131      +/-   ##
==========================================
- Coverage   15.57%   15.33%   -0.25%     
==========================================
  Files         172      173       +1     
  Lines        9727     9782      +55     
==========================================
- Hits         1515     1500      -15     
- Misses       8103     8174      +71     
+ Partials      109      108       -1

Impacted Files	Coverage Δ
cli/container.go	`0% <0%> (ø)`	⬆️
cli/common_flags.go	`0% <0%> (ø)`	⬆️
daemon/mgr/container.go	`0% <0%> (-4.6%)`	⬇️
pkg/opts/mountpoint.go	`84.61% <84.61%> (ø)`
pkg/utils/utils.go	`75.64% <0%> (-2.52%)`	⬇️
cli/cli.go	`0% <0%> (ø)`	⬆️
pkg/randomid/id.go	`75% <0%> (ø)`	⬆️
client/client.go	`39.39% <0%> (+10.82%)`	⬆️

allencloud · 2018-04-16T08:33:09Z

cli/common_flags.go

 	flagSet.StringVar(&c.utsMode, "uts", "", "UTS namespace to use")

 	flagSet.StringSliceVarP(&c.volume, "volume", "v", nil, "Bind mount volumes to container, format is: [source:]<destination>[:mode], [source] can be volume or host's path, <destination> is container's path, [mode] can be \"ro/rw/dr/rr/z/Z/nocopy/private/rprivate/slave/rslave/shared/rshared\"")
+	flagSet.StringSliceVar(&c.volumesFrom, "volumes-from", nil, "set volumes from other containers, format is <container>[:mode]")


I think it is other container, since we cannot set volumes for multiple containers.

I don't think so, the api VolumesFrom is a slice, it means it can use more than one container's volumes.

allencloud · 2018-04-16T08:35:03Z

daemon/mgr/container.go

 	return arr, nil
 }

+func parseVolumesFrom(volume string) (string, string, error) {


Please move this part into pkg opts.

allencloud · 2018-04-16T08:38:51Z

Could we add some integration test for this PR. Thanks a lot. @rudyfly

Letty5411 · 2018-04-17T03:03:25Z

May I give some suggestions about integration tests, I would prefer tests like following:
Negative test:
2. create a container with volumes from a non-existing container
3. create a container with volumes from a container without volumes
4. create container A with volumes V1, remove V1, create container B with volumes from container A

Positive test:

create container with volumes from a container with more than two volumes, verify these volumes
create container B with volumes from container A, then create container C with volumes from container B?
upgrade container with different volumes from different container
create container A with volumes from two containers B and C
test different mode could work, like mode="" default is RW, mode="R", can't write
create a container with volumes from a running/stopped/exited/paused container
create a volume V1, create a container A with volume V1, create container B with volume V1 and volumes from container A.

allencloud · 2018-04-17T06:36:20Z

test/cli_run_test.go

+	for _, line := range strings.Split(out, "\n") {
+		if strings.Contains(line, "\"volumesfrom-test-volume\": \"/mnt\"") {
+			volumeFound = true
+			continue


Add volumes-from for container, container can use the volumes of another containers. The format is "<containerID>[:mode]" Signed-off-by: Rudy Zhang <[email protected]>

allencloud · 2018-04-17T08:10:02Z

LGTM

pouchrobot added areas/storage kind/feature size/L labels Apr 16, 2018

rudyfly requested review from allencloud and yyb196 April 16, 2018 06:59

allencloud reviewed Apr 16, 2018

View reviewed changes

rudyfly force-pushed the volume branch from 97bc780 to d70df79 Compare April 17, 2018 03:37

pouchrobot added size/XL and removed size/L labels Apr 17, 2018

allencloud reviewed Apr 17, 2018

View reviewed changes

pouchrobot added conflict/needs-rebase and removed conflict/needs-rebase labels Apr 17, 2018

feature: add volumes-from for container

d84951a

Add volumes-from for container, container can use the volumes of another containers. The format is "<containerID>[:mode]" Signed-off-by: Rudy Zhang <[email protected]>

rudyfly force-pushed the volume branch from d70df79 to d84951a Compare April 17, 2018 07:27

pouchrobot removed the conflict/needs-rebase label Apr 17, 2018

pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Apr 17, 2018

allencloud merged commit dc75691 into AliyunContainerService:master Apr 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feature: add volumes-from for container #1131

feature: add volumes-from for container #1131

Uh oh!

rudyfly commented Apr 16, 2018 •

edited by allencloud

Loading

Uh oh!

codecov-io commented Apr 16, 2018 •

edited

Loading

Uh oh!

allencloud Apr 16, 2018

Uh oh!

rudyfly Apr 17, 2018

Uh oh!

allencloud Apr 16, 2018

Uh oh!

rudyfly Apr 17, 2018

Uh oh!

allencloud commented Apr 16, 2018

Uh oh!

Letty5411 commented Apr 17, 2018 •

edited

Loading

Uh oh!

allencloud Apr 17, 2018

Uh oh!

allencloud commented Apr 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

feature: add volumes-from for container #1131

feature: add volumes-from for container #1131

Uh oh!

Conversation

rudyfly commented Apr 16, 2018 • edited by allencloud Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Uh oh!

codecov-io commented Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

allencloud Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

rudyfly Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

allencloud Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

rudyfly Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

allencloud commented Apr 16, 2018

Uh oh!

Letty5411 commented Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allencloud Apr 17, 2018

Choose a reason for hiding this comment

Uh oh!

allencloud commented Apr 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rudyfly commented Apr 16, 2018 •

edited by allencloud

Loading

codecov-io commented Apr 16, 2018 •

edited

Loading

Letty5411 commented Apr 17, 2018 •

edited

Loading