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

containerd/console is broken for mac consider creack/pty #1449

Open
1 task
TerryHowe opened this issue Jul 18, 2024 · 7 comments
Open
1 task

containerd/console is broken for mac consider creack/pty #1449

TerryHowe opened this issue Jul 18, 2024 · 7 comments
Labels
enhancement New feature or request test General testing related issues or pull requests
Milestone

Comments

@TerryHowe
Copy link
Member

What is the version of your ORAS CLI

1.2.0

What would you like to be added?

Working console

Why is this needed for ORAS?

Tests are broken for mac. They will not fix.

containerd/console#79

Are you willing to submit PRs to contribute to this feature?

  • Yes, I am willing to implement it.
@TerryHowe TerryHowe added enhancement New feature or request triage New issues or PRs to be acknowledged by maintainers labels Jul 18, 2024
@FeynmanZhou
Copy link
Member

@TerryHowe How could a macOS user reproduces this issue? I am a macOS user but seems have not encountered this issue before.

@qweeah qweeah added test General testing related issues or pull requests and removed triage New issues or PRs to be acknowledged by maintainers labels Jul 23, 2024
@qweeah
Copy link
Contributor

qweeah commented Jul 23, 2024

This should only impact unit tests run on macOS. IMHO since there is no automated CI run on macOS, we can simply exclude macOS from PTY-related tests. Refactoring test infra to creack/pty costs extra effort and most importantly it's managed by individual not CNCF.

@TerryHowe
Copy link
Member Author

Yes, as far as I know only the unit tests are broken, but the way I read this containerd console was not really being supported like it is pretty much a dead project.

Something needs to be done at least to minimize the impact of the broken tests. If one class wrapped the console in a way that could be mocked, we could run cp_test.go on mac.

@qweeah
Copy link
Contributor

qweeah commented Jul 24, 2024

I read this containerd console was not really being supported like it is pretty much a dead project.

Originally choosing containerd/console since it's a CNCF project and reached 1.0. The bugs related to ORAS (e.g. containerd/console#79 and containerd/console#83) receives no support because they are all provider specific bugs. I guess OS experts from Apple and Microsoft should fix those issues but there isn't any.

Something needs to be done at least to minimize the impact of the broken tests.

Again, to me there is no macOS CI for unit test so it's easier to just exclude Darwin out of unit test. We might do need to switch to creack/pty someday but IMHO there isn't enough motivation.

@qweeah qweeah added this to the future milestone Jul 24, 2024
@TerryHowe
Copy link
Member Author

The problem is there are tests up and down (from console_test to cp_test) that are not run on mac because of this.

No need to change containerd/console, just need to isolate the implementation so it can be mocked and only low level tests like console_test will not run on mac.

@slonopotamus
Copy link

As an original reporter of containerd/console#79, I strongly recommend creack/pty (that's what I ended up using in my program). It Just Works. containerd/console is a total SNAFU currently on Darwin + FreeBSD and has a severe Windows issue that might or might not affect you: containerd/console#83

@TerryHowe
Copy link
Member Author

If I can complete #1461 to isolate the separation of concerns, trying out creak/pty would probably be easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request test General testing related issues or pull requests
Projects
None yet
Development

No branches or pull requests

4 participants