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

feature: finish restart interface #944

Conversation

HusterWan
Copy link
Contributor

Signed-off-by: Michael Wan zirenwan@gmail.com

Ⅰ. Describe what this PR did

finish restart interface daemon part

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@HusterWan
Copy link
Contributor Author

HusterWan commented Mar 22, 2018

there is a problem: If start container too quickly after stop it, the snapshot of the container will be deleted by the containerd gc. this pr should not be merged utill i fix the problem

@codecov-io
Copy link

codecov-io commented Mar 22, 2018

Codecov Report

Merging #944 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #944      +/-   ##
==========================================
- Coverage   13.16%   13.12%   -0.04%     
==========================================
  Files         123      123              
  Lines        8175     8197      +22     
==========================================
  Hits         1076     1076              
- Misses       7003     7025      +22     
  Partials       96       96
Impacted Files Coverage Δ
cli/restart.go 0% <0%> (ø) ⬆️
daemon/mgr/container.go 2.25% <0%> (-0.06%) ⬇️
ctrd/container.go 0% <0%> (ø) ⬆️
client/container.go 0% <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 ecdb925...59fa836. Read the comment docs.

@HusterWan HusterWan changed the title [WIP]feature: finish restart interface feature: finish restart interface Mar 23, 2018
@HusterWan
Copy link
Contributor Author

HusterWan commented Mar 23, 2018

there is a problem: If start container too quickly after stop it, the snapshot of the container will be deleted by the containerd gc. this pr should not be merged utill i fix the problem

i have figured it out: when stopping a container we should add lease in ctx to prevent the snapshot being deleted by gc.

Signed-off-by: Michael Wan <zirenwan@gmail.com>
if err := mgr.stop(ctx, c, timeout); err != nil {
return errors.Wrapf(err, "failed to stop container")
}
logrus.Debug("Restart: container " + c.ID() + " stopped succeeded")
Copy link
Collaborator

Choose a reason for hiding this comment

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

we usually use logrus.Debugf() here.

@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Mar 23, 2018
@allencloud allencloud merged commit 359ad41 into AliyunContainerService:master Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants