From 3d50185b172c1c04cc51dd0e338b1e5f42fd9935 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 18 May 2026 17:49:55 -0500 Subject: [PATCH 1/3] feat: auto-overwrite agent binary on version mismatch in Docker delivery When the named Docker volume already contains an agent binary with a different version, force-overwrite it instead of leaving the stale binary in place. Logs an INFO message with the old and new versions. If versions match, skip re-delivery entirely as an optimization. --- pkg/agent/delivery/local_docker.go | 73 +++++++++-- pkg/agent/delivery/local_docker_test.go | 166 +++++++++++++++++++++++- 2 files changed, 230 insertions(+), 9 deletions(-) diff --git a/pkg/agent/delivery/local_docker.go b/pkg/agent/delivery/local_docker.go index 3149b3a24..74a7a7cc7 100644 --- a/pkg/agent/delivery/local_docker.go +++ b/pkg/agent/delivery/local_docker.go @@ -13,6 +13,7 @@ import ( "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/log" + "github.com/devsy-org/devsy/pkg/version" ) var _ AgentDelivery = (*LocalDockerDelivery)(nil) @@ -26,9 +27,10 @@ const ( ) type LocalDockerDelivery struct { - DockerCommand string - Environment []string - HelperImage string + DockerCommand string + Environment []string + HelperImage string + ExpectedVersion string } func (d *LocalDockerDelivery) Phase() DeliveryPhase { @@ -46,11 +48,8 @@ func (d *LocalDockerDelivery) DeliverPreStart(ctx context.Context, opts PreStart return fmt.Errorf("create agent volume: %w", err) } - if err := d.populateVolume(ctx, volumeName, opts.BinarySource, opts.Arch); err != nil { - if removeErr := d.removeVolume(ctx, volumeName); removeErr != nil { - log.Debugf("failed to clean up volume after populate failure: %v", removeErr) - } - return fmt.Errorf("populate agent volume: %w", err) + if err := d.ensureCurrentBinary(ctx, volumeName, opts.BinarySource, opts.Arch); err != nil { + return err } opts.RunOptions.Mounts = append(opts.RunOptions.Mounts, &config.Mount{ @@ -75,6 +74,36 @@ func (d *LocalDockerDelivery) Cleanup(ctx context.Context, workspaceID string) e return d.removeVolume(ctx, workspaceID) } +func (d *LocalDockerDelivery) ensureCurrentBinary( + ctx context.Context, + volumeName string, + binarySource BinarySourceFunc, + arch string, +) error { + expected := d.expectedVersion() + actual := d.detectVolumeVersion(ctx, volumeName) + + if actual != "" && actual == expected { + log.Debugf( + "remote agent version matches expected version %s, skipping delivery", + expected, + ) + return nil + } + + if actual != "" { + log.Infof("upgraded remote agent from %s → %s", actual, expected) + } + + if err := d.populateVolume(ctx, volumeName, binarySource, arch); err != nil { + if removeErr := d.removeVolume(ctx, volumeName); removeErr != nil { + log.Debugf("failed to clean up volume after populate failure: %v", removeErr) + } + return fmt.Errorf("populate agent volume: %w", err) + } + return nil +} + func (d *LocalDockerDelivery) createVolume(ctx context.Context, name string) error { out, err := d.cmd(ctx, "volume", "create", name).CombinedOutput() if err != nil { @@ -90,6 +119,34 @@ func (d *LocalDockerDelivery) helperImageName() string { return defaultHelperImage } +func (d *LocalDockerDelivery) expectedVersion() string { + if d.ExpectedVersion != "" { + return d.ExpectedVersion + } + return version.GetVersion() +} + +func (d *LocalDockerDelivery) detectVolumeVersion(ctx context.Context, volumeName string) string { + binaryPath := volumeMountPath + "/" + binaryName() + script := fmt.Sprintf( + `[ -x "%s" ] && "%s" version 2>/dev/null || true`, + binaryPath, binaryPath, + ) + args := []string{ + "run", "--rm", + "-v", volumeName + ":" + volumeMountPath, + d.helperImageName(), + "sh", "-c", script, + } + + out, err := d.cmd(ctx, args...).CombinedOutput() + if err != nil { + log.Debugf("failed to detect agent version in volume: %v", err) + return "" + } + return strings.TrimSpace(string(out)) +} + func (d *LocalDockerDelivery) populateVolume( ctx context.Context, volumeName string, diff --git a/pkg/agent/delivery/local_docker_test.go b/pkg/agent/delivery/local_docker_test.go index 744fcb46c..8bf425200 100644 --- a/pkg/agent/delivery/local_docker_test.go +++ b/pkg/agent/delivery/local_docker_test.go @@ -8,11 +8,14 @@ import ( "path/filepath" "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" ) +const testArch = "amd64" + func TestLocalDockerDelivery_Phase(t *testing.T) { d := &LocalDockerDelivery{} assert.Equal(t, PhasePreStart, d.Phase()) @@ -112,7 +115,7 @@ func TestPopulateVolume_FallbackToDirectCopy(t *testing.T) { DockerCommand: scriptPath, } - err := d.populateVolume(context.Background(), "test-vol", binarySource, "amd64") + err := d.populateVolume(context.Background(), "test-vol", binarySource, testArch) require.NoError(t, err) destPath := filepath.Join(mountDir, binaryName()) @@ -222,3 +225,164 @@ func TestPopulateVolumeViaUnshare_FailureReturnsError(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "podman unshare write failed") } + +func TestDetectVolumeVersion_ReturnsVersionFromHelper(t *testing.T) { + tmpDir := t.TempDir() + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " run) echo \"v1.2.3\" ;;\n" + + " *) exit 1 ;;\n" + + "esac\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + d := &LocalDockerDelivery{DockerCommand: scriptPath} + ver := d.detectVolumeVersion(context.Background(), "test-vol") + assert.Equal(t, "v1.2.3", ver) +} + +func TestDetectVolumeVersion_ReturnsEmptyOnFailure(t *testing.T) { + tmpDir := t.TempDir() + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + script := "#!/bin/sh\nexit 1\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + d := &LocalDockerDelivery{DockerCommand: scriptPath} + ver := d.detectVolumeVersion(context.Background(), "test-vol") + assert.Empty(t, ver) +} + +func TestDetectVolumeVersion_ReturnsEmptyWhenNoBinary(t *testing.T) { + tmpDir := t.TempDir() + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " run) echo \"\" ;;\n" + + " *) exit 1 ;;\n" + + "esac\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + d := &LocalDockerDelivery{DockerCommand: scriptPath} + ver := d.detectVolumeVersion(context.Background(), "test-vol") + assert.Empty(t, ver) +} + +func TestDeliverPreStart_VersionMatch_SkipsPopulate(t *testing.T) { + tmpDir := t.TempDir() + + populateCalled := false + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + markerPath := filepath.Join(tmpDir, "populate-called") + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " volume) echo \"ok\" ;;\n" + + " run)\n" + + " if echo \"$@\" | grep -q \"\\-i\"; then\n" + + " touch \"" + markerPath + "\"\n" + + " cat > /dev/null\n" + + " else\n" + + " echo \"v2.0.0\"\n" + + " fi\n" + + " ;;\n" + + " *) exit 1 ;;\n" + + "esac\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + binarySource := func(_ context.Context, _ string) (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader([]byte("binary"))), nil + } + + d := &LocalDockerDelivery{ + DockerCommand: scriptPath, + ExpectedVersion: "v2.0.0", + } + + opts := PreStartOptions{ + WorkspaceID: "test-ws", + RunOptions: &driver.RunOptions{}, + BinarySource: binarySource, + Arch: testArch, + } + + err := d.DeliverPreStart(context.Background(), opts) + require.NoError(t, err) + + _, statErr := os.Stat(markerPath) + populateCalled = statErr == nil + assert.False(t, populateCalled, "populateVolume should not be called when versions match") +} + +func TestDeliverPreStart_VersionMismatch_Overwrites(t *testing.T) { + tmpDir := t.TempDir() + mountDir := filepath.Join(tmpDir, "mount") + require.NoError(t, os.MkdirAll(mountDir, 0o750)) + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " volume)\n" + + " case \"$2\" in\n" + + " create) echo \"ok\" ;;\n" + + " inspect) echo \"" + mountDir + "\" ;;\n" + + " *) exit 1 ;;\n" + + " esac\n" + + " ;;\n" + + " run)\n" + + " if echo \"$@\" | grep -q \"\\-i\"; then\n" + + " echo \"image not found\" >&2; exit 1\n" + + " else\n" + + " echo \"v1.0.0\"\n" + + " fi\n" + + " ;;\n" + + " *) exit 1 ;;\n" + + "esac\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + binaryContent := []byte("new-agent-binary") + binarySource := func(_ context.Context, _ string) (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(binaryContent)), nil + } + + d := &LocalDockerDelivery{ + DockerCommand: scriptPath, + ExpectedVersion: "v2.0.0", + } + + opts := PreStartOptions{ + WorkspaceID: "test-ws", + RunOptions: &driver.RunOptions{}, + BinarySource: binarySource, + Arch: testArch, + } + + err := d.DeliverPreStart(context.Background(), opts) + require.NoError(t, err) + + destPath := filepath.Join(mountDir, binaryName()) + data, err := os.ReadFile(destPath) //nolint:gosec // test reads from a temp directory we control + require.NoError(t, err) + assert.Equal(t, binaryContent, data) +} + +func TestExpectedVersion_UsesFieldWhenSet(t *testing.T) { + d := &LocalDockerDelivery{ExpectedVersion: "v3.0.0"} + assert.Equal(t, "v3.0.0", d.expectedVersion()) +} + +func TestExpectedVersion_FallsBackToGetVersion(t *testing.T) { + d := &LocalDockerDelivery{} + assert.NotEmpty(t, d.expectedVersion()) +} From 6886369cbc979a7a3ca71822c527304ebc828932 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 18 May 2026 18:01:26 -0500 Subject: [PATCH 2/3] feat: add `devsy workspace clean` subcommand Adds a command to remove the agent binary from the Docker named volume for a workspace, forcing a fresh injection on next start. Useful when the binary becomes stale and the automatic version-mismatch detection is not sufficient. --- cmd/agent/workspace/clean.go | 118 ++++++++++++++++++++++++++++ cmd/agent/workspace/clean_test.go | 124 ++++++++++++++++++++++++++++++ cmd/agent/workspace/workspace.go | 1 + 3 files changed, 243 insertions(+) create mode 100644 cmd/agent/workspace/clean.go create mode 100644 cmd/agent/workspace/clean_test.go diff --git a/cmd/agent/workspace/clean.go b/cmd/agent/workspace/clean.go new file mode 100644 index 000000000..c649cde79 --- /dev/null +++ b/cmd/agent/workspace/clean.go @@ -0,0 +1,118 @@ +package workspace + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "github.com/devsy-org/devsy/cmd/flags" + "github.com/devsy-org/devsy/pkg/log" + "github.com/spf13/cobra" +) + +const ( + cleanVolumePrefix = "devsy-agent-" + cleanVolumeMountPath = "/opt/devsy" + cleanBinaryName = "devsy" + cleanHelperImage = "busybox:latest" +) + +// CleanCmd holds the cmd flags. +type CleanCmd struct { + *flags.GlobalFlags + + DockerCommand string + HelperImage string +} + +// NewCleanCmd creates a new command. +func NewCleanCmd(globalFlags *flags.GlobalFlags) *cobra.Command { + cmd := &CleanCmd{ + GlobalFlags: globalFlags, + } + cleanCmd := &cobra.Command{ + Use: "clean [workspace-id]", + Short: "Removes the agent binary from the Docker volume for a workspace", + Long: `Removes the agent binary from the Docker named volume for the specified workspace. +This forces a fresh binary injection on the next workspace start.`, + Args: cobra.ExactArgs(1), + RunE: func(cobraCmd *cobra.Command, args []string) error { + return cmd.Run(cobraCmd.Context(), args[0]) + }, + } + cleanCmd.Flags(). + StringVar(&cmd.DockerCommand, "docker-command", "docker", "Docker command to use") + cleanCmd.Flags(). + StringVar(&cmd.HelperImage, "helper-image", cleanHelperImage, "Helper image for volume operations") + return cleanCmd +} + +func (cmd *CleanCmd) Run(ctx context.Context, workspaceID string) error { + if workspaceID == "" { + return fmt.Errorf("workspace ID must not be empty") + } + + volumeName := cleanVolumePrefix + workspaceID + log.Infof("Removing agent binary from volume %s", volumeName) + + if err := cmd.removeBinaryFromVolume(ctx, volumeName); err != nil { + return fmt.Errorf("remove agent binary from volume %s: %w", volumeName, err) + } + + log.Infof("Successfully removed agent binary from volume %s", volumeName) + return nil +} + +func (cmd *CleanCmd) removeBinaryFromVolume(ctx context.Context, volumeName string) error { + if err := cmd.checkVolumeExists(ctx, volumeName); err != nil { + return err + } + + binaryPath := cleanVolumeMountPath + "/" + cleanBinaryName + script := fmt.Sprintf(`rm -f "%s"`, binaryPath) + args := []string{ + "run", "--rm", + "-v", volumeName + ":" + cleanVolumeMountPath, + cmd.helperImage(), + "sh", "-c", script, + } + + out, err := cmd.dockerCmd(ctx, args...).CombinedOutput() + if err != nil { + return fmt.Errorf("docker run failed: %s: %w", strings.TrimSpace(string(out)), err) + } + return nil +} + +func (cmd *CleanCmd) checkVolumeExists(ctx context.Context, volumeName string) error { + out, err := cmd.dockerCmd(ctx, "volume", "inspect", volumeName).CombinedOutput() + if err != nil { + return fmt.Errorf( + "volume %s not found (is Docker running?): %s: %w", + volumeName, + strings.TrimSpace(string(out)), + err, + ) + } + return nil +} + +func (cmd *CleanCmd) dockerCmd(ctx context.Context, args ...string) *exec.Cmd { + // #nosec G204 -- args are constructed internally, not from user input + return exec.CommandContext(ctx, cmd.dockerCommand(), args...) +} + +func (cmd *CleanCmd) dockerCommand() string { + if cmd.DockerCommand != "" { + return cmd.DockerCommand + } + return "docker" +} + +func (cmd *CleanCmd) helperImage() string { + if cmd.HelperImage != "" { + return cmd.HelperImage + } + return cleanHelperImage +} diff --git a/cmd/agent/workspace/clean_test.go b/cmd/agent/workspace/clean_test.go new file mode 100644 index 000000000..70877bdb9 --- /dev/null +++ b/cmd/agent/workspace/clean_test.go @@ -0,0 +1,124 @@ +package workspace + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/devsy-org/devsy/cmd/flags" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCleanCmd_EmptyWorkspaceID(t *testing.T) { + cmd := &CleanCmd{GlobalFlags: &flags.GlobalFlags{}} + err := cmd.Run(context.Background(), "") + require.Error(t, err) + assert.Contains(t, err.Error(), "must not be empty") +} + +func TestCleanCmd_VolumeNotFound(t *testing.T) { + tmpDir := t.TempDir() + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + script := "#!/bin/sh\necho 'no such volume' >&2; exit 1\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + cmd := &CleanCmd{ + GlobalFlags: &flags.GlobalFlags{}, + DockerCommand: scriptPath, + } + err := cmd.Run(context.Background(), "nonexistent-ws") + require.Error(t, err) + assert.Contains(t, err.Error(), "not found") +} + +func TestCleanCmd_RemoveBinarySuccess(t *testing.T) { + tmpDir := t.TempDir() + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + markerPath := filepath.Join(tmpDir, "rm-called") + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " volume) echo '{}' ;;\n" + + " run) touch \"" + markerPath + "\" ;;\n" + + " *) exit 1 ;;\n" + + "esac\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + cmd := &CleanCmd{ + GlobalFlags: &flags.GlobalFlags{}, + DockerCommand: scriptPath, + } + err := cmd.Run(context.Background(), "test-workspace-123") + require.NoError(t, err) + + _, statErr := os.Stat(markerPath) + assert.NoError(t, statErr, "docker run should have been called to remove the binary") +} + +func TestCleanCmd_DockerRunFails(t *testing.T) { + tmpDir := t.TempDir() + + scriptPath := filepath.Join(tmpDir, "fake-docker.sh") + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " volume) echo '{}' ;;\n" + + " run) echo 'container error' >&2; exit 1 ;;\n" + + " *) exit 1 ;;\n" + + "esac\n" + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + // #nosec G302 -- test script must be executable + require.NoError(t, os.Chmod(scriptPath, 0o755)) + + cmd := &CleanCmd{ + GlobalFlags: &flags.GlobalFlags{}, + DockerCommand: scriptPath, + } + err := cmd.Run(context.Background(), "test-workspace") + require.Error(t, err) + assert.Contains(t, err.Error(), "docker run failed") +} + +func TestCleanCmd_VolumeName(t *testing.T) { + assert.Equal(t, "devsy-agent-", cleanVolumePrefix) + assert.Equal(t, "devsy-agent-my-ws", cleanVolumePrefix+"my-ws") +} + +func TestCleanCmd_HelperImage_Default(t *testing.T) { + cmd := &CleanCmd{GlobalFlags: &flags.GlobalFlags{}} + assert.Equal(t, "busybox:latest", cmd.helperImage()) +} + +func TestCleanCmd_HelperImage_Custom(t *testing.T) { + cmd := &CleanCmd{ + GlobalFlags: &flags.GlobalFlags{}, + HelperImage: "alpine:latest", + } + assert.Equal(t, "alpine:latest", cmd.helperImage()) +} + +func TestCleanCmd_DockerCommand_Default(t *testing.T) { + cmd := &CleanCmd{GlobalFlags: &flags.GlobalFlags{}} + assert.Equal(t, "docker", cmd.dockerCommand()) +} + +func TestCleanCmd_DockerCommand_Custom(t *testing.T) { + cmd := &CleanCmd{ + GlobalFlags: &flags.GlobalFlags{}, + DockerCommand: "podman", + } + assert.Equal(t, "podman", cmd.dockerCommand()) +} + +func TestNewCleanCmd_CobraSetup(t *testing.T) { + cobraCmd := NewCleanCmd(&flags.GlobalFlags{}) + assert.Equal(t, "clean [workspace-id]", cobraCmd.Use) + assert.NotEmpty(t, cobraCmd.Short) + assert.NotEmpty(t, cobraCmd.Long) +} diff --git a/cmd/agent/workspace/workspace.go b/cmd/agent/workspace/workspace.go index 15378e4d9..dfdfe9261 100644 --- a/cmd/agent/workspace/workspace.go +++ b/cmd/agent/workspace/workspace.go @@ -22,5 +22,6 @@ func NewWorkspaceCmd(flags *flags.GlobalFlags) *cobra.Command { workspaceCmd.AddCommand(NewInstallDotfilesCmd(flags)) workspaceCmd.AddCommand(NewSetupGPGCmd(flags)) workspaceCmd.AddCommand(NewLogsCmd(flags)) + workspaceCmd.AddCommand(NewCleanCmd(flags)) return workspaceCmd } From ebf6a65585d4d67368932760f16f116becb2e7b1 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 18 May 2026 18:11:27 -0500 Subject: [PATCH 3/3] fix: extract docker command string to constant (goconst lint) --- cmd/agent/workspace/clean.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/agent/workspace/clean.go b/cmd/agent/workspace/clean.go index c649cde79..10e2e9ada 100644 --- a/cmd/agent/workspace/clean.go +++ b/cmd/agent/workspace/clean.go @@ -12,10 +12,11 @@ import ( ) const ( - cleanVolumePrefix = "devsy-agent-" - cleanVolumeMountPath = "/opt/devsy" - cleanBinaryName = "devsy" - cleanHelperImage = "busybox:latest" + cleanVolumePrefix = "devsy-agent-" + cleanVolumeMountPath = "/opt/devsy" + cleanBinaryName = "devsy" + cleanHelperImage = "busybox:latest" + cleanDefaultDockerCmd = "docker" ) // CleanCmd holds the cmd flags. @@ -42,7 +43,7 @@ This forces a fresh binary injection on the next workspace start.`, }, } cleanCmd.Flags(). - StringVar(&cmd.DockerCommand, "docker-command", "docker", "Docker command to use") + StringVar(&cmd.DockerCommand, "docker-command", cleanDefaultDockerCmd, "Docker command to use") cleanCmd.Flags(). StringVar(&cmd.HelperImage, "helper-image", cleanHelperImage, "Helper image for volume operations") return cleanCmd @@ -107,7 +108,7 @@ func (cmd *CleanCmd) dockerCommand() string { if cmd.DockerCommand != "" { return cmd.DockerCommand } - return "docker" + return cleanDefaultDockerCmd } func (cmd *CleanCmd) helperImage() string {