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: avoid task exit channel exit unexpected #1765

Merged
merged 1 commit into from
Jul 26, 2018
Merged

bugfix: avoid task exit channel exit unexpected #1765

merged 1 commit into from
Jul 26, 2018

Conversation

Ace-Tang
Copy link
Contributor

@Ace-Tang Ace-Tang commented Jul 22, 2018

when containerd stop unexpected, we should judge exit time is zero,
zero exit time means grpc connection is broken.

Signed-off-by: Ace-Tang aceapril@126.com

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

step to verify the patch

  1. start a pouchd
  2. create some containers
  3. kill containerd, then you should not see log like
INFO[2018-07-23T21:18:54.972425728+08:00] the task has quit, id: 0bf3ba4b31438679c7905466afe15699fe85eb4dfac3e74e3b7ee4e0f823a99e, err: rpc error: code = Internal desc = transport is closing, exitcode: 255, time: 0001-01-01 00:00:00 +0000 UTC 

Ⅴ. Special notes for reviews

when containerd stop unexpected, we should judge exit time is zero,
zero exit time means grpc connection is broken.

Signed-off-by: Ace-Tang <aceapril@126.com>
@pouchrobot pouchrobot added kind/bug This is bug report for project size/XS labels Jul 22, 2018
@codecov-io
Copy link

Codecov Report

Merging #1765 into master will increase coverage by 0.06%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1765      +/-   ##
==========================================
+ Coverage   56.32%   56.38%   +0.06%     
==========================================
  Files         200      200              
  Lines       15657    15659       +2     
==========================================
+ Hits         8819     8830      +11     
+ Misses       5742     5736       -6     
+ Partials     1096     1093       -3
Flag Coverage Δ
#critest 33.54% <0%> (+0.06%) ⬆️
#integrationtest 37.69% <0%> (ø) ⬆️
#unittest 20.32% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
ctrd/watch.go 75.75% <0%> (+0.75%) ⬆️
daemon/mgr/container.go 53.3% <0%> (ø) ⬆️
cri/v1alpha2/cri.go 66.13% <0%> (+0.17%) ⬆️
ctrd/container.go 50.17% <0%> (+1.37%) ⬆️
ctrd/image.go 81.18% <0%> (+1.98%) ⬆️

@Ace-Tang
Copy link
Contributor Author

PTAL, @HusterWan

// isContainerdDead only take effect when contained stop normal, if containerd
// stop unexpected, judge exit time is zero, zero exit time means grpc connection
// is broken.
if status.ExitTime().IsZero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

should also create a pr in gitlab?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, if you have look at it.

@HusterWan
Copy link
Contributor

lgtm

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jul 26, 2018
@HusterWan HusterWan merged commit e8179b3 into AliyunContainerService:master Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
containerd-related 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.

5 participants