fix: remove devsy managed workspace volumes

cmd/internal/agentworkspace/delete.go

@@ -2,6 +2,7 @@ package agentworkspace
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -56,7 +57,7 @@ func NewDeleteCmd(flags *flags.GlobalFlags) *cobra.Command {
 			&cmd.RemoveVolumes,
 			names.RemoveVolumes,
 			false,
-			"Remove named volumes associated with the workspace",
+			"Remove declared volumes when deleting docker compose workspaces",
 		),
 		cliflags.String(&cmd.WorkspaceInfo, names.WorkspaceInfo, "", "The workspace info"),
 	)
@@ -65,7 +66,6 @@ func NewDeleteCmd(flags *flags.GlobalFlags) *cobra.Command {
 }
 
 func (cmd *DeleteCmd) Run(ctx context.Context) error {
-	// get workspace
 	shouldExit, workspaceInfo, err := agent.WorkspaceInfo(
 		cmd.WorkspaceInfo,
 	)
@@ -75,25 +75,21 @@ func (cmd *DeleteCmd) Run(ctx context.Context) error {
 		return nil
 	}
 
-	// remove daemon
+	var errs []error
 	if cmd.Daemon {
-		err = removeDaemon(workspaceInfo)
-		if err != nil {
-			return fmt.Errorf("remove daemon: %w", err)
+		if err := removeDaemon(workspaceInfo); err != nil {
+			errs = append(errs, fmt.Errorf("remove daemon: %w", err))
 		}
 	}
-
-	// cleanup docker container
 	if cmd.Container {
-		err = removeContainer(ctx, workspaceInfo, cmd.RemoveVolumes)
-		if err != nil {
-			return fmt.Errorf("remove container: %w", err)
+		if err := removeContainer(ctx, workspaceInfo, cmd.RemoveVolumes); err != nil {
+			errs = append(errs, fmt.Errorf("remove container: %w", err))
 		}
 	}
 
 	removeWorkspaceFolders(workspaceInfo)
 
-	return nil
+	return errors.Join(errs...)
 }
 
 // removeWorkspaceFolders deletes the workspace's config folder and cached
@@ -129,17 +125,12 @@ func removeContainer(
 		return err
 	}
 
-	if workspaceInfo.Workspace.Source.Container != "" {
-		log.Info("skipping container deletion, since it was not created by Devsy")
-	} else {
-		err = runner.Delete(ctx, devcontainer.DeleteOptions{
-			RemoveVolumes: removeVolumes,
-		})
-		if err != nil {
-			return err
-		}
-		log.Debug("removed Devsy container from server")
+	if err := runner.Delete(ctx, devcontainer.DeleteOptions{
+		RemoveVolumes: removeVolumes,
+	}); err != nil {
+		return err
 	}
+	log.Debug("removed Devsy container from server")
 
 	return nil
 }

cmd/workspace/delete.go

@@ -59,7 +59,8 @@ Use --ignore-not-found to treat a missing workspace as success.`,
 		cliflags.Bool(&cmd.Force, names.Force, false,
 			"Delete workspace even if it is not found remotely anymore"),
 		cliflags.Bool(&cmd.RemoveVolumes, names.RemoveVolumes, false,
-			"Remove named volumes associated with the workspace"),
+			"Remove named volumes associated with the workspace "+
+				"(docker compose only)"),
 	)
 	return deleteCmd
 }

e2e/tests/down/down.go

@@ -4,11 +4,14 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"strings"
 
 	"github.com/devsy-org/devsy/e2e/framework"
 	pkgconfig "github.com/devsy-org/devsy/pkg/config"
 	docker "github.com/devsy-org/devsy/pkg/docker"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/mount"
 	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 )
@@ -138,5 +141,51 @@ var _ = ginkgo.Describe(
 				"container should still exist after stop (only stopped, not deleted)",
 			)
 		}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
+
+		ginkgo.It("workspace delete removes anonymous volumes declared by the image",
+			func(ctx context.Context) {
+				f, err := framework.SetupDockerProvider(initialDir+"/bin", "docker")
+				framework.ExpectNoError(err)
+
+				tempDir, err := framework.CopyToTempDir("tests/down/testdata/docker-anon-volume")
+				framework.ExpectNoError(err)
+				ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir)
+
+				err = f.DevsyUp(ctx, tempDir)
+				framework.ExpectNoError(err)
+
+				workspace, err := f.FindWorkspace(ctx, tempDir)
+				framework.ExpectNoError(err)
+				ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, tempDir)
+
+				ids, err := dockerHelper.FindContainer(ctx, []string{
+					fmt.Sprintf("%s=%s", pkgconfig.DevcontainerIDLabel, workspace.UID),
+				})
+				framework.ExpectNoError(err)
+				gomega.Expect(ids).NotTo(gomega.BeEmpty())
+
+				var details []container.InspectResponse
+				err = dockerHelper.Inspect(ctx, ids, "container", &details)
+				framework.ExpectNoError(err)
+
+				var volumeName string
+				for _, m := range details[0].Mounts {
+					if m.Type == mount.TypeVolume && m.Destination == "/data" {
+						volumeName = m.Name
+					}
+				}
+				gomega.Expect(volumeName).NotTo(gomega.BeEmpty(),
+					"container should have an anonymous volume mounted at /data")
+
+				err = f.DevsyWorkspaceDelete(ctx, tempDir)
+				framework.ExpectNoError(err)
+
+				// #nosec G204
+				cmd := exec.CommandContext(ctx, "docker", "volume", "inspect", volumeName)
+				out, _ := cmd.CombinedOutput()
+				gomega.Expect(strings.ToLower(string(out))).To(
+					gomega.ContainSubstring("no such volume"),
+					"anonymous volume should be removed along with its container")
+			}, ginkgo.SpecTimeout(framework.TimeoutModerate()))
 	},
 )

e2e/tests/down/testdata/docker-anon-volume/.devcontainer.json

@@ -0,0 +1,6 @@
+{
+  "name": "anon-volume",
+  "build": {
+    "dockerfile": "Dockerfile"
+  }
+}

e2e/tests/down/testdata/docker-anon-volume/Dockerfile

@@ -0,0 +1,2 @@
+FROM ghcr.io/devsy-org/test-images/base:alpine
+VOLUME /data

pkg/agent/delivery/local_docker.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	pkgconfig "github.com/devsy-org/devsy/pkg/config"
@@ -75,17 +76,17 @@ func (d *LocalDockerDelivery) DeliverPostStart(_ context.Context, _ PostStartOpt
 	return fmt.Errorf("LocalDockerDelivery does not support post-start delivery")
 }
 
-// Cleanup removes every devsy-managed volume owned by the workspace (the agent
-// volume and any seeded workspace volume), identified by labels. Only labeled
-// volumes are removed, so foreign/external volumes are left untouched.
+// only labeled devsy-managed volumes and the canonical agent volume are
+// removed.
 func (d *LocalDockerDelivery) Cleanup(ctx context.Context, workspaceID string) error {
 	volumes, err := d.listManagedVolumes(ctx, workspaceID)
 	if err != nil {
 		return err
 	}
-	// Attempt every volume so one transient failure does not orphan the rest.
 	var errs []error
-	for _, name := range volumes {
+	volumes = append(volumes, volumePrefix+workspaceID)
+	slices.Sort(volumes)
+	for _, name := range slices.Compact(volumes) {
 		if err := d.removeVolume(ctx, name); err != nil {
 			errs = append(errs, err)
 			continue

pkg/agent/delivery/local_docker_test.go

@@ -482,6 +482,29 @@ func TestLocalDockerDelivery_Cleanup_RemovesManagedVolumes(t *testing.T) {
 	assert.Contains(t, removed, "ws1-workspace")
 }
 
+func TestLocalDockerDelivery_Cleanup_RemovesLegacyUnlabeledAgentVolume(t *testing.T) {
+	tmpDir := t.TempDir()
+	logPath := filepath.Join(tmpDir, "calls.log")
+
+	scriptPath := filepath.Join(tmpDir, "fake-docker.sh")
+	script := "#!/bin/sh\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"ls\" ]; then exit 0; fi\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"rm\" ]; then\n" +
+		"  echo \"$@\" >> \"" + logPath + "\"; exit 0\n" +
+		"fi\n" +
+		"exit 0\n"
+	require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600))
+	// #nosec G302
+	require.NoError(t, os.Chmod(scriptPath, 0o755))
+
+	d := &LocalDockerDelivery{DockerCommand: scriptPath}
+	require.NoError(t, d.Cleanup(context.Background(), "ws-legacy"))
+
+	logged, err := os.ReadFile(logPath) //nolint:gosec
+	require.NoError(t, err)
+	assert.Equal(t, "volume rm -f devsy-agent-ws-legacy\n", string(logged))
+}
+
 func TestLocalDockerDelivery_SeedExcludesBuildInternal(t *testing.T) {
 	tmpDir := t.TempDir()
 	logPath := filepath.Join(tmpDir, "run.log")

pkg/devcontainer/delete.go

@@ -9,51 +9,23 @@ import (
 )
 
 func (r *runner) Delete(ctx context.Context, options DeleteOptions) error {
-	containerDetails, err := r.driver.FindDevContainer(ctx, r.id)
+	details, err := r.findTeardownContainer(ctx)
 	if err != nil {
 		return fmt.Errorf("find dev container: %w", err)
 	}
-	defer r.cleanupDeliveryVolume(ctx)
-	defer r.cleanupImportedDevContainer()
-	if containerDetails == nil {
-		return nil
-	}
-
-	log.Infof("deleting devcontainer: devcontainerID=%s", containerDetails.ID)
-	if isDockerCompose, projectName := getDockerComposeProject(containerDetails); isDockerCompose {
-		return r.deleteDockerCompose(ctx, projectName, options.RemoveVolumes)
+	if details != nil {
+		log.Infof("deleting devcontainer: devcontainerID=%s", details.ID)
 	}
-	return r.stopAndDeleteContainer(ctx, containerDetails)
+	return r.buildTeardownPlan(details, options).execute(ctx)
 }
 
-// stopAndDeleteContainer stops containerDetails' devcontainer if it
-// is running, then deletes it.
-func (r *runner) stopAndDeleteContainer(
-	ctx context.Context, containerDetails *config.ContainerDetails,
-) error {
-	if containerDetails.State.Status == config.ContainerStatusRunning {
-		if err := r.driver.StopDevContainer(ctx, r.id); err != nil {
-			return err
-		}
-	}
-	return r.driver.DeleteDevContainer(ctx, r.id)
-}
-
-func (r *runner) cleanupDeliveryVolume(ctx context.Context) {
-	if err := r.newAgentDelivery().Cleanup(ctx, r.id); err != nil {
-		log.Debugf("delivery volume cleanup: %v", err)
-	}
-}
-
-func (r *runner) cleanupImportedDevContainer() {
+func (r *runner) cleanupImportedDevContainer() error {
 	if r.workspaceConfig == nil ||
 		r.workspaceConfig.Workspace == nil ||
 		r.workspaceConfig.Workspace.Source.LocalFolder == "" {
-		return
-	}
-	if err := CleanupImportedDevContainers(r.localWorkspaceFolder); err != nil {
-		log.Debugf("imported devcontainer cleanup: %v", err)
+		return nil
 	}
+	return CleanupImportedDevContainers(r.localWorkspaceFolder)
 }
 
 func (r *runner) Stop(ctx context.Context) error {