Skip to content

runtime/v2: add comment for checkCopyShimLogError#5222

Merged
estesp merged 1 commit intocontainerd:masterfrom
fuweid:follow-up-5174
Mar 18, 2021
Merged

runtime/v2: add comment for checkCopyShimLogError#5222
estesp merged 1 commit intocontainerd:masterfrom
fuweid:follow-up-5174

Conversation

@fuweid
Copy link
Copy Markdown
Member

@fuweid fuweid commented Mar 18, 2021

After #4906, containerd opens fifo in read/write mode in linux platform
The original comment doesn't correct and is removed by #5174.

// original comment

// When using a multi-container shim, the fifo of the 2nd to Nth
// container will not be opened when the ctx is done. This will
// cause an ErrReadClosed that can be ignored.

However, we should add comment for checkCopyShimLogError to mention why
we call checkCopyShimLogError. The checkCopyShimLogError, it is to prevent
the flood of expected error messages after task die and the expected
errors depend on platform.

Signed-off-by: Wei Fu [email protected]

@fuweid fuweid requested review from estesp and mikebrow March 18, 2021 04:58
@fuweid
Copy link
Copy Markdown
Member Author

fuweid commented Mar 18, 2021

/ok-to-test

After containerd#4906, containerd opens fifo in read/write mode in linux platform
The original comment doesn't correct and is removed by containerd#5174.

```
// original comment

// When using a multi-container shim, the fifo of the 2nd to Nth
// container will not be opened when the ctx is done. This will
// cause an ErrReadClosed that can be ignored.
```

However, we should add comment for checkCopyShimLogError to mention why
we call checkCopyShimLogError. The checkCopyShimLogError, it is to prevent
the flood of expected error messages after task die and the expected
errors depend on platform.

Signed-off-by: Wei Fu <[email protected]>
@fuweid fuweid changed the title runtime/v2: add comment for checkCopyShimLogError when reload shim runtime/v2: add comment for checkCopyShimLogError Mar 18, 2021
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 92c1bbb into containerd:master Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants