internal/relui: add full-fledged release workflows
Add release workflows that fully replace cmd/releasebot and cmd/release
as far as I know. Also a bit of a mess of random touchups.
We now have beta, RC, major, and minor release workflows. Because we
need to know what major version at workflow definition time (to pick the
target platforms) the major versions are currently hardcoded. We have
all the code we need to make that automatic at some point.
Misc changes:
- use Actions and Sub in places where it makes sense.
- Redesign the GetNextVersion API to make workflow definitions easier.
- Search downward for the current major version to make testing easier.
For golang/go#51797.
Change-Id: Ie5c6f975ac0d38f6da664cc500beb508a643768f
Reviewed-on: https://go-review.googlesource.com/c/build/+/410826
Auto-Submit: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/cmd/relui/deployment-prod.yaml b/cmd/relui/deployment-prod.yaml
index 09c8bef..fd1b3fd 100644
--- a/cmd/relui/deployment-prod.yaml
+++ b/cmd/relui/deployment-prod.yaml
@@ -35,6 +35,7 @@
- "--gerrit-api-secret=secret:symbolic-datum-552/gobot-password"
- "--twitter-api-secret=secret:symbolic-datum-552/twitter-api-secret"
- "--builder-master-key=secret:symbolic-datum-552/builder-master-key"
+ - "--github-token=secret:symbolic-datum-552/maintner-github-token"
- "--scratch-files-base=gs://golang-release-staging/relui-scratch"
- "--staging-files-base=gs://golang-release-staging"
- "--serving-files-base=gs://golang"
diff --git a/cmd/relui/main.go b/cmd/relui/main.go
index 671dc67..395eb26 100644
--- a/cmd/relui/main.go
+++ b/cmd/relui/main.go
@@ -22,7 +22,9 @@
"time"
"cloud.google.com/go/storage"
+ "github.com/google/go-github/github"
"github.com/jackc/pgx/v4/pgxpool"
+ "github.com/shurcooL/githubv4"
"golang.org/x/build"
"golang.org/x/build/buildlet"
"golang.org/x/build/gerrit"
@@ -30,6 +32,7 @@
"golang.org/x/build/internal/relui"
"golang.org/x/build/internal/secret"
"golang.org/x/build/internal/task"
+ "golang.org/x/oauth2"
)
var (
@@ -57,6 +60,7 @@
var twitterAPI secret.TwitterCredentials
secret.JSONVarFlag(&twitterAPI, "twitter-api-secret", "Twitter API secret to use for workflows involving tweeting.")
masterKey := secret.Flag("builder-master-key", "Builder master key")
+ githubToken := secret.Flag("github-token", "GitHub API token")
https.RegisterFlags(flag.CommandLine)
flag.Parse()
@@ -111,7 +115,8 @@
if err != nil {
log.Fatalf("Could not connect to GCS: %v", err)
}
- releaseTasks := &relui.BuildReleaseTasks{
+
+ buildTasks := &relui.BuildReleaseTasks{
GerritURL: "https://go.googlesource.com",
CreateBuildlet: coordinator.CreateBuildlet,
GCSClient: gcsClient,
@@ -123,7 +128,23 @@
return publishFile(*websiteUploadURL, userPassAuth, f)
},
}
- releaseTasks.RegisterBuildReleaseWorkflows(dh)
+ githubHTTPClient := oauth2.NewClient(ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: *githubToken}))
+ milestoneTasks := &task.MilestoneTasks{
+ Client: &task.GitHubClient{
+ V3: github.NewClient(githubHTTPClient),
+ V4: githubv4.NewClient(githubHTTPClient),
+ },
+ RepoOwner: "golang",
+ RepoName: "go",
+ }
+ versionTasks := &task.VersionTasks{
+ Gerrit: &task.RealGerritClient{
+ Client: gerrit.NewClient("https://go-review.googlesource.com", gerrit.BasicAuth("git-gobot.golang.org", *gerritAPIFlag)),
+ },
+ Project: "go",
+ }
+
+ relui.RegisterReleaseWorkflows(dh, buildTasks, milestoneTasks, versionTasks)
db, err := pgxpool.Connect(ctx, *pgConnect)
if err != nil {
log.Fatal(err)
diff --git a/internal/relui/buildrelease_test.go b/internal/relui/buildrelease_test.go
index 669ce14..fcf3714 100644
--- a/internal/relui/buildrelease_test.go
+++ b/internal/relui/buildrelease_test.go
@@ -28,14 +28,26 @@
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
+ "github.com/google/go-github/github"
"github.com/google/uuid"
"golang.org/x/build/buildlet"
+ "golang.org/x/build/gerrit"
"golang.org/x/build/internal"
+ "golang.org/x/build/internal/task"
"golang.org/x/build/internal/untar"
"golang.org/x/build/internal/workflow"
)
func TestRelease(t *testing.T) {
+ t.Run("beta", func(t *testing.T) {
+ testRelease(t, "go1.18beta1", task.KindBeta)
+ })
+ t.Run("rc", func(t *testing.T) {
+ testRelease(t, "go1.18rc1", task.KindRC)
+ })
+}
+
+func testRelease(t *testing.T, wantVersion string, kind task.ReleaseKind) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
if runtime.GOOS != "linux" {
@@ -54,7 +66,7 @@
// Set up the fake signing process.
stagingDir := t.TempDir()
- go fakeSign(ctx, t, filepath.Join(stagingDir, "go1.18releasetest1"))
+ go fakeSign(ctx, t, filepath.Join(stagingDir, wantVersion))
signingPollDuration = 100 * time.Millisecond
// Set up the fake CDN publishing process.
@@ -71,11 +83,21 @@
publishFile := func(f *WebsiteFile) error {
filesMu.Lock()
defer filesMu.Unlock()
- files[f.Filename] = f
+ files[strings.TrimPrefix(f.Filename, wantVersion+".")] = f
return nil
}
- tasks := BuildReleaseTasks{
+ gerrit := &fakeGerrit{createdTags: map[string]string{}}
+ versionTasks := &task.VersionTasks{
+ Gerrit: gerrit,
+ Project: "go",
+ }
+ milestoneTasks := &task.MilestoneTasks{
+ Client: &fakeGitHub{},
+ RepoOwner: "golang",
+ RepoName: "go",
+ }
+ buildTasks := &BuildReleaseTasks{
GerritURL: tarballServer.URL,
GCSClient: nil,
ScratchURL: "file://" + filepath.ToSlash(t.TempDir()),
@@ -85,13 +107,11 @@
DownloadURL: dlServer.URL,
PublishFile: publishFile,
}
- wd, err := tasks.newBuildReleaseWorkflow("go1.18")
- if err != nil {
+ wd := workflow.New()
+ if err := addSingleReleaseWorkflow(buildTasks, milestoneTasks, versionTasks, wd, "go1.18", kind); err != nil {
t.Fatal(err)
}
w, err := workflow.Start(wd, map[string]interface{}{
- "Revision": "0",
- "Version": "go1.18releasetest1",
"Targets to skip testing (or 'all') (optional)": []string(nil),
})
if err != nil {
@@ -107,50 +127,62 @@
}
}
- checkTGZ(t, dlDir, files, "go1.18releasetest1.src.tar.gz", &WebsiteFile{
+ checkTGZ(t, dlDir, files, "src.tar.gz", &WebsiteFile{
OS: "",
Arch: "",
Kind: "source",
}, map[string]string{
- "go/VERSION": "go1.18releasetest1",
+ "go/VERSION": wantVersion,
"go/src/make.bash": makeScript,
})
- checkContents(t, dlDir, files, "go1.18releasetest1.windows-amd64.msi", &WebsiteFile{
+ checkContents(t, dlDir, files, "windows-amd64.msi", &WebsiteFile{
OS: "windows",
Arch: "amd64",
Kind: "installer",
}, "I'm an MSI!\n")
- checkTGZ(t, dlDir, files, "go1.18releasetest1.linux-amd64.tar.gz", &WebsiteFile{
+ checkTGZ(t, dlDir, files, "linux-amd64.tar.gz", &WebsiteFile{
OS: "linux",
Arch: "amd64",
Kind: "archive",
}, map[string]string{
- "go/VERSION": "go1.18releasetest1",
+ "go/VERSION": wantVersion,
"go/tool/something_orother/compile": "",
"go/pkg/something_orother/race.a": "",
})
- checkZip(t, dlDir, files, "go1.18releasetest1.windows-arm64.zip", &WebsiteFile{
+ checkZip(t, dlDir, files, "windows-arm64.zip", &WebsiteFile{
OS: "windows",
Arch: "arm64",
Kind: "archive",
}, map[string]string{
- "go/VERSION": "go1.18releasetest1",
+ "go/VERSION": wantVersion,
"go/tool/something_orother/compile": "",
})
- checkTGZ(t, dlDir, files, "go1.18releasetest1.linux-armv6l.tar.gz", &WebsiteFile{
+ checkTGZ(t, dlDir, files, "linux-armv6l.tar.gz", &WebsiteFile{
OS: "linux",
Arch: "armv6l",
Kind: "archive",
}, map[string]string{
- "go/VERSION": "go1.18releasetest1",
+ "go/VERSION": wantVersion,
"go/tool/something_orother/compile": "",
})
- checkContents(t, dlDir, files, "go1.18releasetest1.darwin-amd64.pkg", &WebsiteFile{
+ checkContents(t, dlDir, files, "darwin-amd64.pkg", &WebsiteFile{
OS: "darwin",
Arch: "amd64",
Kind: "installer",
}, "I'm a .pkg!\n")
+ wantCLs := 1 // VERSION
+ if kind == task.KindBeta {
+ wantCLs = 0
+ }
+ if gerrit.changesCreated != wantCLs {
+ t.Errorf("workflow sent %v changes to Gerrit, want %v", gerrit.changesCreated, wantCLs)
+ }
+
+ if len(gerrit.createdTags) != 1 {
+ t.Errorf("workflow created %v tags, want 1", gerrit.createdTags)
+ }
+
// TODO: consider logging this to golden files?
for name, logs := range fakeBuildlets.logs {
t.Logf("%v buildlets:", name)
@@ -333,6 +365,48 @@
})
}
+type fakeGerrit struct {
+ changesCreated int
+ createdTags map[string]string
+}
+
+func (g *fakeGerrit) CreateAutoSubmitChange(ctx context.Context, input gerrit.ChangeInput, contents map[string]string) (string, error) {
+ g.changesCreated++
+ return "fake~12345", nil
+}
+
+func (g *fakeGerrit) AwaitSubmit(ctx context.Context, changeID string) (string, error) {
+ return "fakehash", nil
+}
+
+func (g *fakeGerrit) ListTags(ctx context.Context, project string) ([]string, error) {
+ return []string{"go1.17"}, nil
+}
+
+func (g *fakeGerrit) Tag(ctx context.Context, project, tag, commit string) error {
+ g.createdTags[tag] = commit
+ return nil
+}
+
+type fakeGitHub struct {
+}
+
+func (g *fakeGitHub) FetchMilestone(ctx context.Context, owner, repo, name string, create bool) (int, error) {
+ return 0, nil
+}
+
+func (g *fakeGitHub) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error {
+ return nil
+}
+
+func (g *fakeGitHub) EditIssue(ctx context.Context, owner string, repo string, number int, issue *github.IssueRequest) (*github.Issue, *github.Response, error) {
+ return nil, nil, nil
+}
+
+func (g *fakeGitHub) EditMilestone(ctx context.Context, owner string, repo string, number int, milestone *github.Milestone) (*github.Milestone, *github.Response, error) {
+ return nil, nil, nil
+}
+
type fakeBuildlets struct {
t *testing.T
dir string
diff --git a/internal/relui/workflows.go b/internal/relui/workflows.go
index deec414..a1cd987 100644
--- a/internal/relui/workflows.go
+++ b/internal/relui/workflows.go
@@ -12,7 +12,6 @@
"math/rand"
"net/http"
"path"
- "strings"
"sync"
"time"
@@ -184,46 +183,102 @@
return arg, nil
}
-func (tasks *BuildReleaseTasks) RegisterBuildReleaseWorkflows(h *DefinitionHolder) {
- go117, err := tasks.newBuildReleaseWorkflow("go1.17")
- if err != nil {
- panic(err)
+func RegisterReleaseWorkflows(h *DefinitionHolder, build *BuildReleaseTasks, milestone *task.MilestoneTasks, version *task.VersionTasks) error {
+ createSingle := func(name, major string, kind task.ReleaseKind) error {
+ wd := workflow.New()
+ err := addSingleReleaseWorkflow(build, milestone, version, wd, "go1.19", task.KindMajor)
+ if err != nil {
+ return err
+ }
+ h.RegisterDefinition(name, wd)
+ return nil
}
- h.RegisterDefinition("Release Go 1.17", go117)
- go118, err := tasks.newBuildReleaseWorkflow("go1.18")
- if err != nil {
- panic(err)
+ if err := createSingle("Go 1.19 final", "go1.19", task.KindMajor); err != nil {
+ return err
}
- h.RegisterDefinition("Release Go 1.18", go118)
-
+ if err := createSingle("Go 1.19 next RC", "go1.19", task.KindRC); err != nil {
+ return err
+ }
+ if err := createSingle("Go 1.19 next beta", "go1.19", task.KindBeta); err != nil {
+ return err
+ }
+ wd, err := createMinorReleaseWorkflow(build, milestone, version, "go1.17", "go1.18")
+ if err != nil {
+ return err
+ }
+ h.RegisterDefinition("Minor releases for Go 1.17 and 1.18", wd)
+ return nil
}
-func (tasks *BuildReleaseTasks) newBuildReleaseWorkflow(majorVersion string) (*workflow.Definition, error) {
+func createMinorReleaseWorkflow(build *BuildReleaseTasks, milestone *task.MilestoneTasks, version *task.VersionTasks, prev, current string) (*workflow.Definition, error) {
wd := workflow.New()
+ if err := addSingleReleaseWorkflow(build, milestone, version, wd.Sub(current), current, task.KindCurrentMinor); err != nil {
+ return nil, err
+ }
+ if err := addSingleReleaseWorkflow(build, milestone, version, wd.Sub(prev), prev, task.KindPrevMinor); err != nil {
+ return nil, err
+ }
+ return wd, nil
+}
+
+func addSingleReleaseWorkflow(build *BuildReleaseTasks, milestone *task.MilestoneTasks, version *task.VersionTasks, wd *workflow.Definition, major string, kind task.ReleaseKind) error {
+ skipTests := wd.Parameter(workflow.Parameter{Name: "Targets to skip testing (or 'all') (optional)", ParameterType: workflow.SliceShort})
+
+ kindVal := wd.Constant(kind)
+ branch := fmt.Sprintf("release-branch.%v", major)
+ if kind == task.KindBeta {
+ branch = "master"
+ }
+ branchVal := wd.Constant(branch)
+ // TODO(heschi): read the real branch HEAD here, and check that it's the base commit for the version CL if there is one.
+ tagCommit := wd.Constant(branch)
+
+ // Select version, check milestones.
+ nextVersion := wd.Task("Get next version", version.GetNextVersion, kindVal)
+ milestones := wd.Task("Pick milestones", milestone.FetchMilestones, nextVersion, kindVal)
+ checked := wd.Action("Check blocking issues", milestone.CheckBlockers, milestones, nextVersion, kindVal)
+ // TODO(heschi): add mail-dl-cl here.
+
+ // Build, test, and sign release.
+ signedAndTestedArtifacts, err := build.addBuildTasks(wd, "go1.19", nextVersion, branchVal, skipTests, checked)
+ if err != nil {
+ return err
+ }
+
+ // Tag version and upload to CDN/website.
+ uploaded := wd.Action("Upload artifacts to CDN", build.uploadArtifacts, signedAndTestedArtifacts)
+ if branch != "master" {
+ versionCL := wd.Task("Mail version CL", version.CreateAutoSubmitVersionCL, branchVal, nextVersion, uploaded)
+ tagCommit = wd.Task("Wait for version CL submission", version.AwaitCL, versionCL)
+ }
+ tagged := wd.Action("Tag version", version.TagRelease, nextVersion, tagCommit, uploaded)
+ pushed := wd.Action("Push issues", milestone.PushIssues, milestones, nextVersion, kindVal, tagged)
+ published := wd.Task("Publish to website", build.publishArtifacts, nextVersion, signedAndTestedArtifacts, pushed)
+ wd.Output("Publish results", published)
+ return nil
+}
+
+func (tasks *BuildReleaseTasks) addBuildTasks(wd *workflow.Definition, majorVersion string, version, revision, skipTests workflow.Value, dependency workflow.Dependency) (workflow.Value, error) {
targets, ok := releasetargets.TargetsForVersion(majorVersion)
if !ok {
return nil, fmt.Errorf("malformed/unknown version %q", majorVersion)
}
- version := wd.Parameter(workflow.Parameter{Name: "Version", Example: "go1.10.1"})
- revision := wd.Parameter(workflow.Parameter{Name: "Revision", Example: "release-branch.go1.10"})
- skipTests := wd.Parameter(workflow.Parameter{Name: "Targets to skip testing (or 'all') (optional)", ParameterType: workflow.SliceShort})
- source := wd.Task("Build source archive", tasks.buildSource, revision, version)
+ source := wd.Task("Build source archive", tasks.buildSource, revision, version, dependency)
// Artifact file paths.
artifacts := []workflow.Value{source}
var darwinTargets []*releasetargets.Target
- // Empty values that represent the dependency on tests passing.
- var testResults []workflow.Value
+ var testsPassed []workflow.TaskInput
for _, target := range targets {
targetVal := wd.Constant(target)
- taskName := func(step string) string { return target.Name + ": " + step }
+ wd := wd.Sub(target.Name)
// Build release artifacts for the platform.
- bin := wd.Task(taskName("Build binary archive"), tasks.buildBinary, targetVal, source)
+ bin := wd.Task("Build binary archive", tasks.buildBinary, targetVal, source)
switch target.GOOS {
case "windows":
- zip := wd.Task(taskName("Convert to .zip"), tasks.convertToZip, targetVal, bin)
- msi := wd.Task(taskName("Build MSI"), tasks.buildMSI, targetVal, bin)
+ zip := wd.Task("Convert to .zip", tasks.convertToZip, targetVal, bin)
+ msi := wd.Task("Build MSI", tasks.buildMSI, targetVal, bin)
artifacts = append(artifacts, msi, zip)
case "darwin":
artifacts = append(artifacts, bin)
@@ -235,21 +290,19 @@
if target.BuildOnly {
continue
}
- short := wd.Task(taskName("Run short tests"), tasks.runTests, targetVal, wd.Constant(target.Builder), skipTests, bin)
- testResults = append(testResults, short)
+ short := wd.Action("Run short tests", tasks.runTests, targetVal, wd.Constant(target.Builder), skipTests, bin)
+ testsPassed = append(testsPassed, short)
if target.LongTestBuilder != "" {
- long := wd.Task(taskName("Run long tests"), tasks.runTests, targetVal, wd.Constant(target.LongTestBuilder), skipTests, bin)
- testResults = append(testResults, long)
+ long := wd.Action("Run long tests", tasks.runTests, targetVal, wd.Constant(target.LongTestBuilder), skipTests, bin)
+ testsPassed = append(testsPassed, long)
}
}
stagedArtifacts := wd.Task("Stage artifacts for signing", tasks.copyToStaging, version, wd.Slice(artifacts))
signedArtifacts := wd.Task("Wait for signed artifacts", tasks.awaitSigned, version, wd.Constant(darwinTargets), stagedArtifacts)
- wd.Output("Signed artifacts", signedArtifacts)
- wait := wd.Task("Wait for signing and tests", combineResults, signedArtifacts, wd.Slice(testResults))
- uploaded := wd.Task("Upload artifacts to CDN", tasks.uploadArtifacts, signedArtifacts, wait)
- published := wd.Task("Publish to website", tasks.publishArtifacts, version, signedArtifacts, uploaded)
- wd.Output("Published", published)
- return wd, nil
+ signedAndTested := wd.Task("Wait for signing and tests", func(ctx *workflow.TaskContext, artifacts []artifact) ([]artifact, error) {
+ return artifacts, nil
+ }, append([]workflow.TaskInput{signedArtifacts}, testsPassed...)...)
+ return signedAndTested, nil
}
// BuildReleaseTasks serves as an adapter to the various build tasks in the task package.
@@ -286,7 +339,7 @@
})
}
-func (b *BuildReleaseTasks) runTests(ctx *workflow.TaskContext, target *releasetargets.Target, buildlet string, skipTests []string, binary artifact) (string, error) {
+func (b *BuildReleaseTasks) runTests(ctx *workflow.TaskContext, target *releasetargets.Target, buildlet string, skipTests []string, binary artifact) error {
skipped := false
for _, skip := range skipTests {
if skip == "all" || target.Name == skip {
@@ -296,12 +349,12 @@
}
if skipped {
ctx.Printf("Skipping test")
- return "skipped", nil
+ return nil
}
_, err := b.runBuildStep(ctx, target, buildlet, binary, "", func(bs *task.BuildletStep, r io.Reader, _ io.Writer) error {
return bs.TestTarget(ctx, r)
})
- return "", err
+ return err
}
// runBuildStep is a convenience function that manages resources a build step might need.
@@ -431,8 +484,8 @@
return len(p), nil
}
-func combineResults(ctx *workflow.TaskContext, artifacts []artifact, tests []string) (string, error) {
- return fmt.Sprintf("%#v\n\n", artifacts) + strings.Join(tests, "\n"), nil
+func combineResults(ctx *workflow.TaskContext, artifacts []artifact) ([]artifact, error) {
+ return artifacts, nil
}
func (tasks *BuildReleaseTasks) copyToStaging(ctx *workflow.TaskContext, version string, artifacts []artifact) ([]artifact, error) {
@@ -592,20 +645,20 @@
var uploadPollDuration = 30 * time.Second
-func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *workflow.TaskContext, artifacts []artifact, _ string) (string, error) {
+func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *workflow.TaskContext, artifacts []artifact) error {
stagingFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.StagingURL)
if err != nil {
- return "", err
+ return err
}
servingFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.ServingURL)
if err != nil {
- return "", err
+ return err
}
todo := map[artifact]bool{}
for _, a := range artifacts {
if err := uploadArtifact(stagingFS, servingFS, a); err != nil {
- return "", err
+ return err
}
todo[a] = true
}
@@ -614,7 +667,7 @@
for _, a := range artifacts {
resp, err := http.Head(tasks.DownloadURL + "/" + a.filename)
if err != nil {
- return "", err
+ return err
}
resp.Body.Close()
if resp.StatusCode == http.StatusOK {
@@ -623,11 +676,11 @@
}
if len(todo) == 0 {
- return "", nil
+ return nil
}
select {
case <-ctx.Done():
- return "", ctx.Err()
+ return ctx.Err()
case <-time.After(uploadPollDuration):
ctx.Printf("Still waiting for %v artifacts to be published", len(todo))
}
@@ -681,7 +734,7 @@
return nil
}
-func (tasks *BuildReleaseTasks) publishArtifacts(ctx *workflow.TaskContext, version string, artifacts []artifact, _ string) (string, error) {
+func (tasks *BuildReleaseTasks) publishArtifacts(ctx *workflow.TaskContext, version string, artifacts []artifact) (string, error) {
for _, a := range artifacts {
f := &WebsiteFile{
Filename: a.filename,
@@ -708,7 +761,7 @@
return "", err
}
}
- return "", nil
+ return fmt.Sprintf("Uploaded %v artifacts for %v", len(artifacts), version), nil
}
// WebsiteFile represents a file on the go.dev downloads page.
diff --git a/internal/task/dlcl.go b/internal/task/dlcl.go
index 65fcebc..e70df6c 100644
--- a/internal/task/dlcl.go
+++ b/internal/task/dlcl.go
@@ -79,7 +79,7 @@
Subject: "dl: add " + strings.Join(versions, " and "),
Branch: "master",
}
- changeID, err := (&realGerritClient{client: cl}).CreateAutoSubmitChange(ctx, changeInput, files)
+ changeID, err := (&RealGerritClient{Client: cl}).CreateAutoSubmitChange(ctx, changeInput, files)
if err != nil {
return "", err
}
diff --git a/internal/task/gerrit.go b/internal/task/gerrit.go
index 7874e2b..8b4dcee 100644
--- a/internal/task/gerrit.go
+++ b/internal/task/gerrit.go
@@ -23,32 +23,32 @@
ListTags(ctx context.Context, project string) ([]string, error)
}
-type realGerritClient struct {
- client *gerrit.Client
+type RealGerritClient struct {
+ Client *gerrit.Client
}
-func (c *realGerritClient) CreateAutoSubmitChange(ctx context.Context, input gerrit.ChangeInput, files map[string]string) (string, error) {
- change, err := c.client.CreateChange(ctx, input)
+func (c *RealGerritClient) CreateAutoSubmitChange(ctx context.Context, input gerrit.ChangeInput, files map[string]string) (string, error) {
+ change, err := c.Client.CreateChange(ctx, input)
if err != nil {
return "", err
}
changeID := fmt.Sprintf("%s~%d", change.Project, change.ChangeNumber)
for path, content := range files {
if content == "" {
- if err := c.client.DeleteFileInChangeEdit(ctx, changeID, path); err != nil {
+ if err := c.Client.DeleteFileInChangeEdit(ctx, changeID, path); err != nil {
return "", err
}
} else {
- if err := c.client.ChangeFileContentInChangeEdit(ctx, changeID, path, content); err != nil {
+ if err := c.Client.ChangeFileContentInChangeEdit(ctx, changeID, path, content); err != nil {
return "", err
}
}
}
- if err := c.client.PublishChangeEdit(ctx, changeID); err != nil {
+ if err := c.Client.PublishChangeEdit(ctx, changeID); err != nil {
return "", err
}
- if err := c.client.SetReview(ctx, changeID, "current", gerrit.ReviewInput{
+ if err := c.Client.SetReview(ctx, changeID, "current", gerrit.ReviewInput{
Labels: map[string]int{
"Run-TryBot": 1,
"Auto-Submit": 1,
@@ -59,9 +59,9 @@
return changeID, nil
}
-func (c *realGerritClient) AwaitSubmit(ctx context.Context, changeID string) (string, error) {
+func (c *RealGerritClient) AwaitSubmit(ctx context.Context, changeID string) (string, error) {
for {
- detail, err := c.client.GetChangeDetail(ctx, changeID, gerrit.QueryChangesOpt{
+ detail, err := c.Client.GetChangeDetail(ctx, changeID, gerrit.QueryChangesOpt{
Fields: []string{"CURRENT_REVISION", "DETAILED_LABELS"},
})
if err != nil {
@@ -84,15 +84,15 @@
}
}
-func (c *realGerritClient) Tag(ctx context.Context, project, tag, commit string) error {
- _, err := c.client.CreateTag(ctx, project, tag, gerrit.TagInput{
+func (c *RealGerritClient) Tag(ctx context.Context, project, tag, commit string) error {
+ _, err := c.Client.CreateTag(ctx, project, tag, gerrit.TagInput{
Revision: commit,
})
return err
}
-func (c *realGerritClient) ListTags(ctx context.Context, project string) ([]string, error) {
- tags, err := c.client.GetProjectTags(ctx, project)
+func (c *RealGerritClient) ListTags(ctx context.Context, project string) ([]string, error) {
+ tags, err := c.Client.GetProjectTags(ctx, project)
if err != nil {
return nil, err
}
diff --git a/internal/task/milestones.go b/internal/task/milestones.go
index f8ac385..1943f2f 100644
--- a/internal/task/milestones.go
+++ b/internal/task/milestones.go
@@ -26,7 +26,8 @@
KindBeta
KindRC
KindMajor
- KindMinor
+ KindCurrentMinor
+ KindPrevMinor
)
type ReleaseMilestones struct {
@@ -81,10 +82,10 @@
// CheckBlockers returns an error if there are open release blockers in
// the current milestone.
-func (m *MilestoneTasks) CheckBlockers(ctx *workflow.TaskContext, milestones ReleaseMilestones, version string, kind ReleaseKind) (string, error) {
+func (m *MilestoneTasks) CheckBlockers(ctx *workflow.TaskContext, milestones ReleaseMilestones, version string, kind ReleaseKind) error {
issues, err := m.loadMilestoneIssues(ctx, milestones.Current, kind)
if err != nil {
- return "", err
+ return err
}
var blockers []string
for number, labels := range issues {
@@ -98,9 +99,9 @@
}
sort.Strings(blockers)
if len(blockers) != 0 {
- return "", fmt.Errorf("open release blockers:\n%v", strings.Join(blockers, "\n"))
+ return fmt.Errorf("open release blockers:\n%v", strings.Join(blockers, "\n"))
}
- return "", nil
+ return nil
}
// loadMilestoneIssues returns all the open issues in the specified milestone
@@ -161,15 +162,15 @@
// PushIssues updates issues to reflect a finished release. For beta1 releases,
// it removes the okay-after-beta1 label. For major and minor releases,
// it moves them to the next milestone and closes the current one.
-func (m *MilestoneTasks) PushIssues(ctx *workflow.TaskContext, milestones ReleaseMilestones, version string, kind ReleaseKind) (string, error) {
+func (m *MilestoneTasks) PushIssues(ctx *workflow.TaskContext, milestones ReleaseMilestones, version string, kind ReleaseKind) error {
// For RCs we don't change issues at all.
if kind == KindRC {
- return "", nil
+ return nil
}
issues, err := m.loadMilestoneIssues(ctx, milestones.Current, KindUnknown)
if err != nil {
- return "", err
+ return err
}
for issueNumber, labels := range issues {
var newLabels *[]string
@@ -184,7 +185,7 @@
*newLabels = append(*newLabels, label)
}
}
- } else if kind == KindMajor || kind == KindMinor {
+ } else if kind == KindMajor || kind == KindCurrentMinor || kind == KindPrevMinor {
newMilestone = &milestones.Next
}
_, _, err := m.Client.EditIssue(ctx, m.RepoOwner, m.RepoName, issueNumber, &github.IssueRequest{
@@ -192,18 +193,18 @@
Labels: newLabels,
})
if err != nil {
- return "", err
+ return err
}
}
- if kind == KindMajor || kind == KindMinor {
+ if kind == KindMajor || kind == KindCurrentMinor || kind == KindPrevMinor {
_, _, err := m.Client.EditMilestone(ctx, m.RepoOwner, m.RepoName, milestones.Current, &github.Milestone{
State: github.String("closed"),
})
if err != nil {
- return "", err
+ return err
}
}
- return "", nil
+ return nil
}
// GitHubClientInterface is a wrapper around the GitHub v3 and v4 APIs, for
diff --git a/internal/task/milestones_test.go b/internal/task/milestones_test.go
index 1e06119..2509f70 100644
--- a/internal/task/milestones_test.go
+++ b/internal/task/milestones_test.go
@@ -57,7 +57,7 @@
if err != nil {
t.Fatalf("GetMilestones: %v", err)
}
- if _, err := tasks.PushIssues(ctx, milestones, "go1.20beta1", KindBeta); err != nil {
+ if err := tasks.PushIssues(ctx, milestones, "go1.20beta1", KindBeta); err != nil {
t.Fatalf("Pushing issues for beta release: %v", err)
}
pushedBlocker, _, err := client3.Issues.Get(ctx, *flagOwner, *flagRepo, blocker.GetNumber())
@@ -67,17 +67,17 @@
if len(pushedBlocker.Labels) != 1 || *pushedBlocker.Labels[0].Name != "release-blocker" {
t.Errorf("release blocking issue has labels %#v, should only have release-blocker", pushedBlocker.Labels)
}
- _, err = tasks.CheckBlockers(ctx, milestones, "go1.20", KindMajor)
+ err = tasks.CheckBlockers(ctx, milestones, "go1.20", KindMajor)
if err == nil || !strings.Contains(err.Error(), "open release blockers") {
t.Fatalf("CheckBlockers with an open release blocker didn't give expected error: %v", err)
}
if _, _, err := client3.Issues.Edit(ctx, *flagOwner, *flagRepo, *blocker.Number, &github.IssueRequest{State: github.String("closed")}); err != nil {
t.Fatal(err)
}
- if _, err := tasks.CheckBlockers(ctx, milestones, "go1.20", KindMajor); err != nil {
+ if err := tasks.CheckBlockers(ctx, milestones, "go1.20", KindMajor); err != nil {
t.Fatalf("CheckBlockers with no release blockers failed: %v", err)
}
- if _, err := tasks.PushIssues(ctx, milestones, "go1.20", KindMajor); err != nil {
+ if err := tasks.PushIssues(ctx, milestones, "go1.20", KindMajor); err != nil {
t.Fatalf("PushIssues for major release failed: %v", err)
}
milestone, _, err := client3.Issues.GetMilestone(ctx, *flagOwner, *flagRepo, milestones.Current)
diff --git a/internal/task/version.go b/internal/task/version.go
index d9f9a20..bf1c548 100644
--- a/internal/task/version.go
+++ b/internal/task/version.go
@@ -15,54 +15,48 @@
Project string
}
-type NextVersions struct {
- CurrentMinor string
- PreviousMinor string
- Beta string
- RC string
- Major string
-}
-
-// GetNextVersions returns the next version for each type of release.
-func (t *VersionTasks) GetNextVersions(ctx *workflow.TaskContext) (NextVersions, error) {
+// GetNextVersion returns the next for the given type of release.
+func (t *VersionTasks) GetNextVersion(ctx *workflow.TaskContext, kind ReleaseKind) (string, error) {
tags, err := t.Gerrit.ListTags(ctx, t.Project)
if err != nil {
- return NextVersions{}, err
+ return "", err
}
tagSet := map[string]bool{}
for _, tag := range tags {
tagSet[tag] = true
}
// Find the most recently released major version.
- currentMajor := 0
- for ; ; currentMajor++ {
- if !tagSet[fmt.Sprintf("go1.%d", currentMajor+1)] {
+ // Going down from a high number is convenient for testing.
+ currentMajor := 100
+ for ; ; currentMajor-- {
+ if tagSet[fmt.Sprintf("go1.%d", currentMajor)] {
break
}
}
- var savedError error
- findUnused := func(v string) string {
+ findUnused := func(v string) (string, error) {
for {
if !tagSet[v] {
- return v
+ return v, nil
}
- var err error
v, err = nextVersion(v)
if err != nil {
- savedError = err
- return ""
+ return "", err
}
}
}
- // Find the next missing tag for each release type.
- result := NextVersions{
- CurrentMinor: findUnused(fmt.Sprintf("go1.%d.1", currentMajor)),
- PreviousMinor: findUnused(fmt.Sprintf("go1.%d.1", currentMajor-1)),
- Beta: findUnused(fmt.Sprintf("go1.%dbeta1", currentMajor+1)),
- RC: findUnused(fmt.Sprintf("go1.%drc1", currentMajor+1)),
- Major: fmt.Sprintf("go1.%d", currentMajor+1),
+ switch kind {
+ case KindCurrentMinor:
+ return findUnused(fmt.Sprintf("go1.%d.1", currentMajor))
+ case KindPrevMinor:
+ return findUnused(fmt.Sprintf("go1.%d.1", currentMajor-1))
+ case KindBeta:
+ return findUnused(fmt.Sprintf("go1.%dbeta1", currentMajor+1))
+ case KindRC:
+ return findUnused(fmt.Sprintf("go1.%drc1", currentMajor+1))
+ case KindMajor:
+ return fmt.Sprintf("go1.%d", currentMajor+1), nil
}
- return result, savedError
+ return "", fmt.Errorf("unknown release kind %v", kind)
}
func nextVersion(version string) (string, error) {
@@ -97,6 +91,6 @@
}
// TagRelease tags commit as version.
-func (t *VersionTasks) TagRelease(ctx *workflow.TaskContext, version, commit string) (string, error) {
- return "", t.Gerrit.Tag(ctx, t.Project, version, commit)
+func (t *VersionTasks) TagRelease(ctx *workflow.TaskContext, version, commit string) error {
+ return t.Gerrit.Tag(ctx, t.Project, version, commit)
}
diff --git a/internal/task/version_test.go b/internal/task/version_test.go
index 6d1fd32..9652799 100644
--- a/internal/task/version_test.go
+++ b/internal/task/version_test.go
@@ -13,14 +13,14 @@
var flagRunVersionTest = flag.Bool("run-version-test", false, "run version test, which will submit CLs to go.googlesource.com/scratch. Must have a Gerrit cookie in gitcookies.")
-func TestGetNextVersionsLive(t *testing.T) {
+func TestGetNextVersionLive(t *testing.T) {
if !*flagRunVersionTest {
t.Skip("Not enabled by flags")
}
cl := gerrit.NewClient("https://go-review.googlesource.com", gerrit.GitCookiesAuth())
tasks := &VersionTasks{
- Gerrit: &realGerritClient{client: cl},
+ Gerrit: &RealGerritClient{Client: cl},
Project: "go",
}
ctx := &workflow.TaskContext{
@@ -28,19 +28,22 @@
Logger: &testLogger{t},
}
- versions, err := tasks.GetNextVersions(ctx)
- if err != nil {
- t.Fatal(err)
+ versions := map[ReleaseKind]string{}
+ for kind := ReleaseKind(0); kind <= KindPrevMinor; kind++ {
+ var err error
+ versions[kind], err = tasks.GetNextVersion(ctx, kind)
+ if err != nil {
+ t.Fatal(err)
+ }
}
// It's hard to check correctness automatically.
t.Errorf("manually verify results: %#v", versions)
}
-func TestGetNextVersions(t *testing.T) {
+func TestGetNextVersion(t *testing.T) {
tasks := &VersionTasks{
Gerrit: &versionsClient{
tags: []string{
- "go1.1", "go1.2",
"go1.3beta1", "go1.3beta2", "go1.3rc1", "go1.3", "go1.3.1", "go1.3.2", "go1.3.3",
"go1.4beta1", "go1.4beta2", "go1.4rc1", "go1.4", "go1.4.1",
"go1.5beta1", "go1.5rc1",
@@ -52,18 +55,22 @@
Context: context.Background(),
Logger: &testLogger{t},
}
- versions, err := tasks.GetNextVersions(ctx)
- if err != nil {
- t.Fatal(err)
+ versions := map[ReleaseKind]string{}
+ for kind := ReleaseKind(1); kind <= KindPrevMinor; kind++ {
+ var err error
+ versions[kind], err = tasks.GetNextVersion(ctx, kind)
+ if err != nil {
+ t.Fatal(err)
+ }
}
- want := NextVersions{
- CurrentMinor: "go1.4.2",
- PreviousMinor: "go1.3.4",
- Beta: "go1.5beta2",
- RC: "go1.5rc2",
- Major: "go1.5",
+ want := map[ReleaseKind]string{
+ KindBeta: "go1.5beta2",
+ KindRC: "go1.5rc2",
+ KindMajor: "go1.5",
+ KindCurrentMinor: "go1.4.2",
+ KindPrevMinor: "go1.3.4",
}
- if diff := cmp.Diff(versions, want); diff != "" {
+ if diff := cmp.Diff(want, versions); diff != "" {
t.Fatalf("GetNextVersions mismatch (-want +got):\n%s", diff)
}
}
@@ -83,7 +90,7 @@
}
cl := gerrit.NewClient("https://go-review.googlesource.com", gerrit.GitCookiesAuth())
tasks := &VersionTasks{
- Gerrit: &realGerritClient{client: cl},
+ Gerrit: &RealGerritClient{Client: cl},
Project: "scratch",
}
ctx := &workflow.TaskContext{
