Skip to content

Commit c4e3711

Browse files
cagedmantiscagedmantis
committed
cmd/coordinator, internal/coordinator/pool: move pool buildlet func
This change moves the function which returns the appropriate pool for the configuration passed in into the pool package. This work is being done as part of a project to break the coordinator into seperate parts. Updates golang/go#48742 Updates golang/go#38337 Change-Id: Ie5b3fc2da6534fca6e55ba6bb710db5e206efe00 Reviewed-on: https://go-review.googlesource.com/c/build/+/368674 Trust: Carlos Amedee <[email protected]> Run-TryBot: Carlos Amedee <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> Reviewed-by: Alex Rakoczy <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent f1ed2d1 commit c4e3711

File tree

14 files changed

+56
-52
lines changed

14 files changed

+56
-52
lines changed

‎cmd/coordinator/buildstatus.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func (st *buildStatus) start() {
213213
}
214214

215215
func (st *buildStatus) buildletPool() pool.Buildlet {
216-
return poolForConf(st.conf.HostConfig())
216+
return pool.ForHost(st.conf.HostConfig())
217217
}
218218

219219
// parentRev returns the parent of this build's commit (but only if this build comes from a trySet).
@@ -589,7 +589,7 @@ func (st *buildStatus) buildRecord() *types.BuildRecord {
589589

590590
// Log whether we used COS, so we can do queries to analyze
591591
// Kubernetes vs COS performance for containers.
592-
if st.conf.IsContainer() && poolForConf(st.conf.HostConfig()) == pool.NewGCEConfiguration().BuildletPool() {
592+
if st.conf.IsContainer() && pool.ForHost(st.conf.HostConfig()) == pool.NewGCEConfiguration().BuildletPool() {
593593
rec.ContainerHost = "cos"
594594
}
595595

‎cmd/coordinator/coordinator.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,33 +1778,6 @@ func getBuildlets(ctx context.Context, n int, schedTmpl *SchedItem, lg pool.Logg
17781778
return ch
17791779
}
17801780

1781-
var testPoolHook func(*dashboard.HostConfig) pool.Buildlet
1782-
1783-
func poolForConf(conf *dashboard.HostConfig) pool.Buildlet {
1784-
if testPoolHook != nil {
1785-
return testPoolHook(conf)
1786-
}
1787-
if conf == nil {
1788-
panic("nil conf")
1789-
}
1790-
switch {
1791-
case conf.IsEC2():
1792-
return pool.EC2BuildetPool()
1793-
case conf.IsVM():
1794-
return pool.NewGCEConfiguration().BuildletPool()
1795-
case conf.IsContainer():
1796-
if pool.NewGCEConfiguration().BuildEnv().PreferContainersOnCOS || pool.KubeErr() != nil {
1797-
return pool.NewGCEConfiguration().BuildletPool() // it also knows how to do containers.
1798-
} else {
1799-
return pool.KubePool()
1800-
}
1801-
case conf.IsReverse:
1802-
return pool.ReversePool()
1803-
default:
1804-
panic(fmt.Sprintf("no buildlet pool for host type %q", conf.HostType))
1805-
}
1806-
}
1807-
18081781
// noCommitDetail is just a nice name for nil at call sites.
18091782
var noCommitDetail *commitDetail = nil
18101783

‎cmd/coordinator/remote_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,11 @@ func TestHandleBuildletCreate_PreStream(t *testing.T) {
126126
defer log.SetOutput(os.Stderr)
127127
addBuilder(buildName)
128128
remoteBuildlets.m = map[string]*remoteBuildlet{}
129-
testPoolHook = func(_ *dashboard.HostConfig) pool.Buildlet { return testPool }
129+
pool.TestPoolHook = func(_ *dashboard.HostConfig) pool.Buildlet { return testPool }
130130
defer func() {
131131
timeNow = time.Now
132132
removeBuilder(buildName)
133-
testPoolHook = nil
133+
pool.TestPoolHook = nil
134134
}()
135135
timeNow = func() time.Time { return time.Unix(123, 0).In(time.UTC) }
136136
data := url.Values{}
@@ -155,11 +155,11 @@ func TestHandleBuildletCreate_Stream(t *testing.T) {
155155
defer log.SetOutput(os.Stderr)
156156
addBuilder(buildName)
157157
remoteBuildlets.m = map[string]*remoteBuildlet{}
158-
testPoolHook = func(_ *dashboard.HostConfig) pool.Buildlet { return testPool }
158+
pool.TestPoolHook = func(_ *dashboard.HostConfig) pool.Buildlet { return testPool }
159159
defer func() {
160160
timeNow = time.Now
161161
removeBuilder(buildName)
162-
testPoolHook = nil
162+
pool.TestPoolHook = nil
163163
}()
164164
timeNow = func() time.Time { return time.Unix(123, 0).In(time.UTC) }
165165
data := url.Values{}

‎cmd/coordinator/sched.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func (s *Scheduler) scheduleLocked() {
113113
if need <= 0 {
114114
continue
115115
}
116-
pool := poolForConf(dashboard.Hosts[hostType])
116+
pool := pool.ForHost(dashboard.Hosts[hostType])
117117
// TODO: recognize certain pools like the reverse pool
118118
// that have finite capacity and will just queue up
119119
// GetBuildlet calls anyway and avoid extra goroutines
@@ -362,10 +362,10 @@ type SchedItem struct {
362362
// The provided si must be newly allocated; ownership passes to the scheduler.
363363
func (s *Scheduler) GetBuildlet(ctx context.Context, si *SchedItem) (*buildlet.Client, error) {
364364
hostConf, ok := dashboard.Hosts[si.HostType]
365-
if !ok && testPoolHook == nil {
365+
if !ok && pool.TestPoolHook == nil {
366366
return nil, fmt.Errorf("invalid SchedItem.HostType %q", si.HostType)
367367
}
368-
pool := poolForConf(hostConf)
368+
pool := pool.ForHost(hostConf)
369369

370370
si.pool = pool
371371
si.s = s

‎cmd/coordinator/sched_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func (m poolChan) GetBuildlet(ctx context.Context, hostType string, lg cpool.Log
228228
func (poolChan) String() string { return "testing poolChan" }
229229

230230
func TestScheduler(t *testing.T) {
231-
defer func() { testPoolHook = nil }()
231+
defer func() { cpool.TestPoolHook = nil }()
232232

233233
var pool poolChan // initialized per test below
234234
// buildletAvailable is a step that creates a buildlet to the pool.
@@ -309,7 +309,7 @@ func TestScheduler(t *testing.T) {
309309
pool["test-host-foo"] = make(chan interface{}, 1)
310310
pool["test-host-bar"] = make(chan interface{}, 1)
311311

312-
testPoolHook = func(*dashboard.HostConfig) cpool.Buildlet { return pool }
312+
cpool.TestPoolHook = func(*dashboard.HostConfig) cpool.Buildlet { return pool }
313313
t.Run(tt.name, func(t *testing.T) {
314314
s := NewScheduler()
315315
for i, st := range tt.steps() {

‎internal/coordinator/pool/ec2.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
package pool

‎internal/coordinator/pool/ec2_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
package pool

‎internal/coordinator/pool/gce.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
// Code interacting with Google Compute Engine (GCE) and

‎internal/coordinator/pool/kube.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
package pool

‎internal/coordinator/pool/ledger.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
package pool

‎internal/coordinator/pool/ledger_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
package pool

‎internal/coordinator/pool/pool.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5+
//go:build linux || darwin
6+
// +build linux darwin
7+
58
package pool
69

710
import (
@@ -13,6 +16,7 @@ import (
1316
"time"
1417

1518
"golang.org/x/build/buildlet"
19+
"golang.org/x/build/dashboard"
1620
)
1721

1822
// BuildletTimeoutOpt is a context.Value key for BuildletPool.GetBuildlet.
@@ -85,3 +89,34 @@ func deleteTimeoutFromContextOrValue(ctx context.Context, timeout time.Duration)
8589
func isBuildlet(name string) bool {
8690
return strings.HasPrefix(name, "buildlet-")
8791
}
92+
93+
// TestPoolHook is used to override the buildlet returned by ForConf. It should only be used for
94+
// testing purposes.
95+
var TestPoolHook func(*dashboard.HostConfig) Buildlet
96+
97+
// ForHost returns the appropriate buildlet depending on the host configuration that is passed it.
98+
// The returned buildlet can be overriden for testing purposes by registering a test hook.
99+
func ForHost(conf *dashboard.HostConfig) Buildlet {
100+
if TestPoolHook != nil {
101+
return TestPoolHook(conf)
102+
}
103+
if conf == nil {
104+
panic("nil conf")
105+
}
106+
switch {
107+
case conf.IsEC2():
108+
return EC2BuildetPool()
109+
case conf.IsVM():
110+
return NewGCEConfiguration().BuildletPool()
111+
case conf.IsContainer():
112+
if NewGCEConfiguration().BuildEnv().PreferContainersOnCOS || KubeErr() != nil {
113+
return NewGCEConfiguration().BuildletPool() // it also knows how to do containers.
114+
} else {
115+
return KubePool()
116+
}
117+
case conf.IsReverse:
118+
return ReversePool()
119+
default:
120+
panic(fmt.Sprintf("no buildlet pool for host type %q", conf.HostType))
121+
}
122+
}

‎internal/coordinator/pool/pool_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5+
//go:build linux || darwin
6+
// +build linux darwin
7+
58
package pool
69

710
import (

‎internal/coordinator/pool/reverse.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build go1.13 && (linux || darwin)
6-
// +build go1.13
5+
//go:build linux || darwin
76
// +build linux darwin
87

98
package pool

0 commit comments

Comments
 (0)