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

test: add test for different volume sources #1553

Merged
merged 1 commit into from
Jun 20, 2018

Conversation

shaloulcy
Copy link
Contributor

the volumes of container have four different sources(volumes-from,
binds, images and Config.Volume). If two volumes have the same destination,
only one volume is reserved. The order is
volumes-from > binds > images > Config.Volume

Signed-off-by: Eric Li [email protected]

Ⅰ. Describe what this PR did

the volumes of container have four different sources(volumes-from, binds, images and Config.Volume). If two volumes have the same destination, only one volume is reserved. The order is
volumes-from > binds > images > Config.Volume

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how you did it

NONE

Ⅳ. Describe how to verify it

NONE

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jun 19, 2018

Codecov Report

Merging #1553 into master will decrease coverage by 1.32%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1553      +/-   ##
==========================================
- Coverage    41.3%   39.98%   -1.33%     
==========================================
  Files         267      267              
  Lines       17331    17975     +644     
==========================================
+ Hits         7159     7187      +28     
- Misses       9282     9898     +616     
  Partials      890      890
Impacted Files Coverage Δ
apis/opts/shm_size.go 0% <0%> (-100%) ⬇️
daemon/mgr/container_types.go 55.85% <0%> (-20.27%) ⬇️
daemon/mgr/spec_mount.go 49.63% <0%> (-19.76%) ⬇️
daemon/mgr/container.go 35.99% <0%> (-13.42%) ⬇️
daemon/mgr/container_utils.go 47.38% <0%> (-7.6%) ⬇️
cli/common_flags.go 0% <0%> (ø) ⬆️
cli/container.go 0% <0%> (ø) ⬆️
daemon/mgr/volume.go 81.25% <0%> (+2.08%) ⬆️
daemon/mgr/image_utils.go 85.45% <0%> (+3.63%) ⬆️
pkg/errtypes/errors.go 100% <0%> (+10.52%) ⬆️

DelContainerForceMultyTime(c, containerName1)
DelContainerForceMultyTime(c, containerName2)
RemoveVolume(c, volumeName1)
command.PouchRun("rmi", "-f", imageWithVolume).Assert(c, icmd.Success)
Copy link
Contributor

Choose a reason for hiding this comment

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

For the cleanup code, please use defer and put them in the right place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Letty5411 The volume must be deleted after all containers has been deleted

but the volume name is generated after run container, So if I want to use defer, the logic is:

pull image
defer rm image
run container1
run container2
getVolumeName
defer rm volume
defer rm container1
defer rm container2

I think it's ugly. So I don't use defer

@@ -170,6 +172,53 @@ func (suite *PouchRunVolumeSuite) TestRunWithVolumesFromWithDupclicate(c *check.
c.Assert(volumeFound, check.Equals, true)
}

func (suite *PouchRunVolumeSuite) TestRunWithVolumesFromDifferentSources(c *check.C) {
imageWithVolume := "registry.hub.docker.com/shaloulcy/busybox:with-volume"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a todo here, we'd better build image explicitly when build CMD is supported in pouch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fine

the volumes of container have four different sources(volumes-from,
binds, images and Config.Volume). If two volumes have the same destination,
only one volume is reserved. The order is
volumes-from > binds > images > Config.Volume

Signed-off-by: Eric Li <[email protected]>
@shaloulcy
Copy link
Contributor Author

@Letty5411 I have updated the code, PTAL

@Letty5411
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/storage areas/test LGTM one maintainer or community participant agrees to merge the pull reuqest. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants