dashboard: move buildlet exec code from coordinator to client package
Change-Id: I778ac78ed02be9f67436ec045a3816dfc24afda3
Reviewed-on: https://go-review.googlesource.com/2923
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/builders.go b/builders.go
index bfa4d25..e238d2d 100644
--- a/builders.go
+++ b/builders.go
@@ -47,6 +47,21 @@
return arch[:i]
}
+// AllScript returns the relative path to the operating system's script to
+// do the build and run its standard set of tests.
+// Example values are "src/all.bash", "src/all.bat", "src/all.rc".
+func (c *BuildConfig) AllScript() string {
+ if strings.HasPrefix(c.Name, "windows-") {
+ return "src/all.bat"
+ }
+ if strings.HasPrefix(c.Name, "plan9-") {
+ return "src/all.rc"
+ }
+ // TODO(bradfitz): race.bash, etc, once the race builder runs
+ // via the buildlet.
+ return "src/all.bash"
+}
+
func (c *BuildConfig) UsesDocker() bool { return c.VMImage == "" }
func (c *BuildConfig) UsesVM() bool { return c.VMImage != "" }
diff --git a/buildlet/buildletclient.go b/buildlet/buildletclient.go
index ae0e87b..3ab091f 100644
--- a/buildlet/buildletclient.go
+++ b/buildlet/buildletclient.go
@@ -9,10 +9,12 @@
package buildlet // import "golang.org/x/tools/dashboard/buildlet"
import (
+ "errors"
"fmt"
"io"
"io/ioutil"
"net/http"
+ "net/url"
"strings"
)
@@ -54,6 +56,8 @@
return "https://" + strings.TrimSuffix(c.ipPort, ":443")
}
+// PutTarball writes files to the remote buildlet.
+// The Reader must be of a tar.gz file.
func (c *Client) PutTarball(r io.Reader) error {
req, err := http.NewRequest("PUT", c.URL()+"/writetgz", r)
if err != nil {
@@ -70,3 +74,64 @@
}
return nil
}
+
+// ExecOpts are options for a remote command invocation.
+type ExecOpts struct {
+ // Output is the output of stdout and stderr.
+ // If nil, the output is discarded.
+ Output io.Writer
+
+ // OnStartExec is an optional hook that runs after the 200 OK
+ // response from the buildlet, but before the output begins
+ // writing to Output.
+ OnStartExec func()
+}
+
+// Exec runs cmd on the buildlet.
+//
+// Two errors are returned: one is whether the command succeeded
+// remotely (remoteErr), and the second (execErr) is whether there
+// were system errors preventing the command from being started or
+// seen to completition. If execErr is non-nil, the remoteErr is
+// meaningless.
+func (c *Client) Exec(cmd string, opts ExecOpts) (remoteErr, execErr error) {
+ res, err := http.PostForm(c.URL()+"/exec", url.Values{"cmd": {cmd}})
+ if err != nil {
+ return nil, err
+ }
+ defer res.Body.Close()
+ if res.StatusCode != http.StatusOK {
+ slurp, _ := ioutil.ReadAll(io.LimitReader(res.Body, 4<<10))
+ return nil, fmt.Errorf("buildlet: HTTP status %v: %s", res.Status, slurp)
+ }
+ condRun(opts.OnStartExec)
+
+ // Stream the output:
+ out := opts.Output
+ if out == nil {
+ out = ioutil.Discard
+ }
+ if _, err := io.Copy(out, res.Body); err != nil {
+ return nil, fmt.Errorf("error copying response: %v", err)
+ }
+
+ // Don't record to the dashboard unless we heard the trailer from
+ // the buildlet, otherwise it was probably some unrelated error
+ // (like the VM being killed, or the buildlet crashing due to
+ // e.g. https://golang.org/issue/9309, since we require a tip
+ // build of the buildlet to get Trailers support)
+ state := res.Trailer.Get("Process-State")
+ if state == "" {
+ return nil, errors.New("missing Process-State trailer from HTTP response; buildlet built with old (<= 1.4) Go?")
+ }
+ if state != "ok" {
+ return errors.New(state), nil
+ }
+ return nil, nil
+}
+
+func condRun(fn func()) {
+ if fn != nil {
+ fn()
+ }
+}
diff --git a/buildlet/gce.go b/buildlet/gce.go
index 325eb35..3e7366f 100644
--- a/buildlet/gce.go
+++ b/buildlet/gce.go
@@ -19,6 +19,7 @@
"google.golang.org/api/compute/v1"
)
+// VMOpts control how new VMs are started.
type VMOpts struct {
// Zone is the GCE zone to create the VM in. Required.
Zone string
@@ -149,9 +150,7 @@
if err != nil {
return nil, fmt.Errorf("Failed to create instance: %v", err)
}
- if fn := opts.OnInstanceRequested; fn != nil {
- fn()
- }
+ condRun(opts.OnInstanceRequested)
createOp := op.Name
// Wait for instance create operation to succeed.
@@ -177,9 +176,7 @@
return nil, fmt.Errorf("Unknown create status %q: %+v", op.Status, op)
}
}
- if fn := opts.OnInstanceCreated; fn != nil {
- fn()
- }
+ condRun(opts.OnInstanceCreated)
inst, err := computeService.Instances.Get(projectID, zone, instName).Do()
if err != nil {
@@ -207,9 +204,7 @@
buildletURL = "http://" + ip
ipPort = ip + ":80"
}
- if fn := opts.OnGotInstanceInfo; fn != nil {
- fn()
- }
+ condRun(opts.OnGotInstanceInfo)
const timeout = 90 * time.Second
var alive bool
diff --git a/cmd/coordinator/coordinator.go b/cmd/coordinator/coordinator.go
index 40d58fe..e21f7f7 100644
--- a/cmd/coordinator/coordinator.go
+++ b/cmd/coordinator/coordinator.go
@@ -769,47 +769,29 @@
}
st.logEventTime("end_write_tar")
- // TODO(bradfitz): add an Exec method to buildlet.Client and update this code.
- // Run the builder
- cmd := "all.bash"
- if strings.HasPrefix(st.name, "windows-") {
- cmd = "all.bat"
- } else if strings.HasPrefix(st.name, "plan9-") {
- cmd = "all.rc"
- }
execStartTime := time.Now()
- st.logEventTime("start_exec")
- res, err := http.PostForm(bc.URL()+"/exec", url.Values{"cmd": {"src/" + cmd}})
- if !goodRes(res, err, "running "+cmd) {
- return
- }
- defer res.Body.Close()
- st.logEventTime("running_exec")
- // Stream the output:
- if _, err := io.Copy(st, res.Body); err != nil {
- return fmt.Errorf("error copying response: %v", err)
+ st.logEventTime("pre_exec")
+
+ remoteErr, err := bc.Exec(conf.AllScript(), buildlet.ExecOpts{
+ Output: st,
+ OnStartExec: func() { st.logEventTime("running_exec") },
+ })
+ if err != nil {
+ return err
}
st.logEventTime("done")
-
- // Don't record to the dashboard unless we heard the trailer from
- // the buildlet, otherwise it was probably some unrelated error
- // (like the VM being killed, or the buildlet crashing due to
- // e.g. https://golang.org/issue/9309, since we require a tip
- // build of the buildlet to get Trailers support)
- state := res.Trailer.Get("Process-State")
- if state == "" {
- return errors.New("missing Process-State trailer from HTTP response; buildlet built with old (<= 1.4) Go?")
- }
-
var log string
- if state != "ok" {
+ if remoteErr != nil {
log = st.logs()
}
- if err := recordResult(st.name, state == "ok", st.rev, log, time.Since(execStartTime)); err != nil {
- return fmt.Errorf("Status was %q but failed to report it to the dashboard: %v", state, err)
+ if err := recordResult(st.name, remoteErr == nil, st.rev, log, time.Since(execStartTime)); err != nil {
+ if remoteErr != nil {
+ return fmt.Errorf("Remote error was %q but failed to report it to the dashboard: %v", remoteErr, err)
+ }
+ return fmt.Errorf("Build succeeded but failed to report it to the dashboard: %v", err)
}
- if state != "ok" {
- return fmt.Errorf("%s failed: %v", cmd, state)
+ if remoteErr != nil {
+ return fmt.Errorf("%s failed: %v", conf.AllScript(), remoteErr)
}
return nil
}
@@ -1081,10 +1063,6 @@
return fmt.Errorf("listing instances: %v", err)
}
for _, inst := range list.Items {
- if !strings.HasPrefix(inst.Name, "buildlet-") {
- // We only delete ones we created.
- continue
- }
if inst.Metadata == nil {
// Defensive. Not seen in practice.
continue
@@ -1103,7 +1081,12 @@
}
}
}
- if sawDeleteAt && !vmIsBuilding(inst.Name) {
+ // Delete buildlets (things we made) from previous
+ // generations. Thenaming restriction (buildlet-*)
+ // prevents us from deleting buildlet VMs used by
+ // Gophers for interactive development & debugging
+ // (non-builder users); those are named "mote-*".
+ if sawDeleteAt && strings.HasPrefix(inst.Name, "buildlet-") && !vmIsBuilding(inst.Name) {
log.Printf("Deleting VM %q in zone %q from an earlier coordinator generation ...", inst.Name, zone)
deleteVM(zone, inst.Name)
}
