Skip to content

Commit

Permalink
Adds CI for remote execution (thought-machine#882)
Browse files Browse the repository at this point in the history
* Move wrapper script to the root as we would normally have it

* Adding test setup for remote execution

* Add test script for remote execution

* make dir configurable

* Make it not interactive

* Quieter output

* Filegroups don't need TmpOutput

* Simpler killing protocol

* Add ability to detach when targets have spawned

* create the symlink

* Detach processes better.

* use exec instead of ForkExec which is inexplicably not working

* Tweak script a bit

* Fix test

* gofmt

* buildify

* Fix up script a bit

* Remove sandbox from remote testing

* Add psmisc to Docker container
  • Loading branch information
peterebden authored Mar 26, 2020
1 parent 4c410d1 commit 6f3348c
Show file tree
Hide file tree
Showing 16 changed files with 149 additions and 21 deletions.
25 changes: 25 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,27 @@ jobs:
key: python-darwin-py370-{{ checksum "third_party/python/BUILD" }}
paths: [ ".plz-cache/third_party/python" ]

test-rex:
working_directory: ~/please
docker:
- image: thoughtmachine/please_ubuntu:20200326
steps:
- checkout
- attach_workspace:
at: /tmp/workspace
- restore_cache:
key: elan-v1-{{ checksum "third_party/go/BUILD" }}
- run:
name: Test Remote Execution
command: ./.circleci/rex_test.sh
- store_test_results:
path: plz-out/results
- store_artifacts:
path: plz-out/log
- save_cache:
key: elan-v1-{{ checksum "third_party/go/BUILD" }}
paths: [ "plz-out/elan" ]

release:
docker:
- image: thoughtmachine/please_docs:20190318
Expand Down Expand Up @@ -156,11 +177,15 @@ workflows:
requires:
- build-linux
- build-linux-alt
- test-rex:
requires:
- build-linux
- release:
requires:
- build-linux
- build-linux-alt
- build-darwin
- test-rex
filters:
branches:
only: master
Expand Down
22 changes: 22 additions & 0 deletions .circleci/rex_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash

set -eu

trap 'killall elan mettle zeal' SIGINT SIGTERM EXIT

DIR="${1:-/tmp/please}"

# Extract the plz installation from earlier step
rm -rf "$DIR"
mkdir "$DIR"
tar -xzf /tmp/workspace/linux_amd64/please_*.tar.gz --strip-components=1 -C "$DIR"
ln -s "${DIR}/please" "${DIR}/plz"
export PATH="$DIR:$PATH"

# Start the servers in the background
plz run parallel -p -v notice --colour --detach //test/remote:run_elan //test/remote:run_zeal //test/remote:run_mettle

# Test we can rebuild plz itself.
plz build --profile ci_remote -p -v notice --colour //src:please
# Check we can actually run some tests
plz test --profile ci_remote -p -v notice --colour //src/core:all
14 changes: 14 additions & 0 deletions .plzconfig.ci_remote
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
; This is used for CI of the remote execution code.
[remote]
url = localhost:7778
casurl = localhost:7777
asseturl = localhost:7776
instance = mettle
secure = false
numexecutors = 10

[build]
hashfunction = sha256

[please]
location = ~/.please
7 changes: 7 additions & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,10 @@ filegroup(
binary = True,
visibility = ["//third_party/go:all"],
)

filegroup(
name = "pleasew",
srcs = ["pleasew"],
binary = True,
visibility = ["//src/utils:all"],
)
14 changes: 12 additions & 2 deletions package/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ subinclude("//build_defs:version")

# This is the master list of files that get installed.
filegroup(
name = "installed_files",
name = "tools",
srcs = [
"//src:please",
"//tools/build_langserver",
"//tools/go_buildid_replacer",
"//tools/jarcat",
Expand All @@ -15,6 +14,17 @@ filegroup(
"//tools/please_pex",
"//tools/sandbox:please_sandbox",
],
binary = True,
visibility = ["//src:tools"],
)

filegroup(
name = "installed_files",
srcs = [
":tools",
"//src:please",
],
binary = True,
visibility = ["//:install"],
)

Expand Down
File renamed without changes.
8 changes: 8 additions & 0 deletions src/BUILD.plz
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,11 @@ go_binary(
"//third_party/go:logging",
],
)

# This is handy for things like plz plz --repo_root
filegroup(
name = "tools",
srcs = ["//package:tools"],
visibility = ["//:install"],
binary = True,
)
4 changes: 3 additions & 1 deletion src/core/build_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,9 @@ func (target *BuildTarget) NamedOutputs(name string) []string {
// GetTmpOutput takes the original output filename as an argument, and returns a temporary output
// filename(plz-out/tmp/) if output has the same name as the package, this avoids the name conflict issue
func (target *BuildTarget) GetTmpOutput(parseOutput string) string {
if parseOutput == target.Label.PackageName {
if target.IsFilegroup {
return parseOutput // Filegroups never need this.
} else if parseOutput == target.Label.PackageName {
return parseOutput + tempOutputSuffix
} else if target.Label.PackageName == "" && target.HasSource(parseOutput) {
// This also fixes the case where source and output are the same, which can happen
Expand Down
5 changes: 3 additions & 2 deletions src/please.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ var opts struct {
PositionalArgs struct {
Targets []core.BuildLabel `positional-arg-name:"target" description:"Targets to run"`
} `positional-args:"true" required:"true"`
Args cli.Filepaths `short:"a" long:"arg" description:"Arguments to pass to the called processes."`
Args cli.Filepaths `short:"a" long:"arg" description:"Arguments to pass to the called processes."`
Detach bool `long:"detach" description:"Detach from the parent process when all children have spawned"`
} `command:"parallel" description:"Runs a sequence of targets in parallel"`
Sequential struct {
Quiet bool `short:"q" long:"quiet" description:"Suppress output from successful subprocesses."`
Expand Down Expand Up @@ -439,7 +440,7 @@ var buildFunctions = map[string]func() int{
},
"parallel": func() int {
if success, state := runBuild(opts.Run.Parallel.PositionalArgs.Targets, true, false, false); success {
os.Exit(run.Parallel(context.Background(), state, state.ExpandOriginalTargets(), opts.Run.Parallel.Args.AsStrings(), opts.Run.Parallel.NumTasks, opts.Run.Parallel.Quiet, opts.Run.Env))
os.Exit(run.Parallel(context.Background(), state, state.ExpandOriginalTargets(), opts.Run.Parallel.Args.AsStrings(), opts.Run.Parallel.NumTasks, opts.Run.Parallel.Quiet, opts.Run.Env, opts.Run.Parallel.Detach))
}
return 1
},
Expand Down
16 changes: 11 additions & 5 deletions src/run/run_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ var log = logging.MustGetLogger("run")

// Run implements the running part of 'plz run'.
func Run(state *core.BuildState, label core.BuildLabel, args []string, env bool) {
run(context.Background(), state, label, args, false, false, env)
run(context.Background(), state, label, args, false, false, env, false)
}

// Parallel runs a series of targets in parallel.
// Returns a relevant exit code (i.e. if at least one subprocess exited unsuccessfully, it will be
// that code, otherwise 0 if all were successful).
// The given context can be used to control the lifetime of the subprocesses.
func Parallel(ctx context.Context, state *core.BuildState, labels []core.BuildLabel, args []string, numTasks int, quiet, env bool) int {
func Parallel(ctx context.Context, state *core.BuildState, labels []core.BuildLabel, args []string, numTasks int, quiet, env, detach bool) int {
pool := NewGoroutinePool(numTasks)
var g errgroup.Group
for _, label := range labels {
Expand All @@ -40,7 +40,7 @@ func Parallel(ctx context.Context, state *core.BuildState, labels []core.BuildLa
var wg sync.WaitGroup
wg.Add(1)
pool.Submit(func() {
if e := run(ctx, state, label, args, true, quiet, env); e != nil {
if e := run(ctx, state, label, args, true, quiet, env, detach); e != nil {
err = e
}
wg.Done()
Expand All @@ -64,7 +64,7 @@ func Parallel(ctx context.Context, state *core.BuildState, labels []core.BuildLa
func Sequential(state *core.BuildState, labels []core.BuildLabel, args []string, quiet, env bool) int {
for _, label := range labels {
log.Notice("Running %s", label)
if err := run(context.Background(), state, label, args, true, quiet, env); err != nil {
if err := run(context.Background(), state, label, args, true, quiet, env, false); err != nil {
log.Error("%s", err)
return err.code
}
Expand All @@ -76,7 +76,7 @@ func Sequential(state *core.BuildState, labels []core.BuildLabel, args []string,
// If fork is true then we fork to run the target and return any error from the subprocesses.
// If it's false this function never returns (because we either win or die; it's like
// Game of Thrones except rather less glamorous).
func run(ctx context.Context, state *core.BuildState, label core.BuildLabel, args []string, fork, quiet, setenv bool) *exitError {
func run(ctx context.Context, state *core.BuildState, label core.BuildLabel, args []string, fork, quiet, setenv, detach bool) *exitError {
target := state.Graph.TargetOrDie(label)
if !target.IsBinary {
log.Fatalf("Target %s cannot be run; it's not marked as binary", label)
Expand Down Expand Up @@ -105,6 +105,12 @@ func run(ctx context.Context, state *core.BuildState, label core.BuildLabel, arg
if !fork {
// Plain 'plz run'. One way or another we never return from the following line.
must(syscall.Exec(splitCmd[0], args, env), args)
} else if detach {
// Bypass the whole process management system since we explicitly aim not to manage this subprocess.
cmd := exec.Command(splitCmd[0], args[1:]...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return toExitError(cmd.Start(), args, nil)
}
// Run as a normal subcommand.
// Note that we don't connect stdin. It doesn't make sense for multiple processes.
Expand Down
4 changes: 2 additions & 2 deletions src/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ func TestSequential(t *testing.T) {

func TestParallel(t *testing.T) {
state, labels1, labels2 := makeState()
code := Parallel(context.Background(), state, labels1, nil, 5, false, false)
code := Parallel(context.Background(), state, labels1, nil, 5, false, false, false)
assert.Equal(t, 0, code)
code = Parallel(context.Background(), state, labels2, nil, 5, true, false)
code = Parallel(context.Background(), state, labels2, nil, 5, true, false, false)
assert.Equal(t, 1, code)
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ go_library(
go_bindata(
name = "bindata",
srcs = [
"//:pleasew",
"//tools/misc:completion_script",
"//tools/misc:wrapper_script",
],
prefix = "tools/misc",
)
2 changes: 1 addition & 1 deletion src/watch/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,6 @@ func build(ctx context.Context, state *core.BuildState, labels []core.BuildLabel
callback(ns, labels)
if state.NeedRun {
// Don't wait for this, its lifetime will be controlled by the context.
go run.Parallel(ctx, state, labels, nil, state.Config.Please.NumThreads, false, false)
go run.Parallel(ctx, state, labels, nil, state.Config.Please.NumThreads, false, false, false)
}
}
39 changes: 39 additions & 0 deletions test/remote/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Various helpers for testing remote execution.
remote_file(
name = "elan",
binary = True,
hashes = ["ac113612e582055302fc732512a31aeb548f6c8837da39f03067419862fb7c14"],
url = "https://github.com/thought-machine/please-servers/releases/download/v2.3.0/elan",
)

remote_file(
name = "mettle",
binary = True,
hashes = ["0f65f009a3dd71e120ec9cba233f3a756a7a17b21b53e8297fd49ea1c38886ad"],
url = "https://github.com/thought-machine/please-servers/releases/download/v2.3.0/mettle",
)

remote_file(
name = "zeal",
binary = True,
hashes = ["b61f5f23b1c82e9aac4f5b9ad282e03112eeff8fe1441c7bf2e960fefd3c6c1b"],
url = "https://github.com/thought-machine/please-servers/releases/download/v2.3.0/zeal",
)

sh_cmd(
name = "run_elan",
srcs = [":elan"],
cmd = "mkdir -p plz-out/elan && exec $(out_location :elan) -s file://\\\\$PWD/plz-out/elan -v warning --log_file plz-out/log/elan.log",
)

sh_cmd(
name = "run_mettle",
srcs = [":mettle"],
cmd = "exec $(out_location :mettle) dual -s 127.0.0.1:7777 -q mem://requests -r mem://responses -m 13434 -d plz-out/mettle -v warning --log_file plz-out/log/mettle.log --browser http://127.0.0.1:7779 --num_workers 8",
)

sh_cmd(
name = "run_zeal",
srcs = [":zeal"],
cmd = "exec $(out_location :zeal) -v warning -s 127.0.0.1:7777 --log_file plz-out/log/zeal.log",
)
2 changes: 1 addition & 1 deletion tools/images/ubuntu/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ MAINTAINER peter.ebden@gmail.com
# Go, Python, Java and other dependencies.
RUN apt-get update && \
apt-get install -y python3 python3-dev python3-pip openjdk-8-jdk-headless \
curl unzip git locales pkg-config zlib1g-dev && \
curl unzip git locales pkg-config zlib1g-dev psmisc && \
apt-get clean

# Go - we want 1.14 here but the latest package available is 1.10.
Expand Down
6 changes: 0 additions & 6 deletions tools/misc/BUILD
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
filegroup(
name = "wrapper_script",
srcs = ["pleasew"],
visibility = ["PUBLIC"],
)

filegroup(
name = "completion_script",
srcs = ["plz_complete.sh"],
Expand Down

0 comments on commit 6f3348c

Please sign in to comment.