feat(executor): add option for additional mounts in docker runner

Author: rdeusser

cmd/executor/internal/config/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
     visibility = ["//cmd/executor:__subpackages__"],
     deps = [
         "//internal/conf/confdefaults",
+        "//internal/docker",
         "//internal/env",
         "//internal/executor/types",
         "//internal/hostname",
@@ -34,6 +35,7 @@ go_test(
     ],
     deps = [
         ":config",
+        "//internal/docker",
         "//internal/env",
         "//lib/errors",
         "@com_github_stretchr_testify//assert",

cmd/executor/internal/config/config.go

@@ -16,6 +16,7 @@ import (
 	"k8s.io/utils/strings/slices"
 
 	"github.com/sourcegraph/sourcegraph/internal/conf/confdefaults"
+	"github.com/sourcegraph/sourcegraph/internal/docker"
 	"github.com/sourcegraph/sourcegraph/internal/env"
 	"github.com/sourcegraph/sourcegraph/internal/executor/types"
 	"github.com/sourcegraph/sourcegraph/internal/hostname"
@@ -55,6 +56,8 @@ type Config struct {
 	DockerRegistryMirrorURL                        string
 	DockerAddHostGateway                           bool
 	DockerAuthConfig                               types.DockerAuthConfig
+	DockerAdditionalMounts                         []string
+	DockerAdditionalMountsStr                      string
 	KubernetesConfigPath                           string
 	KubernetesNodeName                             string
 	KubernetesNodeSelector                         string
@@ -142,6 +145,7 @@ func (c *Config) Load() {
 	c.NumTotalJobs = c.GetInt("EXECUTOR_NUM_TOTAL_JOBS", "0", "The maximum number of jobs that will be dequeued by the worker.")
 	c.NodeExporterURL = c.GetOptional("NODE_EXPORTER_URL", "The URL of the node_exporter instance, without the /metrics path.")
 	c.DockerRegistryNodeExporterURL = c.GetOptional("DOCKER_REGISTRY_NODE_EXPORTER_URL", "The URL of the Docker Registry instance's node_exporter, without the /metrics path.")
+	c.DockerAdditionalMountsStr = c.GetOptional("EXECUTOR_DOCKER_ADDITIONAL_MOUNTS", "Attach filesystem mounts to the container (e.g. type=bind,source=/foo,destination=/bar), semicolon-separated")
 	c.MaxActiveTime = c.GetInterval("EXECUTOR_MAX_ACTIVE_TIME", "0", "The maximum time that can be spent by the worker dequeueing records to be handled.")
 	c.DockerRegistryMirrorURL = c.GetOptional("EXECUTOR_DOCKER_REGISTRY_MIRROR_URL", "The address of a docker registry mirror to use in firecracker VMs. Supports multiple values, separated with a comma.")
 	c.KubernetesConfigPath = c.GetOptional("EXECUTOR_KUBERNETES_CONFIG_PATH", "The path to the Kubernetes config file.")
@@ -184,6 +188,10 @@ func (c *Config) Load() {
 		c.dockerAuthConfigUnmarshalError = json.Unmarshal([]byte(c.dockerAuthConfigStr), &c.DockerAuthConfig)
 	}
 
+	if c.DockerAdditionalMountsStr != "" {
+		c.DockerAdditionalMounts = strings.Split(c.DockerAdditionalMountsStr, ";")
+	}
+
 	if c.kubernetesNodeRequiredAffinityMatchExpressions != "" {
 		c.kubernetesNodeRequiredAffinityMatchExpressionsUnmarshalError = json.Unmarshal([]byte(c.kubernetesNodeRequiredAffinityMatchExpressions), &c.KubernetesNodeRequiredAffinityMatchExpressions)
 	}
@@ -261,6 +269,29 @@ func (c *Config) Validate() error {
 		c.AddError(errors.Wrap(c.dockerAuthConfigUnmarshalError, "invalid EXECUTOR_DOCKER_AUTH_CONFIG, failed to parse"))
 	}
 
+	if c.DockerAdditionalMountsStr != "" {
+		// Target is a mandatory field so we can rely on it showing up if
+		// multiple mounts are present, but we need to check for all forms.
+		countTargets := func(text string, patterns ...string) int {
+			count := 0
+			for _, pattern := range patterns {
+				count += strings.Count(text, pattern)
+			}
+			return count
+		}
+
+		if len(c.DockerAdditionalMounts) == 1 && countTargets(c.DockerAdditionalMountsStr, "target", "dst", "destination") > 1 {
+			c.AddError(errors.New("invalid EXECUTOR_DOCKER_ADDITIONAL_MOUNTS, failed to parse due to incorrect separator"))
+		}
+
+		for _, mount := range c.DockerAdditionalMounts {
+			_, err := docker.ParseMount(mount)
+			if err != nil {
+				c.AddError(errors.Wrap(err, "invalid EXECUTOR_DOCKER_ADDITIONAL_MOUNTS, failed to parse mount spec"))
+			}
+		}
+	}
+
 	if c.kubernetesNodeRequiredAffinityMatchExpressionsUnmarshalError != nil {
 		c.AddError(errors.Wrap(c.kubernetesNodeRequiredAffinityMatchExpressionsUnmarshalError, "invalid EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS, failed to parse"))
 	}

cmd/executor/internal/config/config_test.go

@@ -94,6 +94,7 @@ func TestConfig_Load(t *testing.T) {
 	assert.Equal(t, "EXECUTOR_VM_PREFIX", cfg.VMPrefix)
 	assert.True(t, cfg.KeepWorkspaces)
 	assert.Equal(t, "EXECUTOR_DOCKER_HOST_MOUNT_PATH", cfg.DockerHostMountPath)
+	assert.Equal(t, "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS", cfg.DockerAdditionalMountsStr)
 	assert.Equal(t, 8, cfg.JobNumCPUs)
 	assert.Equal(t, "EXECUTOR_JOB_MEMORY", cfg.JobMemory)
 	assert.Equal(t, "EXECUTOR_FIRECRACKER_DISK_SPACE", cfg.FirecrackerDiskSpace)
@@ -203,6 +204,7 @@ func TestConfig_Load_Defaults(t *testing.T) {
 	assert.Equal(t, "executor", cfg.VMPrefix)
 	assert.False(t, cfg.KeepWorkspaces)
 	assert.Empty(t, cfg.DockerHostMountPath)
+	assert.Empty(t, cfg.DockerAdditionalMountsStr)
 	assert.Equal(t, 4, cfg.JobNumCPUs)
 	assert.Equal(t, "12G", cfg.JobMemory)
 	assert.Equal(t, "20G", cfg.FirecrackerDiskSpace)
@@ -253,6 +255,8 @@ func TestConfig_Validate(t *testing.T) {
 				switch name {
 				case "EXECUTOR_QUEUE_NAME":
 					return "batches"
+				case "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS":
+					return "type=bind,source=/foo,target=/bar"
 				case "EXECUTOR_FRONTEND_URL":
 					return "http://some-url.com"
 				case "EXECUTOR_FRONTEND_PASSWORD":
@@ -348,6 +352,59 @@ func TestConfig_Validate(t *testing.T) {
 			},
 			expectedErr: errors.New("EXECUTOR_QUEUE_NAMES contains invalid queue name 'batches;codeintel', valid names are 'batches, codeintel' and should be comma-separated"),
 		},
+		{
+			name: "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS using invalid separator",
+			getterFunc: func(name, defaultValue, description string) string {
+				switch name {
+				case "EXECUTOR_QUEUE_NAME":
+					return "batches"
+				case "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS":
+					return "type=bind,source=/foo,target=/bar:type=volume,source=gomodcache,target=/gomodcache"
+				case "EXECUTOR_FRONTEND_URL":
+					return "http://some-url.com"
+				case "EXECUTOR_FRONTEND_PASSWORD":
+					return "some-password"
+				default:
+					return defaultValue
+				}
+			},
+			expectedErr: errors.New("invalid EXECUTOR_DOCKER_ADDITIONAL_MOUNTS, failed to parse due to incorrect separator"),
+		},
+		{
+			name: "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS using incorrect format",
+			getterFunc: func(name, defaultValue, description string) string {
+				switch name {
+				case "EXECUTOR_QUEUE_NAME":
+					return "batches"
+				case "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS":
+					return "source=/foo;/bar"
+				case "EXECUTOR_FRONTEND_URL":
+					return "http://some-url.com"
+				case "EXECUTOR_FRONTEND_PASSWORD":
+					return "some-password"
+				default:
+					return defaultValue
+				}
+			},
+			expectedErr: errors.New("2 errors occurred:\n\t* invalid EXECUTOR_DOCKER_ADDITIONAL_MOUNTS, failed to parse mount spec: target is required\n\t* invalid EXECUTOR_DOCKER_ADDITIONAL_MOUNTS, failed to parse mount spec: invalid field '/bar' must be a key=value pair"),
+		},
+		{
+			name: "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS using volume options",
+			getterFunc: func(name, defaultValue, description string) string {
+				switch name {
+				case "EXECUTOR_QUEUE_NAME":
+					return "batches"
+				case "EXECUTOR_DOCKER_ADDITIONAL_MOUNTS":
+					return "type=volume,source=sshvolume,target=/app,volume-opt=sshcmd=test@node2:/home/test,volume-opt=password=testpassword"
+				case "EXECUTOR_FRONTEND_URL":
+					return "http://some-url.com"
+				case "EXECUTOR_FRONTEND_PASSWORD":
+					return "some-password"
+				default:
+					return defaultValue
+				}
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

cmd/executor/internal/run/BUILD.bazel

@@ -27,6 +27,7 @@ go_library(
         "//cmd/executor/internal/worker/runner",
         "//cmd/executor/internal/worker/workspace",
         "//internal/conf/deploy",
+        "//internal/docker",
         "//internal/download",
         "//internal/executor",
         "//internal/executor/types",

cmd/executor/internal/run/util.go

@@ -20,6 +20,7 @@ import (
 	"github.com/sourcegraph/sourcegraph/cmd/executor/internal/worker/command"
 	"github.com/sourcegraph/sourcegraph/cmd/executor/internal/worker/runner"
 	"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
+	"github.com/sourcegraph/sourcegraph/internal/docker"
 	executorutil "github.com/sourcegraph/sourcegraph/internal/executor/util"
 	"github.com/sourcegraph/sourcegraph/internal/observation"
 	"github.com/sourcegraph/sourcegraph/internal/version"
@@ -107,9 +108,23 @@ func dockerOptions(c *config.Config) command.DockerOptions {
 		DockerAuthConfig: c.DockerAuthConfig,
 		AddHostGateway:   c.DockerAddHostGateway,
 		Resources:        resourceOptions(c),
+		Mounts:           dockerAdditionalMounts(c),
 	}
 }
 
+func dockerAdditionalMounts(c *config.Config) []docker.MountOptions {
+	opts := make([]docker.MountOptions, 0)
+
+	for _, mount := range c.DockerAdditionalMounts {
+		// No need to check for parsing errors here. We're already doing that
+		// during the config validation phase.
+		m, _ := docker.ParseMount(mount)
+		opts = append(opts, *m)
+	}
+
+	return opts
+}
+
 func firecrackerOptions(c *config.Config) runner.FirecrackerOptions {
 	var dockerMirrors []string
 	if len(c.DockerRegistryMirrorURL) > 0 {

cmd/executor/internal/worker/command/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//cmd/executor/internal/util",
         "//cmd/executor/internal/worker/cmdlogger",
         "//cmd/executor/internal/worker/files",
+        "//internal/docker",
         "//internal/executor/types",
         "//internal/metrics",
         "//internal/observation",

cmd/executor/internal/worker/command/docker.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"github.com/sourcegraph/sourcegraph/cmd/executor/internal/worker/files"
+	"github.com/sourcegraph/sourcegraph/internal/docker"
 	"github.com/sourcegraph/sourcegraph/internal/executor/types"
 )
 
@@ -15,6 +16,7 @@ type DockerOptions struct {
 	ConfigPath       string
 	AddHostGateway   bool
 	Resources        ResourceOptions
+	Mounts           []docker.MountOptions
 }
 
 // ResourceOptions are the resource limits that can be applied to a container or VM.
@@ -80,6 +82,7 @@ func formatDockerCommand(hostDir string, image string, scriptPath string, spec S
 		"--rm",
 		dockerHostGatewayFlag(options.AddHostGateway),
 		dockerResourceFlags(options.Resources),
+		dockerMountFlags(options.Mounts),
 		dockerVolumeFlags(hostDir),
 		dockerWorkingDirectoryFlags(spec.Dir),
 		dockerEnvFlags(spec.Env),
@@ -115,6 +118,17 @@ func dockerResourceFlags(options ResourceOptions) []string {
 	return flags
 }
 
+func dockerMountFlags(options []docker.MountOptions) []string {
+	mounts := make([]string, 0)
+
+	for _, option := range options {
+		mounts = append(mounts, "--mount")
+		mounts = append(mounts, option.String())
+	}
+
+	return mounts
+}
+
 func dockerVolumeFlags(wd string) []string {
 	return []string{"-v", wd + ":/data"}
 }