feat: manage krelay-server via Job with idle-timeout self-exit by knight42 · Pull Request #101 · knight42/krelay

knight42 · 2026-04-16T17:48:29Z

Summary

Replace the krelay-server's bare Pod with a batch/v1.Job and add a client-side heartbeat so the server can detect when the port-forward tunnel drops and self-exit. Together these ensure orphaned server instances get cleaned up when the client crashes.

Server (`cmd/server/main.go`)

New --idle-timeout flag (default 5m, 0 disables).
idleTracker tracks activeConns and lastActivity with atomics. Background monitor ticks every max(timeout/4, 1s); shuts down when activeConns == 0 && idle >= timeout.
ProtocolKeepalive case in handleConn: returns immediately (no dial, no ACK).
onDisconnect updates lastActivity before decrementing activeConns to prevent a race where the monitor sees zero connections with a stale timestamp.

Client (`cmd/client/*.go`)

sendHeartbeats(conn, interval): goroutine sends a ProtocolKeepalive stream every 5s. Logs and exits on createStream or write failure. Drains errCh to avoid goroutine leaks.
RunServerJob replaces RunServerPod. Builds a batch/v1.Job (backoffLimit=0, ttlSecondsAfterFinished=10, restartPolicy=Never). Watches pods by job-name label. Explicit cleanup() on every error path after Job creation.
waitForServerJobPod handles watch.Deleted / watch.Error explicitly; distinguishes timeout from pod failure in error messages.
Close() deletes the Job with PropagationPolicy=Background.

RBAC (`manifests/rbac.yaml`)

Drop pods: create/delete (Job controller handles pod lifecycle).
Add batch/jobs: create, delete.
Keep pods: watch + pods/portforward: create.
Fix pre-existing typo: "requried" → "required".

Misc

Bump go.mod to go 1.26.0, manifests/Dockerfile-server to golang:1.26.
Replace toPtr[T any] helper with Go 1.26's new(value) builtin.
Adopt sync.WaitGroup.Go in pkg/xnet/udp_test.go; enable modernize linter.
Bump default --server.image to v0.0.5.
Add ProtocolKeepalive=2 to wire protocol docs in ARCHITECTURE.md.
Update CLAUDE.md Go version to 1.26.

Compatibility notes

Old server + new client: heartbeats hit AckCodeUnknownProtocol (client never reads it — harmless). Idle-timeout won't exist on the old server, so a client crash leaves the Job running until TTL expires after the server's process exits for other reasons. Normal graceful exit still works.
RBAC: users with custom RBAC bindings need batch/jobs: create, delete before upgrading.

Test plan

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR updates krelay’s in-cluster server lifecycle management by running krelay-server as a batch/v1.Job (instead of a standalone Pod) and adding a server-side idle timeout so the Job can complete and be TTL-cleaned up if the client crashes, reducing orphaned workloads.

Changes:

Replace server Pod creation/deletion with Job creation/deletion and update client code paths accordingly.
Add --idle-timeout to the server to self-exit after being idle, enabling Job completion + TTL cleanup.
Update RBAC, docs, lint config, and Go/toolchain versions to support the new approach.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`cmd/server/main.go`	Adds idle-tracking and listener self-close behavior via `--idle-timeout`.
`pkg/kube/flags.go`	Creates a `batch/v1.Job` wrapping the prior pod spec; updates port-forward target resolution.
`pkg/kube/utils.go`	Adds pod-wait logic for Job-owned pods; deletes Jobs with background propagation.
`cmd/client/main.go`	Switches client to create/close `ServerJob` rather than `ServerPod`.
`cmd/client/command_proxy.go`	Same as above for the proxy subcommand.
`manifests/rbac.yaml`	Updates permissions to create/delete Jobs; keeps pod watch + portforward.
`docs/ARCHITECTURE.md`	Documents Job-based server lifecycle and idle-timeout behavior.
`manifests/Dockerfile-server`	Updates server build image to Go 1.26.
`go.mod`	Bumps module Go version to 1.26.0.
`.golangci.yaml`	Enables `modernize` linter.
`pkg/xnet/udp_test.go`	Modernizes goroutine/waitgroup usage in tests.
`CLAUDE.md`	Updates package description to reflect Job lifecycle.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-16T17:52:27Z

-			slog.Debug("Pod is not running. Will retry.", slog.String("pod", podObj.Name))
 		}
+		slog.Debug("Pod is not running. Will retry.", slog.String("pod", podObj.Name))
 	}


When the watch channel closes (e.g., context deadline exceeded), the function returns a generic "krelay-server pod is not running" error. This makes timeouts hard to diagnose. Consider checking timeoutCtx.Err() after the loop and returning/wrapping it so callers can see whether this was a timeout vs. watch termination for another reason.

Suggested change

}

}

if err := timeoutCtx.Err(); err != nil {

return "", fmt.Errorf("wait for krelay-server pod: %w", err)

}

Copilot · 2026-04-16T17:52:27Z

-  # clean the krelay-server pod
-  - delete

 # The following permissions is only requried if you want to forward the local port to the respective objects.


Spelling/grammar in this comment: "permissions is only requried" → "permissions are only required" (and fix "requried" → "required").

Suggested change

# The following permissions is only requried if you want to forward the local port to the respective objects.

# The following permissions are only required if you want to forward the local port to the respective objects.

Copilot · 2026-04-16T17:52:28Z

 	for ev := range w.ResultChan() {
 		switch ev.Type {
-		case watch.Deleted, watch.Error:
-			break loop
-		case watch.Modified, watch.Added:
+		case watch.Added, watch.Modified:
 		default:
 			continue
 		}


waitForServerJobPod currently ignores watch.Deleted and watch.Error events (it only reacts to Added/Modified). If the Job's pod is deleted/failed quickly or the watch stream errors, this will just keep waiting until the 5m timeout instead of failing fast. Consider explicitly handling Deleted/Error (and possibly returning a clearer error) to avoid long hangs on immediate failures.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T02:02:39Z

+			RestartPolicy:                corev1.RestartPolicyNever,
+			AutomountServiceAccountToken: new(false),
+			EnableServiceLinks:           new(false),
 			SecurityContext: &corev1.PodSecurityContext{
-				RunAsNonRoot: toPtr(true),
+				RunAsNonRoot: new(true),
 			},


new(false) / new(true) will not compile on current Go versions because the built-in new takes a type (e.g. new(bool)), not a value. Use an address-of temp, a small helper (like the removed toPtr), or a dedicated pointer helper (e.g. k8s.io/utils/ptr.To) for these *bool fields.

new(value) is valid Go 1.26 syntax — go.mod specifies go 1.26.0. See the Go 1.26 release notes.

Copilot · 2026-04-19T02:02:39Z

+		Spec: batchv1.JobSpec{
+			BackoffLimit:            new(jobBackoffLimit),
+			TTLSecondsAfterFinished: new(ttlSecondsAfterFinished),
+			Template: corev1.PodTemplateSpec{


new(jobBackoffLimit) / new(ttlSecondsAfterFinished) has the same issue as new(false): new expects a type, so this likely won’t compile. Use a pointer helper to produce *int32 for these JobSpec fields.

new(value) is valid Go 1.26 syntax — go.mod specifies go 1.26.0. See the Go 1.26 release notes.

Copilot · 2026-04-19T02:02:39Z

+	err := cs.BatchV1().Jobs(namespace).Delete(ctx, jobName, metav1.DeleteOptions{
+		GracePeriodSeconds: new(int64(0)),


GracePeriodSeconds: new(int64(0)) is not valid Go on current toolchains (new requires a type). Use a pointer helper (or reintroduce the generic toPtr) to set *int64 here.

Suggested change

err := cs.BatchV1().Jobs(namespace).Delete(ctx, jobName, metav1.DeleteOptions{

GracePeriodSeconds: new(int64(0)),

gracePeriodSeconds := int64(0)

err := cs.BatchV1().Jobs(namespace).Delete(ctx, jobName, metav1.DeleteOptions{

GracePeriodSeconds: &gracePeriodSeconds,

new(value) is valid Go 1.26 syntax — go.mod specifies go 1.26.0. See the Go 1.26 release notes.

Copilot · 2026-04-19T02:02:40Z

+func openKeepalive(c httpstream.Connection) {
+	reqID := xnet.NewRequestID()
+	stream, _, err := createStream(c, reqID)
+	if err != nil {
+		return
+	}
+	hdr := xnet.Header{
+		RequestID: reqID,
+		Protocol:  xnet.ProtocolKeepalive,
+	}
+	_, _ = xio.WriteFull(stream, hdr.Marshal())
+	// stream stays open until the connection closes — do not defer Close.
+}


openKeepalive ignores the errCh returned by createStream. If the error-stream reader goroutine sends an error (channel is unbuffered), it can block forever because nobody receives from it, leaking a goroutine (and potentially holding resources). Either drain errCh in openKeepalive or make createStream’s errCh buffered / non-blocking for single-send use.

Copilot · 2026-04-19T02:02:40Z

+	flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")
 	flags.IntP("v", "v", 0, "bogus flag to keep backward compatibility. This flag will be removed in the future.")


PR description says the server --idle-timeout default is 1h, but the code sets it to 5*time.Minute here (and docs also mention 5m). Please align the default value and/or update the PR description so they’re consistent.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T02:22:32Z

+The client sends a `ProtocolKeepalive` heartbeat every 5 seconds over the port-forward stream. Each heartbeat refreshes the server's `lastActivity` timestamp. When the port-forward drops (client exit or crash), heartbeats stop. If no connections (including heartbeats) arrive within `--idle-timeout` (default 5m), the server closes the listener, `run()` returns nil, and the process exits 0 — the Job transitions to `Complete` and is garbage-collected by `ttlSecondsAfterFinished`.
+
 ## Wire protocol (`pkg/xnet/header.go`)


This section introduces ProtocolKeepalive heartbeats, but the wire-protocol table later in this doc still only lists 0=TCP and 1=UDP. Please update the protocol value list to include the keepalive protocol (iota=2) so the documentation is internally consistent.

Copilot · 2026-04-19T02:22:32Z

+func sendHeartbeats(c httpstream.Connection) {
+	tick := time.NewTicker(5 * time.Second)
+	defer tick.Stop()
+	for {
+		select {
+		case <-c.CloseChan():
+			return
+		case <-tick.C:
+			reqID := xnet.NewRequestID()
+			stream, _, err := createStream(c, reqID)
+			if err != nil {
+				slog.Error("Fail to create heartbeat stream", slogutil.Error(err))
+				return
+			}
+			hdr := xnet.Header{
+				RequestID: reqID,
+				Protocol:  xnet.ProtocolKeepalive,
+			}
+			if _, err := xio.WriteFull(stream, hdr.Marshal()); err != nil {
+				slog.Error("Fail to send heartbeat", slogutil.Error(err))
+			}
+			_ = stream.Close()
+		}


sendHeartbeats calls createStream and discards the returned errorChan. If the error-stream goroutine in createStream ever tries to send a non-nil error (e.g., on port-forward teardown), it can block forever on the unbuffered channel, leaking a goroutine per heartbeat. Consider draining errorChan (even just in a background goroutine) or adding a createStream variant for fire-and-forget heartbeats that doesn’t spawn/return an error channel.

Copilot · 2026-04-19T02:22:32Z

 	}
 	flags := c.Flags()
 	flags.DurationVar(&o.connectTimeout, "connect-timeout", time.Second*10, "Timeout for connecting to upstream")
+	flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")


The PR description says --idle-timeout defaults to 1h, but the flag is registered here with a 5m default. Please align the implementation and PR description (and any other docs) so users don’t get surprised by a shorter/longer default than advertised.

Copilot · 2026-04-19T02:22:32Z

+func sendHeartbeats(c httpstream.Connection) {
+	tick := time.NewTicker(5 * time.Second)
+	defer tick.Stop()
+	for {
+		select {
+		case <-c.CloseChan():
+			return
+		case <-tick.C:
+			reqID := xnet.NewRequestID()
+			stream, _, err := createStream(c, reqID)
+			if err != nil {
+				slog.Error("Fail to create heartbeat stream", slogutil.Error(err))
+				return
+			}
+			hdr := xnet.Header{
+				RequestID: reqID,
+				Protocol:  xnet.ProtocolKeepalive,
+			}
+			if _, err := xio.WriteFull(stream, hdr.Marshal()); err != nil {
+				slog.Error("Fail to send heartbeat", slogutil.Error(err))
+			}
+			_ = stream.Close()
+		}
+	}
+}


sendHeartbeats is new behavior that affects session liveness and server shutdown, but there’s no accompanying test coverage validating that heartbeats are sent periodically and stop cleanly when the stream closes. Since this package already has tests (e.g., utils_test.go), consider adding a small unit test with a fake httpstream.Connection/Stream to cover the happy path and shutdown path.

Copilot · 2026-04-19T02:22:33Z

-  # clean the krelay-server pod
-  - delete

 # The following permissions is only requried if you want to forward the local port to the respective objects.


Typo in comment: “requried” → “required”.

Suggested change

# The following permissions is only requried if you want to forward the local port to the respective objects.

# The following permissions is only required if you want to forward the local port to the respective objects.

Copilot · 2026-04-19T02:22:33Z

 ## Client (`cmd/client`)

-Binary: `kubectl-relay`. Parses `TYPE/NAME [LOCAL:]REMOTE[@PROTO]` args (or `-f targets.txt`), resolves each target to a remote address, creates a `krelay-server` pod in the active namespace, opens a single SPDY (or SPDY-over-websocket) stream via the `/portforward` subresource, and listens locally for TCP/UDP. The pod is deleted with `GracePeriodSeconds: 0` on exit.
+Binary: `kubectl-relay`. Parses `TYPE/NAME [LOCAL:]REMOTE[@PROTO]` args (or `-f targets.txt`), resolves each target to a remote address, creates a `krelay-server` **Job** in the active namespace, waits for the Job's pod to become Running, opens a single SPDY (or SPDY-over-websocket) stream via the `/portforward` subresource, and listens locally for TCP/UDP. On graceful exit the Job is deleted with `PropagationPolicy=Background`; on crash the server self-terminates on idle (see below) and the Job's `ttlSecondsAfterFinished` cleans it up.


The architecture doc says the client creates the krelay-server Job “in the active namespace”, but buildServerJob hard-codes metav1.NamespaceDefault unless the user overrides it via --patch/--patch-file. Either update the documentation to describe the actual behavior (default namespace unless patched), or change buildServerJob to default to the kubeconfig/--namespace value so this statement is true.

Suggested change

Binary: `kubectl-relay`. Parses `TYPE/NAME [LOCAL:]REMOTE[@PROTO]` args (or `-f targets.txt`), resolves each target to a remote address, creates a `krelay-server` **Job** in the active namespace, waits for the Job's pod to become Running, opens a single SPDY (or SPDY-over-websocket) stream via the `/portforward` subresource, and listens locally for TCP/UDP. On graceful exit the Job is deleted with `PropagationPolicy=Background`; on crash the server self-terminates on idle (see below) and the Job's `ttlSecondsAfterFinished` cleans it up.

Binary: `kubectl-relay`. Parses `TYPE/NAME [LOCAL:]REMOTE[@PROTO]` args (or `-f targets.txt`), resolves each target to a remote address, creates a `krelay-server` **Job** in the default namespace unless overridden via `--patch` / `--patch-file`, waits for the Job's pod to become Running, opens a single SPDY (or SPDY-over-websocket) stream via the `/portforward` subresource, and listens locally for TCP/UDP. On graceful exit the Job is deleted with `PropagationPolicy=Background`; on crash the server self-terminates on idle (see below) and the Job's `ttlSecondsAfterFinished` cleans it up.

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T07:41:14Z

 	l.Info("Creating port-forward stream to krelay-server pod")
 	streamConn, _, err := dialer.Dial(constants.PortForwardProtocolV1Name)
 	if err != nil {
 		return nil, fmt.Errorf("dial: %w", err)
 	}


RunServerJob defines a cleanup() for failures after Job creation, but the Job is not cleaned up if dialer.Dial fails. This can leave an orphaned Job until idle-timeout/TTL kicks in; call cleanup() on the dial error path to match the comment’s intent.

Copilot · 2026-04-19T07:41:15Z

 	flags.DurationVar(&o.connectTimeout, "connect-timeout", time.Second*10, "Timeout for connecting to upstream")
+	flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")


The --idle-timeout default here is 5m, but the PR description claims the default is 1h. Please align the description/docs or change the default so they match (and consider whether 5m vs 1h is the intended operational behavior).

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T07:53:41Z

@@ -126,30 +133,47 @@ func (f *Flags) buildServerPod() (*corev1.Pod, error) {
 			},
 		},
 	}
-	if len(f.patch) == 0 && len(f.patchFile) == 0 {
-		return &origPod, nil
-	}

-	var patchBytes []byte
-	if len(f.patch) > 0 {
-		patchBytes = []byte(f.patch)
-	} else if len(f.patchFile) > 0 {
-		var err error
-		patchBytes, err = os.ReadFile(f.patchFile)
+	if len(f.patch) > 0 || len(f.patchFile) > 0 {
+		var patchBytes []byte
+		if len(f.patch) > 0 {
+			patchBytes = []byte(f.patch)
+		} else {
+			var err error
+			patchBytes, err = os.ReadFile(f.patchFile)
+			if err != nil {
+				return nil, fmt.Errorf("read file: %w", err)
+			}
+		}
+		patched, err := patchPod(patchBytes, origPod)
 		if err != nil {
-			return nil, fmt.Errorf("read file: %w", err)
+			return nil, fmt.Errorf("patch server pod: %w", err)
 		}
+		origPod = *patched
 	}

-	patched, err := patchPod(patchBytes, origPod)
-	if err != nil {
-		return nil, fmt.Errorf("patch server pod: %w", err)
+	job := &batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:    origPod.Namespace,
+			GenerateName: constants.ServerName + "-",
+			Labels:       podLabels,
+		},
+		Spec: batchv1.JobSpec{
+			BackoffLimit:            new(jobBackoffLimit),
+			TTLSecondsAfterFinished: new(ttlSecondsAfterFinished),
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels:      origPod.Labels,
+					Annotations: origPod.Annotations,
+				},
+				Spec: origPod.Spec,
+			},
+		},
 	}
-
-	return patched, nil
+	return job, nil


buildServerJob now wraps the pod template in a Job and relies on pod-shaped patches to drive behavior like namespace selection. There are existing unit tests for patchPod, but none that cover the new Job-wrapping logic (e.g., TTL/backoff settings and propagation of patched metadata.namespace into Job.ObjectMeta.Namespace). Adding a focused unit test for buildServerJob would help ensure this backward-compatibility contract doesn’t drift.

Copilot · 2026-04-19T07:53:41Z

 	}
 	flags := c.Flags()
 	flags.DurationVar(&o.connectTimeout, "connect-timeout", time.Second*10, "Timeout for connecting to upstream")
+	flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")


The PR description says --idle-timeout defaults to 1h, but the actual flag default here is 5*time.Minute (and docs/ARCHITECTURE.md also describes 5m). Please align the PR description and/or the flag default so users don’t get surprised by an earlier-than-advertised server exit.

Copilot · 2026-04-19T07:53:41Z

+			reqID := xnet.NewRequestID()
+			stream, errCh, err := createStream(c, reqID)
+			if err != nil {
+				slog.Error("Fail to create heartbeat stream", slogutil.Error(err))
+				return
+			}
+			go func() { <-errCh }()
+			hdr := xnet.Header{
+				RequestID: reqID,
+				Protocol:  xnet.ProtocolKeepalive,
+			}
+			if _, err := xio.WriteFull(stream, hdr.Marshal()); err != nil {


sendHeartbeats spawns an extra goroutine on every tick just to drain errCh (go func() { <-errCh }()). Over long runs this adds avoidable goroutine churn, and if errCh ever fails to close promptly it can leak goroutines indefinitely. Consider adjusting the stream/error-channel plumbing so heartbeats can drain errors without per-tick goroutines (e.g., make the error channel buffered / non-blocking, or wait for it with a bounded timeout after closing the stream).

Copilot · 2026-04-19T07:53:41Z

+	case xnet.ProtocolKeepalive:
+		l.Debug("Heartbeat received")
+


For ProtocolKeepalive, dstAddr is still computed from an empty Addr/Port (yielding something like "<nil>:0"). It’s currently unused in the keepalive case, but computing it unconditionally makes the keepalive path harder to reason about and risks confusing logs if this value gets included later. Consider moving dstAddr construction into the TCP/UDP cases (or guarding it) so keepalives don’t fabricate a destination address.

Copilot · 2026-04-19T07:53:42Z

+		case watch.Deleted:
+			return "", fmt.Errorf("krelay-server pod was deleted before becoming ready")
+		case watch.Error:
+			return "", fmt.Errorf("watch error for krelay-server pod: %v", ev.Object)


In the watch.Error case, ev.Object is typically a *metav1.Status; formatting it with %v often produces a noisy struct dump rather than the actionable status message/reason. Consider extracting the Status fields (e.g., message/reason/code) when ev.Object is a *metav1.Status, and fall back to %T/%v only when it’s not.

Suggested change

return "", fmt.Errorf("watch error for krelay-server pod: %v", ev.Object)

if status, ok := ev.Object.(*metav1.Status); ok {

return "", fmt.Errorf("watch error for krelay-server pod: %s (reason=%s, code=%d)", status.Message, status.Reason, status.Code)

}

return "", fmt.Errorf("watch error for krelay-server pod: %T: %v", ev.Object, ev.Object)

Copilot · 2026-04-19T07:53:42Z

+// idleTracker closes the listener when no connections have been active for
+// idleTimeout, so the Job can transition to Complete and be garbage-collected
+// by its TTL if the client crashed without cleaning up.
+type idleTracker struct {


idleTracker + --idle-timeout and the new ProtocolKeepalive path introduce new shutdown/lifecycle behavior, but cmd/server currently only has a TCP proxy test. Adding unit tests that assert (1) keepalive requests don’t attempt upstream dials / don’t write ACKs unexpectedly and (2) the listener is closed after the configured idle period would help prevent regressions.

Replaces the bare Pod with a batch/v1.Job whose server self-terminates after --idle-timeout (default 1h) of no active connections. If the client crashes without cleanup, the server exits, the Job transitions to Complete, and ttlSecondsAfterFinished garbage-collects it. - cmd/server: idleTracker (atomic activeConns + lastActivity) closes the listener when idle; run() exits 0 on net.ErrClosed. - pkg/kube: buildServerJob wraps the pod template in a Job (backoffLimit=0, ttlSecondsAfterFinished=10s, restartPolicy=Never); patchPod still targets the pod so pod-shaped --patch/--patch-file patches keep working, with namespace propagated to the Job. waitForServerJobPod watches pods by job-name label for readiness. - manifests/rbac.yaml: drop pods create/delete, add batch/jobs create/delete, keep pods watch + pods/portforward create. - Bump go.mod to 1.26 and Dockerfile-server to golang:1.26; replace the local toPtr helper with Go 1.26's new(value) builtin. - Adopt sync.WaitGroup.Go in udp_test; enable modernize linter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The server used to start counting idle time from process startup, which could cause it to exit before any client connected. Now the idle timer is gated behind a hadConn flag that flips on the first Accept. Also reduce the default --idle-timeout from 1h to 5m — once all connections close after the port-forward stream breaks, 5 minutes of silence is enough to conclude the client is gone. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The client now opens a ProtocolKeepalive stream immediately after the port-forward is established. This stream blocks on the server side and counts as activeConns=1, so the idle timer never fires while the tunnel is alive. When the port-forward drops (client exit or crash), the keepalive closes, activeConns falls to 0, and the 5-minute idle countdown begins. Old servers that don't recognise ProtocolKeepalive will respond with AckCodeUnknownProtocol, which the client never reads — backwards compatible; the idle timer simply falls back to the hadConn-gated behaviour from the previous commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Instead of holding a single long-lived stream open, the client now sends a ProtocolKeepalive heartbeat every 5 seconds. Each heartbeat opens a stream, writes the header, and closes — refreshing the server's lastActivity timestamp. When the port-forward drops, heartbeats stop and the idle timer fires after --idle-timeout. This is simpler: no blocking goroutine on the server, no hadConn gate, and the idle tracker's existing logic works unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- #2: Clean up orphaned Job when waitForServerJobPod or later steps fail in RunServerJob — the caller never gets a ServerJob handle to call Close() on, so the Job would linger indefinitely (TTL only applies to finished Jobs, not stuck-Pending ones). - #3: sendHeartbeats now returns on WriteFull error (not just createStream error). A write failure signals a broken tunnel; silently continuing would let the server idle-timeout while the client still thinks it's forwarding. - #4: waitForServerJobPod restores explicit handling of watch.Deleted and watch.Error events instead of silently continuing until the 5-minute timeout. Also distinguishes timeout from pod deletion in the error message. Also: update CLAUDE.md Go version to 1.26, log lis.Close() error in idle monitor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

New server image includes --idle-timeout and ProtocolKeepalive support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Fix pre-existing typo "requried" → "required" in rbac.yaml. - Add ProtocolKeepalive=2 to wire protocol table in ARCHITECTURE.md. - Clarify Job namespace defaults to "default" (not "active namespace"). - Drain errCh in sendHeartbeats to prevent goroutine leak. - Add missing cleanup() on Dial error path in RunServerJob. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Extract heartbeat interval as a parameter so tests can use a short tick (10ms) instead of the production 5s. Four test cases: - CloseChan: returns immediately when connection is closed - SendsHeartbeats: verifies multiple heartbeats are sent over time - CreateStreamError: returns on stream creation failure - WriteError: returns on header write failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Four new e2e tests verifying the Pod→Job migration: - TestJobCreatedNotPod: krelay-server runs as a Job, not a bare Pod - TestJobCleanedUpOnSIGINT: SIGINT deletes the Job (pod cascades) - TestIdleTimeoutExitsAfterClientKilled: SIGKILL → server idle-exits → Job completes → TTL GC removes it - TestHeartbeatKeepsServerAlive: client heartbeats prevent server idle-exit even when no forwarding traffic flows Framework additions: - serverPodPatchWithArgs: injects custom server container args and sets imagePullPolicy=IfNotPresent when KRELAY_SERVER_IMAGE is set - startKrelayWithServerArgs: variant of startKrelay for custom server args - krelayInstance.kill(): SIGKILL for crash-simulation tests - listKrelayJobs / waitForNoKrelayJobs: Job assertion helpers All 13 e2e tests pass on OrbStack k8s v1.33.5 (arm64). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Prevents a race where the idle monitor sees activeConns==0 with a stale lastActivity timestamp from before the connection was established. If a long-lived connection (open longer than idle-timeout) closes, the monitor could shut down immediately instead of waiting a full timeout window. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Captures pitfalls discovered during the Job+heartbeat implementation: atomic ordering in idleTracker, errCh drain requirement, Job cleanup on setup failure, and e2e imagePullPolicy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T03:04:14Z

+func waitForServerJobPod(ctx context.Context, cs kubernetes.Interface, namespace, jobName string) (string, error) {
 	timeoutCtx, cancel := context.WithTimeout(ctx, time.Minute*5)
 	defer cancel()

 	w, err := cs.CoreV1().Pods(namespace).Watch(timeoutCtx, metav1.ListOptions{
-		FieldSelector: fmt.Sprintf("metadata.name=%s", podName),
+		LabelSelector: fmt.Sprintf("job-name=%s", jobName),
 	})
 	if err != nil {


waitForServerJobPod starts a watch without an initial List. If the Job's pod is created and reaches Running before the watch is established (or no further updates occur), this can time out even though a Running pod exists. Consider doing an initial Pods(namespace).List with the same labelSelector, return immediately if any matching pod is already Running, then start a watch from the list's ResourceVersion (classic list+watch pattern).

Copilot · 2026-04-20T03:04:14Z

+func sendHeartbeats(c httpstream.Connection, interval time.Duration) {
+	tick := time.NewTicker(interval)
+	defer tick.Stop()
+	for {
+		select {
+		case <-c.CloseChan():
+			return
+		case <-tick.C:
+			reqID := xnet.NewRequestID()
+			stream, errCh, err := createStream(c, reqID)
+			if err != nil {
+				slog.Error("Fail to create heartbeat stream", slogutil.Error(err))
+				return
+			}
+			go func() { <-errCh }()
+			hdr := xnet.Header{
+				RequestID: reqID,
+				Protocol:  xnet.ProtocolKeepalive,
+			}
+			if _, err := xio.WriteFull(stream, hdr.Marshal()); err != nil {
+				slog.Error("Fail to send heartbeat", slogutil.Error(err))
+				_ = stream.Close()
+				return
+			}
+			_ = stream.Close()
+		}


sendHeartbeats discards the value from errCh (go func() { <-errCh }()), so port-forward error-stream messages are never surfaced and the goroutine pattern is harder to reason about. Instead, read errCh and log/stop on a non-nil error (or select on errCh vs CloseChan) to avoid silently continuing when the port-forward stream reports an error.

Copilot · 2026-04-20T03:04:14Z

 	flags.DurationVar(&o.connectTimeout, "connect-timeout", time.Second*10, "Timeout for connecting to upstream")
+	flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")
 	flags.IntP("v", "v", 0, "bogus flag to keep backward compatibility. This flag will be removed in the future.")


The PR description says --idle-timeout defaults to 1h, but the flag default here is 5*time.Minute. Please align the implementation + docs/PR notes on the intended default (either change the default to 1h or update the documentation/description accordingly).

Copilot · 2026-04-20T03:04:15Z

+func TestJobCreatedNotPod(t *testing.T) {
+	ki := startKrelay(t, 1, "Forwarding", "-n", testNS, "svc/test-nginx-svc", ":80")
+
+	jobs := listKrelayJobs(t)
+	require.NotEmpty(t, jobs, "expected a krelay-server Job in namespace %s", testNS)


This test name says "NotPod" but the assertions only verify that a Job exists; it doesn't actually check that the client isn't directly creating a standalone Pod. Either rename the test to reflect what it asserts, or add an explicit assertion that there is no krelay-server Pod without a Job owner reference.

Copilot · 2026-04-20T03:04:15Z

+	// Server should idle-timeout (15s) + TTL GC (10s) = ~25s.
+	// Give generous margin.
+	waitForNoKrelayJobs(t, 60*time.Second)


This test requires the Job to disappear (waitForNoKrelayJobs) after the client is killed, which depends on the cluster's TTL-after-finish controller being enabled and timely. To reduce e2e flakiness across clusters, consider asserting that the Job reaches Complete (or the Pod terminates) and only optionally asserting TTL-based deletion when supported.

Copilot · 2026-04-20T03:04:15Z

+	close(conn.closeCh)
+	<-done
+
+	// createStream is called twice per heartbeat (error stream + data stream)


The comment says "createStream is called twice per heartbeat", but sendHeartbeats calls createStream once per heartbeat; it's CreateStream that is called twice inside createStream (error + data). Updating this comment would avoid confusion when interpreting the expected createCount.

Suggested change

// createStream is called twice per heartbeat (error stream + data stream)

// Each heartbeat creates two streams: an error stream and a data stream.

Copilot AI review requested due to automatic review settings April 16, 2026 17:48

Copilot started reviewing on behalf of knight42 April 16, 2026 17:48 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 19, 2026 01:59

Copilot started reviewing on behalf of knight42 April 19, 2026 01:59 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 19, 2026 02:15

Copilot started reviewing on behalf of knight42 April 19, 2026 02:16 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 19, 2026 07:36

Copilot started reviewing on behalf of knight42 April 19, 2026 07:37 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 19, 2026 07:48

Copilot started reviewing on behalf of knight42 April 19, 2026 07:48 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

knight42 and others added 10 commits April 19, 2026 19:33

fix: log heartbeat failures

ee22def

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: bump default server image to v0.0.5

9e93807

New server image includes --idle-timeout and ProtocolKeepalive support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

knight42 force-pushed the feat/server-job-with-idle-timeout branch from cb4753d to c0d5802 Compare April 20, 2026 02:48

Copilot AI review requested due to automatic review settings April 20, 2026 02:59

Copilot started reviewing on behalf of knight42 April 20, 2026 02:59 View session

knight42 merged commit d818454 into main Apr 20, 2026
2 checks passed

knight42 deleted the feat/server-job-with-idle-timeout branch April 20, 2026 03:03

Copilot AI reviewed Apr 20, 2026

View reviewed changes

-	}
+	}
+	if err := timeoutCtx.Err(); err != nil {
+		return "", fmt.Errorf("wait for krelay-server pod: %w", err)
+	}

	# The following permissions is only requried if you want to forward the local port to the respective objects.
	# The following permissions are only required if you want to forward the local port to the respective objects.

		err := cs.BatchV1().Jobs(namespace).Delete(ctx, jobName, metav1.DeleteOptions{
		GracePeriodSeconds: new(int64(0)),

		flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")
		flags.IntP("v", "v", 0, "bogus flag to keep backward compatibility. This flag will be removed in the future.")

		The client sends a `ProtocolKeepalive` heartbeat every 5 seconds over the port-forward stream. Each heartbeat refreshes the server's `lastActivity` timestamp. When the port-forward drops (client exit or crash), heartbeats stop. If no connections (including heartbeats) arrive within `--idle-timeout` (default 5m), the server closes the listener, `run()` returns nil, and the process exits 0 — the Job transitions to `Complete` and is garbage-collected by `ttlSecondsAfterFinished`.

		## Wire protocol (`pkg/xnet/header.go`)

		flags.DurationVar(&o.connectTimeout, "connect-timeout", time.Second*10, "Timeout for connecting to upstream")
		flags.DurationVar(&o.idleTimeout, "idle-timeout", 5*time.Minute, "Exit when no connections have been active for this duration after the last client disconnects. 0 disables.")

-			return "", fmt.Errorf("watch error for krelay-server pod: %v", ev.Object)
+			if status, ok := ev.Object.(*metav1.Status); ok {
+				return "", fmt.Errorf("watch error for krelay-server pod: %s (reason=%s, code=%d)", status.Message, status.Reason, status.Code)
+			}
+			return "", fmt.Errorf("watch error for krelay-server pod: %T: %v", ev.Object, ev.Object)

	// createStream is called twice per heartbeat (error stream + data stream)
	// Each heartbeat creates two streams: an error stream and a data stream.

Conversation

knight42 commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Server (cmd/server/main.go)

Client (cmd/client/*.go)

RBAC (manifests/rbac.yaml)

Misc

Compatibility notes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

knight42 Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

knight42 Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

knight42 Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 19, 2026

knight42 commented Apr 16, 2026 •

edited

Loading

Server (`cmd/server/main.go`)

Client (`cmd/client/*.go`)

RBAC (`manifests/rbac.yaml`)