Skip to content

Commit

Permalink
Let every build action have its own temporary directory
Browse files Browse the repository at this point in the history
As an alternative to enabling cleaning of temporary directories, it may
be of interest to simply remove write access from it entirely. We could
let bb_runner override the TMPDIR environment variable for all build
actions. This variable could point to a directly placed in the build
directory. This directly will be purged automatically when the action
completes.

The advantage of this approach is that it's a lot more granular than
having simple temporary directory cleaning. There is a lower chance of
accidental sharing of data.
  • Loading branch information
EdSchouten committed Apr 2, 2020
1 parent 94b9fb6 commit 5df73f0
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 8 deletions.
3 changes: 2 additions & 1 deletion cmd/bb_runner/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ func main() {

r := runner.NewLocalRunner(
buildDirectory,
configuration.BuildDirectoryPath)
configuration.BuildDirectoryPath,
configuration.SetTmpdirEnvironmentVariable)

// When temporary directories need cleaning prior to executing a build
// action, attach a series of TemporaryDirectoryCleaningRunners.
Expand Down
13 changes: 13 additions & 0 deletions pkg/builder/local_build_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,18 @@ func (be *localBuildExecutor) Execute(ctx context.Context, filePool re_filesyste
}
}

// Create a directory inside the build directory that build
// actions may use to store temporary files. This ensures that
// temporary files are automatically removed when the build
// action completes. When using FUSE, it also causes quotas to
// be applied to them.
if err := buildDirectory.Mkdir("tmp", 0777); err != nil {
attachErrorToExecuteResponse(
response,
util.StatusWrap(err, "Failed to create temporary directory inside build directory"))
return response
}

executionStateUpdates <- &remoteworker.CurrentState_Executing{
ActionDigest: request.ActionDigest,
ExecutionState: &remoteworker.CurrentState_Executing_Running{
Expand All @@ -312,6 +324,7 @@ func (be *localBuildExecutor) Execute(ctx context.Context, filePool re_filesyste
StdoutPath: path.Join(buildDirectoryPath, "stdout"),
StderrPath: path.Join(buildDirectoryPath, "stderr"),
InputRootDirectory: path.Join(buildDirectoryPath, "root"),
TemporaryDirectory: path.Join(buildDirectoryPath, "tmp"),
})

// Attach the exit code or execution error.
Expand Down
8 changes: 8 additions & 0 deletions pkg/builder/local_build_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ func TestLocalBuildExecutorOutputSymlinkReadingFailure(t *testing.T) {
digest.MustNewDigest("nintendo64", "7777777777777777777777777777777777777777777777777777777777777777", 42),
).Return(nil)
inputRootDirectory.EXPECT().Mkdir("foo", os.FileMode(0777)).Return(nil)
buildDirectory.EXPECT().Mkdir("tmp", os.FileMode(0777))
runner := mock.NewMockRunner(ctrl)
runner.EXPECT().Run(gomock.Any(), &runner_pb.RunRequest{
Arguments: []string{"touch", "foo"},
Expand All @@ -340,6 +341,7 @@ func TestLocalBuildExecutorOutputSymlinkReadingFailure(t *testing.T) {
StdoutPath: "stdout",
StderrPath: "stderr",
InputRootDirectory: "root",
TemporaryDirectory: "tmp",
}).Return(&runner_pb.RunResponse{
ExitCode: 0,
}, nil)
Expand Down Expand Up @@ -464,6 +466,7 @@ func TestLocalBuildExecutorSuccess(t *testing.T) {
ctx,
digest.MustNewDigest("ubuntu1804", "0000000000000000000000000000000000000000000000000000000000000003", 345),
).Return(nil)
buildDirectory.EXPECT().Mkdir("tmp", os.FileMode(0777))
resourceUsage, err := ptypes.MarshalAny(&empty.Empty{})
require.NoError(t, err)
runner := mock.NewMockRunner(ctrl)
Expand All @@ -487,6 +490,7 @@ func TestLocalBuildExecutorSuccess(t *testing.T) {
StdoutPath: "0000000000000000/stdout",
StderrPath: "0000000000000000/stderr",
InputRootDirectory: "0000000000000000/root",
TemporaryDirectory: "0000000000000000/tmp",
}).Return(&runner_pb.RunResponse{
ExitCode: 0,
ResourceUsage: []*any.Any{resourceUsage},
Expand Down Expand Up @@ -682,6 +686,7 @@ func TestLocalBuildExecutorWithWorkingDirectorySuccess(t *testing.T) {
ctx,
digest.MustNewDigest("ubuntu1804", "0000000000000000000000000000000000000000000000000000000000000003", 345),
).Return(nil)
buildDirectory.EXPECT().Mkdir("tmp", os.FileMode(0777))
resourceUsage, err := ptypes.MarshalAny(&empty.Empty{})
require.NoError(t, err)
runner := mock.NewMockRunner(ctrl)
Expand All @@ -704,6 +709,7 @@ func TestLocalBuildExecutorWithWorkingDirectorySuccess(t *testing.T) {
StdoutPath: "stdout",
StderrPath: "stderr",
InputRootDirectory: "root",
TemporaryDirectory: "tmp",
}).Return(&runner_pb.RunResponse{
ExitCode: 0,
ResourceUsage: []*any.Any{resourceUsage},
Expand Down Expand Up @@ -924,6 +930,7 @@ func TestLocalBuildExecutorTimeoutDuringExecution(t *testing.T) {
ctx,
digest.MustNewDigest("ubuntu1804", "0000000000000000000000000000000000000000000000000000000000000003", 345),
).Return(nil)
buildDirectory.EXPECT().Mkdir("tmp", os.FileMode(0777))

// Simulate a timeout by running the command with a timeout of
// zero seconds. This should cause an immediate build failure.
Expand All @@ -935,6 +942,7 @@ func TestLocalBuildExecutorTimeoutDuringExecution(t *testing.T) {
StdoutPath: "stdout",
StderrPath: "stderr",
InputRootDirectory: "root",
TemporaryDirectory: "tmp",
}).DoAndReturn(func(ctx context.Context, request *runner_pb.RunRequest) (*runner_pb.RunResponse, error) {
<-ctx.Done()
return nil, util.StatusFromContext(ctx)
Expand Down
7 changes: 7 additions & 0 deletions pkg/proto/configuration/bb_runner/bb_runner.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,11 @@ message ApplicationConfiguration {

// Common configuration options that apply to all Buildbarn binaries.
buildbarn.configuration.global.Configuration global = 4;

// Run every build action with the TMPDIR environment variable set to
// point to a location inside the build directory. This causes
// temporary files to be cleaned up automatically on the build
// action's behalf, assuming it properly respects the environment
// variable.
bool set_tmpdir_environment_variable = 5;
}
4 changes: 4 additions & 0 deletions pkg/proto/runner/runner.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ message RunRequest {

// Path of the input root, relative to the build directory.
string input_root_directory = 6;

// Path of a scratch space directory that may be used by the build
// action, relative to the build directory.
string temporary_directory = 7;
}

message RunResponse {
Expand Down
19 changes: 12 additions & 7 deletions pkg/runner/local_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ import (
)

type localRunner struct {
buildDirectory filesystem.Directory
buildDirectoryPath string
buildDirectory filesystem.Directory
buildDirectoryPath string
setTmpdirEnvironmentVariable bool
}

// NewLocalRunner returns a Runner capable of running commands on the
// local system directly.
func NewLocalRunner(buildDirectory filesystem.Directory, buildDirectoryPath string) Runner {
func NewLocalRunner(buildDirectory filesystem.Directory, buildDirectoryPath string, setTmpdirEnvironmentVariable bool) Runner {
return &localRunner{
buildDirectory: buildDirectory,
buildDirectoryPath: buildDirectoryPath,
buildDirectory: buildDirectory,
buildDirectoryPath: buildDirectoryPath,
setTmpdirEnvironmentVariable: setTmpdirEnvironmentVariable,
}
}

Expand Down Expand Up @@ -69,9 +71,12 @@ func (r *localRunner) Run(ctx context.Context, request *runner.RunRequest) (*run
return nil, status.Error(codes.InvalidArgument, "Insufficient number of command arguments")
}
cmd := exec.CommandContext(ctx, request.Arguments[0], request.Arguments[1:]...)
// TODO: Convert WorkingDirectory to use platform specific path
// delimiters.
// TODO: Convert WorkingDirectory and TemporaryDirectory to use
// platform specific path delimiters.
cmd.Dir = filepath.Join(r.buildDirectoryPath, request.InputRootDirectory, request.WorkingDirectory)
if r.setTmpdirEnvironmentVariable && request.TemporaryDirectory != "" {
cmd.Env = append(cmd.Env, "TMPDIR="+filepath.Join(r.buildDirectoryPath, request.TemporaryDirectory))
}
for name, value := range request.EnvironmentVariables {
cmd.Env = append(cmd.Env, name+"="+value)
}
Expand Down

0 comments on commit 5df73f0

Please sign in to comment.