portforward: Auto-reconnect service-backed port-forwards on pod restart - #7273
portforward: Auto-reconnect service-backed port-forwards on pod restart#7273Slambot01 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Slambot01 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
The GitHub CI test job has snapshot failures. Run cd frontend && npm run test -- -u to regenerate the snapshots.
How to update snapshots
Run cd frontend && npm run test -- -u to regenerate all snapshots. Review the diff to make sure the visual changes are intentional, then commit the updated snapshot files.
The backend test job in CI is failing. Run cd backend && go test ./... to reproduce the errors locally.
How to run the backend tests
Run cd backend && go test ./... to see all failures. Fix the failing tests and commit the result.
There was a problem hiding this comment.
Pull request overview
Adds automatic recovery for service-backed port-forwards when their target pod restarts.
Changes:
- Resolves replacement pods and retries forwarding with backoff.
- Adds backend resolver tests.
- Displays reconnecting states in the frontend.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
backend/pkg/portforward/handler.go |
Implements reconnection lifecycle and status handling. |
backend/pkg/portforward/reconnect_test.go |
Tests service endpoint resolution. |
frontend/src/components/common/Resource/PortForward.tsx |
Adds reconnecting status and spinner. |
frontend/src/components/portforward/index.tsx |
Adds reconnecting list status. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
backend/pkg/portforward/handler.go:717
- Capturing the old channel prevents this goroutine from closing the replacement channel, but it still mutates the shared
pfDetailsbelow. If the oldForwardPortsexits after a replacement has becomeRunning, either its error path or normal-exit path can mark the replacementStoppedand overwrite the cache. Associate status updates with a forwarder generation (or verify channel ownership undermu) before allowing this goroutine to mutate shared state.
// Capture closeChan now so the deferred close targets THIS forwarder's
// channel even if pfDetails.closeChan is swapped during reconnect.
closeOnExit := pfDetails.closeChan
backend/pkg/portforward/handler.go:525
- The added tests cover only EndpointSlice resolution; none executes this retry/cancellation lifecycle, which allowed the already-closed channel above to make every retry exit immediately. Add deterministic coverage for a successful retry, exhaustion, readiness failure followed by another attempt, and user cancellation, using injectable backoffs/forwarder setup so the tests remain fast.
select {
case <-time.After(delay):
case <-pfDetails.closeChan:
frontend/src/components/common/Resource/PortForward.tsx:492
- This reconnecting branch removes the Stop action from the Service/Pod detail view for the entire retry window, even though the port-forward list deliberately treats
ReconnectinglikeRunningand retains Stop. Keep the stop button available beside the spinner so users can cancel a reconnect without navigating away.
) : portForward.status === PORT_FORWARD_RECONNECTING_STATUS ? (
<Box display={'flex'} alignItems="center" gap={1}>
<CircularProgress size={16} />
backend/pkg/portforward/handler.go:506
- The commit sequence needs cleanup:
9279bf8broadly rewrites/fixes the feature introduced by84cd8f5, and108b0b0then fixes lint/i18n fallout from that corrective commit. Please interactively rebase and squash/reorder these corrections into coherent feature commits so the reviewed history does not preserve intermediate broken states.
// attemptReconnect tries to re-establish a port-forward by finding a new pod
// behind the same service. It retries up to 3 times with exponential backoff
// (5s, 10s, 20s). Returns true if reconnection succeeded.
func attemptReconnect(
0a13856 to
a84695c
Compare
a84695c to
2c0182f
Compare
Squashes previous commits to fix i18n formatting diff issues
2c0182f to
b2fc735
Compare
Summary
This PR adds automatic reconnection for service-backed port-forwards by resolving a new pod via the Kubernetes Endpoints API when the target pod is deleted or restarted (e.g. during a rolling update). Previously, the port-forward was marked as
Stoppedand required manual restart.Related Issue
Fixes #7272
Changes
RECONNECTINGstatus constant andrConffield toportForwardstruct inbackend/pkg/portforward/handler.goresolveServicePod()to query the Endpoints API for a ready pod backing a serviceattemptReconnect()with exponential backoff (3 attempts: 5s → 10s → 20s)monitorPodAndManagePortForward()to attempt auto-reconnect for service-backed port-forwards instead of immediately stoppingreconnect_test.gowith 5 unit tests forresolveServicePodusing fake clientsetsPORT_FORWARD_RECONNECTING_STATUSconstant and reconnecting UI (spinner + "Reconnecting..." text) infrontend/src/components/common/Resource/PortForward.tsxfrontend/src/components/portforward/index.tsxto show a warning-colored status label for reconnecting port-forwardsSteps to Test
kubectl create deployment nginx --image=nginx && kubectl expose deployment nginx --port=80kubectl delete pod -l app=nginxNotes for the Reviewer
resolveServicePodacceptskubernetes.Interfaceinstead of*kubernetes.Clientsetto enable testing with fake clientsets. All callers pass*kubernetes.Clientsetwhich satisfies the interface.rConffield is taggedjson:"-"so it is excluded from all API responses.