campaigns: fall back to bind workspaces when running steps with mixed users

.goreleaser.yml

@@ -12,7 +12,7 @@ builds:
   main: ./cmd/src/
   binary: src
   ldflags:
-    - -X main.buildTag={{.Version}}
+    - -X github.com/sourcegraph/src-cli/internal/version.BuildTag={{.Version}}
   goos:
     - linux
     - windows

CHANGELOG.md

@@ -23,7 +23,7 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Added
 
-- `src campaign [apply|preview]` can now make use of Docker volumes, rather than bind-mounting the host filesystem. This is now the default on macOS, as volume mounts have generally better performance there. The optional `-workspace` flag can be used to override the default. [#412](https://github.com/sourcegraph/src-cli/pull/412)
+- `src campaign [apply|preview]` can now make use of Docker volumes, rather than bind-mounting the host filesystem. This is now the default on Intel macOS so long as the Docker images used in the campaign steps run as the same user, as volume mounts have generally better performance there. The optional `-workspace` flag can be used to override the default. [#412](https://github.com/sourcegraph/src-cli/pull/412)
 
 ### Changed
 

cmd/src/campaigns_common.go

@@ -101,18 +101,9 @@ func newCampaignsApplyFlags(flagSet *flag.FlagSet, cacheDir, tempDir string) *ca
 		"If true, errors encountered while executing steps in a repository won't stop the execution of the campaign spec but only cause that repository to be skipped.",
 	)
 
-	// We default to bind workspaces on everything except ARM64 macOS at
-	// present. In the future, we'll likely want to switch the default for ARM64
-	// macOS as well, but this requires access to the hardware for testing.
-	var defaultWorkspace string
-	if runtime.GOOS == "darwin" && runtime.GOARCH == "amd64" {
-		defaultWorkspace = "volume"
-	} else {
-		defaultWorkspace = "bind"
-	}
 	flagSet.StringVar(
-		&caf.workspace, "workspace", defaultWorkspace,
-		`Workspace mode to use ("bind" or "volume")`,
+		&caf.workspace, "workspace", "auto",
+		`Workspace mode to use ("auto", "bind", or "volume")`,
 	)
 
 	flagSet.BoolVar(verbose, "v", false, "print verbose output")
@@ -180,36 +171,13 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
 		return "", "", err
 	}
 
-	// Parse flags and build up our service options.
-	var errs *multierror.Error
+	// Parse flags and build up our service and executor options.
 
 	specFile, err := campaignsOpenFileFlag(&flags.file)
 	if err != nil {
-		errs = multierror.Append(errs, err)
-	} else {
-		defer specFile.Close()
-	}
-
-	opts := campaigns.ExecutorOpts{
-		Cache:       svc.NewExecutionCache(flags.cacheDir),
-		Creator:     svc.NewWorkspaceCreator(flags.cacheDir),
-		RepoFetcher: svc.NewRepoFetcher(flags.cacheDir, flags.cleanArchives),
-		ClearCache:  flags.clearCache,
-		KeepLogs:    flags.keepLogs,
-		Timeout:     flags.timeout,
-		TempDir:     flags.tempDir,
-	}
-	if flags.parallelism <= 0 {
-		opts.Parallelism = runtime.GOMAXPROCS(0)
-	} else {
-		opts.Parallelism = flags.parallelism
-	}
-	out.VerboseLine(output.Linef("🚧", output.StyleSuccess, "Workspace creator: %T", opts.Creator))
-	executor := svc.NewExecutor(opts)
-
-	if errs != nil {
-		return "", "", errs
+		return "", "", err
 	}
+	defer specFile.Close()
 
 	pending := campaignsCreatePending(out, "Parsing campaign spec")
 	campaignSpec, rawSpec, err := campaignsParseSpec(out, svc, specFile)
@@ -229,7 +197,7 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
 		Label: "Preparing container images",
 		Max:   1.0,
 	}}, nil)
-	err = svc.SetDockerImages(ctx, opts.Creator, campaignSpec, func(perc float64) {
+	err = svc.SetDockerImages(ctx, campaignSpec, func(perc float64) {
 		imageProgress.SetValue(0, perc)
 	})
 	if err != nil {
@@ -255,6 +223,27 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
 		campaignsCompletePending(pending, "Resolved repositories")
 	}
 
+	pending = campaignsCreatePending(out, "Preparing workspaces")
+	workspaceCreator := svc.NewWorkspaceCreator(ctx, flags.cacheDir, campaignSpec.Steps)
+	pending.VerboseLine(output.Linef("🚧", output.StyleSuccess, "Workspace creator: %T", workspaceCreator))
+	campaignsCompletePending(pending, "Prepared workspaces")
+
+	opts := campaigns.ExecutorOpts{
+		Cache:       svc.NewExecutionCache(flags.cacheDir),
+		Creator:     workspaceCreator,
+		RepoFetcher: svc.NewRepoFetcher(flags.cacheDir, flags.cleanArchives),
+		ClearCache:  flags.clearCache,
+		KeepLogs:    flags.keepLogs,
+		Timeout:     flags.timeout,
+		TempDir:     flags.tempDir,
+	}
+	if flags.parallelism <= 0 {
+		opts.Parallelism = runtime.GOMAXPROCS(0)
+	} else {
+		opts.Parallelism = flags.parallelism
+	}
+
+	executor := svc.NewExecutor(opts)
 	p := newCampaignProgressPrinter(out, *verbose, opts.Parallelism)
 	specs, err := svc.ExecuteCampaignSpec(ctx, repos, executor, campaignSpec, p.PrintStatuses, flags.skipErrors)
 	if err != nil && !flags.skipErrors {

cmd/src/version.go

@@ -9,13 +9,9 @@ import (
 	"net/http"
 
 	"github.com/sourcegraph/src-cli/internal/api"
+	"github.com/sourcegraph/src-cli/internal/version"
 )
 
-// buildTag is the git tag at the time of build and is used to
-// denote the binary's current version. This value is supplied
-// as an ldflag at compile time in travis.
-var buildTag = "dev"
-
 func init() {
 	usage := `
 Examples:
@@ -30,7 +26,7 @@ Examples:
 	var apiFlags = api.NewFlags(flagSet)
 
 	handler := func(args []string) error {
-		fmt.Printf("Current version: %s\n", buildTag)
+		fmt.Printf("Current version: %s\n", version.BuildTag)
 
 		client := cfg.apiClient(apiFlags, flagSet.Output())
 		recommendedVersion, err := getRecommendedVersion(context.Background(), client)

internal/campaigns/bind_workspace.go

@@ -30,8 +30,6 @@ func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.
 	return w, errors.Wrap(wc.prepareGitRepo(ctx, w), "preparing local git repo")
 }
 
-func (*dockerBindWorkspaceCreator) DockerImages() []string { return []string{} }
-
 func (*dockerBindWorkspaceCreator) prepareGitRepo(ctx context.Context, w *dockerBindWorkspace) error {
 	if _, err := runGitCmd(ctx, w.dir, "init"); err != nil {
 		return errors.Wrap(err, "git init failed")

internal/campaigns/service.go

@@ -211,8 +211,17 @@ func (svc *Service) NewRepoFetcher(dir string, cleanArchives bool) RepoFetcher {
 	}
 }
 
-func (svc *Service) NewWorkspaceCreator(dir string) WorkspaceCreator {
+func (svc *Service) NewWorkspaceCreator(ctx context.Context, dir string, steps []Step) WorkspaceCreator {
+	var workspace workspaceCreatorType
 	if svc.workspace == "volume" {
+		workspace = workspaceCreatorVolume
+	} else if svc.workspace == "bind" {
+		workspace = workspaceCreatorBind
+	} else {
+		workspace = bestWorkspaceCreator(ctx, steps)
+	}
+
+	if workspace == workspaceCreatorVolume {
 		return &dockerVolumeWorkspaceCreator{}
 	}
 	return &dockerBindWorkspaceCreator{dir: dir}
@@ -243,20 +252,18 @@ func (dis dockerImageSet) add(image string, digestPtr *string) {
 
 // SetDockerImages updates the steps within the campaign spec to include the
 // exact content digest to be used when running each step, and ensures that all
-// Docker images are available, including any required by the workspace creator.
+// Docker images are available, including any required by the service itself.
 //
 // Progress information is reported back to the given progress function: perc
 // will be a value between 0.0 and 1.0, inclusive.
-func (svc *Service) SetDockerImages(ctx context.Context, creator WorkspaceCreator, spec *CampaignSpec, progress func(perc float64)) error {
+func (svc *Service) SetDockerImages(ctx context.Context, spec *CampaignSpec, progress func(perc float64)) error {
 	images := dockerImageSet{}
 	for i, step := range spec.Steps {
 		images.add(step.Container, &spec.Steps[i].image)
 	}
 
-	// The workspace creator may have its own dependencies.
-	for _, image := range creator.DockerImages() {
-		images.add(image, nil)
-	}
+	// We also need to ensure we have our own utility images available.
+	images.add(dockerVolumeWorkspaceImage, nil)
 
 	progress(0)
 	i := 0

internal/campaigns/volume_workspace.go

@@ -10,9 +10,9 @@ import (
 
 	"github.com/sourcegraph/src-cli/internal/campaigns/graphql"
 	"github.com/sourcegraph/src-cli/internal/exec"
+	"github.com/sourcegraph/src-cli/internal/version"
 )
 
-// dockerVolumeWorkspaceCreator creates dockerVolumeWorkspace instances.
 type dockerVolumeWorkspaceCreator struct{}
 
 var _ WorkspaceCreator = &dockerVolumeWorkspaceCreator{}
@@ -31,10 +31,6 @@ func (wc *dockerVolumeWorkspaceCreator) Create(ctx context.Context, repo *graphq
 	return w, errors.Wrap(wc.prepareGitRepo(ctx, w), "preparing local git repo")
 }
 
-func (*dockerVolumeWorkspaceCreator) DockerImages() []string {
-	return []string{dockerWorkspaceImage}
-}
-
 func (*dockerVolumeWorkspaceCreator) createVolume(ctx context.Context) (string, error) {
 	out, err := exec.CommandContext(ctx, "docker", "volume", "create").CombinedOutput()
 	if err != nil {
@@ -77,7 +73,7 @@ func (*dockerVolumeWorkspaceCreator) unzipRepoIntoVolume(ctx context.Context, w
 		"--workdir", "/work",
 		"--mount", "type=bind,source=" + zip + ",target=/tmp/zip,ro",
 	}, common...)
-	opts = append(opts, dockerWorkspaceImage, "unzip", "/tmp/zip")
+	opts = append(opts, dockerVolumeWorkspaceImage, "unzip", "/tmp/zip")
 
 	if out, err := exec.CommandContext(ctx, "docker", opts...).CombinedOutput(); err != nil {
 		return errors.Wrapf(err, "unzip output:\n\n%s\n\n", string(out))
@@ -151,9 +147,19 @@ exec git diff --cached --no-prefix --binary
 	return out, nil
 }
 
-// dockerWorkspaceImage is the Docker image we'll run our unzip and git commands
-// in. This needs to match the name defined in .github/workflows/docker.yml.
-const dockerWorkspaceImage = "sourcegraph/src-campaign-volume-workspace"
+// dockerVolumeWorkspaceImage is the Docker image we'll run our unzip and git
+// commands in. This needs to match the name defined in
+// .github/workflows/docker.yml.
+var dockerVolumeWorkspaceImage = "sourcegraph/src-campaign-volume-workspace"
+
+func init() {
+	dockerTag := version.BuildTag
+	if version.BuildTag == version.DefaultBuildTag {
+		dockerTag = "latest"
+	}
+
+	dockerVolumeWorkspaceImage = dockerVolumeWorkspaceImage + ":" + dockerTag
+}
 
 // runScript is a utility function to mount the given shell script into a Docker
 // container started from the dockerWorkspaceImage, then run it and return the
@@ -183,7 +189,7 @@ func (w *dockerVolumeWorkspace) runScript(ctx context.Context, target, script st
 		"--workdir", target,
 		"--mount", "type=bind,source=" + name + ",target=/run.sh,ro",
 	}, common...)
-	opts = append(opts, dockerWorkspaceImage, "sh", "/run.sh")
+	opts = append(opts, dockerVolumeWorkspaceImage, "sh", "/run.sh")
 
 	out, err := exec.CommandContext(ctx, "docker", opts...).CombinedOutput()
 	if err != nil {

internal/campaigns/volume_workspace_test.go

@@ -51,15 +51,15 @@ func TestVolumeWorkspaceCreator(t *testing.T) {
 				"docker", "run", "--rm", "--init", "--workdir", "/work",
 				"--mount", "type=bind,source=*,target=/tmp/zip,ro",
 				"--mount", "type=volume,source="+volumeID+",target=/work",
-				dockerWorkspaceImage,
+				dockerVolumeWorkspaceImage,
 				"unzip", "/tmp/zip",
 			),
 			expect.NewGlob(
 				expect.Behaviour{},
 				"docker", "run", "--rm", "--init", "--workdir", "/work",
 				"--mount", "type=bind,source=*,target=/run.sh,ro",
 				"--mount", "type=volume,source="+volumeID+",target=/work",
-				dockerWorkspaceImage,
+				dockerVolumeWorkspaceImage,
 				"sh", "/run.sh",
 			),
 		)
@@ -97,7 +97,7 @@ func TestVolumeWorkspaceCreator(t *testing.T) {
 				"docker", "run", "--rm", "--init", "--workdir", "/work",
 				"--mount", "type=bind,source=*,target=/tmp/zip,ro",
 				"--mount", "type=volume,source="+volumeID+",target=/work",
-				dockerWorkspaceImage,
+				dockerVolumeWorkspaceImage,
 				"unzip", "/tmp/zip",
 			),
 		)
@@ -119,15 +119,15 @@ func TestVolumeWorkspaceCreator(t *testing.T) {
 				"docker", "run", "--rm", "--init", "--workdir", "/work",
 				"--mount", "type=bind,source=*,target=/tmp/zip,ro",
 				"--mount", "type=volume,source="+volumeID+",target=/work",
-				dockerWorkspaceImage,
+				dockerVolumeWorkspaceImage,
 				"unzip", "/tmp/zip",
 			),
 			expect.NewGlob(
 				expect.Behaviour{ExitCode: 1},
 				"docker", "run", "--rm", "--init", "--workdir", "/work",
 				"--mount", "type=bind,source=*,target=/run.sh,ro",
 				"--mount", "type=volume,source="+volumeID+",target=/work",
-				dockerWorkspaceImage,
+				dockerVolumeWorkspaceImage,
 				"sh", "/run.sh",
 			),
 		)
@@ -232,7 +232,7 @@ M  internal/campaigns/volume_workspace_test.go
 						"docker", "run", "--rm", "--init", "--workdir", "/work",
 						"--mount", "type=bind,source=*,target=/run.sh,ro",
 						"--mount", "type=volume,source="+volumeID+",target=/work",
-						dockerWorkspaceImage,
+						dockerVolumeWorkspaceImage,
 						"sh", "/run.sh",
 					),
 				)
@@ -263,7 +263,7 @@ M  internal/campaigns/volume_workspace_test.go
 						"docker", "run", "--rm", "--init", "--workdir", "/work",
 						"--mount", "type=bind,source=*,target=/run.sh,ro",
 						"--mount", "type=volume,source="+volumeID+",target=/work",
-						dockerWorkspaceImage,
+						dockerVolumeWorkspaceImage,
 						"sh", "/run.sh",
 					),
 				)
@@ -308,7 +308,7 @@ index 06471f4..5f9d3fa 100644
 						"docker", "run", "--rm", "--init", "--workdir", "/work",
 						"--mount", "type=bind,source=*,target=/run.sh,ro",
 						"--mount", "type=volume,source="+volumeID+",target=/work",
-						dockerWorkspaceImage,
+						dockerVolumeWorkspaceImage,
 						"sh", "/run.sh",
 					),
 				)
@@ -334,7 +334,7 @@ index 06471f4..5f9d3fa 100644
 				"docker", "run", "--rm", "--init", "--workdir", "/work",
 				"--mount", "type=bind,source=*,target=/run.sh,ro",
 				"--mount", "type=volume,source="+volumeID+",target=/work",
-				dockerWorkspaceImage,
+				dockerVolumeWorkspaceImage,
 				"sh", "/run.sh",
 			),
 		)
@@ -362,7 +362,7 @@ func TestVolumeWorkspace_runScript(t *testing.T) {
 					"docker", "run", "--rm", "--init", "--workdir", "/work",
 					"--mount", "type=bind,source=*,target=/run.sh,ro",
 					"--mount", "type=volume,source="+volumeID+",target=/work",
-					dockerWorkspaceImage,
+					dockerVolumeWorkspaceImage,
 					"sh", "/run.sh",
 				)
 				if err := glob(name, arg...); err != nil {