Skip to content

Commit 79dd4f3

Browse files
dmitshurgopherbot
authored andcommitted
cmd/coordinator/internal/legacydash: add LUCI build result support
The better way to do this would've been to separate the display logic from database reading/writing logic. But that's what the incomplete dashboard v2 attempt is about. It's a bit too late to try to do more here, so instead try to make minimal changes to get the job done and make it easier to be confident it won't break existing functionality. For golang/go#65913. Change-Id: I8c43c25759929c8bb2c4bb18613258f4401577f9 Reviewed-on: https://go-review.googlesource.com/c/build/+/567577 Reviewed-by: Dmitri Shuralyov <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent 8e6bb5e commit 79dd4f3

File tree

6 files changed

+306
-59
lines changed

6 files changed

+306
-59
lines changed

cmd/coordinator/coordinator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func main() {
360360
// grpcServer is a shared gRPC server. It is global, as it needs to be used in places that aren't factored otherwise.
361361
grpcServer := grpc.NewServer(opts...)
362362

363-
dashV1 := legacydash.Handler(gce.GoDSClient(), maintnerClient, string(masterKey()), grpcServer)
363+
dashV1 := legacydash.Handler(gce.GoDSClient(), maintnerClient, nil, string(masterKey()), grpcServer)
364364
dashV2 := &builddash.Handler{Datastore: gce.GoDSClient(), Maintner: maintnerClient}
365365
gs := &gRPCServer{dashboardURL: "https://build.golang.org"}
366366
setSessionPool(sp)

cmd/coordinator/internal/legacydash/build.go

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ type Commit struct {
138138
// For non-Go commits, only the Results for the current Go tip, weekly,
139139
// and release Tags are stored here. This is purely de-normalized data.
140140
// The complete data set is stored in Result entities.
141+
//
142+
// Each string is formatted as builder|OK|LogHash|GoHash.
141143
ResultData []string `datastore:",noindex"`
142144
}
143145

@@ -225,49 +227,66 @@ func min(a, b int) int {
225227
//
226228
// For the main Go repo, goHash is the empty string.
227229
func (c *Commit) Result(builder, goHash string) *Result {
228-
return result(c.ResultData, c.Hash, c.PackagePath, builder, goHash)
230+
r := result(c.ResultData, c.Hash, c.PackagePath, builder, goHash)
231+
if r == nil {
232+
return nil
233+
}
234+
return &r.Result
229235
}
230236

231237
// Result returns the build Result for this commit for the given builder/goHash.
232238
//
233239
// For the main Go repo, goHash is the empty string.
234-
func (c *CommitInfo) Result(builder, goHash string) *Result {
240+
func (c *CommitInfo) Result(builder, goHash string) *DisplayResult {
235241
if r := result(c.ResultData, c.Hash, c.PackagePath, builder, goHash); r != nil {
236242
return r
237243
}
238244
if u, ok := c.BuildingURLs[builderAndGoHash{builder, goHash}]; ok {
239-
return &Result{
245+
return &DisplayResult{Result: Result{
240246
Builder: builder,
241247
BuildingURL: u,
242248
Hash: c.Hash,
243249
GoHash: goHash,
244-
}
250+
}}
245251
}
246252
if fakeResults {
247253
// Create a fake random result.
248254
switch rand.Intn(3) {
249255
default:
250256
return nil
251257
case 1:
252-
return &Result{
258+
return &DisplayResult{Result: Result{
253259
Builder: builder,
254260
Hash: c.Hash,
255261
GoHash: goHash,
256262
OK: true,
257-
}
263+
}}
258264
case 2:
259-
return &Result{
265+
return &DisplayResult{Result: Result{
260266
Builder: builder,
261267
Hash: c.Hash,
262268
GoHash: goHash,
263269
LogHash: "fakefailureurl",
264-
}
270+
}}
265271
}
266272
}
267273
return nil
268274
}
269275

270-
func result(resultData []string, hash, packagePath, builder, goHash string) *Result {
276+
type DisplayResult struct {
277+
Result
278+
Noise bool
279+
}
280+
281+
func (r *DisplayResult) LogURL() string {
282+
if strings.HasPrefix(r.LogHash, "https://") {
283+
return r.LogHash
284+
} else {
285+
return "/log/" + r.LogHash
286+
}
287+
}
288+
289+
func result(resultData []string, hash, packagePath, builder, goHash string) *DisplayResult {
271290
for _, r := range resultData {
272291
if !strings.HasPrefix(r, builder) {
273292
// Avoid strings.SplitN alloc in the common case.
@@ -292,6 +311,14 @@ func result(resultData []string, hash, packagePath, builder, goHash string) *Res
292311
// As a special case, "tip" is an alias for "master", since this app
293312
// still uses a bunch of hg terms from when we used hg.
294313
func isUntested(builder, repo, branch, goBranch string) bool {
314+
if strings.HasSuffix(builder, "-🐇") {
315+
// LUCI builders are never considered untested.
316+
//
317+
// Note: It would be possible to improve this by reporting
318+
// whether a given LUCI builder exists for a given x/ repo.
319+
// That needs more bookkeeping and code, so left for later.
320+
return false
321+
}
295322
if branch == "tip" {
296323
branch = "master"
297324
}
@@ -320,8 +347,8 @@ func knownIssue(builder string) int {
320347
return 0
321348
}
322349

323-
// Results returns the build Results for this Commit.
324-
func (c *CommitInfo) Results() (results []*Result) {
350+
// Results returns the build results for this Commit.
351+
func (c *CommitInfo) Results() (results []*DisplayResult) {
325352
for _, r := range c.ResultData {
326353
p := strings.SplitN(r, "|", 4)
327354
if len(p) != 4 {
@@ -376,15 +403,18 @@ func reverse(s []string) {
376403
}
377404
}
378405

379-
// partsToResult creates a Result from ResultData substrings.
380-
func partsToResult(hash, packagePath string, p []string) *Result {
381-
return &Result{
382-
Builder: p[0],
383-
Hash: hash,
384-
PackagePath: packagePath,
385-
GoHash: p[3],
386-
OK: p[1] == "true",
387-
LogHash: p[2],
406+
// partsToResult creates a DisplayResult from ResultData substrings.
407+
func partsToResult(hash, packagePath string, p []string) *DisplayResult {
408+
return &DisplayResult{
409+
Result: Result{
410+
Builder: p[0],
411+
Hash: hash,
412+
PackagePath: packagePath,
413+
GoHash: p[3],
414+
OK: p[1] == "true",
415+
LogHash: p[2],
416+
},
417+
Noise: p[1] == "infra_failure",
388418
}
389419
}
390420

cmd/coordinator/internal/legacydash/dash.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,16 @@ import (
1919

2020
"cloud.google.com/go/datastore"
2121
"github.com/NYTimes/gziphandler"
22+
"golang.org/x/build/cmd/coordinator/internal/lucipoll"
2223
"golang.org/x/build/maintner/maintnerd/apipb"
2324
"golang.org/x/build/repos"
2425
"google.golang.org/grpc"
2526
)
2627

28+
type luciClient interface {
29+
PostSubmitSnapshot() lucipoll.Snapshot
30+
}
31+
2732
type handler struct {
2833
mux *http.ServeMux
2934

@@ -34,6 +39,9 @@ type handler struct {
3439
// Maintner client for the maintner service.
3540
// Typically the one at maintner.golang.org.
3641
maintnerCl apipb.MaintnerServiceClient
42+
43+
// LUCI is a client for LUCI, used for fetching build results from there.
44+
LUCI luciClient
3745
}
3846

3947
func (h handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { h.mux.ServeHTTP(w, req) }
@@ -43,11 +51,12 @@ const fakeResults = false
4351

4452
// Handler sets a datastore client, maintner client, builder master key and
4553
// GRPC server at the package scope, and returns an HTTP mux for the legacy dashboard.
46-
func Handler(dc *datastore.Client, mc apipb.MaintnerServiceClient, key string, grpcServer *grpc.Server) http.Handler {
54+
func Handler(dc *datastore.Client, mc apipb.MaintnerServiceClient, lc luciClient, key string, grpcServer *grpc.Server) http.Handler {
4755
h := handler{
4856
mux: http.NewServeMux(),
4957
datastoreCl: dc,
5058
maintnerCl: mc,
59+
LUCI: lc,
5160
}
5261
kc := keyCheck{masterKey: key}
5362

0 commit comments

Comments
 (0)