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.

Sign up for GitHub

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

KEP-4006: PortForward Websockets graduates to Beta #4666

Merged
merged 1 commit into from
Jun 10, 2024
Merged

KEP-4006: PortForward Websockets graduates to Beta #4666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 25 additions & 31 deletions keps/sig-api-machinery/4006-transition-spdy-to-websockets/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ Currently, the bi-directional streaming protocols (either SPDY or WebSockets) ar
initiated from clients, proxied by the API Server and Kubelet, and terminated at
the Container Runtime (e.g. containerd or CRI-O). This enhancement proposes to 1)
modify `kubectl` to request a WebSocket based streaming connection, and to 2) modify
the current API Server proxy to translate the `kubectl` WebSockets data stream to
the current API Server proxy to translate or tunnel the `kubectl` WebSockets data stream to
a SPDY upstream connection. In this way, the cluster components upstream from the
API Server will not initially need to be changed. We intend to extend the communication
path for WebSockets streaming from `kubectl` to Kubelet once the the initial leg
Expand Down Expand Up @@ -317,22 +317,6 @@ is redirected to other API endpoints.

- Mitigation: Upgraded connections are disallowed from redirecting.

- Risk: Overloaded Concurrency
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, since this risk was associated with StreamTranslation, and it was fixed when the design was updated to StreamTunneling.


PortForward subrequests (e.g. `curl http://localhost:8080/index.html` after the connection
upgrade) can occur concurrently over the the upgraded streaming connection, and these
subrequests can be long-lasting. Each of these subrequests creates two streams (an
error stream and a data stream) over the connection, and there are four goroutines spawned
to service this subrequest and its associated streams. After the completion of the
subrequest, the associated resources are reclaimed.

- Mitigation: Throttling the number of concurrent subrequests will limit the
number of concurrent streams and the number of concurrent goroutines on the
API Server. This throttling will ensure the server does not get overloaded.
If we need to the reduce number of concurrent goroutines even further we can
explore goroutine pools so that the number of goroutines will grow sublinearly
with the number of subrequests and streams.

- Risk: Performance

When transitioning from the SPDY streaming protocol to WebSockets, there may be a
Expand Down Expand Up @@ -591,13 +575,16 @@ extending the production code to implement this enhancement.
The following packages (including current test coverage) will be modified to implement
this SDPY to WebSockets migration.

- `k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/portforward`: `2024-05-27` - `86.3%`
- `k8s.io/kubernetes/staging/src/k8s.io/client-go/tools/remotecommand`: `2023-05-31` - `57.3%`
- `k8s.io/kubernetes/staging/src/k8s.io/client-go/transport`: `2023-05-31` - `57.7%`
- `k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/httpstream`: `2023-05-31` - `76.7%`
- `k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/util/proxy`: `2023-05-31` - `59.1%`
- `k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/util/proxy`: `2024-05-27` - `81.5%`
- `k8s.io/kubernetes/staging/src/k8s.io/kubectl/pkg/cmd/attach`: `2023-06-05` - `43.4%`
- `k8s.io/kubernetes/staging/src/k8s.io/kubectl/pkg/cmd/cp`: `2023-06-05` - `66.3%`
- `k8s.io/kubernetes/staging/src/k8s.io/kubectl/pkg/cmd/exec`: `2023-06-05` - `70.0%`
- `k8s.io/kubernetes/staging/src/k8s.io/kubectl/pkg/cmd/portforward`: `2024-05-27` - `74.7%`

An important set of tests for this migration will be **loopback** tests, which exercise the
WebSocket client and the StreamTranslator proxy. These tests create two test servers: a
Expand Down Expand Up @@ -633,8 +620,7 @@ https://storage.googleapis.com/k8s-triage/index.html

-->

No integration tests are planned for alpha. Previously mentioned unit tests and current
e2e tests provide sufficient.
`PortForward: https://github.com/kubernetes/kubernetes/blob/master/test/integration/apiserver/portforward/portforward_test.go`

##### e2e tests

Expand All @@ -650,7 +636,7 @@ We expect no non-infra related flakes in the last month as a GA graduation crite
- `<test>: <link to test coverage>`
-->

While there are already numerous current e2e tests for `kubectl exec, cp, attach`,
While there are already numerous current e2e tests for `kubectl exec, cp, attach, and port-forward`,
we will enhance these tests with the permutations of the feature flags for `kubectl`
and the API Server. We will add e2e test coverage for flags and arguments that are
not already covered for these commands.
Expand Down Expand Up @@ -740,9 +726,9 @@ in back-to-back releases.
`kubectl port-forward` behind the `kubectl` environment variable KUBECTL_PORT_FORWARD_WEBSOCKETS
which is **OFF** by default.
- FallbackDialer is completed and functional behind the `kubectl` environment variable
KUBECTL_PORT_FORWARD which if **OFF** by default. The FallbackDialer executes legacy
KUBECTL_PORT_FORWARD which is **OFF** by default. The FallbackDialer executes legacy
SPDY `port-forward` if the server does not support the new WebSockets functionality.
- PortForward `StreamTranslatorProxy` successfully added and integrated, living
- PortForward `StreamTunnelingProxy` successfully added and integrated, living
behind the API Server feature flag `PortForwardWebsockets` which is **OFF** by default.

#### Beta
Expand All @@ -755,6 +741,13 @@ in back-to-back releases.

##### v1.31 PortForward Subprotocol (port-forward)

- `kubectl port-forward` is behind the `kubectl` environment variable KUBECTL_PORT_FORWARD_WEBSOCKETS
which is **ON** by default.
- FallbackDialer is completed and functional behind the `kubectl` environment variable
KUBECTL_PORT_FORWARD which is **ON** by default. The FallbackDialer executes legacy
SPDY `port-forward` if the server does not support the new WebSockets functionality.
- PortForward `StreamTunnelingProxy` successfully added and integrated, living
behind the API Server feature flag `PortForwardWebsockets` which is **ON** by default.
- Additional `port-forward` unit tests completed and enabled.
- Additional `port-forward` integration tests completed and enabled.
- Additional `port-forward` e2e tests completed and enabled.
Expand Down Expand Up @@ -825,7 +818,7 @@ just as it has for the last several years.
#### PortForward Subprotocol

1. A newer WebSockets enabled `kubectl` communicating with an older API Server that
does not support the newer PortForward `StreamTranslator` proxy.
does not support the newer PortForward `StreamTunneling` proxy.

In this case, the initial upgrade request for `PortForward` WebSockets will
fail, because the `WebSockets` upgrade request `v2.portforward.k8s.io` will be proxied
Expand All @@ -835,19 +828,19 @@ legacy SPDY `v1.portforward.k8s.io`. In this fallback case, the PortForward stre
functionality in this case will work exactly as it has for the last several years.

2. A legacy non-WebSockets enabled `kubectl` communicating with a newer API Server that
supports the newer PortForward `StreamTranslator` proxy.
supports the newer PortForward `StreamTunneling` proxy.

The `kubectl port-forward` will successfully request an upgrade for legacy
`SPDY/PortForward - V1`, just as it has for the last several years.

#### Version Skew within the Control Plane and Nodes

These proposals do not modify intra-cluster version skew behavior. The entire reason
for the current `StreamTranslatorProxy` design is to ensure no modifications
to communication within the Control Plane. The `StreamTranslatorProxy` can update
for the current `StreamTranslatorProxy` and `StreamTunnelingProxy` design is to ensure no modifications
to communication within the Control Plane. The `StreamTranslatorProxy` or `StreamTunnelingProxy` can update
streaming between the client and the API Server, but it is designed to provide legacy
SPDY streaming from the API Server to the other components within the ControlPlane.
Once this `StreamTranslatorProxy` is moved to the kubelet, we will have to address
Once these `StreamTranslatorProxy` and `StreamTunnelingProxy` are moved to the kubelet, we will have to address
the possibility of intra-cluster version skew.

## Production Readiness Review Questionnaire
Expand Down Expand Up @@ -917,10 +910,6 @@ KUBECTL_PORT_FORWARD_WEBSOCKETS environment variable must be set to **ON** for
user unless the `kubectl`/API Server communication is communicating through an
intermediary such as a proxy (which is the whole reason for the feature).

**NOTE** These two sets of feature flags are currently at different maturity levels.
As of v1.30, `RemoteCommand` feature flags are **enabled** by default (Beta), while
`PortFoward` features flags are **disabled** by default (Alpha).

###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)?

<!--
Expand Down Expand Up @@ -959,9 +948,13 @@ https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05
-->

- There will be unit tests for the `kubectl` environment variable KUBECTL_REMOTE_COMMAND_WEBSOCKETS.
- There are unit tests for the `kubectl` environment variable KUBECTL_PORT_FORWARD_WEBSOCKETS.
- There will be unit tests in the API Server which exercise the feature gate within
the `UpgradeAwareProxy`, which conditionally delegates to the `StreamTranslator`
proxy (depending on the feature gate and the upgrade parameters).
- There are unit tests in the API Server which exercise the feature gate within
the `UpgradeAwareProxy`, which conditionally delegates to the `StreamTunneling`
proxy for the PortForward subprotocol.

### Rollout, Upgrade and Rollback Planning

Expand Down Expand Up @@ -1459,6 +1452,7 @@ Major milestones might include:
- RemoteCommand over WebSockets shipped as beta: v1.30
- First Kubernetes release where PortForward over WebSockets described in KEP: v1.30
- PortForward over WebSockets shipped as alpha: v1.30
- PortForward over WebSockets shipped as beta: v1.31

## Drawbacks

Expand Down
6 changes: 3 additions & 3 deletions keps/sig-api-machinery/4006-transition-spdy-to-websockets/kep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ stage: beta
# The most recent milestone for which work toward delivery of this KEP has been
# done. This can be the current (upcoming) milestone, if it is being actively
# worked on.
latest-milestone: "v1.30"
latest-milestone: "v1.31"

# The milestone at which this feature was, or is targeted to be, at each stage.
milestone:
alpha: "v1.29"
beta: "v1.30"
stable: "v1.31"
beta: "v1.31"
stable: "v1.32"

# The following PRR answers are required at alpha release
# List the feature gate name and the components for which it must be enabled
Expand Down