Support mounts field on docker_environment

pants.toml

@@ -114,6 +114,8 @@ root_patterns = [
 docker = "//:docker_env"
 # Used for iterating on remote-execution.
 remote = "//:buildgrid_remote"
+# Used for testing
+docker_for_testing = "//:docker_env_for_testing"
 
 [tailor]
 build_file_header = """\

src/python/pants/backend/shell/goals/test.py

@@ -15,6 +15,7 @@
 from pants.backend.shell.util_rules import shell_command
 from pants.backend.shell.util_rules.shell_command import ShellCommandProcessFromTargetRequest
 from pants.core.goals.test import TestExtraEnv, TestFieldSet, TestRequest, TestResult, TestSubsystem
+from pants.core.util_rules.environments import EnvironmentField
 from pants.engine.internals.selectors import Get
 from pants.engine.process import FallibleProcessResult, Process, ProcessCacheScope
 from pants.engine.rules import collect_rules, rule
@@ -23,12 +24,15 @@
 from pants.util.logging import LogLevel
 
 
+@dataclasses.dataclass(frozen=True)
 class TestShellCommandFieldSet(TestFieldSet):
     required_fields = (
         ShellCommandCommandField,
         ShellCommandTestDependenciesField,
     )
 
+    environment: EnvironmentField
+
     @classmethod
     def opt_out(cls, tgt: Target) -> bool:
         return tgt.get(SkipShellCommandTestsField).value

src/python/pants/core/util_rules/BUILD

@@ -4,3 +4,16 @@
 python_sources()
 
 python_tests(name="tests")
+
+docker_environment(
+    name="docker_env_for_testing",
+    image="debian:stable-slim",
+    mounts=["/etc/passwd:/mount_dir/testfile:ro"],
+)
+
+experimental_test_shell_command(
+    name="test-docker-environment-bind-mounts",
+    tools=["test"],
+    command="test -f /mount_dir/testfile",
+    environment="docker_for_testing"
+)

src/python/pants/core/util_rules/environments.py

@@ -201,6 +201,19 @@ class DockerImageField(StringField):
         """
     )
 
+class DockerMountsField(StringSequenceField):
+    alias = "mounts"
+    help = help_text(
+        """
+        Mounts to use when starting the container.
+
+        The values should be in the form "<path on host>:<path on container>(:<options>)".
+
+        For more information, see the documentation for the Rust Crate which powers the underlying
+        docker integration: https://docs.rs/bollard/0.15.0/bollard/models/struct.HostConfig.html#structfield.binds
+        and the Docker Bind mounts documentation: https://docs.docker.com/storage/bind-mounts/.
+        """
+    )
 
 class DockerPlatformField(StringField):
     alias = "platform"
@@ -263,6 +276,7 @@ class DockerEnvironmentTarget(Target):
     core_fields = (
         *COMMON_TARGET_FIELDS,
         DockerImageField,
+        DockerMountsField,
         DockerPlatformField,
         DockerFallbackEnvironmentField,
     )
@@ -896,6 +910,7 @@ async def extract_process_config_from_environment(
     environments_subsystem: EnvironmentsSubsystem,
 ) -> ProcessExecutionEnvironment:
     docker_image = None
+    docker_mounts = None
     remote_execution = False
     raw_remote_execution_extra_platform_properties: tuple[str, ...] = ()
 
@@ -909,6 +924,7 @@ async def extract_process_config_from_environment(
         docker_image = (
             tgt.val[DockerImageField].value if tgt.val.has_field(DockerImageField) else None
         )
+        docker_mounts = tgt.val[DockerMountsField].value if tgt.val.has_field(DockerMountsField) else None
 
         # If a docker image name is provided, convert to an image ID so caching works properly.
         # TODO(17104): Append image ID instead to the image name.
@@ -937,6 +953,7 @@ async def extract_process_config_from_environment(
         environment_name=tgt.name,
         platform=platform.value,
         docker_image=docker_image,
+        docker_mounts=docker_mounts,
         remote_execution=remote_execution,
         remote_execution_extra_platform_properties=[
             tuple(pair.split("=", maxsplit=1))  # type: ignore[misc]

src/python/pants/engine/internals/native_engine.pyi

@@ -493,6 +493,7 @@ class ProcessExecutionEnvironment:
         environment_name: str | None,
         platform: str,
         docker_image: str | None,
+        docker_mounts: Sequence[str] | None,
         remote_execution: bool,
         remote_execution_extra_platform_properties: Sequence[tuple[str, str]],
     ) -> None: ...

src/rust/engine/process_execution/docker/src/docker.rs

@@ -410,7 +410,7 @@ impl<'a> process_execution::CommandRunner for CommandRunner<'a> {
 
                 // Obtain ID of the base container in which to run the execution for this process.
                 let (container_id, named_caches) = {
-                    let ProcessExecutionStrategy::Docker(image) =
+                    let ProcessExecutionStrategy::Docker { image, mounts } =
                         &req.execution_environment.strategy
                     else {
                         return Err(ProcessError::Unclassified(
@@ -423,6 +423,7 @@ impl<'a> process_execution::CommandRunner for CommandRunner<'a> {
                     self.container_cache
                         .container_for_image(
                             image,
+                            mounts,
                             &req.execution_environment.platform,
                             &context.build_id,
                         )
@@ -740,6 +741,8 @@ impl Command {
 /// multiple tasks trying to access an initializing container do not try to start multiple
 /// containers.
 type CachedContainer = Arc<OnceCell<(String, NamedCaches)>>;
+/// Cache that maps (image name / bind mounts / platform) to a cached container.
+type ContainerMap = BTreeMap<(String, Option<Vec<String>>, Platform), CachedContainer>;
 
 /// Caches running containers so that build actions can be invoked by running "executions"
 /// within those cached containers.
@@ -749,8 +752,7 @@ pub(crate) struct ContainerCache<'a> {
     executor: Executor,
     work_dir_base: String,
     immutable_inputs_base_dir: String,
-    /// Cache that maps image name / platform to a cached container.
-    containers: Mutex<BTreeMap<(String, Platform), CachedContainer>>,
+    containers: Mutex<ContainerMap>,
 }
 
 impl<'a> ContainerCache<'a> {
@@ -794,6 +796,7 @@ impl<'a> ContainerCache<'a> {
         docker: Docker,
         executor: Executor,
         image_name: String,
+        mut mounts: Vec<String>,
         platform: Platform,
         image_pull_scope: ImagePullScope,
         image_pull_cache: ImagePullCache,
@@ -816,17 +819,16 @@ impl<'a> ContainerCache<'a> {
             .await
             .map_err(|e| format!("Failed to create named cache volume for {image_name}: {e}"))?;
 
+        mounts.extend(vec![
+            format!("{work_dir_base}:{SANDBOX_BASE_PATH_IN_CONTAINER}"),
+            format!("{named_cache_volume_name}:{NAMED_CACHES_BASE_PATH_IN_CONTAINER}",),
+            // DOCKER-TODO: Consider making this bind mount read-only.
+            format!("{immutable_inputs_base_dir}:{IMMUTABLE_INPUTS_BASE_PATH_IN_CONTAINER}"),
+        ]);
         let config = bollard::container::Config {
             entrypoint: Some(vec!["/bin/sh".to_string()]),
             host_config: Some(bollard::service::HostConfig {
-                binds: Some(vec![
-                    format!("{work_dir_base}:{SANDBOX_BASE_PATH_IN_CONTAINER}"),
-                    format!("{named_cache_volume_name}:{NAMED_CACHES_BASE_PATH_IN_CONTAINER}",),
-                    // DOCKER-TODO: Consider making this bind mount read-only.
-                    format!(
-                        "{immutable_inputs_base_dir}:{IMMUTABLE_INPUTS_BASE_PATH_IN_CONTAINER}"
-                    ),
-                ]),
+                binds: Some(mounts),
                 // The init process ensures that child processes are properly reaped.
                 init: Some(true),
                 ..bollard::service::HostConfig::default()
@@ -942,6 +944,7 @@ impl<'a> ContainerCache<'a> {
     pub async fn container_for_image(
         &self,
         image_name: &str,
+        mounts: &Option<Vec<String>>,
         platform: &Platform,
         build_generation: &str,
     ) -> Result<(String, NamedCaches), String> {
@@ -951,7 +954,7 @@ impl<'a> ContainerCache<'a> {
         let container_id_cell = {
             let mut containers = self.containers.lock();
             let cell = containers
-                .entry((image_name.to_string(), *platform))
+                .entry((image_name.to_string(), mounts.clone(), *platform))
                 .or_insert_with(|| Arc::new(OnceCell::new()));
             cell.clone()
         };
@@ -966,6 +969,7 @@ impl<'a> ContainerCache<'a> {
                     docker.clone(),
                     executor,
                     image_name.to_string(),
+                    mounts.clone().unwrap_or_default(),
                     *platform,
                     image_pull_scope,
                     self.image_pull_cache.clone(),