search

blazzy / podman-rest-client

MIT License
6 stars 1 forks source link

Exec attachment support #22

Open kanpov opened 2 months ago

kanpov commented 2 months ago

Similarly to bollard (I'm noticing all this since my use case is abstracting between bollard and podman-rest-client), the exec APIs must not simply be direct REST mappings as they (except for when detaching from the exec) hijack the socket stream for communication to the exec via stdout/stderr/stdin. This functionality needs to exist in podman-rest-client, although it'd understandably be less trivial to implement as the library seems to mostly consist of direct (and sometimes unfortunately incorrectly generated) API bindings.

kanpov commented 2 months ago

The big issue with not having this in the library is that if a library user calls exec with detached being false (which is the default), no attachment support is activated and yet the socket is still hijacked, so (as far as I've understood of dockerd and libpod docs) trying to use it for HTTP afterwards will always fail.

blazzy commented 2 months ago

v0.13.0 has experimental support for this. How does this API look?

exec_start_libpod, exec_start, and container_attach_libpod will return something that implements AsyncRead + AsyncWrite. This allows a user to read and write raw bytes to the hijacked connection. Which is fine if you connect with tty: true. This is all the auto generated code provides. Here is a test that starts a shell and interacts with it.

If you're not connecting to a tty though, raw bytes are not good enough. There is a more complicated multiplexed response. That's where AttachFrameStream helper comes in. let mut reader = AttachFrameStream::new(conn); This will produce a stream of AttachFrames. Example test here.

kanpov commented 2 months ago

image Not related to the library, but one strange thing I noticed is that creating and starting the exec both have the TTY property, for some reason. If I already configured a TTY when creating the exec, why do I have to do it again when starting it?

kanpov commented 2 months ago

@blazzy There is an actual issue I encountered, that being that if tty is enabled, awaiting .next() on the stream awaits the end of the entire exec and then returns None, instead of correctly streaming everything in which happens just fine when tty is disabled (default).

blazzy commented 2 months ago

Not related to the library, but one strange thing I noticed is that creating and starting the exec both have the TTY property, for some reason. If I already configured a TTY when creating the exec, why do I have to do it again when starting it?

Yeah. I don't understand the tty flag on exec either. 😕

@blazzy There is an actual issue I encountered, that being that if tty is enabled, awaiting .next() on the stream awaits the end of the entire exec and then returns None, instead of correctly streaming everything in which happens just fine when tty is disabled (default).

The AttachFrame stream doesn't work with the tty enabled. The tokio_util crate with the io flag provides tokio_util::io::ReaderStream that can be used to stream all the raw bytes. The library should document that better if this works. I'm not sure if it should export something like ReaderStream for convenience.

kanpov commented 2 months ago

The AttachFrame stream doesn't work with the tty enabled. The tokio_util crate with the io flag provides tokio_util::io::ReaderStream that can be used to stream all the raw bytes. The library should document that better if this works. I'm not sure if it should export something like ReaderStream for convenience.

Bollard works on both tty and non-tty without any changes needed, so I (or you if you have the time and will to do so) should probably look at how they do it.