Exec can return stdout data even after stdin is closed.

[esw-amzn]

#1689

This change allows stdout data to be read even after stdin has been closed for clusters that support the v5.channel.k8s.io protocol.

Motivation

This will allow clients to implement actions similar to this.

echo hello world | kubectl  exec --stdin my_pod -- sh -c "sleep 4; echo done"

Solution

This works by setting the protocol header to allow using the v5.channel.k8s.io protocol. When v5.channel.k8s.io is used, if stdin is closed then the client will now send the correct message indicating it has been closed.

These protocols are also used for portforward and it looks like portforward only supports v4.channel.k8s.io. So this change includes some logic to allow for different protocols when exec and portforward are used.

Testing

Ran these unit test commands.

cargo test -p kube-client --all-features
cargo test -p kube-client --all-features pod_can_exec_and_write_to_stdin -- --ignored
cargo test -p kube-client --doc

Ran these related examples.

cargo run --example pod_exec
cargo run --example pod_shell
cargo run --example pod_shell_crossterm
cargo run --example pod_cp
cargo run --example pod_attach
cargo run --example pod_portforward
cargo run --example pod_portforward_bind
cargo run --example pod_portforward_hyper_http

[clux]

Thanks a lot for this. I think this looks largely sensible. Very clean change.

Have only put in one question and one small optional interface nit. Sorry for the delay.
Happy to merge this with clarity and green CI.

[clux]

how supported is this, do you know? from browsing from the SPDY blog post in https://kubernetes.io/blog/2024/08/20/websockets-transition/ it mentions 1.31

the KEP mentions 1.33 as stable: kubernetes/enhancements#4006

are you testing this with any feature flags?

[esw-amzn]

I retested this with k3d with K8s version 1.27 to 1.32. This only works with K8s >= 1.30. Second revision includes a condition in the test so that this part of the test only runs when stream closing is supported.

[clux]

Perfect thank you. I'll leave this unresolved so that I remember this for the release notes.

[clux]

Some of the stdin test do seem to fail with integration tests;

test test::pod_can_exec_and_write_to_stdin_from_node_proxy ... FAILED
test test::pod_can_exec_and_write_to_stdin ... FAILED

the second one there being a standard ws only one so it would be good to understand if that requires any changes, or if there's something legit there.

[esw-amzn]

Thanks for the feedback! This revision should hopefully pass all the tests.

[codecov]

Codecov Report

Attention: Patch coverage is 66.27907% with 29 lines in your changes missing coverage. Please review.

Project coverage is 75.8%. Comparing base (8876639) to head (864af30).

Files with missing lines	Patch %	Lines
kube-client/src/lib.rs	36.9%	12 Missing ⚠️
kube-client/src/api/remote_command.rs	46.7%	8 Missing ⚠️
kube-client/src/client/upgrade.rs	80.6%	7 Missing ⚠️
kube-client/src/api/subresource.rs	0.0%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1693     +/-   ##
=======================================
- Coverage   75.9%   75.8%   -0.0%     
=======================================
  Files         84      84             
  Lines       7685    7745     +60     
=======================================
+ Hits        5827    5866     +39     
- Misses      1858    1879     +21     

Files with missing lines	Coverage Δ
kube-client/src/client/kubelet_debug.rs	55.6% <ø> (ø)
kube-client/src/client/mod.rs	76.3% <100.0%> (+0.6%)	⬆️
kube-client/src/api/subresource.rs	52.7% <0.0%> (ø)
kube-client/src/client/upgrade.rs	78.5% <80.6%> (+3.5%)	⬆️
kube-client/src/api/remote_command.rs	59.5% <46.7%> (-0.5%)	⬇️
kube-client/src/lib.rs	90.6% <36.9%> (-3.5%)	⬇️

[clux]

Thank you very much. This looks very sensible now. Rustfmt is red and clippy is complaining a little bit, but nothing major, but if you have time it's appreciated. No need to force push.

(Also if you have time to write a few lines to describe the breaking changes, it'll also make my time easier releasing this later.)

[clux]

just for checking my understanding; the idea here is that we send a comma separated list of protocols we support, and the far end gives us the most recent one they can handle, which we can inspect viasupports_stream_close later for feature selection?

[esw-amzn]

Yes, that is correct (or at least correct as far as my understanding goes).

I found this doc which explains how the protocol header is intended to operate. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-WebSocket-Protocol

[esw-amzn]

The only part of this that is a breaking change is the Client.connect function. It now returns the new Connection struct. The Connection struct can be converted into the old return type by calling Connection's into_stream() function.

[esw-amzn]

I'm not sure why the test failed. Here is what happens for me.

# Setup 1.28 cluster
$ k3d cluster create main --servers 1 --registry-create main --image rancher/k3s:v1.28.15-k3s1 \
    -p 10250:10250 --no-rollback \
    --k3s-arg "--disable=traefik,servicelb,metrics-server@server:*" \
    --k3s-arg '--kubelet-arg=eviction-hard=imagefs.available<1%,nodefs.available<1%@agent:*' \
    --k3s-arg '--kubelet-arg=eviction-minimum-reclaim=imagefs.available=1%,nodefs.available=1%@agent:*' \
    --k3s-arg '--kube-apiserver-arg=feature-gates=WatchList=true@server:*'
...

# Check version
$ kubectl version
Client Version: v1.31.1
Kustomize Version: v5.4.2
Server Version: v1.28.15+k3s1
WARNING: version difference between client (1.31) and server (1.28) exceeds the supported minor version skew of +/-1

# Run tests
$ cargo test --lib --workspace --exclude e2e --all-features -j6 -- --ignored
...
     Running unittests src/lib.rs (target/debug/deps/kube-a90ce5fede1995b2)

running 6 tests
test test::custom_resource_generates_correct_core_structs ... ok
test test::api_get_opt_handles_404 ... ok
test test::custom_serialized_objects_are_queryable_and_iterable ... ok
test test::derived_resource_queriable_and_has_subresources ... ok
test test::derived_resources_discoverable ... ok
test test::pod_can_await_conditions ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 8.57s

     Running unittests src/lib.rs (target/debug/deps/kube_client-397062828ab0569d)

running 18 tests
test client::auth::test::exec_auth_command ... ok
test client::client_ext::test::fetch_fails ... ok
test test::group_discovery_oneshot ... ok
test test::custom_client_openssl_tls_configuration ... ok
test api::util::test::create_token_request ... ok
test test::csr_can_be_approved ... ok
test api::entry::tests::entry_update_existing_object ... ok
test client::client_ext::test::client_ext_fetch_ref_pods_svcs ... ok
test test::can_operate_on_pod_metadata ... ok
test api::entry::tests::entry_create_missing_object ... ok
test client::client_ext::test::client_ext_list_get_pods_svcs ... ok
test api::util::test::node_cordon_and_uncordon_works ... ok
test test::can_operate_on_ephemeral_containers ... ok
test api::entry::tests::entry_create_dry_run ... ok
test test::pod_can_use_core_apis ... ok
test test::pod_can_exec_and_write_to_stdin_from_node_proxy ... ok
test test::pod_can_exec_and_write_to_stdin ... ok
test test::can_get_pod_logs_and_evict ... ok

test result: ok. 18 passed; 0 failed; 0 ignored; 0 measured; 33 filtered out; finished in 3.08s

     Running unittests src/lib.rs (target/debug/deps/kube_core-6726d09c868e6ba2)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 82 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/kube_derive-78dd92a520505743)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/kube_runtime-570e8ac4e792a44a)

running 3 tests
test events::test::event_recorder_cache_retain ... ok
test events::test::event_recorder_attaches_events ... ok
test events::test::event_recorder_attaches_events_without_namespace ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 49 filtered out; finished in 0.04s

[clux]

hm. yeah. beats me. I'll give it a re-run.