Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/build/cmd/coordinator: update build.golang.org dashboard during LUCI migration #65913

Closed
dmitshur opened this issue Feb 23, 2024 · 21 comments
Closed

x/build/cmd/coordinator: update build.golang.org dashboard during LUCI migration #65913

dmitshur opened this issue Feb 23, 2024 · 21 comments
Assignees
@dmitshur
Labels
Builders x/build issues (builders, bots, dashboards) FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Feb 23, 2024 •
edited
Loading

We're getting close to the post-submit phase of the migration to LUCI, with many LUCI builders ported to https://ci.chromium.org/p/golang and ready to replace the previous coordinator-powered counterparts. This is the tracking issue to update the build.golang.org dashboard during the transition.

The plan is to gradually start displaying LUCI build results on the existing build.golang.org dashboard, and work towards removing duplicate builders from coordinator over time. There are some known issues in the LUCI post-submit UI that we'll be tracking separately (#66036), so fully moving to that UI is out of scope for this issue.

CC @golang/release.
@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 23, 2024
@dmitshur dmitshur added this to the Unreleased milestone Feb 23, 2024
@dmitshur dmitshur self-assigned this Feb 23, 2024
@dmitshur dmitshur moved this to In Progress in Go Release Feb 23, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567499 mentions this issue: cmd/coordinator/internal/legacydash: refactor package-scoped variables

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567498 mentions this issue: cmd/coordinator, perf/app: prepare a placeholder banner

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567496 mentions this issue: cmd/coordinator: disable Kubernetes mode in local "dev" mode

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567497 mentions this issue: cmd/coordinator: rewrite scheme-less URLs and redirects in dev mode

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567578 mentions this issue: cmd/coordinator: display LUCI build results on build dashboards

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567577 mentions this issue: WIP: cmd/coordinator/internal/legacydash: add LUCI build result support

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567575 mentions this issue: WIP: cmd/coordinator/internal/lucipoll: add package

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567576 mentions this issue: WIP: cmd/coordinator/internal/dashboard: add LUCI build result support

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 27, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/568196 mentions this issue: dashboard: add a map for hiding builders that migrated to LUCI

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/568195 mentions this issue: cmd/coordinator: display LUCI build results on build dashboards (part 2)

gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator: disable Kubernetes mode in local "dev" mode
9d46ba7 
Restore the local "dev" mode after the refactor in CL 422956.
The development environment doesn't have KubeServices set to
anything, so KubeServices.Location() was otherwise panicking.

While here, also do fewer things in background of "dev" mode
unless appropriate dev flags are turned on.

For golang/go#65913.

Change-Id: Ib7409c28e39f2dbf98e08a4bcbd21cd45cb81aa1
Reviewed-on: https://go-review.googlesource.com/c/build/+/567496
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator: rewrite scheme-less URLs and redirects in dev mode
bf23401 
This was created in preparation of removal of dashboard v2, which was
going to use a redirect. However dashboard v2 turned out to be useful
enough to warrant keeping and maintaining it.

Apply this enhancement to the local development serving path since it
is generally useful and already prepared.

For golang/go#65913.

Change-Id: I6006bfa02d512675b63773c22c9ed21d8d5b4ab4
Reviewed-on: https://go-review.googlesource.com/c/build/+/567497
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator, perf/app: prepare a placeholder banner
f15cdea 
We initially intended for a banner to show up to notify users about
a migration affecting the old dashboard. A banner turned out not to
be needed at this time. Since it was already prepared and might end
up being useful later on, add an invisible placeholder for now.

Unfortunately, it would take more refactoring than I was willing to
take on at this stage to make it possible for the unified banner to
be implemented in one place, so it is instead implemented inline in
multiple places.

For golang/go#65913.

Change-Id: I86fbf17b56711f0855e206e803eff8271019b6a8
Reviewed-on: https://go-review.googlesource.com/c/build/+/567498
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/legacydash: refactor package-scoped variables
efa540e 
I was on the of fence whether to take this on so late in the game; it
seems to favor slightly towards doing it anyway. This makes it easier
to see where the variables are used, and will make the future changes
easier to reason about.

For golang/go#65913.

Change-Id: I3c274d2aa7174a9fbd6be91869ce4b1da0dfecaa
Reviewed-on: https://go-review.googlesource.com/c/build/+/567499
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/lucipoll: create a Go LUCI dashboard poller
8d59e02 
Fetching builds (and builders) via the BuildBucket API isn't as quick
and inexpensive as from Datastore, so it's necessary to add a caching
layer.

Use a simple polling approach to facilitate the migration to LUCI and
make it possible to show LUCI build results on the current dashboard.

There are various ways it can be improved, but all polling options are
worse than proactively pushing test results. We'll evaluate what the
long term future needs are later, so for now avoid investing too much.

For golang/go#65913.

Change-Id: Ib131a90b1108036b1d9618ea706959cbda2a0eac
Reviewed-on: https://go-review.googlesource.com/c/build/+/567575
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]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/dashboard: add LUCI build result support
8e6bb5e 
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]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/legacydash: add LUCI build result support
79dd4f3 
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]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator: display LUCI build results on build dashboards (part 1)
8eaaf2a 
This connects the pieces together, enabling the new behavior.
It can be opted-out via a 'legacyonly=1' URL query parameter.

In part 1 (this commit), show the old build.golang.org/ view
by default. Part 2 (next commit) flips the default.

For golang/go#65913.

Change-Id: I04be3b9d345b97892bb18751bf8bd717481235de
Reviewed-on: https://go-review.googlesource.com/c/build/+/567578
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
cmd/coordinator: display LUCI build results on build dashboards (part 2)
253aa5b 
This makes the new view on by default. As before, it remains
possible to opt-out via a 'legacyonly=1' URL query parameter.

For golang/go#65913.

Change-Id: I52b0a7e206a1d8bdf4645d4e1f91cadebb446c0e
Reviewed-on: https://go-review.googlesource.com/c/build/+/568195
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Feb 29, 2024
@dmitshur @gopherbot
dashboard: add a map for hiding builders that migrated to LUCI
09eab47 
This sets a path forward for hiding builders that have migrated.
Outright deleting their configuration is left for a later phase.

For golang/go#65913.
Updates golang/go#63471.

Change-Id: Icaa40cbaf5e9395ef6f82fc70a6febc2ec8bc838
Reviewed-on: https://go-review.googlesource.com/c/build/+/568196
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
@dmitshur
Copy link
Contributor Author

dmitshur commented Feb 29, 2024 •
edited
Loading

This is done, and announced at https://groups.google.com/g/golang-dev/c/CqVVuxngzDU/m/MD4qrteuAQAJ.

There may be some more follow up in general, but bulk of the work is done, so I'll close this. We can file a new issue if there's something specific.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Feb 29, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/574558 mentions this issue: cmd/coordinator: improve availability and precision of x/ repo results

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/574557 mentions this issue: cmd/coordinator/internal/legacydash: don't clobber ResultData slice

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/574555 mentions this issue: cmd/coordinator: use authenticated BuildBucket clients

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/574556 mentions this issue: all: switch to newer, preferred API for BuildBucket API clients

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/574497 mentions this issue: dashboard: mark more builders as ported to LUCI

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/574496 mentions this issue: dashboard: mark all WebAssembly builders as ported to LUCI

gopherbot pushed a commit to golang/build that referenced this issue Mar 27, 2024
@dmitshur @gopherbot
cmd/coordinator: use authenticated BuildBucket clients
8c9726c 
We're seeing an odd situation where a fraction of BuildBucket API calls
will sometimes take 24 hours and a minute (instead of the usual ~minute)
to complete. Start by making authenticated BuildBucket clients, so that
if the problem continues, we would have an easier time reporting it and
investigating further.

For golang/go#65913.

Change-Id: Ief18f32f582ec82fcdb207f1ec9612f84c961b5c
Reviewed-on: https://go-review.googlesource.com/c/build/+/574555
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]>
gopherbot pushed a commit to golang/build that referenced this issue Mar 27, 2024
@dmitshur @gopherbot
all: switch to newer, preferred API for BuildBucket API clients
17ad1fa 
The PRPCClient variant is generated by a custom plugin and predates
the raw Client variant, which is preferred in newer code. Switch to
it before doing more to understand the previously-mentioned strange
intermittent issue of queries sometimes taking an extra 24 hours to
successfully complete.

For golang/go#65913.

Change-Id: Ibb8495a939bb80f445261cddda9d900c82ec7e62
Reviewed-on: https://go-review.googlesource.com/c/build/+/574556
Reviewed-by: Michael Knyszek <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Mar 27, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/legacydash: don't clobber ResultData slice
6a613cb 
during append

Back when there was only one source of result data (Datastore), the
CommitInfo.ResultData field was only assigned to. By now, it's also
sometimes appended to. Remove extra capacity in the shared slice so
that appending results to the same x/ repo commit (i.e., a total of
three times: for the main branch, and two release branches) is fine.

This happened not to come up during local development, at least not
until Datastore permissions were setup to permit its use, but it is
easy to see on build.golang.org.

For golang/go#65913.

Change-Id: Id19ddf389f8d7ac4d69f2ac8477443005b9671ef
Reviewed-on: https://go-review.googlesource.com/c/build/+/574557
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Mar 27, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/lucipoll: improve availability and precision
88066a8 
of x/ repo build results

When the build.golang.org dashboard front page shows x/ repo results,
it shows builds for the very latest x/ repo head commit built at the
head release branch (or main branch) of the main Go repo.

There are two ways a LUCI build becomes available for the corresponding
"latest x/ repo head and latest Go repo release branch" pair of commits:

1. a new Go repo commit invokes a build for each x/ repo main head
2. a new x/ repo commit invokes a build for Go release branch head

Some x/ repos are low volume, so most of the time its (latest, latest)
pair comes from first type of trigger. Other x/ repos are high volume,
and their (latest, latest) pair often comes from the second trigger.

Previously, package lucipoll was considering x/ repo LUCI builds that
came from the second trigger, and so high volume x/ repos had results
visible most of the time. This change adds a loop (over the main branch
and two supported release branches) to fetch x/ repo results that were
invoked via the Go repo side, so now low volume x/ repos are handled as
well. It also reads build output properties and uses them to filter out
builds that weren't for the exact (Go repo, x/ repo) commit pair that
the build dashboard intends to display.

For golang/go#65913.

Change-Id: I231e0b1c49bfafb792301dd7dad9a301aca4b924
Reviewed-on: https://go-review.googlesource.com/c/build/+/574558
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Mar 27, 2024
@dmitshur @gopherbot
dashboard: mark all WebAssembly builders as ported to LUCI
65a1126 
These builders are fully ported to LUCI. Given we're now displaying
build results from LUCI, start hiding the legacy builder duplicates.

For golang/go#65913.
For golang/go#63471.

Change-Id: I57698e225a28f386740c46936f1b97e16351faee
Reviewed-on: https://go-review.googlesource.com/c/build/+/574496
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
gopherbot pushed a commit to golang/build that referenced this issue Mar 27, 2024
@dmitshur @gopherbot
dashboard: mark more builders as ported to LUCI
f6df090 
Hide additional duplicate builders to reduce noise and make progress
on the migration to LUCI.

This helped me notice that we haven't turned on linux-arm64-longtest
and linux-arm64-race in LUCI yet, so I mailed CL 574559 for that.

For golang/go#65913.
For golang/go#63471.

Change-Id: If90aeb37235fc07e399980bed3096a36a3e53fe2
Reviewed-on: https://go-review.googlesource.com/c/build/+/574497
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/575855 mentions this issue: dashboard: mark linux-arm64-{longtest,race} builders as ported to LUCI

gopherbot pushed a commit to golang/build that referenced this issue Apr 2, 2024
@dmitshur @gopherbot
dashboard: mark linux-arm64-{longtest,race} builders as ported to LUCI
7718c45 
They're available as of CL 574559.

For golang/go#65913.
For golang/go#63471.

Change-Id: I916d9db6d3fefe6f07a363adcce7085b6c72a5b7
Reviewed-on: https://go-review.googlesource.com/c/build/+/575855
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/578042 mentions this issue: dashboard: mark recent clang/boringcrypto builders as ported to LUCI

gopherbot pushed a commit to golang/build that referenced this issue Apr 12, 2024
@dmitshur @gopherbot
dashboard: mark recent clang/boringcrypto builders as ported to LUCI
68f5572 
For golang/go#65913.
For golang/go#63471.

Change-Id: I8db307a98804422e1b004df2a3a9723048e7744e
Reviewed-on: https://go-review.googlesource.com/c/build/+/578042
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
@dmitshur dmitshur mentioned this issue May 31, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/589658 mentions this issue: internal/migration: mark darwin-amd64-race as ported to LUCI

gopherbot pushed a commit to golang/build that referenced this issue Jun 3, 2024
@dmitshur @gopherbot
internal/migration: mark darwin-amd64-race as ported to LUCI
69093f3 
Thanks to Michael, this done as of CL 588939.

For golang/go#65913.
For golang/go#60440.

Change-Id: I0596549d90a449a114cb3451e8d775ea31132cb3
Reviewed-on: https://go-review.googlesource.com/c/build/+/589658
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/615976 mentions this issue: cmd/coordinator/internal/lucipoll: update for new JSONPB field naming

gopherbot pushed a commit to golang/build that referenced this issue Sep 27, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/lucipoll: update for new JSONPB field naming
a411a18 
As of crrev.com/c/5875121, golangbuild switched to the new default
behavior of using 'proto' field naming (such as "gitiles_commit")
instead of using protobuf default names (such as "gitilesCommit").

The previous behavior can still be achieved by explicitly using the
https://pkg.go.dev/go.chromium.org/luci/luciexe/build/properties#OptProtoUseJSONNames
option, but we choose to accept the new default behavior since it's
fine for our needs.

So, update a few places in lucipoll for the new field naming pattern
as was also recently done for watchflakes in CL 615515. Not handling
the old pattern here since lucipoll only needs to handle new builds.

Also drop the unused "omitempty" option from BuilderConfigProperties
while here - it's never used for marshaling, only unmarshaling.

For golang/go#69609.
For golang/go#65913.

Change-Id: Icf2f942fa9821f8781c9a2ecaddd2a3e10496ba2
Reviewed-on: https://go-review.googlesource.com/c/build/+/615976
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: David Chase <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmitshur dmitshur

Labels
Builders x/build issues (builders, bots, dashboards) FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
2 participants
@dmitshur @gopherbot