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

pillar: containerd: Create a pipe for container's stdin #4234

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

rene
Copy link
Contributor

@rene rene commented Sep 11, 2024

Currently the device /dev/null is used as the standard input for a container task. However, any process reading from /dev/null will always get 0 bytes as the result, which makes interactive containers, e.g., those that runs a shell as the entrypoint (and expects data on the standard input), to exit right after starting. In order to keep the container task blocked (in these situations), a valid standard input device must be provided. This commit creates a fake stdin that reads from an unpopulated pipe so it blocks the reader process.

This issue is not observed on standard containers because qemu's process doesn't expect any inputs on stdin. It only affects native containers.

Currently the device /dev/null is used as the standard input for a
container task. However, any process reading from /dev/null will always get
0 bytes as the result, which makes interactive containers, e.g., those that
runs a shell as the entrypoint (and expects data on the standard input), to
exit right after starting. In order to keep the container task blocked (in
these situations), a valid standard input device must be provided.  This
commit creates a fake stdin that reads from an unpopulated pipe so it
blocks the reader process.

Signed-off-by: Renê de Souza Pinto <rene@renesp.com.br>
@rene rene requested review from christoph-zededa and removed request for deitch and rouming September 11, 2024 08:18
Copy link
Member

@OhmSpectator OhmSpectator left a comment

Choose a reason for hiding this comment

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

Looks fine, although I'm not sure about the use case.

pkg/pillar/containerd/containerd.go Show resolved Hide resolved
@OhmSpectator
Copy link
Member

It looks fine; I just want to see the Eden tests run.
@yash-zededa, could you please check why they are skipped here?

Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

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

And it is easier to read as well.

@OhmSpectator
Copy link
Member

All green!

@OhmSpectator OhmSpectator merged commit 9a29580 into lf-edge:master Sep 11, 2024
54 of 77 checks passed
@rene rene deleted the fix-bm-stdin branch September 16, 2024 12:10
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.

3 participants