Clean up volumes if workspace preparation fails (#1021)

Author: lowjoel

internal/batches/workspace/volume_workspace.go

@@ -30,16 +30,22 @@ type dockerVolumeWorkspaceCreator struct {
 
 var _ Creator = &dockerVolumeWorkspaceCreator{}
 
-func (wc *dockerVolumeWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, steps []batcheslib.Step, archive repozip.Archive) (Workspace, error) {
+func (wc *dockerVolumeWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository,
+	steps []batcheslib.Step, archive repozip.Archive) (ws Workspace, err error) {
 	volume, err := wc.createVolume(ctx)
 	if err != nil {
 		return nil, errors.Wrap(err, "creating Docker volume")
 	}
 
+	defer func() {
+		if err != nil {
+			deleteVolume(ctx, volume)
+		}
+	}()
+
 	// Figure out the user that containers will be run as.
 	ug := docker.UIDGID{}
 	if len(steps) > 0 {
-		var err error
 		img, err := wc.EnsureImage(ctx, steps[0].Container)
 		if err != nil {
 			return nil, err
@@ -74,6 +80,10 @@ func (*dockerVolumeWorkspaceCreator) createVolume(ctx context.Context) (string,
 	return string(bytes.TrimSpace(out)), nil
 }
 
+func deleteVolume(ctx context.Context, volume string) error {
+	return exec.CommandContext(ctx, "docker", "volume", "rm", volume).Run()
+}
+
 func (*dockerVolumeWorkspaceCreator) prepareGitRepo(ctx context.Context, w *dockerVolumeWorkspace) error {
 	script := `#!/bin/sh
 	
@@ -219,7 +229,7 @@ var _ Workspace = &dockerVolumeWorkspace{}
 
 func (w *dockerVolumeWorkspace) Close(ctx context.Context) error {
 	// Cleanup here is easy: we just get rid of the Docker volume.
-	return exec.CommandContext(ctx, "docker", "volume", "rm", w.volume).Run()
+	return deleteVolume(ctx, w.volume)
 }
 
 func (w *dockerVolumeWorkspace) DockerRunOpts(ctx context.Context, target string) ([]string, error) {

internal/batches/workspace/volume_workspace_test.go

@@ -211,6 +211,10 @@ func TestVolumeWorkspaceCreator(t *testing.T) {
 					DockerVolumeWorkspaceImage,
 					"sh", "-c", "touch /work/*; chown -R 0:0 /work",
 				),
+				expect.NewGlob(
+					expect.Behaviour{ExitCode: 0},
+					"docker", "volume", "rm", volumeID,
+				),
 			},
 			steps: []batcheslib.Step{
 				{},
@@ -253,6 +257,10 @@ func TestVolumeWorkspaceCreator(t *testing.T) {
 					DockerVolumeWorkspaceImage,
 					"sh", "/run.sh",
 				),
+				expect.NewGlob(
+					expect.Behaviour{ExitCode: 0},
+					"docker", "volume", "rm", volumeID,
+				),
 			},
 			steps: []batcheslib.Step{
 				{},
@@ -286,6 +294,10 @@ func TestVolumeWorkspaceCreator(t *testing.T) {
 					DockerVolumeWorkspaceImage,
 					"sh", "-c", "unzip /tmp/zip; rm /work/*",
 				),
+				expect.NewGlob(
+					expect.Behaviour{ExitCode: 0},
+					"docker", "volume", "rm", volumeID,
+				),
 			},
 			steps: []batcheslib.Step{
 				{},