SPDY -> WebSocket change

[Mikhail Mazurskiy]

Hello,

I'd like to change kubectl commands that use SPDY today to use WebSocket. I have a draft PR for discussion here [1]. There are some protocol issues [2] that I encountered that I'd like to discuss and come up with a plan to address. Please see the PR and issue for details.

Cheers,

Mikhail.

[1]: https://github.com/kubernetes/kubernetes/pull/110142

[2]: https://github.com/kubernetes/kubernetes/issues/89899

[Monis Khan]

I would expect to see a KEP that describes the proposed changes.

--
You received this message because you are subscribed to the Google Groups "K8s API Machinery SIG" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-api-m...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/790f78e6-4a59-429f-9e8c-88ec498afe79n%40googlegroups.com.

[David Eads]

In concept, I'd be happy to see spdy retired.  I agree that a KEP outlining the change could help show how small it is.  I think our server already makes this possible.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/CALrOjAC%2BVBY2S6c5S%2BLKdJYi6H9QWCTrFMVs1AjN1dHsPpY9rA%40mail.gmail.com.

[Maciej Szulik]

There's also prior work about it in https://github.com/kubernetes/kubernetes/pull/107125

although currently outdated and abandoned, it seems.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/CAFS1MjLuwBjVfR1hKkMLPNWqNkcLQa3x2qoJdfH5gbAYDni9hw%40mail.gmail.com.

[Daniel Smith]

I'm happy to see SPDY replaced with websocket calls. If you need to make protocol level changes (which it seems you do), I'd expect a KEP to be modified or created.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/CALFFA0Og-rakoTkKMHA0hstwuC4_D%3DKRYQY_0%3DOF8NQ9QaGChQ%40mail.gmail.com.

[Karl Isenberg]

Perhaps a better topic for a KEP, but why WebSocket instead of HTTP2?

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/CAB_J3bbvw_mDtWP6%2By%2B50ZSBGEmDR3AXp5X4urnBKzZ4Q7B9Zg%40mail.gmail.com.

[Daniel Smith]

We already use HTTP2 for regular calls.

Websocket plays nice with JS / browsers and weird uses of HTTP2 don't at all.

[Antonio Ojea]

HTTP/2 doesn't expose Streams, and doesn't seem it will do it https://github.com/golang/go/issues/26574

 so you'll need to add a lot of plumbery to be able to use streams with HTTP2, using io.Pipe on the request and response body, there are some PRs with POCs from Mike Danese here https://github.com/kubernetes/kubernetes/issues/7452#issuecomment-439596601

or accessing the http2 framer, like the SPDY implementation is doing, similar to what this project is doing https://github.com/cloudflare/cloudflared/blob/12302ba1bf741af0f607b6031923a31cd44d2ac5/connection/h2mux.go#L63

But those approaches sound like more of the same, after a few years we'll be tied to a custom code that nobody will remember how it works :)

However, I think that the problem is not about the protocol itself, but about how this is implemented, these are my notes, feel free to correct me if I'm wrong:

client - SPDY-- apiserver (proxy) ---- SPDY---- kubelet (proxy) ---- GRPC handshake then SPDY----- (CRI containerd/crio/..)

since the dockershim deprecation, kubelet uses GRPC to communicate with the Container Runtime ... and it receives a "streaming URL" to forward the connection

https://github.com/kubernetes/cri-api/blob/1c90a6f7abe8f5b7331c95960a5106c9876a9e4f/pkg/apis/runtime/v1/api.pb.go#L5729-L5749

 // PortForward prepares a streaming endpoint to forward ports from a PodSandbox, and returns the address.
PortForward(*runtimeapi.PortForwardRequest) (*runtimeapi.PortForwardResponse, error)

 // Exec prepares a streaming endpoint to execute a command in the container, and returns the address.
Exec(*runtimeapi.ExecRequest) (*runtimeapi.ExecResponse, error)
// Attach prepares a streaming endpoint to attach to a running container, and returns the address.
Attach(req *runtimeapi.AttachRequest) (*runtimeapi.AttachResponse, error)

This situation forces the runtimes to implement the streaming server and carry all the websockets and SPDY logic (I think Clayton already implemented websockets from kubelet to apiserver, there is websocket code in the CRI and kubelet streaming server, but I may be wrong)

Since the kubelet to runtime communication already uses GRPC (HTTP2 under the hood) and GRPC already has streaming, should it be simpler to implement 3 new CRI commands PortForwardStream(), ExecStream(), AttachStream() that returns a stream instead of an URL, so we can break this dependency on SPDY maintaining backwards compatibility? (we need to do the GRPC to WS later too , but we start from a clean environment)

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/CAB_J3baXq%3DbohCGPu6wZYN%2BoWoyzsmJmDGZ9VfTnX4ZLCmRLdw%40mail.gmail.com.

[Antonio Ojea]

On Tue, 7 Jun 2022 at 08:06, Mikhail Mazurskiy <mmazu...@gitlab.com> wrote:

On Thu, Jun 2, 2022 at 8:30 AM Antonio Ojea <antonio.o...@gmail.com> wrote:

But those approaches sound like more of the same, after a few years we'll be tied to a custom code that nobody will remember how it works :)

However, I think that the problem is not about the protocol itself, but about how this is implemented, these are my notes, feel free to correct me if I'm wrong:

client - SPDY-- apiserver (proxy) ---- SPDY---- kubelet (proxy) ---- GRPC handshake then SPDY----- (CRI containerd/crio/..)

since the dockershim deprecation, kubelet uses GRPC to communicate with the Container Runtime ... and it receives a "streaming URL" to forward the connection

https://github.com/kubernetes/cri-api/blob/1c90a6f7abe8f5b7331c95960a5106c9876a9e4f/pkg/apis/runtime/v1/api.pb.go#L5729-L5749

 // PortForward prepares a streaming endpoint to forward ports from a PodSandbox, and returns the address.
PortForward(*runtimeapi.PortForwardRequest) (*runtimeapi.PortForwardResponse, error)

 // Exec prepares a streaming endpoint to execute a command in the container, and returns the address.
Exec(*runtimeapi.ExecRequest) (*runtimeapi.ExecResponse, error)
// Attach prepares a streaming endpoint to attach to a running container, and returns the address.
Attach(req *runtimeapi.AttachRequest) (*runtimeapi.AttachResponse, error)

This situation forces the runtimes to implement the streaming server and carry all the websockets and SPDY logic (I think Clayton already implemented websockets from kubelet to apiserver, there is websocket code in the CRI and kubelet streaming server, but I may be wrong)

Since the kubelet to runtime communication already uses GRPC (HTTP2 under the hood) and GRPC already has streaming, should it be simpler to implement 3 new CRI commands PortForwardStream(), ExecStream(), AttachStream() that returns a stream instead of an URL, so we can break this dependency on SPDY maintaining backwards compatibility? (we need to do the GRPC to WS later too , but we start from a clean environment)

I agree with this. At the same time, as noted above, WebSocket-based protocol will still be useful to other languages and platforms, such as browsers. The refactoring you are suggesting still should be done in my opinion, but it's a "backend" change. API server should still expose WebSocket protocol on top of that (at least removing it is a separate story, if we ever would want to do it).

I'm happy to prepare a KEP but what the scope should be? WebSocket protocol change only? SPDY removal from the API? Or complete SPDY removal? I'm happy to help but I don't have the bandwidth to boil the ocean. I want to solve the problem I have - some kubectl commands don't work via Google Cloud Load Balancer because of SPDY. This is not just GCP LB, other proxies and LBs are too not passing SPDY through, based on the issue. So it's not just my personal problem.

if you have to keep backwards compatibility, how can you remove SPDY without breaking old clients?

I was not suggesting a refactoring, on the opposite, I was suggesting a kind of new V2 API where you can use websockets directly instead of SPDY, and let SPDY die slowly using the old API

[Antonio Ojea]

On Tue, 7 Jun 2022 at 11:16, Mikhail Mazurskiy <mmazu...@gitlab.com> wrote:

You cannot remove something without breaking the users, obviously. But Kubernetes does have a deprecation and removal policy for APIs/versions. I thought we could use the same approach? That's why I'm asking what goal should be in the KEP.

 

I was not suggesting a refactoring, on the opposite, I was suggesting a kind of new V2 API where you can use websockets directly instead of SPDY, and let SPDY die slowly using the old API

Could you elaborate more on what you are suggesting in terms of client-facing WebSocket API? Aren't the clients using WebSocket directly today?

The only things that are missing from the current WebSocket API are i) a way to half-close a "stream" and ii) a way to reset a stream. I think these can be implemented in a way that is trivial for existing clients/libraries to migrate to.

ah, sorry, I was assuming that the problem was that you couldn't implement the " i) a way to half-close a "stream" and ii) a way to reset a stream" on current httpstream.Stream, so I was thinking that if you get the stream directly in the apiserver replacing current plumbery it will be easier to "plug" it into a websocket so users can consume it

[Antonio Ojea]

actually, scratch my comment, you still need to implement the protocol for websockets ... 

I think that is more my desire to simplify/remove the internal streaming logic, Sorry :)

[Mikhail Mazurskiy]

On Tue, Jun 7, 2022 at 4:30 PM Antonio Ojea <antonio.o...@gmail.com> wrote:

You cannot remove something without breaking the users, obviously. But Kubernetes does have a deprecation and removal policy for APIs/versions. I thought we could use the same approach? That's why I'm asking what goal should be in the KEP.

 

I was not suggesting a refactoring, on the opposite, I was suggesting a kind of new V2 API where you can use websockets directly instead of SPDY, and let SPDY die slowly using the old API

Could you elaborate more on what you are suggesting in terms of client-facing WebSocket API? Aren't the clients using WebSocket directly today?

The only things that are missing from the current WebSocket API are i) a way to half-close a "stream" and ii) a way to reset a stream. I think these can be implemented in a way that is trivial for existing clients/libraries to migrate to.

--

Mikhail Mazurskiy
Staff Backend Engineer, Configure

[Mikhail Mazurskiy]

On Thu, Jun 2, 2022 at 8:30 AM Antonio Ojea <antonio.o...@gmail.com> wrote:

But those approaches sound like more of the same, after a few years we'll be tied to a custom code that nobody will remember how it works :)

However, I think that the problem is not about the protocol itself, but about how this is implemented, these are my notes, feel free to correct me if I'm wrong:

client - SPDY-- apiserver (proxy) ---- SPDY---- kubelet (proxy) ---- GRPC handshake then SPDY----- (CRI containerd/crio/..)

since the dockershim deprecation, kubelet uses GRPC to communicate with the Container Runtime ... and it receives a "streaming URL" to forward the connection

https://github.com/kubernetes/cri-api/blob/1c90a6f7abe8f5b7331c95960a5106c9876a9e4f/pkg/apis/runtime/v1/api.pb.go#L5729-L5749

 // PortForward prepares a streaming endpoint to forward ports from a PodSandbox, and returns the address.
PortForward(*runtimeapi.PortForwardRequest) (*runtimeapi.PortForwardResponse, error)

 // Exec prepares a streaming endpoint to execute a command in the container, and returns the address.
Exec(*runtimeapi.ExecRequest) (*runtimeapi.ExecResponse, error)
// Attach prepares a streaming endpoint to attach to a running container, and returns the address.
Attach(req *runtimeapi.AttachRequest) (*runtimeapi.AttachResponse, error)

This situation forces the runtimes to implement the streaming server and carry all the websockets and SPDY logic (I think Clayton already implemented websockets from kubelet to apiserver, there is websocket code in the CRI and kubelet streaming server, but I may be wrong)

Since the kubelet to runtime communication already uses GRPC (HTTP2 under the hood) and GRPC already has streaming, should it be simpler to implement 3 new CRI commands PortForwardStream(), ExecStream(), AttachStream() that returns a stream instead of an URL, so we can break this dependency on SPDY maintaining backwards compatibility? (we need to do the GRPC to WS later too , but we start from a clean environment)

I agree with this. At the same time, as noted above, WebSocket-based protocol will still be useful to other languages and platforms, such as browsers. The refactoring you are suggesting still should be done in my opinion, but it's a "backend" change. API server should still expose WebSocket protocol on top of that (at least removing it is a separate story, if we ever would want to do it).

I'm happy to prepare a KEP but what the scope should be? WebSocket protocol change only? SPDY removal from the API? Or complete SPDY removal? I'm happy to help but I don't have the bandwidth to boil the ocean. I want to solve the problem I have - some kubectl commands don't work via Google Cloud Load Balancer because of SPDY. This is not just GCP LB, other proxies and LBs are too not passing SPDY through, based on the issue. So it's not just my personal problem.

[Mikhail Mazurskiy]

KEP issue: https://github.com/kubernetes/enhancements/issues/3396

KEP proposal: https://github.com/kubernetes/enhancements/pull/3401

The document is not complete, I need some help to flesh it out. Thank you!