cmd/coordinator: clean up reverse buildlet code, export status JSON

bradfitz · bradfitz · commit 15521bc2599c · 2017-02-25T18:24:32.000Z
Will be used for dynamic creation/destruction of Mac VMs in subsequent CL. Updates golang/go#9495 (Mac virtualization) Updates golang/go#15760 (monitoring) Change-Id: I48b17589b258d5d742bad5a3ddae18de98778149 Reviewed-on: https://go-review.googlesource.com/37457 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/cmd/coordinator/coordinator.go b/cmd/coordinator/coordinator.go
@@ -283,6 +283,7 @@ func main() {
 	http.HandleFunc("/reverse", handleReverse)
 	http.HandleFunc("/style.css", handleStyleCSS)
 	http.HandleFunc("/try", handleTryStatus)
+	http.HandleFunc("/status/reverse.json", reversePool.ServeReverseStatusJSON)
 	http.Handle("/buildlet/create", requireBuildletProxyAuth(http.HandlerFunc(handleBuildletCreate)))
 	http.Handle("/buildlet/list", requireBuildletProxyAuth(http.HandlerFunc(handleBuildletList)))
 	go func() {
diff --git a/cmd/coordinator/reverse.go b/cmd/coordinator/reverse.go
@@ -30,6 +30,7 @@ work, go to:
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -45,50 +46,110 @@ import (
 	"golang.org/x/build/buildlet"
 	"golang.org/x/build/dashboard"
 	"golang.org/x/build/revdial"
+	"golang.org/x/build/types"
 )
 
 const minBuildletVersion = 1
 
-var reversePool = &reverseBuildletPool{
-	available: make(chan token, 1),
-}
+var reversePool = new(reverseBuildletPool)
 
 type token struct{}
 
 type reverseBuildletPool struct {
-	available chan token // best-effort tickle when any buildlet becomes free
-
-	mu        sync.Mutex // guards buildlets and their fields
+	mu sync.Mutex // guards all fields, including fields of *reverseBuildlet
+	// TODO: switch to a map[hostType][]buildlets or map of set.
 	buildlets []*reverseBuildlet
+	wakeChan  map[string]chan token // hostType => best-effort wake-up chan when buildlet free
+	waiters   map[string]int        // hostType => number waiters blocked in GetBuildlet
+}
+
+func (p *reverseBuildletPool) ServeReverseStatusJSON(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("Content-Type", "application/json")
+	status := p.buildReverseStatusJSON()
+	j, _ := json.MarshalIndent(status, "", "\t")
+	w.Write(j)
 }
 
-var errInUse = errors.New("all buildlets are in use")
+func (p *reverseBuildletPool) buildReverseStatusJSON() *types.ReverseBuilderStatus {
+	status := &types.ReverseBuilderStatus{}
 
-func (p *reverseBuildletPool) tryToGrab(hostType string) (*buildlet.Client, error) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	candidates := 0
 	for _, b := range p.buildlets {
-		isCandidate := b.hostType == hostType
-		if isCandidate {
-			candidates++
+		hs := status.Host(b.hostType)
+		if hs.Machines == nil {
+			hs.Machines = make(map[string]*types.ReverseBuilder)
+		}
+		hs.Connected++
+		bs := &types.ReverseBuilder{
+			Name:         b.hostname,
+			HostType:     b.hostType,
+			ConnectedSec: time.Since(b.regTime).Seconds(),
+			Version:      b.version,
+		}
+		if b.inUse && !b.inHealthCheck {
+			hs.Busy++
+			bs.Busy = true
+			bs.BusySec = time.Since(b.inUseTime).Seconds()
+		} else {
+			hs.Idle++
+			bs.IdleSec = time.Since(b.inUseTime).Seconds()
 		}
-		if isCandidate && !b.inUse {
-			// Found an unused match.
-			b.inUse = true
-			b.inUseTime = time.Now()
-			return b.client, nil
+
+		hs.Machines[b.hostname] = bs
+	}
+	for hostType, waiters := range p.waiters {
+		status.Host(hostType).Waiters = waiters
+	}
+	for hostType, hc := range dashboard.Hosts {
+		if hc.ExpectNum > 0 {
+			status.Host(hostType).Expect = hc.ExpectNum
 		}
 	}
-	if candidates == 0 {
-		return nil, fmt.Errorf("no buildlets registered for host type %q", hostType)
+	return status
+}
+
+// tryToGrab returns non-nil bc on success if a buildlet is free.
+//
+// Otherwise it returns how many were busy, which might be 0 if none
+// were (yet?) registered. The busy valid is only valid if bc == nil.
+func (p *reverseBuildletPool) tryToGrab(hostType string) (bc *buildlet.Client, busy int) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	for _, b := range p.buildlets {
+		if b.hostType != hostType {
+			continue
+		}
+		if b.inUse {
+			busy++
+			continue
+		}
+		// Found an unused match.
+		b.inUse = true
+		b.inUseTime = time.Now()
+		return b.client, 0
+	}
+	return nil, busy
+}
+
+func (p *reverseBuildletPool) getWakeChan(hostType string) chan token {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if p.wakeChan == nil {
+		p.wakeChan = make(map[string]chan token)
+	}
+	c, ok := p.wakeChan[hostType]
+	if !ok {
+		c = make(chan token)
+		p.wakeChan[hostType] = c
 	}
-	return nil, errInUse
+	return c
 }
 
-func (p *reverseBuildletPool) noteBuildletAvailable() {
+func (p *reverseBuildletPool) noteBuildletAvailable(hostType string) {
+	wake := p.getWakeChan(hostType)
 	select {
-	case p.available <- token{}:
+	case wake <- token{}:
 	default:
 	}
 }
@@ -168,7 +229,7 @@ func (p *reverseBuildletPool) healthCheckBuildlet(b *reverseBuildlet) bool {
 	b.inUse = false
 	b.inHealthCheck = false
 	b.inUseTime = time.Now()
-	p.noteBuildletAvailable()
+	go p.noteBuildletAvailable(b.hostType)
 	return true
 }
 
@@ -188,45 +249,53 @@ func highPriChan(hostType string) chan *buildlet.Client {
 	return c
 }
 
+func (p *reverseBuildletPool) updateWaiterCounter(hostType string, delta int) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if p.waiters == nil {
+		p.waiters = make(map[string]int)
+	}
+	p.waiters[hostType] += delta
+}
+
 func (p *reverseBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg logger) (*buildlet.Client, error) {
+	p.updateWaiterCounter(hostType, 1)
+	defer p.updateWaiterCounter(hostType, -1)
 	seenErrInUse := false
 	isHighPriority, _ := ctx.Value(highPriorityOpt{}).(bool)
 	sp := lg.createSpan("wait_static_builder", hostType)
 	for {
-		b, err := p.tryToGrab(hostType)
-		if err == errInUse {
-			if !seenErrInUse {
-				lg.logEventTime("waiting_machine_in_use")
-				seenErrInUse = true
-			}
-			var highPri chan *buildlet.Client
-			if isHighPriority {
-				highPri = highPriChan(hostType)
-			}
+		bc, busy := p.tryToGrab(hostType)
+		if bc != nil {
 			select {
-			case <-ctx.Done():
-				return nil, sp.done(ctx.Err())
-			case bc := <-highPri:
+			case highPriChan(hostType) <- bc:
+				// Somebody else was more important.
+			default:
 				sp.done(nil)
 				return p.cleanedBuildlet(bc, lg)
+			}
+		}
+		if busy > 0 && !seenErrInUse {
+			lg.logEventTime("waiting_machine_in_use")
+			seenErrInUse = true
+		}
+		var highPri chan *buildlet.Client
+		if isHighPriority {
+			highPri = highPriChan(hostType)
+		}
+		select {
+		case <-ctx.Done():
+			return nil, sp.done(ctx.Err())
+		case bc := <-highPri:
+			sp.done(nil)
+			return p.cleanedBuildlet(bc, lg)
+
+		case <-time.After(10 * time.Second):
 			// As multiple goroutines can be listening for
 			// the available signal, it must be treated as
 			// a best effort signal. So periodically try
-			// to grab a buildlet again:
-			case <-time.After(10 * time.Second):
-			case <-p.available:
-			}
-		} else if err != nil {
-			sp.done(err)
-			return nil, err
-		} else {
-			select {
-			case highPriChan(hostType) <- b:
-				// Somebody else was more important.
-			default:
-				sp.done(nil)
-				return p.cleanedBuildlet(b, lg)
-			}
+			// to grab a buildlet again.
+		case <-p.getWakeChan(hostType):
 		}
 	}
 }
@@ -329,7 +398,7 @@ func (p *reverseBuildletPool) CanBuild(hostType string) bool {
 
 func (p *reverseBuildletPool) addBuildlet(b *reverseBuildlet) {
 	p.mu.Lock()
-	defer p.noteBuildletAvailable()
+	defer p.noteBuildletAvailable(b.hostType)
 	defer p.mu.Unlock()
 	p.buildlets = append(p.buildlets, b)
 	go p.healthCheckBuildletLoop(b)
diff --git a/dashboard/builders.go b/dashboard/builders.go
@@ -61,6 +61,7 @@ var Hosts = map[string]*HostConfig{
 	},
 	"host-linux-arm": &HostConfig{
 		IsReverse:      true,
+		ExpectNum:      50,
 		env:            []string{"GOROOT_BOOTSTRAP=/usr/local/go"},
 		ReverseAliases: []string{"linux-arm", "linux-arm-arm5"},
 	},
@@ -139,6 +140,7 @@ var Hosts = map[string]*HostConfig{
 	},
 	"host-darwin-10_8": &HostConfig{
 		IsReverse: true,
+		ExpectNum: 1,
 		Notes:     "MacStadium OS X 10.8 VM under VMWare ESXi",
 		env: []string{
 			"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
@@ -147,6 +149,7 @@ var Hosts = map[string]*HostConfig{
 	},
 	"host-darwin-10_10": &HostConfig{
 		IsReverse: true,
+		ExpectNum: 2,
 		Notes:     "MacStadium OS X 10.10 VM under VMWare ESXi",
 		env: []string{
 			"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
@@ -155,6 +158,7 @@ var Hosts = map[string]*HostConfig{
 	},
 	"host-darwin-10_11": &HostConfig{
 		IsReverse: true,
+		ExpectNum: 15,
 		Notes:     "MacStadium OS X 10.11 VM under VMWare ESXi",
 		env: []string{
 			"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
@@ -163,6 +167,7 @@ var Hosts = map[string]*HostConfig{
 	},
 	"host-darwin-10_12": &HostConfig{
 		IsReverse: true,
+		ExpectNum: 2,
 		Notes:     "MacStadium OS X 10.12 VM under VMWare ESXi",
 		env: []string{
 			"GOROOT_BOOTSTRAP=/Users/gopher/go1.4",
@@ -178,30 +183,35 @@ var Hosts = map[string]*HostConfig{
 	"host-linux-ppc64-osu": &HostConfig{
 		Notes:          "Debian jessie; run by Go team on osuosl.org",
 		IsReverse:      true,
+		ExpectNum:      5,
 		env:            []string{"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap"},
 		ReverseAliases: []string{"linux-ppc64-buildlet"},
 	},
 	"host-linux-ppc64le-osu": &HostConfig{
 		Notes:          "Debian jessie; run by Go team on osuosl.org",
 		IsReverse:      true,
+		ExpectNum:      5,
 		env:            []string{"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap"},
 		ReverseAliases: []string{"linux-ppc64le-buildlet"},
 	},
 	"host-linux-arm64-linaro": &HostConfig{
 		Notes:          "Ubuntu wily; run by Go team, from linaro",
 		IsReverse:      true,
+		ExpectNum:      5,
 		env:            []string{"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap"},
 		ReverseAliases: []string{"linux-arm64-buildlet"},
 	},
 	"host-solaris-amd64": &HostConfig{
 		Notes:          "run by Go team on Joyent, on a SmartOS 'infrastructure container'",
 		IsReverse:      true,
+		ExpectNum:      5,
 		env:            []string{"GOROOT_BOOTSTRAP=/root/go-solaris-amd64-bootstrap"},
 		ReverseAliases: []string{"solaris-amd64-smartosbuildlet"},
 	},
 	"host-linux-mips": &HostConfig{
 		Notes:     "Run by Brendan Kirby, imgtec.com",
 		IsReverse: true,
+		ExpectNum: 1,
 		env: []string{
 			"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap-mips",
 			"GOARCH=mips",
@@ -213,6 +223,7 @@ var Hosts = map[string]*HostConfig{
 	"host-linux-mipsle": &HostConfig{
 		Notes:     "Run by Brendan Kirby, imgtec.com",
 		IsReverse: true,
+		ExpectNum: 1,
 		env: []string{
 			"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap-mipsle",
 			"GOARCH=mipsle",
@@ -223,6 +234,7 @@ var Hosts = map[string]*HostConfig{
 	"host-linux-mips64": &HostConfig{
 		Notes:     "Run by Brendan Kirby, imgtec.com",
 		IsReverse: true,
+		ExpectNum: 1,
 		env: []string{
 			"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap-mips64",
 			"GOARCH=mips64",
@@ -234,6 +246,7 @@ var Hosts = map[string]*HostConfig{
 	"host-linux-mips64le": &HostConfig{
 		Notes:     "Run by Brendan Kirby, imgtec.com",
 		IsReverse: true,
+		ExpectNum: 1,
 		env: []string{
 			"GOROOT_BOOTSTRAP=/usr/local/go-bootstrap-mips64le",
 			"GOARCH=mips64le",
@@ -299,6 +312,9 @@ type HostConfig struct {
 	machineType string // optional GCE instance type
 	RegularDisk bool   // if true, use spinning disk instead of SSD
 
+	// ReverseOptions:
+	ExpectNum int // expected number of reverse buildlets of this type
+
 	// Optional base env. GOROOT_BOOTSTRAP should go here if the buildlet
 	// has Go 1.4+ baked in somewhere.
 	env []string
diff --git a/types/types.go b/types/types.go
