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

Fix panic when stdout is not a tty #3300

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Aug 12, 2024

Tentatively fix rootless-containers/usernetes#327 - if that PR unblocks it, my job is done here :).

And some of #3297 is likely fixed as well.

Now, I am pretty sure there are other conditions where we still panic.
I would rather leave this to someone who is more knowledgeable on these areas though (including writing exhaustive tests).

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 13, 2024
@AkihiroSuda AkihiroSuda merged commit 6ba6b3b into containerd:main Aug 13, 2024
19 checks passed
@alitvak69
Copy link

The case I have is using -t in a systemd unit:
/usr/local/bin/nerdctl run -t --rm --name=dns-master-internal --log-driver=journald ...

Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: panic: provided file is not a console
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: goroutine 1 [running]:
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: github.com/containerd/console.Current()
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/go/pkg/mod/github.com/containerd/[email protected]/console.go:80 +0x94
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: github.com/containerd/nerdctl/v2/cmd/nerdctl/container.runAction(0xc0003d1b08, {0xc000264dc0, 0x1, 0x14})
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/work/nerdctl/nerdctl/cmd/nerdctl/container/container_run.go:392 +0x70a
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: github.com/spf13/cobra.(*Command).execute(0xc0003d1b08, {0xc00003e180, 0x14, 0x14})
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:985 +0xaaa
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: github.com/spf13/cobra.(*Command).ExecuteC(0xc0003d1508)
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1117 +0x3ff
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: github.com/spf13/cobra.(*Command).Execute(...)
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1041
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: main.xmain()
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/work/nerdctl/nerdctl/cmd/nerdctl/main.go:144 +0x8f
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: main.main()
Sep 15 01:43:26 sbc21n1-mke dns-master-internal[157367]: /home/runner/work/nerdctl/nerdctl/cmd/nerdctl/main.go:127 +0x13

@apostasie
Copy link
Contributor Author

@alitvak69 this here is a pull request, not an issue.

Also, you do not mention a version, or any other os detail and info, or give a concrete way to reproduce your problem.
Did you try the latest v2-rc with that PR?

Either way, suggesting you open a ticket if you have an issue, with enough info to make it actionnable.

apostasie added a commit to apostasie/nerdctl that referenced this pull request Sep 18, 2024
As outlined in containerd#3433, containerd/console will panic on a call to console.Current().
This patch provides a simple consoleutil wrapper that will return an error instead.

Note that part of containerd#3300 is being reverted, as no longer necessary.

This patch does not try to be "smart" and does not check the status of stdin/out/err otherwise or if it is consistent with user provided flags,
but merely ensures we do not crash.

Signed-off-by: apostasie <[email protected]>
apostasie added a commit to apostasie/nerdctl that referenced this pull request Sep 18, 2024
As outlined in containerd#3433, containerd/console will panic on a call to console.Current().
This patch provides a simple consoleutil wrapper that will return an error instead.

Note that part of containerd#3300 is being reverted, as no longer necessary.

This patch does not try to be "smart" and does not check the status of stdin/out/err otherwise or if it is consistent with user provided flags,
but merely ensures we do not crash.

Signed-off-by: apostasie <[email protected]>
apostasie added a commit to apostasie/nerdctl that referenced this pull request Sep 18, 2024
As outlined in containerd#3433, containerd/console will panic on a call to console.Current().
This patch provides a simple consoleutil wrapper that will return an error instead.

Note that part of containerd#3300 is being reverted, as no longer necessary.

This patch does not try to be "smart" and does not check the status of stdin/out/err otherwise or if it is consistent with user provided flags,
but merely ensures we do not crash.

Signed-off-by: apostasie <[email protected]>
apostasie added a commit to apostasie/nerdctl that referenced this pull request Sep 19, 2024
As outlined in containerd#3433, containerd/console will panic on a call to console.Current().
This patch provides a simple consoleutil wrapper that will return an error instead.

Note that part of containerd#3300 is being reverted, as no longer necessary.

This patch does not try to be "smart" and does not check the status of stdin/out/err otherwise or if it is consistent with user provided flags,
but merely ensures we do not crash.

Signed-off-by: apostasie <[email protected]>
apostasie added a commit to apostasie/nerdctl that referenced this pull request Sep 23, 2024
As outlined in containerd#3433, containerd/console will panic on a call to console.Current().
This patch provides a simple consoleutil wrapper that will return an error instead.

Note that part of containerd#3300 is being reverted, as no longer necessary.

This patch does not try to be "smart" and does not check the status of stdin/out/err otherwise or if it is consistent with user provided flags,
but merely ensures we do not crash.

Signed-off-by: apostasie <[email protected]>
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