Transfer error to ErrNotFound when kill a not exist container by payall4u · Pull Request #4214 · containerd/containerd

payall4u · 2020-04-27T03:04:01Z

From #4020 #4019
We encountered the same problem after upgrading containerd from v1.2.7 to v1.3.3 .Same as pr #4121.

When use cri-containerd and k8s, pod may be stuck in terminating. It happens when container is delete by runc, however containerd unawares of this or deletes container failed.When cri retry to delete container it will got a runc error container does not exist. This is because cri ignore IsNotFound error, but shim is not converting the runc error to a containerd NotFound error.

theopenlab-ci · 2020-04-27T03:12:41Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 8m 06s (non-voting)

fuweid

Please use git commit -s --amend to sign off.

theopenlab-ci · 2020-04-27T10:04:29Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 45s (non-voting)

theopenlab-ci · 2020-04-30T08:14:57Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 28s (non-voting)

payall4u · 2020-04-30T08:32:11Z

runc completely correct, but crun error:

##[error] container_test.go:696: expected error "not found" but received "unknown error after kill: runc did not terminate successfully: error opening file '/tmp/runc-test/testing/TestKillContainerDeletedByRunc/status': No such file or directory\n: unknown"

I think it may need to add an adapter for handling runtime error.

cpuguy83 · 2020-04-30T17:53:08Z

It would be nice if the shim translated this error.

codecov-io · 2020-05-11T06:49:13Z

Codecov Report

Merging #4214 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4214   +/-   ##
=======================================
  Coverage   38.34%   38.34%           
=======================================
  Files          90       90           
  Lines       12728    12728           
=======================================
  Hits         4881     4881           
  Misses       7181     7181           
  Partials      666      666

Flag	Coverage Δ
#windows	`38.34% <ø> (ø)`

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 83084c9...bbfa6fa. Read the comment docs.

theopenlab-ci · 2020-05-11T06:50:23Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 8m 29s (non-voting)

theopenlab-ci · 2020-05-11T07:05:05Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 10s (non-voting)

theopenlab-ci · 2020-05-11T09:44:08Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 9m 05s (non-voting)

theopenlab-ci · 2020-05-11T09:59:18Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 12s (non-voting)

theopenlab-ci · 2020-05-11T10:59:02Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 14s (non-voting)

fuweid

The change is LGTM basically.
Just need to add some comment and adjust error message

fuweid · 2020-05-11T13:46:20Z

could you mind to add More information in https://github.com/containerd/containerd/pull/4214#discussion_r422769497 in other line? thanks

OK, and should I open issue on oci container-spec for runtime exception?

test case. Signed-off-by: payall4u <[email protected]> Add integration test case Signed-off-by: payall4u <[email protected]>

theopenlab-ci · 2020-05-11T14:05:20Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 7m 43s (non-voting)

fuweid

LGTM

@payall4u Thanks for your patience !

estesp

LGTM

dmcgowan · 2020-05-13T02:37:29Z

@payall4u would you mind providing your real name for the author list. Thanks!

payall4u · 2020-05-13T02:59:15Z

@payall4u would you mind providing your real name for the author list. Thanks!

@dmcgowan Zhiyu Li [email protected]. Should I add it into the list?

dmcgowan · 2020-05-13T03:01:57Z

Thanks, I'll take care of it

samba · 2020-07-02T23:36:44Z

Are there any plans to backport this, to fix the problem in 1.3.x as well?

fuweid requested changes Apr 27, 2020

View reviewed changes

payall4u force-pushed the bugfix-check-not-exist branch from 9a311ca to f37377e Compare April 27, 2020 09:56

fuweid reviewed Apr 27, 2020

View reviewed changes

Comment thread pkg/process/utils.go Outdated

BenTheElder mentioned this pull request Apr 27, 2020

upgrade to containerd 1.3.4 kubernetes-sigs/kind#1511

Closed

payall4u force-pushed the bugfix-check-not-exist branch from f37377e to 16ef4d9 Compare April 30, 2020 08:06

fuweid reviewed May 11, 2020

View reviewed changes

Comment thread container_test.go Outdated

Comment thread container_test.go Outdated

payall4u force-pushed the bugfix-check-not-exist branch 2 times, most recently from 20edf5f to cc54d8a Compare May 11, 2020 06:41

payall4u force-pushed the bugfix-check-not-exist branch from cc54d8a to 96a356c Compare May 11, 2020 06:56

fuweid reviewed May 11, 2020

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

payall4u force-pushed the bugfix-check-not-exist branch from 96a356c to 09b93eb Compare May 11, 2020 09:34

payall4u force-pushed the bugfix-check-not-exist branch from 09b93eb to 831bd2d Compare May 11, 2020 09:51

payall4u force-pushed the bugfix-check-not-exist branch from 831bd2d to bbfa6fa Compare May 11, 2020 10:51

fuweid reviewed May 11, 2020

View reviewed changes

Transfer error to ErrNotFound when kill a not exist container, also add

b437938

test case. Signed-off-by: payall4u <[email protected]> Add integration test case Signed-off-by: payall4u <[email protected]>

payall4u force-pushed the bugfix-check-not-exist branch from bbfa6fa to b437938 Compare May 11, 2020 13:56

fuweid approved these changes May 11, 2020

View reviewed changes

estesp approved these changes May 11, 2020

View reviewed changes

estesp merged commit 0498503 into containerd:master May 11, 2020

fuweid mentioned this pull request May 11, 2020

Pod is stuck in terminating due to containerd-shim unmount error. #4020

Closed

fuweid mentioned this pull request Jun 15, 2020

Pod is stuck in terminating: runc did not terminate sucessfully: container does not exist #4016

Closed

joejulian mentioned this pull request Jul 2, 2020

Backport: runc container does not exist fix #4359

Closed

joejulian mentioned this pull request Jul 15, 2020

[release/1.3] backport: Transfer error to ErrNotFound when kill a not exist container, also add test case. #4361

Merged

This was referenced Nov 10, 2020

[supervisor] Ensure timely shutdown gitpod-io/gitpod#2154

Merged

[ws-daemon] Add workaround for containerd ErrNotFound issue gitpod-io/gitpod#2156

Merged

csweichel mentioned this pull request Mar 26, 2021

[ws-manager] Dispose workspace when container termination is not propagated gitpod-io/gitpod#3608

Merged

Conversation

payall4u commented Apr 27, 2020

Uh oh!

theopenlab-ci Bot commented Apr 27, 2020

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci Bot commented Apr 27, 2020

Uh oh!

Uh oh!

theopenlab-ci Bot commented Apr 30, 2020

Uh oh!

payall4u commented Apr 30, 2020

Uh oh!

cpuguy83 commented Apr 30, 2020

Uh oh!

Uh oh!

Uh oh!

codecov-io commented May 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

theopenlab-ci Bot commented May 11, 2020

Uh oh!

theopenlab-ci Bot commented May 11, 2020

Uh oh!

Uh oh!

theopenlab-ci Bot commented May 11, 2020

Uh oh!

theopenlab-ci Bot commented May 11, 2020

Uh oh!

theopenlab-ci Bot commented May 11, 2020

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fuweid May 11, 2020

Choose a reason for hiding this comment

Uh oh!

payall4u May 11, 2020

Choose a reason for hiding this comment

Uh oh!

fuweid May 11, 2020

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci Bot commented May 11, 2020

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

dmcgowan commented May 13, 2020

Uh oh!

payall4u commented May 13, 2020

Uh oh!

dmcgowan commented May 13, 2020

Uh oh!

samba commented Jul 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov-io commented May 11, 2020 •

edited

Loading