all: misc cleanup, docs, actually add Alpine builder

-- move policy of which builders are trybots out of coordinator
   and into dashboard/builders.go.

-- move some GCE-specific code from coordinator.go to gce.go.

-- rename an old "watcher" reference to "gitmirror"

-- add some docs

-- actually add the Alpine builder, missing from https://golang.org/cl/33890

Fixes golang/go#17891

Change-Id: Ia63671ca09aec322ed57b3663e0ac5042cdc56f2
Reviewed-on: https://go-review.googlesource.com/40395
Reviewed-by: Brad Fitzpatrick <[email protected]>

Author: bradfitz

cmd/coordinator/coordinator.go

@@ -104,31 +104,10 @@ var (
 )
 
 func initTryBuilders() {
-	names := []string{
-		"darwin-amd64-10_11",
-		"linux-386",
-		"linux-amd64",
-		"linux-amd64-race",
-		"linux-amd64-ssacheck",
-		"freebsd-amd64-gce101",
-		"windows-386-gce",
-		"windows-amd64-gce",
-		"openbsd-amd64-60",
-		"nacl-386",
-		"nacl-amd64p32",
-		"linux-arm",
-		"misc-vet-vetall",
-	}
-	for name := range dashboard.Builders {
-		if strings.HasPrefix(name, "misc-compile") {
-			names = append(names, name)
-		}
-	}
-	for _, name := range names {
+	for _, name := range dashboard.TrybotBuilderNames() {
 		conf, ok := dashboard.Builders[name]
 		if !ok {
-			log.Printf("ignoring invalid try builder config %q", name)
-			continue
+			panic("bogus")
 		}
 		tryBuilders = append(tryBuilders, conf)
 		if conf.BuildSubrepos() {
@@ -149,27 +128,6 @@ const (
 	podDeleteTimeout = 45 * time.Minute
 )
 
-func readGCSFile(name string) ([]byte, error) {
-	if *mode == "dev" {
-		b, ok := testFiles[name]
-		if !ok {
-			return nil, &os.PathError{
-				Op:   "open",
-				Path: name,
-				Err:  os.ErrNotExist,
-			}
-		}
-		return []byte(b), nil
-	}
-
-	r, err := storageClient.Bucket(buildEnv.BuildletBucket).Object(name).NewReader(context.Background())
-	if err != nil {
-		return nil, err
-	}
-	defer r.Close()
-	return ioutil.ReadAll(r)
-}
-
 // Fake keys signed by a fake CA.
 // These are used in localhost dev mode. (Not to be confused with the
 // staging "dev" instance under GCE project "go-dashboard-dev")
@@ -320,7 +278,7 @@ func main() {
 	workc := make(chan builderRev)
 
 	if *mode == "dev" {
-		// TODO(crawshaw): do more in test mode
+		// TODO(crawshaw): do more in dev mode
 		gcePool.SetEnabled(*devEnableGCE)
 		http.HandleFunc("/dosomework/", handleDoSomeWork(workc))
 	} else {
@@ -3145,6 +3103,8 @@ func (st *buildStatus) Write(p []byte) (n int, err error) {
 	return st.output.Write(p)
 }
 
+// versionTgz returns an io.Reader of a *.tar.gz file containing only
+// a VERSION file containing the contents of the provided rev string.
 func versionTgz(rev string) io.Reader {
 	var buf bytes.Buffer
 	zw := gzip.NewWriter(&buf)
@@ -3175,7 +3135,7 @@ var sourceGroup singleflight.Group
 
 var sourceCache = lru.New(40) // git rev -> []byte
 
-func useWatcher() bool {
+func useGitMirror() bool {
 	return *mode != "dev"
 }
 
@@ -3191,7 +3151,7 @@ func getSourceTgz(sl spanLogger, repo, rev string, isTry bool) (tgz io.Reader, e
 			return tgzBytes, nil
 		}
 
-		if useWatcher() {
+		if useGitMirror() {
 			sp := sl.createSpan("get_source_from_gitmirror")
 			tgzBytes, err := getSourceTgzFromGitMirror(repo, rev)
 			if err == nil {

cmd/coordinator/gce.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"io/ioutil"
 	"log"
+	"os"
 	"path"
 	"sort"
 	"strconv"
@@ -629,3 +630,24 @@ func (c *gcsAutocertCache) Delete(ctx context.Context, key string) error {
 	}
 	return err
 }
+
+func readGCSFile(name string) ([]byte, error) {
+	if *mode == "dev" {
+		b, ok := testFiles[name]
+		if !ok {
+			return nil, &os.PathError{
+				Op:   "open",
+				Path: name,
+				Err:  os.ErrNotExist,
+			}
+		}
+		return []byte(b), nil
+	}
+
+	r, err := storageClient.Bucket(buildEnv.BuildletBucket).Object(name).NewReader(context.Background())
+	if err != nil {
+		return nil, err
+	}
+	defer r.Close()
+	return ioutil.ReadAll(r)
+}

dashboard/builders.go

@@ -8,6 +8,7 @@ package dashboard
 
 import (
 	"fmt"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -17,6 +18,7 @@ import (
 // Builders are the different build configurations.
 // The keys are like "darwin-amd64" or "linux-386-387".
 // This map should not be modified by other packages.
+// Initialization happens below, via calls to addBuilder.
 var Builders = map[string]BuildConfig{}
 
 // Hosts contains the names and configs of all the types of
@@ -119,13 +121,6 @@ var Hosts = map[string]*HostConfig{
 		goBootstrapURLTmpl: "https://storage.googleapis.com/$BUCKET/go1.4-freebsd-amd64.tar.gz",
 		env:                []string{"CC=clang"},
 	},
-	"host-netbsd-70": &HostConfig{
-		VMImage:            "netbsd-amd64-70",
-		Notes:              "NetBSD 7.0_2016Q4; GCE VM is built from script in build/env/netbsd-amd64",
-		machineType:        "n1-highcpu-2",
-		buildletURLTmpl:    "http://storage.googleapis.com/$BUCKET/buildlet.netbsd-amd64",
-		goBootstrapURLTmpl: "https://storage.googleapis.com/$BUCKET/gobootstrap-netbsd-amd64.tar.gz",
-	},
 	"host-netbsd-71": &HostConfig{
 		VMImage:            "netbsd-amd64-71",
 		Notes:              "NetBSD 7.1RC1; GCE VM is built from script in build/env/netbsd-amd64",
@@ -409,6 +404,7 @@ type BuildConfig struct {
 
 	Notes string // notes for humans
 
+	TryBot      bool // be a trybot
 	TryOnly     bool // only used for trybots, and not regular builds
 	CompileOnly bool // if true, compile tests, but don't run them
 	FlakyNet    bool // network tests are flaky (try anyway, but ignore some failures)
@@ -683,6 +679,7 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:              "freebsd-amd64-gce101",
 		HostType:          "host-freebsd-101-gce",
+		TryBot:            true,
 		numTestHelpers:    2,
 		numTryTestHelpers: 4,
 	})
@@ -707,6 +704,7 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:              "linux-386",
 		HostType:          "host-linux-kubestd",
+		TryBot:            true,
 		env:               []string{"GOARCH=386", "GOHOSTARCH=386"},
 		numTestHelpers:    1,
 		numTryTestHelpers: 3,
@@ -720,21 +718,28 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:           "linux-amd64",
 		HostType:       "host-linux-kubestd",
+		TryBot:         true,
 		numTestHelpers: 3,
 	})
+	addBuilder(BuildConfig{
+		Name:     "linux-amd64-alpine",
+		HostType: "host-linux-x86-alpine",
+	})
 	// Add the -vetall builder. The builder name suffix "-vetall" is recognized by cmd/dist/test.go
 	// to only run the "go vet std cmd" test and no others.
 	addBuilder(BuildConfig{
 		Name:           "misc-vet-vetall",
 		HostType:       "host-linux-kubestd",
 		Notes:          "Runs vet over the standard library.",
+		TryBot:         true,
 		numTestHelpers: 5,
 	})
 
 	addMiscCompile := func(suffix, rx string) {
 		addBuilder(BuildConfig{
 			Name:        "misc-compile" + suffix,
 			HostType:    "host-linux-kubestd",
+			TryBot:      true,
 			TryOnly:     true,
 			CompileOnly: true,
 			Notes:       "Runs buildall.sh to cross-compile std packages for " + rx + ", but doesn't run any tests.",
@@ -772,13 +777,15 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:        "linux-amd64-ssacheck",
 		HostType:    "host-linux-kubestd",
+		TryBot:      true,
 		CompileOnly: true,
 		Notes:       "SSA internal checks enabled",
 		env:         []string{"GO_GCFLAGS=-d=ssa/check/on,dclstack"},
 	})
 	addBuilder(BuildConfig{
 		Name:              "linux-amd64-race",
 		HostType:          "host-linux-kubestd",
+		TryBot:            true,
 		numTestHelpers:    2,
 		numTryTestHelpers: 5,
 	})
@@ -808,6 +815,7 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:              "linux-arm",
 		HostType:          "host-linux-arm",
+		TryBot:            true,
 		FlakyNet:          true,
 		numTestHelpers:    2,
 		numTryTestHelpers: 7,
@@ -836,26 +844,27 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:           "nacl-386",
 		HostType:       "host-nacl-kube",
+		TryBot:         true,
 		numTestHelpers: 3,
 		env:            []string{"GOOS=nacl", "GOARCH=386", "GOHOSTOS=linux", "GOHOSTARCH=amd64"},
 	})
 	addBuilder(BuildConfig{
 		Name:           "nacl-amd64p32",
 		HostType:       "host-nacl-kube",
+		TryBot:         true,
 		numTestHelpers: 3,
 		env:            []string{"GOOS=nacl", "GOARCH=amd64p32", "GOHOSTOS=linux", "GOHOSTARCH=amd64"},
 	})
 	addBuilder(BuildConfig{
 		Name:              "openbsd-amd64-60",
 		HostType:          "host-openbsd-amd64-60",
+		TryBot:            true,
 		numTestHelpers:    2,
 		numTryTestHelpers: 5,
 	})
 	addBuilder(BuildConfig{
-		Name:              "openbsd-386-60",
-		HostType:          "host-openbsd-386-60",
-		numTestHelpers:    2,
-		numTryTestHelpers: 5,
+		Name:     "openbsd-386-60",
+		HostType: "host-openbsd-386-60",
 	})
 	addBuilder(BuildConfig{
 		Name:     "netbsd-amd64-71",
@@ -875,6 +884,7 @@ func init() {
 		Name:              "windows-amd64-gce",
 		HostType:          "host-windows-gce",
 		env:               []string{"GOARCH=amd64", "GOHOSTARCH=amd64"},
+		TryBot:            true,
 		numTestHelpers:    1,
 		numTryTestHelpers: 5,
 	})
@@ -888,6 +898,7 @@ func init() {
 		Name:              "windows-386-gce",
 		HostType:          "host-windows-gce",
 		env:               []string{"GOARCH=386", "GOHOSTARCH=386"},
+		TryBot:            true,
 		numTestHelpers:    1,
 		numTryTestHelpers: 5,
 	})
@@ -902,6 +913,7 @@ func init() {
 	addBuilder(BuildConfig{
 		Name:              "darwin-amd64-10_11",
 		HostType:          "host-darwin-10_11",
+		TryBot:            true,
 		numTestHelpers:    2,
 		numTryTestHelpers: 3,
 	})
@@ -1037,6 +1049,8 @@ func (c BuildConfig) isMobile() bool {
 	return strings.HasPrefix(c.Name, "android-") || strings.HasPrefix(c.Name, "darwin-arm")
 }
 
+// addBuilder adds c to the Builders map after doing some sanity
+// checks.
 func addBuilder(c BuildConfig) {
 	if c.Name == "" {
 		panic("empty name")
@@ -1062,5 +1076,29 @@ func addBuilder(c BuildConfig) {
 	if _, ok := Hosts[c.HostType]; !ok {
 		panic(fmt.Sprintf("undefined HostType %q for builder %q", c.HostType, c.Name))
 	}
+
+	types := 0
+	for _, fn := range []func() bool{c.IsReverse, c.IsKube, c.IsGCE} {
+		if fn() {
+			types++
+		}
+	}
+	if types != 1 {
+		panic(fmt.Sprintf("build config %q host type inconsistent (must be Reverse, Kube, or GCE)", c.Name))
+	}
+
 	Builders[c.Name] = c
 }
+
+// TrybotBuilderNames returns the names of the builder configs
+// with the TryBot field set true.
+func TrybotBuilderNames() []string {
+	var ret []string
+	for name, conf := range Builders {
+		if conf.TryBot {
+			ret = append(ret, name)
+		}
+	}
+	sort.Strings(ret)
+	return ret
+}

dashboard/builders_test.go

@@ -20,3 +20,26 @@ func TestOSARCHAccessors(t *testing.T) {
 		}
 	}
 }
+
+func TestListTrybots(t *testing.T) {
+	tryBots := TrybotBuilderNames()
+	t.Logf("Builders:")
+	for _, name := range tryBots {
+		t.Logf("  - %s", name)
+	}
+}
+
+func TestHostConfigsAllUsed(t *testing.T) {
+	used := map[string]bool{}
+	for _, conf := range Builders {
+		used[conf.HostType] = true
+	}
+	for hostType := range Hosts {
+		if !used[hostType] {
+			// Currently host-linux-armhf-cross and host-linux-armel-cross aren't
+			// referenced, but the coordinator hard-codes them, so don't make
+			// this an error for now.
+			t.Logf("warning: host type %q is not referenced from any build config", hostType)
+		}
+	}
+}