search

containerd / nerdctl

contaiNERD CTL - Docker-compatible CLI for containerd, with support for Compose, Rootless, eStargz, OCIcrypt, IPFS, ...
Apache License 2.0
7.62k stars 565 forks source link

[DRAFT] Add support for start -i (help required) #3104

Open apostasie opened 1 week ago

apostasie commented 1 week ago

Hey folks.

This is to tentatively fix #3103

I am slightly out of my comfort zone here and can use some advice and thorough review here, especially on the combination behavior of --attach, --stdin and what that means for detaching or not.

Sending as draft for now for that reason.

apostasie commented 1 week ago

CI is green.

@AkihiroSuda and other good folks here, PTAL at your convenience and lmk if I got this one right.

AkihiroSuda commented 1 week ago

Can we have an integration test to verify this?

apostasie commented 1 week ago

Can we have an integration test to verify this?

Of course. Coming later today or tomorrow.

apostasie commented 1 week ago

@AkihiroSuda test added.

New test is green locally, but let's wait for the CI.

PTAL at your convenience.

apostasie commented 1 week ago

@AkihiroSuda this is a lot more complex than I thought.

The combination matrix of create [--tty] [--interactive] and start [--interactive] [--attach] has significant subtleties and console juggling that requires more research on my side.

I am converting this back to draft for now.

Note: docker behavior is not clear either IMHO - specifically the case of create -t + start -i

apostasie commented 1 week ago

@AkihiroSuda I don't think I can put more work in this in time for v2.

Feel free to remove it from the milestone if you prefer.