Skip to content

Commit

Permalink
internal/task: convert x repo tagging to use LUCI
Browse files Browse the repository at this point in the history
One less thing depending on the old stuff.

The conversion is pretty straightforward. The greenness check selects a
commit at the beginning. Any builders that we want that haven't passed
get rerun with a high priority. If any of those fail, the overall
attempt fails and restarts, at which point it re-selects the commit it's
looking at. That minimizes redundant work, should get a result
relatively quickly, but still allows us to recover from a genuinely
broken build.

Note: prioritization doesn't work quite right yet because golangbuild
doesn't propagate it down to child builds. That's a separate issue.

For golang/go#63147.

Change-Id: I8db7c5a9dfcfef709915f74b7d9d0d4a4edc89bb
Reviewed-on: https://go-review.googlesource.com/c/build/+/544275
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
heschi authored and gopherbot committed Nov 27, 2023
1 parent acb44b5 commit 9d341b1
Show file tree
Hide file tree
Showing 7 changed files with 251 additions and 405 deletions.
21 changes: 11 additions & 10 deletions cmd/relui/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ func main() {
C: luciHTTPClient,
Host: "cr-buildbucket.appspot.com",
})
buildBucketClient := &task.RealBuildBucketClient{
BuildersClient: buildersClient,
BuildsClient: buildsClient,
}

var dbPool db.PGDBTX
dbPool, err = pgxpool.Connect(ctx, *pgConnect)
Expand Down Expand Up @@ -231,15 +235,12 @@ func main() {
BaseURL: *scratchFilesBase,
GCS: gcsClient,
},
SignedURL: *signedFilesBase,
ServingURL: *servingFilesBase,
DownloadURL: *edgeCacheURL,
ProxyPrefix: "https://proxy.golang.org/golang.org/toolchain/@v",
CloudBuildClient: cloudBuildClient,
BuildBucketClient: &task.RealBuildBucketClient{
BuildersClient: buildersClient,
BuildsClient: buildsClient,
},
SignedURL: *signedFilesBase,
ServingURL: *servingFilesBase,
DownloadURL: *edgeCacheURL,
ProxyPrefix: "https://proxy.golang.org/golang.org/toolchain/@v",
CloudBuildClient: cloudBuildClient,
BuildBucketClient: buildBucketClient,
SwarmingClient: &task.RealSwarmingClient{
SwarmingClient: swarmingClient,
SwarmingURL: *swarmingURL,
Expand Down Expand Up @@ -280,8 +281,8 @@ func main() {
tagTasks := &task.TagXReposTasks{
IgnoreProjects: ignoreProjects,
Gerrit: gerritClient,
DashboardURL: "https://build.golang.org",
CloudBuild: cloudBuildClient,
BuildBucket: buildBucketClient,
}
dh.RegisterDefinition("Tag x/ repos", tagTasks.NewDefinition())
dh.RegisterDefinition("Tag a single x/ repo", tagTasks.NewSingleDefinition())
Expand Down
4 changes: 2 additions & 2 deletions internal/relui/buildrelease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ esac
return fmt.Errorf("unexpected approval request for %q", ctx.TaskName)
},
}
buildBucket := task.NewFakeBuildBucketClient(major, fakeGerrit.GerritURL(), "go")
buildBucket := task.NewFakeBuildBucketClient(major, fakeGerrit.GerritURL(), "security-try", []string{"go"})

const dockerProject, dockerTrigger = "docker-build-project", "docker-build-trigger"

Expand Down Expand Up @@ -436,7 +436,7 @@ func testSecurity(t *testing.T, mergeFixes bool) {
privateRef := privateRepo.Commit(securityFix)
privateGerrit := task.NewFakeGerrit(t, privateRepo)
deps.buildBucket.GerritURL = privateGerrit.GerritURL()
deps.buildBucket.GerritProject = "go-private"
deps.buildBucket.Projects = []string{"go-private"}
deps.buildTasks.PrivateGerritClient = privateGerrit
deps.buildTasks.PrivateGerritProject = "go-private"

Expand Down
2 changes: 1 addition & 1 deletion internal/relui/workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,7 @@ func (b *BuildReleaseTasks) readRelevantBuilders(ctx *wf.TaskContext, major int,
return nil, err
}
var builders []string
for _, b := range resp {
for b := range resp {
if strings.HasPrefix(b, prefix) {
builders = append(builders, b)
}
Expand Down
45 changes: 34 additions & 11 deletions internal/task/buildbucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,41 @@ import (
)

type BuildBucketClient interface {
// ListBuilders lists all the builder names in bucket.
ListBuilders(ctx context.Context, bucket string) ([]string, error)
// ListBuilders lists all the builders in bucket, keyed by their builder names.
ListBuilders(ctx context.Context, bucket string) (map[string]*pb.BuilderConfig, error)
// RunBuild runs a builder at commit with properties and returns its ID.
RunBuild(ctx context.Context, bucket, builder string, commit *pb.GitilesCommit, properties map[string]*structpb.Value) (int64, error)
// Completed reports whether a build has finished, returning an error if
// it's failed. It's suitable for use with AwaitCondition.
Completed(ctx context.Context, id int64) (string, bool, error)
// SearchBuilds searches for builds matching pred and returns their IDs.
SearchBuilds(ctx context.Context, pred *pb.BuildPredicate) ([]int64, error)
}

type RealBuildBucketClient struct {
BuildersClient pb.BuildersClient
BuildsClient pb.BuildsClient
}

func (c *RealBuildBucketClient) ListBuilders(ctx context.Context, bucket string) ([]string, error) {
func (c *RealBuildBucketClient) ListBuilders(ctx context.Context, bucket string) (map[string]*pb.BuilderConfig, error) {
var pageToken string
builders := map[string]*pb.BuilderConfig{}
nextPage:
resp, err := c.BuildersClient.ListBuilders(ctx, &pb.ListBuildersRequest{
Project: "golang",
Bucket: bucket,
PageSize: 1000,
Project: "golang",
Bucket: bucket,
PageSize: 1000,
PageToken: pageToken,
})
if err != nil {
return nil, err
}
if resp.NextPageToken != "" {
return nil, fmt.Errorf("page size to ListBuilders insufficient")
}
var builders []string
for _, b := range resp.Builders {
builders = append(builders, b.Id.Builder)
builders[b.Id.Builder] = b.Config
}
if resp.NextPageToken != "" {
pageToken = resp.NextPageToken
goto nextPage
}
return builders, nil
}
Expand Down Expand Up @@ -77,3 +83,20 @@ func (c *RealBuildBucketClient) Completed(ctx context.Context, id int64) (string
}
return build.SummaryMarkdown, true, nil
}

func (c *RealBuildBucketClient) SearchBuilds(ctx context.Context, pred *pb.BuildPredicate) ([]int64, error) {
resp, err := c.BuildsClient.SearchBuilds(ctx, &pb.SearchBuildsRequest{
Predicate: pred,
})
if err != nil {
return nil, err
}
if resp.NextPageToken != "" {
return nil, fmt.Errorf("page size to SearchBuilds insufficient")
}
var results []int64
for _, b := range resp.Builds {
results = append(results, b.Id)
}
return results, nil
}
55 changes: 38 additions & 17 deletions internal/task/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"golang.org/x/build/internal/relui/sign"
"golang.org/x/build/internal/untar"
wf "golang.org/x/build/internal/workflow"
"golang.org/x/exp/slices"
"google.golang.org/protobuf/types/known/structpb"
)

Expand Down Expand Up @@ -924,46 +925,59 @@ func (c *FakeSwarmingClient) Completed(ctx context.Context, id string) (string,
return "here's some build detail", true, result
}

func NewFakeBuildBucketClient(major int, url, project string) *FakeBuildBucketClient {
func NewFakeBuildBucketClient(major int, url, bucket string, projects []string) *FakeBuildBucketClient {
return &FakeBuildBucketClient{
major: major,
GerritURL: url,
GerritProject: project,
results: map[int64]error{},
Bucket: bucket,
major: major,
GerritURL: url,
Projects: projects,
results: map[int64]error{},
}
}

type FakeBuildBucketClient struct {
FailBuilds []string
major int
GerritURL, GerritProject, GerritBranch string
Bucket string
FailBuilds []string
MissingBuilds []string
major int
GerritURL, Branch string
Projects []string

mu sync.Mutex
results map[int64]error
}

var _ BuildBucketClient = (*FakeBuildBucketClient)(nil)

func (c *FakeBuildBucketClient) ListBuilders(ctx context.Context, bucket string) ([]string, error) {
if bucket != "security-try" {
func (c *FakeBuildBucketClient) ListBuilders(ctx context.Context, bucket string) (map[string]*pb.BuilderConfig, error) {
if bucket != c.Bucket {
return nil, fmt.Errorf("unexpected bucket %q", bucket)
}
var res []string
for _, v := range []string{"gotip", fmt.Sprintf("go1.%v", c.major)} {
for _, b := range []string{"linux-amd64", "linux-amd64-longtest", "darwin-amd64_13"} {
res = append(res, v+"-"+b)
res := map[string]*pb.BuilderConfig{}
for _, proj := range c.Projects {
prefix := ""
if proj != "go" {
prefix = "x_" + proj + "-"
}
for _, v := range []string{"gotip", fmt.Sprintf("go1.%v", c.major)} {
for _, b := range []string{"linux-amd64", "linux-amd64-longtest", "darwin-amd64_13"} {
parts := strings.FieldsFunc(b, func(r rune) bool { return r == '-' || r == '_' })
res[prefix+v+"-"+b] = &pb.BuilderConfig{
Properties: fmt.Sprintf(`{"project":%q, "is_google":true, "target":{"goos":%q, "goarch":%q}}`, proj, parts[0], parts[1]),
}
}
}
}
return res, nil
}

func (c *FakeBuildBucketClient) RunBuild(ctx context.Context, bucket string, builder string, commit *pb.GitilesCommit, properties map[string]*structpb.Value) (int64, error) {
if bucket != "security-try" {
if bucket != c.Bucket {
return 0, fmt.Errorf("unexpected bucket %q", bucket)
}
match := regexp.MustCompile(`.*://(.+)`).FindStringSubmatch(c.GerritURL)
if commit.Host != match[1] || commit.Project != c.GerritProject {
return 0, fmt.Errorf("unexpected host or project: got %q, %q want %q, %q", commit.Host, commit.Project, match[1], c.GerritProject)
if commit.Host != match[1] || !slices.Contains(c.Projects, commit.Project) {
return 0, fmt.Errorf("unexpected host or project: got %q, %q want %q, %q", commit.Host, commit.Project, match[1], c.Projects)
}
// It would be nice to validate the commit hash and branch, but it's
// tricky to get the right value because it depends on the release type.
Expand Down Expand Up @@ -994,3 +1008,10 @@ func (c *FakeBuildBucketClient) Completed(ctx context.Context, id int64) (string
}
return "here's some build detail", true, result
}

func (c *FakeBuildBucketClient) SearchBuilds(ctx context.Context, pred *pb.BuildPredicate) ([]int64, error) {
if slices.Contains(c.MissingBuilds, pred.GetBuilder().GetBuilder()) {
return nil, nil
}
return []int64{rand.Int63()}, nil
}
Loading

0 comments on commit 9d341b1

Please sign in to comment.