Skip to content

Fix runc state error handling.#2598

Merged
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:fix-state-error-handling
Aug 31, 2018
Merged

Fix runc state error handling.#2598
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:fix-state-error-handling

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

@Random-Liu Random-Liu commented Aug 30, 2018

The error returned by runtime.State is a composed error fmt.Errorf("%s: %s", err, data). We can't use os.IsNotExist directly on it.

If the runc container is successfully deleted, but rootfs unmount fails here, we won't be able to load the task and retry deletion again, we'll keep getting:

failed to load task for sandbox: OCI runtime state failed: container "6a701add28f27c7e1442fa2c75354c86676ff22f3d9773b4d71f1014100c1205" does not exist: unknown

This is not urgent, because rootfs unmount won't fail in normal cases.

Signed-off-by: Lantao Liu lantaol@google.com

Signed-off-by: Lantao Liu <lantaol@google.com>
@codecov-io
Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2598   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          98       98           
  Lines       10130    10130           
=======================================
  Hits         4463     4463           
  Misses       4945     4945           
  Partials      722      722
Flag Coverage Δ
#linux 47.79% <ø> (ø) ⬆️
#windows 40.97% <ø> (ø) ⬆️

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 7e66292...7a4e080. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@estesp
Copy link
Copy Markdown
Member

estesp commented Aug 31, 2018

What about fixing go-runc code to wrap the error rather than compose a string from the error?

@crosbymichael
Copy link
Copy Markdown
Member

@estesp i think it would be a little more challenging and require some more work to get error codes from runc/parse the output on STDERR and return go level errors.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit a09bad5 into containerd:master Aug 31, 2018
@Random-Liu Random-Liu deleted the fix-state-error-handling branch September 5, 2018 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants