feature: add "volume" flag when remove container #1255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rudyfly
force-pushed
the
remove-volume
branch
2 times, most recently
from
954cd17 to
fe64515
Compare
Codecov Report
@@ Coverage Diff @@
## master #1255 +/- ##
==========================================
+ Coverage 15.24% 15.28% +0.04%
==========================================
Files 172 172
Lines 10715 10684 -31
==========================================
Hits 1633 1633
+ Misses 8962 8930 -32
- Partials 120 121 +1
|
fuweid
reviewed
May 3, 2018
cli/rm.go
Outdated
| flagSet := r.cmd.Flags() | ||
|
|
||
| flagSet.BoolVarP(&r.force, "force", "f", false, "if the container is running, force to remove it") | ||
| flagSet.BoolVarP(&r.removeVolume, "volumes", "v", false, "remove container's volumes that create by the container") |
There was a problem hiding this comment.
it's better to use removeVolume instead of removeVolumes
HusterWan
reviewed
May 3, 2018
daemon/mgr/container.go
Outdated
| @@ -2271,6 +2271,12 @@ func (mgr *ContainerManager) detachVolumes(ctx context.Context, c *ContainerMeta | |||
| } | |||
|
|
|||
| mgr.VolumeMgr.Detach(ctx, name, option) | |||
There was a problem hiding this comment.
Detach is to umount volume mountpoint, so if the volume is attached to multi containers, does this action will cause other containers' volumes unused?
Another thing: do we distinct volumes created by user and pouchd, because users may delete volumes created by themself, so we just need delete volumes created by pouchd?
fuweid
reviewed
May 3, 2018
test/environment/cleanup.go
Outdated
| @@ -4,6 +4,7 @@ import ( | |||
| "context" | |||
| "fmt" | |||
|
|
|||
| "github.com/alibaba/pouch/apis/types" | |||
| "github.com/alibaba/pouch/client" | |||
| "github.com/pkg/errors" | |||
rudyfly
force-pushed
the
remove-volume
branch
2 times, most recently
from
3eed41b to
50fd6cd
Compare
|
PTAL @HusterWan @fuweid |
HusterWan
reviewed
May 3, 2018
client/container_remove_test.go
Outdated
| @@ -24,7 +26,7 @@ func TestContainerRemoveNotFoundError(t *testing.T) { | |||
| client := &APIClient{ | |||
| HTTPCli: newMockClient(errorMockResponse(http.StatusNotFound, "Not Found")), | |||
| } | |||
| err := client.ContainerRemove(context.Background(), "no contaienr", true) | |||
| err := client.ContainerRemove(context.Background(), "no contaienr", &types.ContainerRemoveOptions{Force: true}) | |||
There was a problem hiding this comment.
%s/contaienr/container :)
HusterWan
reviewed
May 3, 2018
daemon/mgr/volume.go
Outdated
| } | ||
|
|
||
| cid, ok := options[types.OptionRef] | ||
| if ok && len(cid) != 0 { |
There was a problem hiding this comment.
cid == "" will be better WDYT???
There was a problem hiding this comment.
@HusterWan I think two kinds are the same. Just keep the consistent in one project.
Add "volume" flag when remove container, it is used to remove these volumes that is created by container. Signed-off-by: Rudy Zhang <rudyflyzhang@gmail.com>
|
LGTM |
pouchrobot
added
the
LGTM
This was referenced May 3, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ⅰ. Describe what this PR did
Add "volume" flag when remove container, it is used to remove these
volumes that is created by container.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
-vⅤ. Special notes for reviews
Signed-off-by: Rudy Zhang rudyflyzhang@gmail.com