Skip to content

Commit

Permalink
Add sandbox_add_mount_pair support to docker sandbox
Browse files Browse the repository at this point in the history
    The flag --sandbox_add_mount_pair allows multiple 'source:target' pairs to be
    added to the Linux sandbox. The docker sandbox should support this as well.

    Closes #9175.

    PiperOrigin-RevId: 264592536
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 4326fce commit e281e22
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.runtime.ProcessWrapper;
import com.google.devtools.build.lib.runtime.ProcessWrapperUtil;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.time.Duration;
Expand All @@ -26,13 +26,14 @@
import java.util.UUID;

final class DockerCommandLineBuilder {
private ProcessWrapper processWrapper;
private Path processWrapper;
private Path dockerClient;
private String imageName;
private List<String> commandArguments;
private Path sandboxExecRoot;
private Map<String, String> environmentVariables;
private Duration timeout;
private Duration killDelay;
private boolean createNetworkNamespace;
private UUID uuid;
private int uid;
Expand All @@ -41,7 +42,7 @@ final class DockerCommandLineBuilder {
private boolean privileged;
private List<Map.Entry<String, String>> additionalMounts;

public DockerCommandLineBuilder setProcessWrapper(ProcessWrapper processWrapper) {
public DockerCommandLineBuilder setProcessWrapper(Path processWrapper) {
this.processWrapper = processWrapper;
return this;
}
Expand Down Expand Up @@ -77,6 +78,11 @@ public DockerCommandLineBuilder setTimeout(Duration timeout) {
return this;
}

public DockerCommandLineBuilder setKillDelay(Duration killDelay) {
this.killDelay = killDelay;
return this;
}

public DockerCommandLineBuilder setCreateNetworkNamespace(boolean createNetworkNamespace) {
this.createNetworkNamespace = createNetworkNamespace;
return this;
Expand Down Expand Up @@ -168,11 +174,15 @@ public List<String> build() {
dockerCmdLine.add(imageName);
dockerCmdLine.addAll(commandArguments);

ProcessWrapper.CommandLineBuilder processWrapperCmdLine =
processWrapper.commandLineBuilder(dockerCmdLine.build());
ProcessWrapperUtil.CommandLineBuilder processWrapperCmdLine =
ProcessWrapperUtil.commandLineBuilder(
this.processWrapper.getPathString(), dockerCmdLine.build());
if (timeout != null) {
processWrapperCmdLine.setTimeout(timeout);
}
if (killDelay != null) {
processWrapperCmdLine.setKillDelay(killDelay);
}
return processWrapperCmdLine.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.platform.PlatformUtils;
Expand All @@ -49,11 +50,9 @@
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;

Expand Down Expand Up @@ -150,7 +149,7 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
private final TreeDeleter treeDeleter;
private final int uid;
private final int gid;
private final Set<UUID> containersToCleanup;
private final List<UUID> containersToCleanup;
private final CommandEnvironment cmdEnv;

/**
Expand Down Expand Up @@ -193,14 +192,14 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
this.uid = -1;
this.gid = -1;
}
this.containersToCleanup = Collections.synchronizedSet(new HashSet<>());
this.containersToCleanup = Collections.synchronizedList(new ArrayList<>());

cmdEnv.getEventBus().register(this);
}

@Override
protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
throws IOException, ExecException {
protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionContext context)
throws IOException, ExecException, InterruptedException {
// Each invocation of "exec" gets its own sandbox base, execroot and temporary directory.
Path sandboxPath =
sandboxBase.getRelative(getName()).getRelative(Integer.toString(context.getId()));
Expand Down Expand Up @@ -264,28 +263,32 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
cmdLine.setTimeout(timeout);
}

// If we were interrupted, it is possible that "docker run" gets killed in exactly the moment
// between the create and the start call, leaving behind a container that is created but never
// ran. This means that Docker won't automatically clean it up (as --rm only affects the start
// phase and has no effect on the create phase of "docker run").
// We register the container UUID for cleanup, but remove the UUID if the process ran
// successfully.
containersToCleanup.add(uuid);
return new CopyingSandboxedSpawn(
sandboxPath,
sandboxExecRoot,
cmdLine.build(),
cmdEnv.getClientEnv(),
SandboxHelpers.processInputFiles(
spawn,
context,
execRoot,
getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree),
outputs,
ImmutableSet.of(),
treeDeleter,
/*statisticsPath=*/ null,
() -> containersToCleanup.remove(uuid));
SandboxedSpawn sandbox =
new CopyingSandboxedSpawn(
sandboxPath,
sandboxExecRoot,
cmdLine.build(),
cmdEnv.getClientEnv(),
SandboxHelpers.processInputFiles(
spawn,
context,
execRoot,
getSandboxOptions().symlinkedSandboxExpandsTreeArtifactsInRunfilesTree),
outputs,
ImmutableSet.of(),
treeDeleter);

try {
return runSpawn(spawn, sandbox, context, execRoot, timeout, null);
} catch (InterruptedException e) {
// If we were interrupted, it is possible that "docker run" gets killed in exactly the moment
// between the create and the start call, leaving behind a container that is created but never
// ran. This means that Docker won't automatically clean it up (as --rm only affects the start
// phase and has no effect on the create phase of "docker run").
// We add the container UUID to a list and clean them up after the execution is over.
containersToCleanup.add(uuid);
throw e;
}
}

private String getOrCreateCustomizedImage(String baseImage) {
Expand Down Expand Up @@ -374,7 +377,8 @@ private String executeCommand(List<String> cmdLine, InputStream stdIn) throws Us

private Optional<String> dockerContainerFromSpawn(Spawn spawn) throws ExecException {
Platform platform =
PlatformUtils.getPlatformProto(spawn, cmdEnv.getOptions().getOptions(RemoteOptions.class));
PlatformUtils.getPlatformProto(
spawn.getExecutionPlatform(), cmdEnv.getOptions().getOptions(RemoteOptions.class));

if (platform != null) {
try {
Expand Down

0 comments on commit e281e22

Please sign in to comment.