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

bugfix: can remove the exited container without flag force. #640

Merged
merged 1 commit into from
Jan 25, 2018

Conversation

skoowoo
Copy link
Contributor

@skoowoo skoowoo commented Jan 25, 2018

Signed-off-by: skoo87 [email protected]

1.Describe what this PR did

2.Does this pull request fix one issue?

fix #639

3.Describe how you did it

4.Describe how to verify it

5.Special notes for reviews

@pouchrobot pouchrobot added kind/bug This is bug report for project size/XS labels Jan 25, 2018
@codecov-io
Copy link

Codecov Report

Merging #640 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #640   +/-   ##
=======================================
  Coverage   14.21%   14.21%           
=======================================
  Files          65       65           
  Lines        3736     3736           
=======================================
  Hits          531      531           
  Misses       3159     3159           
  Partials       46       46
Impacted Files Coverage Δ
daemon/mgr/container.go 3.79% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f51014b...dfc4e70. Read the comment docs.

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jan 25, 2018
@@ -163,7 +163,7 @@ func (mgr *ContainerManager) Remove(ctx context.Context, name string, option *Co
c.Lock()
defer c.Unlock()

if !c.IsStopped() && !c.IsCreated() && !option.Force {
if !c.IsStopped() && !c.IsExited() && !c.IsCreated() && !option.Force {
return fmt.Errorf("container: %s is not stopped, can't remove it without flag force", c.ID())
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to also change the error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's unnecessary ....

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is all right.

@Letty5411 Letty5411 removed the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jan 25, 2018
@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jan 25, 2018
@allencloud allencloud merged commit a983b63 into AliyunContainerService:master Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is bug report for project LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] should also check exited state when remove container
5 participants