Skip to content

Commit c8126a0

Browse files
committed
cmd/coordinator: run subrepo trybots against recent Go release branches
This is the coordinator half of the change to make the coordinator test subrepos against two prior releases of Go. The next change that's required is for the maintnerd API server to return the name of the past two releases. For now this change is a no-op. But once maintnerd starts returning more data, then this change will start causing causing two more linux-amd64 builders per subrepo trybot. Updates golang/go#17626 Change-Id: I1cedbc2e4eee51fb003c8bcc8e072fd10717a833 Reviewed-on: https://go-review.googlesource.com/c/143538 Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent 037bfd7 commit c8126a0

File tree

1 file changed

+62
-21
lines changed

1 file changed

+62
-21
lines changed

cmd/coordinator/coordinator.go

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,9 +1058,9 @@ type trySet struct {
10581058

10591059
type trySetState struct {
10601060
remain int
1061-
failed []string // build names
1061+
failed []string // builder names, with optional " ($branch)" suffix
10621062
builds []*buildStatus
1063-
benchResults []string // builder names
1063+
benchResults []string // builder names, with optional " ($branch)" suffix
10641064
}
10651065

10661066
func (ts trySetState) clone() trySetState {
@@ -1100,33 +1100,66 @@ func newTrySet(work *apipb.GerritTryWorkItem) *trySet {
11001100
tryID: "T" + randHex(9),
11011101
trySetState: trySetState{
11021102
remain: len(builders),
1103-
builds: make([]*buildStatus, len(builders)),
1103+
builds: make([]*buildStatus, 0, len(builders)),
11041104
},
11051105
}
11061106

1107-
// For now, for subrepos, we only support building one repo.
1108-
// TODO: Issue 17626: test subrepos against Go master and past two
1109-
// releases. But to save resources, we'll probably only want
1110-
// to do that for linux-amd64 (the Kubernetes cheap builder) to
1111-
// not blow up usage? Or maybe it doesn't matter.
1107+
// GoCommit is non-empty for x/* repos (aka "subrepos"). It
1108+
// is the Go revision to use to build & test the x/* repo
1109+
// with. The first element is the master branch. We test the
1110+
// master branch against all the normal builders configured to
1111+
// do subrepos (subTryBuilders above). Any GoCommit values past
1112+
// the first are for older release branches, but we use a limited
1113+
// subset of builders for those.
11121114
var goRev string
11131115
if len(work.GoCommit) > 0 {
11141116
goRev = work.GoCommit[0]
11151117
}
11161118

1119+
addBuilderToSet := func(bs *buildStatus, brev buildgo.BuilderRev) {
1120+
bs.trySet = ts
1121+
status[brev] = bs
1122+
1123+
idx := len(ts.builds)
1124+
ts.builds = append(ts.builds, bs)
1125+
go bs.start() // acquires statusMu itself, so in a goroutine
1126+
go ts.awaitTryBuild(idx, bs, brev)
1127+
}
1128+
11171129
go ts.notifyStarting()
1118-
for i, bconf := range builders {
1130+
for _, bconf := range builders {
11191131
brev := tryKeyToBuilderRev(bconf.Name, key, goRev)
11201132
bs, err := newBuild(brev)
11211133
if err != nil {
11221134
log.Printf("can't create build for %q: %v", brev, err)
11231135
continue
11241136
}
1125-
bs.trySet = ts
1126-
status[brev] = bs
1127-
ts.builds[i] = bs
1128-
go bs.start() // acquires statusMu itself, so in a goroutine
1129-
go ts.awaitTryBuild(i, bconf, bs, brev)
1137+
addBuilderToSet(bs, brev)
1138+
}
1139+
1140+
// Defensive check that the input is well-formed and each GoCommit
1141+
// has a GoBranch.
1142+
if len(work.GoBranch) < len(work.GoCommit) {
1143+
log.Printf("WARNING: len(GoBranch) of %d != len(GoCommit) of %d", len(work.GoBranch), len(work.GoCommit))
1144+
work.GoCommit = work.GoCommit[:len(work.GoBranch)]
1145+
}
1146+
1147+
// If there's more than one GoCommit, that means this is an x/* repo
1148+
// and we're testing against previous releases of Go.
1149+
for i, goRev := range work.GoCommit {
1150+
if i == 0 {
1151+
// Skip the i==0 element, which is handled above.
1152+
continue
1153+
}
1154+
branch := work.GoBranch[i]
1155+
brev := tryKeyToBuilderRev("linux-amd64", key, goRev)
1156+
bs, err := newBuild(brev)
1157+
if err != nil {
1158+
log.Printf("can't create build for %q: %v", brev, err)
1159+
continue
1160+
}
1161+
bs.goBranch = branch
1162+
addBuilderToSet(bs, brev)
11301163
}
11311164
return ts
11321165
}
@@ -1184,7 +1217,7 @@ func (ts *trySet) notifyStarting() {
11841217
//
11851218
// If the build fails without getting to the end, it sleeps and
11861219
// reschedules it, as long as it's still wanted.
1187-
func (ts *trySet) awaitTryBuild(idx int, bconf dashboard.BuildConfig, bs *buildStatus, brev buildgo.BuilderRev) {
1220+
func (ts *trySet) awaitTryBuild(idx int, bs *buildStatus, brev buildgo.BuilderRev) {
11881221
for {
11891222
WaitCh:
11901223
for {
@@ -1202,7 +1235,7 @@ func (ts *trySet) awaitTryBuild(idx int, bconf dashboard.BuildConfig, bs *buildS
12021235
}
12031236

12041237
if bs.hasEvent(eventDone) || bs.hasEvent(eventSkipBuildMissingDep) {
1205-
ts.noteBuildComplete(bconf, bs)
1238+
ts.noteBuildComplete(bs)
12061239
return
12071240
}
12081241

@@ -1241,7 +1274,7 @@ func (ts *trySet) cancelBuilds() {
12411274
// TODO(bradfitz): implement
12421275
}
12431276

1244-
func (ts *trySet) noteBuildComplete(bconf dashboard.BuildConfig, bs *buildStatus) {
1277+
func (ts *trySet) noteBuildComplete(bs *buildStatus) {
12451278
bs.mu.Lock()
12461279
succeeded := bs.succeeded
12471280
var buildLog string
@@ -1253,12 +1286,12 @@ func (ts *trySet) noteBuildComplete(bconf dashboard.BuildConfig, bs *buildStatus
12531286

12541287
ts.mu.Lock()
12551288
if hasBenchResults {
1256-
ts.benchResults = append(ts.benchResults, bs.Name)
1289+
ts.benchResults = append(ts.benchResults, bs.NameAndBranch())
12571290
}
12581291
ts.remain--
12591292
remain := ts.remain
12601293
if !succeeded {
1261-
ts.failed = append(ts.failed, bconf.Name)
1294+
ts.failed = append(ts.failed, bs.NameAndBranch())
12621295
}
12631296
numFail := len(ts.failed)
12641297
benchResults := append([]string(nil), ts.benchResults...)
@@ -1282,7 +1315,7 @@ func (ts *trySet) noteBuildComplete(bconf dashboard.BuildConfig, bs *buildStatus
12821315
bs.failURL = failLogURL
12831316
bs.mu.Unlock()
12841317
ts.mu.Lock()
1285-
fmt.Fprintf(&ts.errMsg, "Failed on %s: %s\n", bs.Name, failLogURL)
1318+
fmt.Fprintf(&ts.errMsg, "Failed on %s: %s\n", bs.NameAndBranch(), failLogURL)
12861319
ts.mu.Unlock()
12871320

12881321
if numFail == 1 && remain > 0 {
@@ -1292,7 +1325,7 @@ func (ts *trySet) noteBuildComplete(bconf dashboard.BuildConfig, bs *buildStatus
12921325
"This change failed on %s:\n"+
12931326
"See %s\n\n"+
12941327
"Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.",
1295-
bs.Name, failLogURL),
1328+
bs.NameAndBranch(), failLogURL),
12961329
}); err != nil {
12971330
log.Printf("Failed to call Gerrit: %v", err)
12981331
return
@@ -3251,6 +3284,7 @@ type buildStatus struct {
32513284
// Immutable:
32523285
buildgo.BuilderRev
32533286
buildID string // "B" + 9 random hex
3287+
goBranch string // non-empty for subrepo trybots if not go master branch
32543288
conf dashboard.BuildConfig
32553289
startTime time.Time // actually time of newBuild (~same thing); TODO(bradfitz): rename this createTime
32563290
trySet *trySet // or nil
@@ -3275,6 +3309,13 @@ type buildStatus struct {
32753309
useSnapshotMemo *bool // if non-nil, memoized result of useSnapshot
32763310
}
32773311

3312+
func (st *buildStatus) NameAndBranch() string {
3313+
if st.goBranch != "" {
3314+
return fmt.Sprintf("%s (go branch %s)", st.Name, st.goBranch)
3315+
}
3316+
return st.Name
3317+
}
3318+
32783319
func (st *buildStatus) setDone(succeeded bool) {
32793320
st.mu.Lock()
32803321
defer st.mu.Unlock()

0 commit comments

Comments
 (0)