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't stop a paused container #1269

Merged
merged 1 commit into from
May 4, 2018

Conversation

shaloulcy
Copy link
Contributor

can't stop a paused container
after restart pouch, can't unpause a paused container

Signed-off-by: Eric Li lcy041536@gmail.com

Ⅰ. Describe what this PR did

fixes #1268

  • can't stop a paused container
  • after restart pouch, can't unpause a paused container

Ⅱ. Does this pull request fix one issue?

  • when stop the container, if the container is paused, allow it
  • restore a paused container when start the pouchd daemon

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added kind/bug This is bug report for project size/XS labels May 3, 2018
@HusterWan
Copy link
Contributor

@shaloulcy can you add some test case for your bugfix, thanks a lot!

@codecov-io
Copy link

codecov-io commented May 3, 2018

Codecov Report

Merging #1269 into master will increase coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1269      +/-   ##
==========================================
+ Coverage   15.21%   15.26%   +0.04%     
==========================================
  Files         172      172              
  Lines       10734    10699      -35     
==========================================
  Hits         1633     1633              
+ Misses       8980     8946      -34     
+ Partials      121      120       -1
Impacted Files Coverage Δ
daemon/mgr/container.go 0% <0%> (ø) ⬆️
apis/server/container_bridge.go 0% <0%> (ø) ⬆️
daemon/mgr/spec_hook.go 0% <0%> (ø) ⬆️
cli/inspect.go 0% <0%> (ø) ⬆️
cli/run.go 0% <0%> (ø) ⬆️
daemon/mgr/volume.go 0% <0%> (ø) ⬆️
cli/rm.go 0% <0%> (ø) ⬆️
apis/server/volume_bridge.go 0% <0%> (ø) ⬆️
daemon/mgr/container_types.go 25.31% <0%> (ø) ⬆️
... and 2 more

@pouchrobot pouchrobot added size/S and removed size/XS labels May 3, 2018
@shaloulcy shaloulcy force-pushed the stop_container branch 3 times, most recently from c13ecee to 45e24b7 Compare May 4, 2018 02:12
@pouchrobot pouchrobot added size/L and removed size/S labels May 4, 2018
@shaloulcy shaloulcy force-pushed the stop_container branch 2 times, most recently from 769750d to f1dc4ff Compare May 4, 2018 02:29
@@ -695,7 +696,7 @@ func (mgr *ContainerManager) Stop(ctx context.Context, name string, timeout int6
c.Lock()
defer c.Unlock()

if !c.IsRunning() {
if !c.IsRunning() && !c.IsPaused() {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should also check Restart interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done it

@@ -67,3 +68,31 @@ func (suite *APIContainerStopSuite) TestInvalidParam(c *check.C) {
//TODO
// 1. invalid timeout value
}

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding api test case, it will be great to add more test for cli test :)

@shaloulcy shaloulcy force-pushed the stop_container branch 2 times, most recently from e522d01 to 5b307ca Compare May 4, 2018 03:33
can't stop a paused container
after restart pouch, can't unpause a paused container

Signed-off-by: Eric Li <lcy041536@gmail.com>
@HusterWan
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label May 4, 2018
@HusterWan HusterWan merged commit 4d3ac8b into AliyunContainerService:master May 4, 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/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ bug ] can't stop a paused container
4 participants