cmd/coordinator/internal/dashboard: add LUCI build result support

Make some progress on the dashboard v2 package, which is cleaner¹ and
easier to prototype changes in compared to the original legacydash v1
package. Notably, add support for displaying failing test results and
noise results (e.g., a LUCI build with INFRA_FAILURE status).

For golang/go#65913.

¹ In large part this is due to it being focused on displaying results
  only; it doesn't handle receiving build results and writing them to
  Datastore as legacydash v1 does.

Change-Id: I2f0032a275dc8d41af81865dd6ec1f0ea9ef7997
Reviewed-on: https://go-review.googlesource.com/c/build/+/567576
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>

Author: dmitshur

cmd/coordinator/internal/dashboard/dashboard.html

@@ -165,10 +165,14 @@ <h2 class="Dashboard-packageName">{{$.Package.Name}}</h2>
           {{range $a := .Archs}}
             <td class="Build-result {{if not $a.FirstClass}} unsupported{{end}}" data-builder="{{$a.Name}}">
               {{with $c.ResultForBuilder $a.Name}}
-                  {{if .OK}}
-                    <a class="Build-resultOK" href="https://build.golang.org/log/{{.LogHash}}"
-                      title="Build log of {{$a.Name}} on commit {{shortHash $c.Hash}}.">ok</a>
-                  {{end}}
+                {{if .BuildingURL}}
+                  <a href="{{.BuildingURL}}"><img src="https://go.dev/favicon.ico" height=16 width=16 border=0></a>
+                {{else}}
+                  <a class="Build-result{{if .OK}}OK{{else}}Fail{{end}}
+                    {{- if .Noise}} Build-resultNoise{{end}}"
+                    href="{{.LogURL}}"
+                    title="Build log of {{$a.Name}} on commit {{shortHash $c.Hash}}.">{{if .OK}}ok{{else}}fail{{end}}</a>
+                {{end}}
               {{end}}
             </td>
           {{end}}

cmd/coordinator/internal/dashboard/datastore.go

@@ -9,12 +9,15 @@ package dashboard
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log"
 
 	"cloud.google.com/go/datastore"
+	bbpb "go.chromium.org/luci/buildbucket/proto"
+	"golang.org/x/build/cmd/coordinator/internal/lucipoll"
 )
 
-// getDatastoreResults populates result data on commits, fetched from Datastore.
+// getDatastoreResults populates result data fetched from Datastore into commits.
 func getDatastoreResults(ctx context.Context, cl *datastore.Client, commits []*commit, pkg string) {
 	var keys []*datastore.Key
 	for _, c := range commits {
@@ -27,7 +30,7 @@ func getDatastoreResults(ctx context.Context, cl *datastore.Client, commits []*c
 	out := make([]*Commit, len(keys))
 	// datastore.ErrNoSuchEntity is returned when we ask for a commit that we do not yet have test data.
 	if err := cl.GetMulti(ctx, keys, out); err != nil && filterMultiError(err, ignoreNoSuchEntity) != nil {
-		log.Printf("getResults: error fetching %d results: %v", len(keys), err)
+		log.Printf("getDatastoreResults: error fetching %d results: %v", len(keys), err)
 		return
 	}
 	hashOut := make(map[string]*Commit)
@@ -41,7 +44,48 @@ func getDatastoreResults(ctx context.Context, cl *datastore.Client, commits []*c
 			c.ResultData = result.ResultData
 		}
 	}
-	return
+}
+
+// appendLUCIResults appends result data polled from LUCI to commits.
+func appendLUCIResults(luci lucipoll.Snapshot, commits []*commit, repo string) {
+	commitBuilds, ok := luci.RepoCommitBuilds[repo]
+	if !ok {
+		return
+	}
+	for _, c := range commits {
+		builds, ok := commitBuilds[c.Hash]
+		if !ok {
+			// No builds for this commit.
+			continue
+		}
+		for _, b := range builds {
+			switch b.Status {
+			case bbpb.Status_STARTED:
+				c.ResultData = append(c.ResultData, fmt.Sprintf("%s|%s",
+					b.BuilderName,
+					buildURL(b.ID),
+				))
+			case bbpb.Status_SUCCESS, bbpb.Status_FAILURE:
+				c.ResultData = append(c.ResultData, fmt.Sprintf("%s|%t|%s|%s",
+					b.BuilderName,
+					b.Status == bbpb.Status_SUCCESS,
+					buildURL(b.ID),
+					c.Hash,
+				))
+			case bbpb.Status_INFRA_FAILURE:
+				c.ResultData = append(c.ResultData, fmt.Sprintf("%s|%s|%s|%s",
+					b.BuilderName,
+					"infra_failure",
+					buildURL(b.ID),
+					c.Hash,
+				))
+			}
+		}
+	}
+}
+
+func buildURL(buildID int64) string {
+	return fmt.Sprintf("https://ci.chromium.org/b/%d", buildID)
 }
 
 type ignoreFunc func(err error) error

cmd/coordinator/internal/dashboard/handler.go

@@ -16,10 +16,12 @@ import (
 	"log"
 	"net/http"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
 	"cloud.google.com/go/datastore"
+	"golang.org/x/build/cmd/coordinator/internal/lucipoll"
 	"golang.org/x/build/dashboard"
 	"golang.org/x/build/internal/releasetargets"
 	"golang.org/x/build/maintner/maintnerd/apipb"
@@ -44,39 +46,55 @@ type MaintnerClient interface {
 	GetDashboard(ctx context.Context, in *apipb.DashboardRequest, opts ...grpc.CallOption) (*apipb.DashboardResponse, error)
 }
 
+type luciClient interface {
+	PostSubmitSnapshot() lucipoll.Snapshot
+}
+
 type Handler struct {
 	// Datastore is a client used for fetching build status. If nil, it uses in-memory storage of build status.
 	Datastore *datastore.Client
 	// Maintner is a client for Maintner, used for fetching lists of commits.
 	Maintner MaintnerClient
+	// LUCI is a client for LUCI, used for fetching build results from there.
+	LUCI luciClient
 
 	// memoryResults is an in-memory storage of CI results. Used in development and testing for datastore data.
 	memoryResults map[string][]string
 }
 
-func (d *Handler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
+func (d *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+	showLUCI, _ := strconv.ParseBool(req.URL.Query().Get("showluci"))
+	if legacyOnly, _ := strconv.ParseBool(req.URL.Query().Get("legacyonly")); legacyOnly {
+		showLUCI = false
+	}
+
+	var luci lucipoll.Snapshot
+	if d.LUCI != nil && showLUCI {
+		luci = d.LUCI.PostSubmitSnapshot()
+	}
+
 	dd := &data{
-		Builders: d.getBuilders(dashboard.Builders),
-		Commits:  d.commits(r.Context()),
+		Builders: d.getBuilders(dashboard.Builders, luci),
+		Commits:  d.commits(req.Context(), luci),
 		Package:  dashPackage{Name: "Go"},
 	}
 
 	var buf bytes.Buffer
 	if err := templ.Execute(&buf, dd); err != nil {
 		log.Printf("handleDashboard: error rendering template: %v", err)
-		http.Error(rw, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
 	}
-	buf.WriteTo(rw)
+	buf.WriteTo(w)
 }
 
-func (d *Handler) commits(ctx context.Context) []*commit {
-	var commits []*commit
+func (d *Handler) commits(ctx context.Context, luci lucipoll.Snapshot) []*commit {
 	resp, err := d.Maintner.GetDashboard(ctx, &apipb.DashboardRequest{})
 	if err != nil {
 		log.Printf("handleDashboard: error fetching from maintner: %v", err)
-		return commits
+		return nil
 	}
+	var commits []*commit
 	for _, c := range resp.GetCommits() {
 		commits = append(commits, &commit{
 			Desc: c.Title,
@@ -85,24 +103,26 @@ func (d *Handler) commits(ctx context.Context) []*commit {
 			User: formatGitAuthor(c.AuthorName, c.AuthorEmail),
 		})
 	}
-	d.getResults(ctx, commits)
+	d.getResults(ctx, commits, luci)
 	return commits
 }
 
-// getResults populates result data on commits, fetched from Datastore or in-memory storage.
-func (d *Handler) getResults(ctx context.Context, commits []*commit) {
-	if d.Datastore == nil {
+// getResults populates result data on commits, fetched from Datastore or in-memory storage
+// and, if luci is non-zero, also from LUCI.
+func (d *Handler) getResults(ctx context.Context, commits []*commit, luci lucipoll.Snapshot) {
+	if d.Datastore != nil {
+		getDatastoreResults(ctx, d.Datastore, commits, "go")
+	} else {
 		for _, c := range commits {
 			if result, ok := d.memoryResults[c.Hash]; ok {
 				c.ResultData = result
 			}
 		}
-		return
 	}
-	getDatastoreResults(ctx, d.Datastore, commits, "go")
+	appendLUCIResults(luci, commits, "go")
 }
 
-func (d *Handler) getBuilders(conf map[string]*dashboard.BuildConfig) []*builder {
+func (d *Handler) getBuilders(conf map[string]*dashboard.BuildConfig, luci lucipoll.Snapshot) []*builder {
 	bm := make(map[string]builder)
 	for _, b := range conf {
 		if !b.BuildsRepoPostSubmit("go", "master", "master") {
@@ -111,12 +131,36 @@ func (d *Handler) getBuilders(conf map[string]*dashboard.BuildConfig) []*builder
 		db := bm[b.GOOS()]
 		db.OS = b.GOOS()
 		db.Archs = append(db.Archs, &arch{
-			Arch: b.GOARCH(),
+			os: b.GOOS(), Arch: b.GOARCH(),
 			Name: b.Name,
-			Tag:  strings.TrimPrefix(b.Name, fmt.Sprintf("%s-%s-", b.GOOS(), b.GOARCH())),
+			// Tag is the part after "os-arch", if any, without leading dash.
+			Tag: strings.TrimPrefix(strings.TrimPrefix(b.Name, fmt.Sprintf("%s-%s", b.GOOS(), b.GOARCH())), "-"),
 		})
 		bm[b.GOOS()] = db
 	}
+
+	for _, b := range luci.Builders {
+		if b.Repo != "go" || b.GoBranch != "master" {
+			continue
+		}
+		db := bm[b.Target.GOOS]
+		db.OS = b.Target.GOOS
+		tagFriendly := b.Name + "-🐇"
+		if after, ok := strings.CutPrefix(tagFriendly, fmt.Sprintf("gotip-%s-%s_", b.Target.GOOS, b.Target.GOARCH)); ok {
+			// Convert os-arch_osversion-mod1-mod2 (an underscore at start of "_osversion")
+			// to have os-arch-osversion-mod1-mod2 (a dash at start of "-osversion") form.
+			// The tag computation below uses this to find both "osversion-mod1" or "mod1".
+			tagFriendly = fmt.Sprintf("gotip-%s-%s-", b.Target.GOOS, b.Target.GOARCH) + after
+		}
+		db.Archs = append(db.Archs, &arch{
+			os: b.Target.GOOS, Arch: b.Target.GOARCH,
+			Name: b.Name,
+			// Tag is the part after "os-arch", if any, without leading dash.
+			Tag: strings.TrimPrefix(strings.TrimPrefix(tagFriendly, fmt.Sprintf("gotip-%s-%s", b.Target.GOOS, b.Target.GOARCH)), "-"),
+		})
+		bm[b.Target.GOOS] = db
+	}
+
 	var builders builderSlice
 	for _, db := range bm {
 		db := db
@@ -128,18 +172,12 @@ func (d *Handler) getBuilders(conf map[string]*dashboard.BuildConfig) []*builder
 }
 
 type arch struct {
-	Arch string
-	Name string
-	Tag  string
+	os, Arch string
+	Name     string
+	Tag      string
 }
 
-func (a arch) FirstClass() bool {
-	segs := strings.SplitN(a.Name, "-", 3)
-	if len(segs) < 2 {
-		return false
-	}
-	return releasetargets.IsFirstClass(segs[0], segs[1])
-}
+func (a arch) FirstClass() bool { return releasetargets.IsFirstClass(a.os, a.Arch) }
 
 type archSlice []*arch
 
@@ -217,8 +255,13 @@ type dashPackage struct {
 }
 
 type commit struct {
-	Desc       string
-	Hash       string
+	Desc string
+	Hash string
+	// ResultData is a copy of the [Commit.ResultData] field from datastore,
+	// with an additional rule that the second '|'-separated value may be "infra_failure"
+	// to indicate a problem with the infrastructure rather than the code being tested.
+	//
+	// It can also have the form of "builder|BuildingURL" for in progress builds.
 	ResultData []string
 	Time       string
 	User       string
@@ -236,28 +279,43 @@ func (c *commit) ShortUser() string {
 	return user
 }
 
-func (c *commit) ResultForBuilder(builder string) result {
+func (c *commit) ResultForBuilder(builder string) *result {
 	for _, rd := range c.ResultData {
 		segs := strings.Split(rd, "|")
+		if len(segs) == 2 && segs[0] == builder {
+			return &result{
+				BuildingURL: segs[1],
+			}
+		}
 		if len(segs) < 4 {
 			continue
 		}
 		if segs[0] == builder {
-			return result{
+			return &result{
 				OK:      segs[1] == "true",
+				Noise:   segs[1] == "infra_failure",
 				LogHash: segs[2],
 			}
 		}
 	}
-	return result{}
+	return nil
 }
 
 type result struct {
 	BuildingURL string
 	OK          bool
+	Noise       bool
 	LogHash     string
 }
 
+func (r result) LogURL() string {
+	if strings.HasPrefix(r.LogHash, "https://") {
+		return r.LogHash
+	} else {
+		return "https://build.golang.org/log/" + r.LogHash
+	}
+}
+
 // formatGitAuthor formats the git author name and email (as split by
 // maintner) back into the unified string how they're stored in a git
 // commit, so the shortUser func (used by the HTML template) can parse