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.

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

x/build/cmd/coordinator, x/build/maintner/maintnerd/maintapi: golang.org/x repo trybot on custom release branch should use corresponding Go branch #42127

Closed
dmitshur opened this issue Oct 21, 2020 · 3 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Oct 21, 2020

When trybots are testing a CL in a golang.org/x to a release branch with a custom suffix (e.g., release-branch.go1.14-cmd or release-branch.go1.15-bundle), they should use the mentioned Go version to test that CL, not tip.

See CL 264058 where Go tip (1.16) was used, despite the release branch having a "go1.15" in it.

This is somewhat related to issues #37512 and #38303. Custom release branches are somewhat unusual (now for #41375, last time for #36851) and hopefully won't come up much in the future.

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 Oct 21, 2020
@dmitshur dmitshur added this to the Unreleased milestone Oct 21, 2020
@dmitshur
Copy link
Contributor Author

The problem is on this line:

https://github.com/golang/build/blob/247680342f28bf27b68358c64c491da551864749/cmd/coordinator/coordinator.go#L1220

When a release branch has a suffix (like "-bundle"), the branch == work.Branch equality won't hold.

A fix to allow suffixes separated by a dash could be to change that to work.Branch == branch || strings.HasPrefix(work.Branch, branch+"-").

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 21, 2020
@dmitshur dmitshur self-assigned this Oct 21, 2020
@gopherbot
Copy link

Change https://golang.org/cl/264203 mentions this issue: cmd/coordinator: use right Go for x repo release-branch with suffix

@dmitshur dmitshur changed the title x/build/cmd/coordinator, x/build/maintner/maintnerd/maintapi: pick right Go version for custom release branches x/build/cmd/coordinator, x/build/maintner/maintnerd/maintapi: pick right Go version for custom release branches in golang.org/x repos Oct 21, 2020
@dmitshur dmitshur changed the title x/build/cmd/coordinator, x/build/maintner/maintnerd/maintapi: pick right Go version for custom release branches in golang.org/x repos x/build/cmd/coordinator, x/build/maintner/maintnerd/maintapi: golang.org/x repo trybot on custom release branch should use corresponding Go branch Oct 22, 2020
@gopherbot
Copy link

Change https://golang.org/cl/319789 mentions this issue: cmd/coordinator, maintner/maintnerd/maintapi: consolidate TryBot branch logic

gopherbot pushed a commit to golang/build that referenced this issue May 17, 2021
…ch logic

This change reverts the coordinator side to be much simpler, as it was
before CL 167382, and consolidates all the version selection in maintapi.
This direction is chosen because at this time¹ maintapi is best suited
to perform version selection.

This is a refactor CL that leaves current behavior unmodified, and test
cases provide improved coverage. The following smaller CL will make the
desired changes to behavior.

Remove TestNewTrySetBuildRepoGo110 because it's no longer needed. It was
added when x/build started requiring module mode to build successfully,
given that Go 1.10 didn't have module mode support.

Background

At this time¹ coordinator makes use of maintapi to find TryBot work,
and it is a collaborate effort between the two components that results
in determining what builds will happen. Coordinator is ultimately
responsible for starting and running the builds, but it doesn't have
information about the branches in the Go project (the Go revision at
refs/heads/master, refs/heads/release-branch.go1.16, etc.). Maintapi
has that information via the maintner corpus. So it makes the version
information available to coordinator by populating relevant fields in
apipb.GoFindTryWorkResponse.

Issue golang/go#28891 was about wanting to test golang.org/x repo CLs
on release-branch.go1.n with the corresponding Go 1.n version, rather
than Go tip. Unfortunately, it was implemented on the coordinator side,
resulting in the logic for version selection to be more spread between
the coordinator and maintapi components. There were followup issues
like golang/go#42127 and golang/go#37512, whose fixes built on top of
the coordinator side, and increased complexity there. As a consequence,
making and testing further changes became more difficult than it needs
to be.

¹ After golang/go#34744 is done, I'd like to move all the TryBot version
  selection logic into coordinator, the component responsible for TryBots.
  But not today.

For golang/go#46154.

Change-Id: I93986acefd4bf66b27ccf0323439966122b7989a
Reviewed-on: https://go-review.googlesource.com/c/build/+/319789
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@golang golang locked and limited conversation to collaborators May 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants