Skip to content

Commit 02e10ad

Browse files
cagedmantiscagedmantis
committed
internal/pool: move the gce buildlet pool into a pool package
This CL creates the internal/coordinator/pool package intended to contain all buildlet pool implementations. In order to keep this change small and carefully discover where the interactions are between the gce buildlet pool and the rest of the coordinator are, this change only moves the gce buildlet over to the new package. The next steps will be to move the rest of the buildlet pools over to this package. After that we will restructure the implementations themselves in order to increase test coverage and increase the ease of testing. Updates golang/go#36841 Updates golang/go#38337 Change-Id: If82ae1b584bd77c697aa84fadf9011c9e79fa409 Reviewed-on: https://go-review.googlesource.com/c/build/+/227141 Run-TryBot: Carlos Amedee <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Alexander Rakoczy <[email protected]>
1 parent 6a8b9e1 commit 02e10ad

16 files changed

+370
-216
lines changed

‎cmd/coordinator/coordinator.go

Lines changed: 59 additions & 101 deletions
Large diffs are not rendered by default.

‎cmd/coordinator/coordinator_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"golang.org/x/build/buildenv"
2222
"golang.org/x/build/dashboard"
2323
"golang.org/x/build/internal/buildgo"
24+
"golang.org/x/build/internal/coordinator/pool"
2425
"golang.org/x/build/maintner/maintnerd/apipb"
2526
)
2627

@@ -230,8 +231,9 @@ func TestFindWork(t *testing.T) {
230231
if testing.Short() {
231232
t.Skip("skipping in short mode")
232233
}
233-
defer func(old *buildenv.Environment) { buildEnv = old }(buildEnv)
234-
buildEnv = buildenv.Production
234+
buildEnv := pool.GCEBuildEnv()
235+
defer func(old *buildenv.Environment) { pool.SetGCEBuildEnv(old) }(buildEnv)
236+
pool.SetGCEBuildEnv(buildenv.Production)
235237
defer func() { buildgo.TestHookSnapshotExists = nil }()
236238
buildgo.TestHookSnapshotExists = func(br *buildgo.BuilderRev) bool {
237239
if strings.Contains(br.Name, "android") {

‎cmd/coordinator/dash.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828

2929
"cloud.google.com/go/compute/metadata"
3030
"golang.org/x/build/internal/buildgo"
31+
"golang.org/x/build/internal/coordinator/pool"
3132
"golang.org/x/build/internal/secret"
3233
)
3334

@@ -50,7 +51,7 @@ func dash(meth, cmd string, args url.Values, req, resp interface{}) error {
5051
}
5152
var r *http.Response
5253
var err error
53-
cmd = buildEnv.DashBase() + cmd + "?" + argsCopy.Encode()
54+
cmd = pool.GCEBuildEnv().DashBase() + cmd + "?" + argsCopy.Encode()
5455
switch meth {
5556
case "GET":
5657
if req != nil {

‎cmd/coordinator/debug.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"text/template"
2020

2121
"golang.org/x/build/internal/buildgo"
22+
"golang.org/x/build/internal/coordinator/pool"
2223
"golang.org/x/build/types"
2324
)
2425

@@ -50,8 +51,8 @@ func handleDoSomeWork(work chan<- buildgo.BuilderRev) func(w http.ResponseWriter
5051

5152
// Cap number of jobs that can be scheduled from debug UI. If
5253
// buildEnv.MaxBuilds is zero, there is no cap.
53-
if buildEnv.MaxBuilds > 0 && count > buildEnv.MaxBuilds {
54-
count = buildEnv.MaxBuilds
54+
if pool.GCEBuildEnv().MaxBuilds > 0 && count > pool.GCEBuildEnv().MaxBuilds {
55+
count = pool.GCEBuildEnv().MaxBuilds
5556
}
5657
log.Printf("looking for %v work items for %q", count, mode)
5758

‎cmd/coordinator/kube.go

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"golang.org/x/build/buildlet"
2424
"golang.org/x/build/dashboard"
25+
"golang.org/x/build/internal/coordinator/pool"
2526
"golang.org/x/build/internal/sourcecache"
2627
"golang.org/x/build/kubernetes"
2728
"golang.org/x/build/kubernetes/api"
@@ -44,12 +45,12 @@ var (
4445

4546
// initGCE must be called before initKube
4647
func initKube() error {
47-
if buildEnv.KubeBuild.MaxNodes == 0 {
48+
if pool.GCEBuildEnv().KubeBuild.MaxNodes == 0 {
4849
return errors.New("Kubernetes builders disabled due to KubeBuild.MaxNodes == 0")
4950
}
5051

5152
// projectID was set by initGCE
52-
registryPrefix += "/" + buildEnv.ProjectName
53+
registryPrefix += "/" + pool.GCEBuildEnv().ProjectName
5354
if !hasCloudPlatformScope() {
5455
return errors.New("coordinator not running with access to the Cloud Platform scope.")
5556
}
@@ -58,19 +59,19 @@ func initKube() error {
5859
defer cancel() // ctx is only used for discovery and connect; not retained.
5960
var err error
6061
buildletsKubeClient, err = gke.NewClient(ctx,
61-
buildEnv.KubeBuild.Name,
62-
gke.OptZone(buildEnv.ControlZone),
63-
gke.OptProject(buildEnv.ProjectName),
64-
gke.OptTokenSource(gcpCreds.TokenSource))
62+
pool.GCEBuildEnv().KubeBuild.Name,
63+
gke.OptZone(pool.GCEBuildEnv().ControlZone),
64+
gke.OptProject(pool.GCEBuildEnv().ProjectName),
65+
gke.OptTokenSource(pool.GCPCredentials().TokenSource))
6566
if err != nil {
6667
return err
6768
}
6869

6970
goKubeClient, err = gke.NewClient(ctx,
70-
buildEnv.KubeTools.Name,
71-
gke.OptZone(buildEnv.ControlZone),
72-
gke.OptProject(buildEnv.ProjectName),
73-
gke.OptTokenSource(gcpCreds.TokenSource))
71+
pool.GCEBuildEnv().KubeTools.Name,
72+
gke.OptZone(pool.GCEBuildEnv().ControlZone),
73+
gke.OptProject(pool.GCEBuildEnv().ProjectName),
74+
gke.OptTokenSource(pool.GCPCredentials().TokenSource))
7475
if err != nil {
7576
return err
7677
}
@@ -135,12 +136,12 @@ func (p *kubeBuildletPool) pollCapacityLoop() {
135136
func (p *kubeBuildletPool) pollCapacity(ctx context.Context) {
136137
nodes, err := buildletsKubeClient.GetNodes(ctx)
137138
if err != nil {
138-
log.Printf("failed to retrieve nodes to calculate cluster capacity for %s/%s: %v", buildEnv.ProjectName, buildEnv.Region(), err)
139+
log.Printf("failed to retrieve nodes to calculate cluster capacity for %s/%s: %v", pool.GCEBuildEnv().ProjectName, pool.GCEBuildEnv().Region(), err)
139140
return
140141
}
141142
pods, err := buildletsKubeClient.GetPods(ctx)
142143
if err != nil {
143-
log.Printf("failed to retrieve pods to calculate cluster capacity for %s/%s: %v", buildEnv.ProjectName, buildEnv.Region(), err)
144+
log.Printf("failed to retrieve pods to calculate cluster capacity for %s/%s: %v", pool.GCEBuildEnv().ProjectName, pool.GCEBuildEnv().Region(), err)
144145
return
145146
}
146147

@@ -209,7 +210,7 @@ func (p *kubeBuildletPool) pollCapacity(ctx context.Context) {
209210

210211
}
211212

212-
func (p *kubeBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg logger) (*buildlet.Client, error) {
213+
func (p *kubeBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg pool.Logger) (*buildlet.Client, error) {
213214
hconf, ok := dashboard.Hosts[hostType]
214215
if !ok || !hconf.IsContainer() {
215216
return nil, fmt.Errorf("kubepool: invalid host type %q", hostType)
@@ -221,7 +222,7 @@ func (p *kubeBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg
221222
panic("expect non-nil buildletsKubeClient")
222223
}
223224

224-
deleteIn, ok := ctx.Value(buildletTimeoutOpt{}).(time.Duration)
225+
deleteIn, ok := ctx.Value(pool.BuildletTimeoutOpt{}).(time.Duration)
225226
if !ok {
226227
deleteIn = podDeleteTimeout
227228
}
@@ -236,7 +237,7 @@ func (p *kubeBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg
236237
log.Printf("Creating Kubernetes pod %q for %s", podName, hostType)
237238

238239
bc, err := buildlet.StartPod(ctx, buildletsKubeClient, podName, hostType, buildlet.PodOpts{
239-
ProjectID: buildEnv.ProjectName,
240+
ProjectID: pool.GCEBuildEnv().ProjectName,
240241
ImageRegistry: registryPrefix,
241242
Description: fmt.Sprintf("Go Builder for %s", hostType),
242243
DeleteIn: deleteIn,
@@ -291,7 +292,7 @@ func (p *kubeBuildletPool) WriteHTMLStatus(w io.Writer) {
291292
fmt.Fprintf(w, "<ul>")
292293
for i, pod := range active {
293294
if i < show/2 || i >= len(active)-(show/2) {
294-
fmt.Fprintf(w, "<li>%v, %v</li>\n", pod.name, time.Since(pod.creation))
295+
fmt.Fprintf(w, "<li>%v, %v</li>\n", pod.Name, time.Since(pod.Creation))
295296
} else if i == show/2 {
296297
fmt.Fprintf(w, "<li>... %d of %d total omitted ...</li>\n", len(active)-show, len(active))
297298
}
@@ -353,16 +354,16 @@ func (p *kubeBuildletPool) podUsed(podName string) bool {
353354
return ok
354355
}
355356

356-
func (p *kubeBuildletPool) podsActive() (ret []resourceTime) {
357+
func (p *kubeBuildletPool) podsActive() (ret []pool.ResourceTime) {
357358
p.mu.Lock()
358359
defer p.mu.Unlock()
359360
for name, ph := range p.pods {
360-
ret = append(ret, resourceTime{
361-
name: name,
362-
creation: ph.requestedAt,
361+
ret = append(ret, pool.ResourceTime{
362+
Name: name,
363+
Creation: ph.requestedAt,
363364
})
364365
}
365-
sort.Sort(byCreationTime(ret))
366+
sort.Sort(pool.ByCreationTime(ret))
366367
return ret
367368
}
368369

@@ -437,7 +438,7 @@ func (p *kubeBuildletPool) cleanUpOldPods(ctx context.Context) {
437438
}
438439
if err == nil && time.Now().Unix() > unixDeadline {
439440
stats.DeletedOld++
440-
log.Printf("cleanUpOldPods: Deleting expired pod %q in zone %q ...", pod.Name, buildEnv.ControlZone)
441+
log.Printf("cleanUpOldPods: Deleting expired pod %q in zone %q ...", pod.Name, pool.GCEBuildEnv().ControlZone)
441442
err = buildletsKubeClient.DeletePod(ctx, pod.Name)
442443
if err != nil {
443444
log.Printf("cleanUpOldPods: problem deleting old pod %q: %v", pod.Name, err)
@@ -467,5 +468,5 @@ func (p *kubeBuildletPool) cleanUpOldPods(ctx context.Context) {
467468
}
468469

469470
func hasCloudPlatformScope() bool {
470-
return hasScope(container.CloudPlatformScope)
471+
return pool.HasScope(container.CloudPlatformScope)
471472
}

‎cmd/coordinator/log.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"cloud.google.com/go/datastore"
1717

18+
"golang.org/x/build/internal/coordinator/pool"
1819
"golang.org/x/build/types"
1920
)
2021

@@ -32,13 +33,13 @@ type ProcessRecord struct {
3233
}
3334

3435
func updateInstanceRecord() {
35-
if dsClient == nil {
36+
if pool.GCEDSClient() == nil {
3637
return
3738
}
3839
ctx := context.Background()
3940
for {
4041
key := datastore.NameKey("Process", processID, nil)
41-
_, err := dsClient.Put(ctx, key, &ProcessRecord{
42+
_, err := pool.GCEDSClient().Put(ctx, key, &ProcessRecord{
4243
ID: processID,
4344
Start: processStartTime,
4445
LastHeartbeat: time.Now(),
@@ -51,23 +52,23 @@ func updateInstanceRecord() {
5152
}
5253

5354
func putBuildRecord(br *types.BuildRecord) {
54-
if dsClient == nil {
55+
if pool.GCEDSClient() == nil {
5556
return
5657
}
5758
ctx := context.Background()
5859
key := datastore.NameKey("Build", br.ID, nil)
59-
if _, err := dsClient.Put(ctx, key, br); err != nil {
60+
if _, err := pool.GCEDSClient().Put(ctx, key, br); err != nil {
6061
log.Printf("datastore Build Put: %v", err)
6162
}
6263
}
6364

6465
func putSpanRecord(sr *types.SpanRecord) {
65-
if dsClient == nil {
66+
if pool.GCEDSClient() == nil {
6667
return
6768
}
6869
ctx := context.Background()
6970
key := datastore.NameKey("Span", fmt.Sprintf("%s-%v-%v", sr.BuildID, sr.StartTime.UnixNano(), sr.Event), nil)
70-
if _, err := dsClient.Put(ctx, key, sr); err != nil {
71+
if _, err := pool.GCEDSClient().Put(ctx, key, sr); err != nil {
7172
log.Printf("datastore Span Put: %v", err)
7273
}
7374
}

‎cmd/coordinator/metrics.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"time"
1414

1515
"golang.org/x/build/cmd/coordinator/metrics"
16+
"golang.org/x/build/internal/coordinator/pool"
1617

1718
"github.com/golang/protobuf/ptypes"
1819
metpb "google.golang.org/genproto/googleapis/api/metric"
@@ -66,8 +67,8 @@ func reportReverseCountMetrics(ctx context.Context) error {
6667
})
6768
}
6869

69-
return metricsClient.CreateTimeSeries(ctx, &monpb.CreateTimeSeriesRequest{
70-
Name: m.DescriptorPath(buildEnv.ProjectName),
70+
return pool.MetricsClient().CreateTimeSeries(ctx, &monpb.CreateTimeSeriesRequest{
71+
Name: m.DescriptorPath(pool.GCEBuildEnv().ProjectName),
7172
TimeSeries: ts,
7273
})
7374
}

‎cmd/coordinator/remote.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"github.com/kr/pty"
3838
"golang.org/x/build/buildlet"
3939
"golang.org/x/build/dashboard"
40+
"golang.org/x/build/internal/coordinator/pool"
4041
"golang.org/x/build/internal/gophers"
4142
"golang.org/x/build/internal/secret"
4243
"golang.org/x/build/types"
@@ -525,11 +526,11 @@ func listenAndServeSSH(sc *secret.Client) {
525526
log.Fatal(err)
526527
}
527528
} else {
528-
if storageClient == nil {
529+
if pool.StorageClient() == nil {
529530
log.Printf("GCS storage client not available; not running SSH server.")
530531
return
531532
}
532-
r, err := storageClient.Bucket(buildEnv.BuildletBucket).Object("coordinator-gomote-ssh.key").NewReader(context.Background())
533+
r, err := pool.StorageClient().Bucket(pool.GCEBuildEnv().BuildletBucket).Object("coordinator-gomote-ssh.key").NewReader(context.Background())
533534
if err != nil {
534535
log.Printf("Failed to read ssh host key: %v; not running SSH server.", err)
535536
return

‎cmd/coordinator/remote_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"golang.org/x/build/buildlet"
2323
"golang.org/x/build/dashboard"
24+
"golang.org/x/build/internal/coordinator/pool"
2425
)
2526

2627
type TestBuildletPool struct {
@@ -30,7 +31,7 @@ type TestBuildletPool struct {
3031

3132
// GetBuildlet finds the first available buildlet for the hostType and returns
3233
// it, or an error if no buildlets are available for that hostType.
33-
func (tp *TestBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg logger) (*buildlet.Client, error) {
34+
func (tp *TestBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg pool.Logger) (*buildlet.Client, error) {
3435
tp.mu.Lock()
3536
defer tp.mu.Unlock()
3637
c, ok := tp.clients[hostType]
@@ -123,7 +124,7 @@ func TestHandleBuildletCreate_PreStream(t *testing.T) {
123124
defer log.SetOutput(os.Stderr)
124125
addBuilder(buildName)
125126
remoteBuildlets.m = map[string]*remoteBuildlet{}
126-
testPoolHook = func(_ *dashboard.HostConfig) BuildletPool { return testPool }
127+
testPoolHook = func(_ *dashboard.HostConfig) pool.Buildlet { return testPool }
127128
defer func() {
128129
timeNow = time.Now
129130
removeBuilder(buildName)
@@ -152,7 +153,7 @@ func TestHandleBuildletCreate_Stream(t *testing.T) {
152153
defer log.SetOutput(os.Stderr)
153154
addBuilder(buildName)
154155
remoteBuildlets.m = map[string]*remoteBuildlet{}
155-
testPoolHook = func(_ *dashboard.HostConfig) BuildletPool { return testPool }
156+
testPoolHook = func(_ *dashboard.HostConfig) pool.Buildlet { return testPool }
156157
defer func() {
157158
timeNow = time.Now
158159
removeBuilder(buildName)

‎cmd/coordinator/reverse.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747

4848
"golang.org/x/build/buildlet"
4949
"golang.org/x/build/dashboard"
50+
"golang.org/x/build/internal/coordinator/pool"
5051
"golang.org/x/build/revdial/v2"
5152
"golang.org/x/build/types"
5253
)
@@ -295,7 +296,7 @@ func (p *reverseBuildletPool) updateWaiterCounter(hostType string, delta int) {
295296
p.waiters[hostType] += delta
296297
}
297298

298-
func (p *reverseBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg logger) (*buildlet.Client, error) {
299+
func (p *reverseBuildletPool) GetBuildlet(ctx context.Context, hostType string, lg pool.Logger) (*buildlet.Client, error) {
299300
p.updateWaiterCounter(hostType, 1)
300301
defer p.updateWaiterCounter(hostType, -1)
301302
seenErrInUse := false
@@ -324,7 +325,7 @@ func (p *reverseBuildletPool) GetBuildlet(ctx context.Context, hostType string,
324325
}
325326
}
326327

327-
func (p *reverseBuildletPool) cleanedBuildlet(b *buildlet.Client, lg logger) (*buildlet.Client, error) {
328+
func (p *reverseBuildletPool) cleanedBuildlet(b *buildlet.Client, lg pool.Logger) (*buildlet.Client, error) {
328329
// Clean up any files from previous builds.
329330
sp := lg.CreateSpan("clean_buildlet", b.String())
330331
err := b.RemoveAll(context.Background(), ".")

‎cmd/coordinator/sched.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"golang.org/x/build/buildlet"
1919
"golang.org/x/build/dashboard"
2020
"golang.org/x/build/internal/buildgo"
21+
"golang.org/x/build/internal/coordinator/pool"
2122
"golang.org/x/build/internal/spanlog"
2223
"golang.org/x/build/types"
2324
)
@@ -43,7 +44,7 @@ type Scheduler struct {
4344
// A getBuildletResult is a buildlet that was just created and is up and
4445
// is ready to be assigned to a caller based on priority.
4546
type getBuildletResult struct {
46-
Pool BuildletPool
47+
Pool pool.Buildlet
4748
HostType string
4849

4950
// One of Client or Err gets set:
@@ -142,7 +143,7 @@ func (l stderrLogger) CreateSpan(event string, optText ...string) spanlog.Span {
142143

143144
// getPoolBuildlet is launched as its own goroutine to do a
144145
// potentially long blocking cal to pool.GetBuildlet.
145-
func (s *Scheduler) getPoolBuildlet(pool BuildletPool, hostType string) {
146+
func (s *Scheduler) getPoolBuildlet(pool pool.Buildlet, hostType string) {
146147
res := getBuildletResult{
147148
Pool: pool,
148149
HostType: hostType,
@@ -341,7 +342,7 @@ type SchedItem struct {
341342
s *Scheduler
342343
requestTime time.Time
343344
tryFor string // TODO: which user. (user with 1 trybot >> user with 50 trybots)
344-
pool BuildletPool
345+
pool pool.Buildlet
345346
ctxDone <-chan struct{}
346347

347348
// wantRes is the unbuffered channel that's passed

0 commit comments

Comments
 (0)