From 479d87dfd26db5b8fd982c8d1b7063ce9ea88ec2 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 24 Jun 2026 12:48:32 -0500 Subject: [PATCH 1/2] feat(kubernetes): platform-native agent delivery Route the kubernetes driver to a native exec-stream delivery instead of legacy shell injection, removing the deprecation warning on devsy up. KubernetesDelivery streams the locally-cached agent binary into the pod over the cluster's exec API (no in-container network or shell download), with a version probe for idempotency and an atomic temp-file write. The driver's exec now prefers WebSocket with an SPDY fallback. Also resolves the agent arch from the cluster (TargetArchitecture) rather than the host GOARCH, fixing cross-architecture delivery. --- pkg/agent/delivery/factory.go | 19 ++-- pkg/agent/delivery/factory_test.go | 23 ++++- pkg/agent/delivery/kubernetes.go | 59 ++++++++++-- pkg/agent/delivery/kubernetes_test.go | 125 +++++++++++++++++--------- pkg/devcontainer/setup.go | 38 +++++++- pkg/devcontainer/single.go | 3 +- pkg/driver/kubernetes/client.go | 20 ++++- pkg/driver/kubernetes/driver.go | 20 +++++ pkg/driver/types.go | 7 ++ 9 files changed, 249 insertions(+), 65 deletions(-) diff --git a/pkg/agent/delivery/factory.go b/pkg/agent/delivery/factory.go index c86e94315..f61a0ecaa 100644 --- a/pkg/agent/delivery/factory.go +++ b/pkg/agent/delivery/factory.go @@ -21,6 +21,7 @@ type FactoryOptions struct { IsRemoteDocker bool ContainerID string ExecFunc inject.ExecFunc //nolint:staticcheck // legacy delivery strategies require this type + PodExec PodExecFunc } func NewAgentDelivery(opts FactoryOptions) AgentDelivery { @@ -38,14 +39,18 @@ func NewAgentDelivery(opts FactoryOptions) AgentDelivery { } case driverType == provider.KubernetesDriver: - log.Debugf("using legacy shell delivery for kubernetes driver") - log.Warnf( - "legacy shell delivery is deprecated; platform-native delivery will replace this in a future release", - ) - return &LegacyShellDelivery{ - ExecFunc: opts.ExecFunc, - DownloadURL: "", + if opts.PodExec == nil { + log.Debugf("kubernetes pod exec unavailable, using legacy shell delivery") + log.Warnf( + "legacy shell delivery is deprecated; platform-native delivery will replace this in a future release", + ) + return &LegacyShellDelivery{ + ExecFunc: opts.ExecFunc, + DownloadURL: "", + } } + log.Debugf("using kubernetes-native delivery (exec stream)") + return &KubernetesDelivery{Exec: opts.PodExec} case opts.IsRemoteDocker: log.Debugf("using remote docker delivery (docker cp)") diff --git a/pkg/agent/delivery/factory_test.go b/pkg/agent/delivery/factory_test.go index 0e6cb134f..9b08b69ea 100644 --- a/pkg/agent/delivery/factory_test.go +++ b/pkg/agent/delivery/factory_test.go @@ -5,6 +5,7 @@ import ( "io" "testing" + "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/provider" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -73,7 +74,26 @@ func TestNewAgentDelivery_CustomDriver(t *testing.T) { assert.Equal(t, PhasePostStart, d.Phase()) } -func TestNewAgentDelivery_KubernetesDriver(t *testing.T) { +func TestNewAgentDelivery_KubernetesDriver_Native(t *testing.T) { + podExec := func(_ context.Context, _ []string, _ driver.Streams) error { + return nil + } + + opts := FactoryOptions{ + WorkspaceConfig: &provider.AgentWorkspaceInfo{ + Agent: provider.ProviderAgentConfig{ + Driver: provider.KubernetesDriver, + }, + }, + PodExec: podExec, + } + + d := NewAgentDelivery(opts) + assert.IsType(t, &KubernetesDelivery{}, d) + assert.Equal(t, PhasePostStart, d.Phase()) +} + +func TestNewAgentDelivery_KubernetesDriver_FallsBackWhenNoPodExec(t *testing.T) { execFn := func(ctx context.Context, cmd string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { return nil } @@ -85,6 +105,7 @@ func TestNewAgentDelivery_KubernetesDriver(t *testing.T) { }, }, ExecFunc: execFn, + // PodExec intentionally nil → legacy fallback. } d := NewAgentDelivery(opts) diff --git a/pkg/agent/delivery/kubernetes.go b/pkg/agent/delivery/kubernetes.go index e778bd7bb..b5eb498cf 100644 --- a/pkg/agent/delivery/kubernetes.go +++ b/pkg/agent/delivery/kubernetes.go @@ -1,18 +1,28 @@ package delivery import ( + "bytes" "context" "fmt" + "strings" pkgconfig "github.com/devsy-org/devsy/pkg/config" - "github.com/devsy-org/devsy/pkg/inject" + "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/log" + "github.com/devsy-org/devsy/pkg/version" ) var _ AgentDelivery = (*KubernetesDelivery)(nil) +// PodExecFunc runs argv in the workspace pod's dev container with the given streams. +type PodExecFunc func(ctx context.Context, argv []string, streams driver.Streams) error + +// KubernetesDelivery streams the agent binary into the pod over the cluster's exec API. type KubernetesDelivery struct { - ExecFunc inject.ExecFunc //nolint:staticcheck // K8s exec routing requires this type + Exec PodExecFunc + + // ExpectedVersion defaults to version.GetVersion() when empty. + ExpectedVersion string } func (d *KubernetesDelivery) Phase() DeliveryPhase { @@ -27,31 +37,62 @@ func (d *KubernetesDelivery) DeliverPostStart(ctx context.Context, opts PostStar if opts.BinarySource == nil { return fmt.Errorf("binary source is required for kubernetes delivery") } - if d.ExecFunc == nil { + if d.Exec == nil { return fmt.Errorf("exec function is required for kubernetes delivery") } + destPath := pkgconfig.ContainerDevsyHelperLocation + + // Skip delivery when the in-pod binary already matches. + expected := d.expectedVersion() + if actual := d.detectVersion(ctx, destPath); actual != "" && actual == expected { + log.Debugf("remote agent version matches expected version %s, skipping delivery", expected) + return nil + } + binary, err := opts.BinarySource(ctx, opts.Arch) if err != nil { return fmt.Errorf("acquire binary: %w", err) } defer func() { _ = binary.Close() }() - destPath := pkgconfig.ContainerDevsyHelperLocation + // Write to a temp file and atomically move it into place so a failed stream + // never leaves an executable stub. script := fmt.Sprintf( - `set -e; t=$(mktemp %s.XXXXXX); cat > "$t" && chmod 755 "$t" && mv "$t" %s || { rm -f "$t"; exit 1; }`, - destPath, - destPath, + `set -e; d=$(dirname %s); mkdir -p "$d"; `+ + `t=$(mktemp %s.XXXXXX); `+ + `cat > "$t" && chmod 0755 "$t" && mv -f "$t" %s || { rm -f "$t"; exit 1; }`, + destPath, destPath, destPath, ) - if err := d.ExecFunc(ctx, script, binary, nil, nil); err != nil { + if err := d.Exec(ctx, []string{"sh", "-c", script}, driver.Streams{Stdin: binary}); err != nil { return fmt.Errorf("write binary to container: %w", err) } - log.Debugf("delivered agent binary to kubernetes container via exec") + log.Debugf("delivered agent binary to pod via kubernetes exec") return nil } func (d *KubernetesDelivery) Cleanup(_ context.Context, _ string) error { return nil } + +func (d *KubernetesDelivery) expectedVersion() string { + if d.ExpectedVersion != "" { + return d.ExpectedVersion + } + return version.GetVersion() +} + +// detectVersion returns the agent version in the pod, or "" if absent or unprobeable. +func (d *KubernetesDelivery) detectVersion(ctx context.Context, destPath string) string { + script := fmt.Sprintf(`[ -x "%s" ] && "%s" --version 2>/dev/null || true`, destPath, destPath) + + var stdout bytes.Buffer + err := d.Exec(ctx, []string{"sh", "-c", script}, driver.Streams{Stdout: &stdout}) + if err != nil { + log.Debugf("failed to detect agent version in pod: %v", err) + return "" + } + return strings.TrimSpace(stdout.String()) +} diff --git a/pkg/agent/delivery/kubernetes_test.go b/pkg/agent/delivery/kubernetes_test.go index 28e4152e4..4bf82fddd 100644 --- a/pkg/agent/delivery/kubernetes_test.go +++ b/pkg/agent/delivery/kubernetes_test.go @@ -9,10 +9,46 @@ import ( "testing" pkgconfig "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/driver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +const testVersion = "v1.2.3" + +// recordingExec records each call and replays stdouts[N] as stdout on call N. +type recordingExec struct { + calls []recordedCall + stdouts []string + err error +} + +type recordedCall struct { + argv []string + stdin string +} + +func (r *recordingExec) fn(_ context.Context, argv []string, streams driver.Streams) error { + call := recordedCall{argv: argv} + if streams.Stdin != nil { + var buf bytes.Buffer + _, _ = io.Copy(&buf, streams.Stdin) + call.stdin = buf.String() + } + idx := len(r.calls) + r.calls = append(r.calls, call) + if streams.Stdout != nil && idx < len(r.stdouts) { + _, _ = io.WriteString(streams.Stdout, r.stdouts[idx]) + } + return r.err +} + +func binarySourceFrom(data string) BinarySourceFunc { + return func(_ context.Context, _ string) (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader(data)), nil + } +} + func TestKubernetesDelivery_Phase(t *testing.T) { d := &KubernetesDelivery{} assert.Equal(t, PhasePostStart, d.Phase()) @@ -26,17 +62,14 @@ func TestKubernetesDelivery_DeliverPreStart_ReturnsError(t *testing.T) { } func TestKubernetesDelivery_DeliverPostStart_RequiresBinarySource(t *testing.T) { - d := &KubernetesDelivery{ - ExecFunc: func(_ context.Context, _ string, _ io.Reader, _ io.Writer, _ io.Writer) error { - return nil - }, - } + exec := &recordingExec{} + d := &KubernetesDelivery{Exec: exec.fn} err := d.DeliverPostStart(context.Background(), PostStartOptions{}) require.Error(t, err) assert.Contains(t, err.Error(), "binary source is required") } -func TestKubernetesDelivery_DeliverPostStart_RequiresExecFunc(t *testing.T) { +func TestKubernetesDelivery_DeliverPostStart_RequiresExec(t *testing.T) { d := &KubernetesDelivery{} err := d.DeliverPostStart(context.Background(), PostStartOptions{ BinarySource: fakeBinarySource, @@ -47,63 +80,69 @@ func TestKubernetesDelivery_DeliverPostStart_RequiresExecFunc(t *testing.T) { func TestKubernetesDelivery_DeliverPostStart_WritesBinary(t *testing.T) { binaryData := "test-binary-content" - var capturedCmd string - var capturedStdin bytes.Buffer - - execFn := func(_ context.Context, cmd string, stdin io.Reader, _ io.Writer, _ io.Writer) error { - capturedCmd = cmd - if stdin != nil { - _, _ = io.Copy(&capturedStdin, stdin) - } - return nil - } + // Probe returns nothing → deliver. + exec := &recordingExec{stdouts: []string{""}} - d := &KubernetesDelivery{ExecFunc: execFn} + d := &KubernetesDelivery{Exec: exec.fn, ExpectedVersion: testVersion} err := d.DeliverPostStart(context.Background(), PostStartOptions{ - BinarySource: func(_ context.Context, _ string) (io.ReadCloser, error) { - return io.NopCloser(strings.NewReader(binaryData)), nil - }, - Arch: "amd64", + BinarySource: binarySourceFrom(binaryData), + Arch: testArch, }) - require.NoError(t, err) + require.Len(t, exec.calls, 2) destPath := pkgconfig.ContainerDevsyHelperLocation - assert.Contains(t, capturedCmd, "cat >") - assert.Contains(t, capturedCmd, destPath) - assert.Contains(t, capturedCmd, "chmod 755") - assert.Equal(t, binaryData, capturedStdin.String()) + probeScript := strings.Join(exec.calls[0].argv, " ") + assert.Contains(t, probeScript, "--version") + assert.Contains(t, probeScript, destPath) + + writeScript := strings.Join(exec.calls[1].argv, " ") + assert.Contains(t, writeScript, "mktemp") + assert.Contains(t, writeScript, "chmod 0755") + assert.Contains(t, writeScript, "mv -f") + assert.Contains(t, writeScript, destPath) + assert.Equal(t, binaryData, exec.calls[1].stdin) } -func TestKubernetesDelivery_DeliverPostStart_BinarySourceError(t *testing.T) { - execFn := func(_ context.Context, _ string, _ io.Reader, _ io.Writer, _ io.Writer) error { - return nil - } +func TestKubernetesDelivery_DeliverPostStart_SkipsWhenVersionMatches(t *testing.T) { + exec := &recordingExec{stdouts: []string{testVersion + "\n"}} - d := &KubernetesDelivery{ExecFunc: execFn} + d := &KubernetesDelivery{Exec: exec.fn, ExpectedVersion: testVersion} err := d.DeliverPostStart(context.Background(), PostStartOptions{ - BinarySource: func(_ context.Context, _ string) (io.ReadCloser, error) { - return nil, fmt.Errorf("download failed") - }, + BinarySource: binarySourceFrom("should-not-be-streamed"), + Arch: testArch, }) + require.NoError(t, err) - require.Error(t, err) - assert.Contains(t, err.Error(), "acquire binary") + require.Len(t, exec.calls, 1, "only the version probe should run") + assert.Empty(t, exec.calls[0].stdin) } -func TestKubernetesDelivery_DeliverPostStart_ExecError(t *testing.T) { - execFn := func(_ context.Context, _ string, _ io.Reader, _ io.Writer, _ io.Writer) error { - return fmt.Errorf("exec failed") - } +func TestKubernetesDelivery_DeliverPostStart_DeliversWhenProbeErrors(t *testing.T) { + // A failing probe must not abort delivery; it should still attempt the write. + probeErr := &recordingExec{err: fmt.Errorf("probe boom")} + d := &KubernetesDelivery{Exec: probeErr.fn, ExpectedVersion: testVersion} - d := &KubernetesDelivery{ExecFunc: execFn} err := d.DeliverPostStart(context.Background(), PostStartOptions{ - BinarySource: fakeBinarySource, + BinarySource: binarySourceFrom("data"), + Arch: testArch, }) - require.Error(t, err) assert.Contains(t, err.Error(), "write binary to container") + assert.Len(t, probeErr.calls, 2) +} + +func TestKubernetesDelivery_DeliverPostStart_BinarySourceError(t *testing.T) { + exec := &recordingExec{stdouts: []string{""}} + d := &KubernetesDelivery{Exec: exec.fn} + err := d.DeliverPostStart(context.Background(), PostStartOptions{ + BinarySource: func(_ context.Context, _ string) (io.ReadCloser, error) { + return nil, fmt.Errorf("download failed") + }, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "acquire binary") } func TestKubernetesDelivery_Cleanup_IsNoOp(t *testing.T) { diff --git a/pkg/devcontainer/setup.go b/pkg/devcontainer/setup.go index ec73e4d5c..0c00749b3 100644 --- a/pkg/devcontainer/setup.go +++ b/pkg/devcontainer/setup.go @@ -87,7 +87,7 @@ func (r *runner) injectAgentIntoContainer(ctx context.Context, timeout time.Dura if strategy.Phase() == delivery.PhasePostStart { if err := r.deliverPostStart(ctx, strategy); err != nil { - log.Debugf("post-start delivery failed, falling back to legacy inject: %v", err) + log.Warnf("platform-native delivery failed, falling back to legacy inject: %v", err) return r.legacyInject(ctx, timeout) } return nil @@ -96,6 +96,17 @@ func (r *runner) injectAgentIntoContainer(ctx context.Context, timeout time.Dura return r.legacyInject(ctx, timeout) } +// podExecCapableDriver is implemented by the kubernetes driver, decoupling +// delivery wiring from the driver package. +type podExecCapableDriver interface { + CommandContainerArgv( + ctx context.Context, + workspaceID string, + argv []string, + streams driver.Streams, + ) error +} + func (r *runner) newAgentDelivery() delivery.AgentDelivery { dockerCmd := "docker" var dockerEnv []string @@ -108,6 +119,13 @@ func (r *runner) newAgentDelivery() delivery.AgentDelivery { execFn := delivery.CommandFunc(r.Driver.CommandDevContainer, r.ID) + var podExec delivery.PodExecFunc + if d, ok := r.Driver.(podExecCapableDriver); ok { + podExec = func(ctx context.Context, argv []string, streams driver.Streams) error { + return d.CommandContainerArgv(ctx, r.ID, argv, streams) + } + } + return delivery.NewAgentDelivery(delivery.FactoryOptions{ WorkspaceConfig: r.WorkspaceConfig, WorkspaceID: r.ID, @@ -116,9 +134,25 @@ func (r *runner) newAgentDelivery() delivery.AgentDelivery { HelperImage: r.WorkspaceConfig.Agent.Docker.HelperImage, ContainerID: r.ID, ExecFunc: execFn, + PodExec: podExec, }) } +// deliveryArch returns the target arch for the agent binary. The kubernetes +// cluster arch can differ from the host, so it takes precedence when available. +func (r *runner) deliveryArch(ctx context.Context) string { + arch := runtime.GOARCH + if r.WorkspaceConfig.Agent.Driver != provider2.KubernetesDriver { + return arch + } + if a, err := r.Driver.TargetArchitecture(ctx, r.ID); err == nil && a != "" { + return a + } else if err != nil { + log.Debugf("target architecture lookup failed, using host arch %q: %v", arch, err) + } + return arch +} + func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDelivery) error { binarySource, err := r.newBinarySource() if err != nil { @@ -128,7 +162,7 @@ func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDe err = strategy.DeliverPostStart(ctx, delivery.PostStartOptions{ WorkspaceID: r.ID, BinarySource: binarySource, - Arch: runtime.GOARCH, + Arch: r.deliveryArch(ctx), }) if err != nil { return fmt.Errorf("deliver agent (post-start): %w", err) diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 48f28a622..614a634bc 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "maps" - "runtime" "strings" "time" @@ -406,7 +405,7 @@ func (r *runner) deliverPreStart(ctx context.Context, runOptions *driver.RunOpti WorkspaceID: r.ID, RunOptions: runOptions, BinarySource: binarySource, - Arch: runtime.GOARCH, + Arch: r.deliveryArch(ctx), }) } diff --git a/pkg/driver/kubernetes/client.go b/pkg/driver/kubernetes/client.go index 4f04d6ef2..dc99ce30e 100644 --- a/pkg/driver/kubernetes/client.go +++ b/pkg/driver/kubernetes/client.go @@ -4,9 +4,11 @@ import ( "context" "fmt" "io" + "net/url" "os" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/httpstream" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -119,7 +121,7 @@ func (c *Client) Exec(ctx context.Context, options *ExecStreamOptions) error { Stderr: options.Stderr != nil, }, scheme.ParameterCodec) - exec, err := remotecommand.NewSPDYExecutor(c.config, "POST", execRequest.URL()) + exec, err := newFallbackExecutor(c.config, execRequest.URL()) if err != nil { return err } @@ -141,3 +143,19 @@ func (c *Client) Exec(ctx context.Context, options *ExecStreamOptions) error { return err } } + +// newFallbackExecutor prefers the WebSocket transport and falls back to SPDY, +// which is deprecated and disabled on newer API servers. +func newFallbackExecutor(config *rest.Config, url *url.URL) (remotecommand.Executor, error) { + spdyExec, err := remotecommand.NewSPDYExecutor(config, "POST", url) + if err != nil { + return nil, err + } + + wsExec, err := remotecommand.NewWebSocketExecutor(config, "GET", url.String()) + if err != nil { + return nil, err + } + + return remotecommand.NewFallbackExecutor(wsExec, spdyExec, httpstream.IsUpgradeFailure) +} diff --git a/pkg/driver/kubernetes/driver.go b/pkg/driver/kubernetes/driver.go index 8acfaf2fd..fc08684e5 100644 --- a/pkg/driver/kubernetes/driver.go +++ b/pkg/driver/kubernetes/driver.go @@ -184,6 +184,26 @@ func (k *KubernetesDriver) CommandDevContainer( }) } +// CommandContainerArgv execs argv in the workspace pod's dev container, streaming +// the given stdin/stdout/stderr. Unlike CommandDevContainer it takes explicit +// argv rather than wrapping the command in sh -c. +func (k *KubernetesDriver) CommandContainerArgv( + ctx context.Context, + workspaceID string, + argv []string, + streams driver.Streams, +) error { + return k.client.Exec(ctx, &ExecStreamOptions{ + Pod: getID(workspaceID), + Namespace: k.namespace, + Container: DevContainerName, + Command: argv, + Stdin: streams.Stdin, + Stdout: streams.Stdout, + Stderr: streams.Stderr, + }) +} + func (k *KubernetesDriver) GetDevContainerLogs( ctx context.Context, workspaceID string, diff --git a/pkg/driver/types.go b/pkg/driver/types.go index 988334deb..baf9b876a 100644 --- a/pkg/driver/types.go +++ b/pkg/driver/types.go @@ -56,6 +56,13 @@ type CommandParams struct { Stderr io.Writer } +// Streams bundles the standard IO streams for an exec. +type Streams struct { + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer +} + // RunOptions are the options for running a container. type RunOptions struct { // UID is a unique identifier for this workspace From 4988b9a91397a7c29ce179d841920a4548d4cbe5 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Wed, 24 Jun 2026 14:07:06 -0500 Subject: [PATCH 2/2] fix(kubernetes): surface cluster arch lookup failures; harden tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bubble up a failed TargetArchitecture lookup for the kubernetes delivery path instead of silently falling back to the host arch — streaming a wrong-arch binary would succeed at delivery but fail at agent start, after the legacy fallback can no longer recover. Also harden tests: the probe-error case now fails only the probe and verifies the write succeeds, and the factory tests assert the exec func is actually wired. --- pkg/agent/delivery/factory_test.go | 8 +++++-- pkg/agent/delivery/kubernetes_test.go | 20 +++++++++++------- pkg/devcontainer/setup.go | 30 +++++++++++++++++---------- pkg/devcontainer/single.go | 7 ++++++- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/pkg/agent/delivery/factory_test.go b/pkg/agent/delivery/factory_test.go index 9b08b69ea..cfad6049a 100644 --- a/pkg/agent/delivery/factory_test.go +++ b/pkg/agent/delivery/factory_test.go @@ -89,7 +89,9 @@ func TestNewAgentDelivery_KubernetesDriver_Native(t *testing.T) { } d := NewAgentDelivery(opts) - assert.IsType(t, &KubernetesDelivery{}, d) + native, ok := d.(*KubernetesDelivery) + require.True(t, ok) + assert.NotNil(t, native.Exec) assert.Equal(t, PhasePostStart, d.Phase()) } @@ -109,7 +111,9 @@ func TestNewAgentDelivery_KubernetesDriver_FallsBackWhenNoPodExec(t *testing.T) } d := NewAgentDelivery(opts) - assert.IsType(t, &LegacyShellDelivery{}, d) + legacy, ok := d.(*LegacyShellDelivery) + require.True(t, ok) + assert.NotNil(t, legacy.ExecFunc) assert.Equal(t, PhasePostStart, d.Phase()) } diff --git a/pkg/agent/delivery/kubernetes_test.go b/pkg/agent/delivery/kubernetes_test.go index 4bf82fddd..8ac355c7e 100644 --- a/pkg/agent/delivery/kubernetes_test.go +++ b/pkg/agent/delivery/kubernetes_test.go @@ -16,11 +16,12 @@ import ( const testVersion = "v1.2.3" -// recordingExec records each call and replays stdouts[N] as stdout on call N. +// recordingExec records each call, replays stdouts[N] as stdout, and returns +// errs[N] on call N (nil when unset). type recordingExec struct { calls []recordedCall stdouts []string - err error + errs []error } type recordedCall struct { @@ -40,7 +41,10 @@ func (r *recordingExec) fn(_ context.Context, argv []string, streams driver.Stre if streams.Stdout != nil && idx < len(r.stdouts) { _, _ = io.WriteString(streams.Stdout, r.stdouts[idx]) } - return r.err + if idx < len(r.errs) { + return r.errs[idx] + } + return nil } func binarySourceFrom(data string) BinarySourceFunc { @@ -120,16 +124,18 @@ func TestKubernetesDelivery_DeliverPostStart_SkipsWhenVersionMatches(t *testing. } func TestKubernetesDelivery_DeliverPostStart_DeliversWhenProbeErrors(t *testing.T) { - // A failing probe must not abort delivery; it should still attempt the write. - probeErr := &recordingExec{err: fmt.Errorf("probe boom")} + // A failing probe must not abort delivery; the write still succeeds. + probeErr := &recordingExec{ + stdouts: []string{""}, + errs: []error{fmt.Errorf("probe boom"), nil}, + } d := &KubernetesDelivery{Exec: probeErr.fn, ExpectedVersion: testVersion} err := d.DeliverPostStart(context.Background(), PostStartOptions{ BinarySource: binarySourceFrom("data"), Arch: testArch, }) - require.Error(t, err) - assert.Contains(t, err.Error(), "write binary to container") + require.NoError(t, err) assert.Len(t, probeErr.calls, 2) } diff --git a/pkg/devcontainer/setup.go b/pkg/devcontainer/setup.go index 0c00749b3..f42207f16 100644 --- a/pkg/devcontainer/setup.go +++ b/pkg/devcontainer/setup.go @@ -138,19 +138,22 @@ func (r *runner) newAgentDelivery() delivery.AgentDelivery { }) } -// deliveryArch returns the target arch for the agent binary. The kubernetes -// cluster arch can differ from the host, so it takes precedence when available. -func (r *runner) deliveryArch(ctx context.Context) string { - arch := runtime.GOARCH +// deliveryArch returns the target arch for the agent binary. For kubernetes the +// cluster arch can differ from the host, so a lookup failure is surfaced rather +// than guessing the host arch: streaming a wrong-arch binary would succeed here +// but fail when the agent starts, after the legacy fallback can no longer run. +func (r *runner) deliveryArch(ctx context.Context) (string, error) { if r.WorkspaceConfig.Agent.Driver != provider2.KubernetesDriver { - return arch + return runtime.GOARCH, nil } - if a, err := r.Driver.TargetArchitecture(ctx, r.ID); err == nil && a != "" { - return a - } else if err != nil { - log.Debugf("target architecture lookup failed, using host arch %q: %v", arch, err) + arch, err := r.Driver.TargetArchitecture(ctx, r.ID) + if err != nil { + return "", fmt.Errorf("resolve cluster architecture: %w", err) + } + if arch == "" { + return "", fmt.Errorf("cluster architecture is empty") } - return arch + return arch, nil } func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDelivery) error { @@ -159,10 +162,15 @@ func (r *runner) deliverPostStart(ctx context.Context, strategy delivery.AgentDe return fmt.Errorf("create binary source: %w", err) } + arch, err := r.deliveryArch(ctx) + if err != nil { + return err + } + err = strategy.DeliverPostStart(ctx, delivery.PostStartOptions{ WorkspaceID: r.ID, BinarySource: binarySource, - Arch: r.deliveryArch(ctx), + Arch: arch, }) if err != nil { return fmt.Errorf("deliver agent (post-start): %w", err) diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 614a634bc..34230f4e5 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -401,11 +401,16 @@ func (r *runner) deliverPreStart(ctx context.Context, runOptions *driver.RunOpti return fmt.Errorf("create binary source: %w", err) } + arch, err := r.deliveryArch(ctx) + if err != nil { + return err + } + return strategy.DeliverPreStart(ctx, delivery.PreStartOptions{ WorkspaceID: r.ID, RunOptions: runOptions, BinarySource: binarySource, - Arch: r.deliveryArch(ctx), + Arch: arch, }) }