From 3ecc311976cc3f7c7b7a50314929bdc1b07c4c9d Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 16 Jan 2015 09:54:03 -0800 Subject: 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 --- dashboard/builders.go | 15 ++++++++ dashboard/buildlet/buildletclient.go | 65 ++++++++++++++++++++++++++++++++ dashboard/buildlet/gce.go | 13 ++----- dashboard/cmd/coordinator/coordinator.go | 61 +++++++++++------------------- 4 files changed, 106 insertions(+), 48 deletions(-) diff --git a/dashboard/builders.go b/dashboard/builders.go index bfa4d25..e238d2d 100644 --- a/dashboard/builders.go +++ b/dashboard/builders.go @@ -47,6 +47,21 @@ func (c *BuildConfig) GOARCH() string { 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/dashboard/buildlet/buildletclient.go b/dashboard/buildlet/buildletclient.go index ae0e87b..3ab091f 100644 --- a/dashboard/buildlet/buildletclient.go +++ b/dashboard/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 @@ func (c *Client) URL() string { 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 @@ func (c *Client) PutTarball(r io.Reader) error { } 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/dashboard/buildlet/gce.go b/dashboard/buildlet/gce.go index 325eb35..3e7366f 100644 --- a/dashboard/buildlet/gce.go +++ b/dashboard/buildlet/gce.go @@ -19,6 +19,7 @@ import ( "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 @@ func StartNewVM(ts oauth2.TokenSource, instName, builderType string, opts VMOpts 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 @@ OpLoop: 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 @@ OpLoop: 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/dashboard/cmd/coordinator/coordinator.go b/dashboard/cmd/coordinator/coordinator.go index 40d58fe..e21f7f7 100644 --- a/dashboard/cmd/coordinator/coordinator.go +++ b/dashboard/cmd/coordinator/coordinator.go @@ -769,47 +769,29 @@ func buildInVM(conf dashboard.BuildConfig, st *buildStatus) (retErr error) { } 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("done") + st.logEventTime("pre_exec") - // 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?") + remoteErr, err := bc.Exec(conf.AllScript(), buildlet.ExecOpts{ + Output: st, + OnStartExec: func() { st.logEventTime("running_exec") }, + }) + if err != nil { + return err } - + st.logEventTime("done") 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 @@ func cleanZoneVMs(zone string) error { 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 @@ func cleanZoneVMs(zone string) error { } } } - 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) } -- cgit v1.2.3