From e0ba88339ec82d3b2f9e5ca0037f18fcff0d3f95 Mon Sep 17 00:00:00 2001 From: Hongxiang Jiang Date: Tue, 3 Sep 2024 19:20:14 +0000 Subject: [PATCH] internal/task: add a step to run test for vscode-go extension Add new method to interface CloudBuildClient: - RunCustomSteps accept a function as input parameter. - The input function should return a slice of cloud steps which will be eventually executed by the method. - The input function should accept an input parameter "resultURL" and can assume this will be valid so the caller can write output to it. - The method will generate a random result url and pass that to the input function to generate the cloud steps. A local relui screenshot is at https://github.com/golang/vscode-go/issues/3500#issuecomment-2341760224 For golang/vscode-go#3500 Change-Id: Icdc6411ab8274b71783c15a0ccf7ecae0d61b3a5 Reviewed-on: https://go-review.googlesource.com/c/build/+/610375 LUCI-TryBot-Result: Go LUCI Reviewed-by: Hyang-Ah Hana Kim --- cmd/relui/main.go | 1 + internal/task/cloudbuild.go | 63 ++++++++++++--------- internal/task/fakes.go | 48 ++++++++++++++++ internal/task/releasegopls.go | 8 +-- internal/task/releasevscodego.go | 79 ++++++++++++++++++++++++--- internal/task/releasevscodego_test.go | 71 +++++++++++++++++++++++- 6 files changed, 231 insertions(+), 39 deletions(-) diff --git a/cmd/relui/main.go b/cmd/relui/main.go index e088be1cdf..2e5b2d807d 100644 --- a/cmd/relui/main.go +++ b/cmd/relui/main.go @@ -313,6 +313,7 @@ func main() { V4: githubv4.NewClient(githubHTTPClient), }, Gerrit: gerritClient, + CloudBuild: cloudBuildClient, ApproveAction: relui.ApproveActionDep(dbPool), } dh.RegisterDefinition("Create a vscode-go release candidate", releaseVSCodeGoTasks.NewPrereleaseDefinition()) diff --git a/internal/task/cloudbuild.go b/internal/task/cloudbuild.go index 90a04f88f1..d2fd6bd68a 100644 --- a/internal/task/cloudbuild.go +++ b/internal/task/cloudbuild.go @@ -28,6 +28,13 @@ type CloudBuildClient interface { // If gerritProject is provided, the script operates within a checkout of the // latest commit on the default branch of that repository. RunScript(ctx context.Context, script string, gerritProject string, outputs []string) (CloudBuild, error) + // RunCustomSteps is a low-level API that provides direct control over + // individual Cloud Build steps. It creates a random result directory + // and provides that as a parameter to the steps function, so that it + // may write output to it with 'gsutil cp' for accessing via ResultFS. + // Prefer RunScript for simpler scenarios. + // Reference: https://cloud.google.com/build/docs/build-config-file-schema + RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep) (CloudBuild, 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, build CloudBuild) (detail string, completed bool, _ error) @@ -78,35 +85,32 @@ archive=$(wget -qO - 'https://go.dev/dl/?mode=json' | grep -Eo 'go.*linux-amd64. wget -qO - https://go.dev/dl/${archive} | tar -xz mv go /workspace/released_go ` - const scriptPrefix = `#!/usr/bin/env bash set -eux set -o pipefail export PATH=/workspace/released_go/bin:$PATH ` - // Cloud build loses directory structure when it saves artifacts, which is - // a problem since (e.g.) we have multiple files named go.mod in the - // tagging tasks. It's not very complicated, so reimplement it ourselves. - resultURL := fmt.Sprintf("%v/script-build-%v", c.ScratchURL, rand.Int63()) - var saveOutputsScript strings.Builder - saveOutputsScript.WriteString(scriptPrefix) - for _, out := range outputs { - saveOutputsScript.WriteString(fmt.Sprintf("gsutil cp %q %q\n", out, resultURL+"/"+strings.TrimPrefix(out, "./"))) - } - - var steps []*cloudbuildpb.BuildStep - var dir string - if gerritProject != "" { - steps = append(steps, &cloudbuildpb.BuildStep{ - Name: "gcr.io/cloud-builders/git", - Args: []string{"clone", "https://go.googlesource.com/" + gerritProject, "checkout"}, - }) - dir = "checkout" - } + steps := func(resultURL string) []*cloudbuildpb.BuildStep { + // Cloud build loses directory structure when it saves artifacts, which is + // a problem since (e.g.) we have multiple files named go.mod in the + // tagging tasks. It's not very complicated, so reimplement it ourselves. + var saveOutputsScript strings.Builder + saveOutputsScript.WriteString(scriptPrefix) + for _, out := range outputs { + saveOutputsScript.WriteString(fmt.Sprintf("gsutil cp %q %q\n", out, resultURL+"/"+strings.TrimPrefix(out, "./"))) + } - build := &cloudbuildpb.Build{ - Steps: append(steps, + var steps []*cloudbuildpb.BuildStep + var dir string + if gerritProject != "" { + steps = append(steps, &cloudbuildpb.BuildStep{ + Name: "gcr.io/cloud-builders/git", + Args: []string{"clone", "https://go.googlesource.com/" + gerritProject, "checkout"}, + }) + dir = "checkout" + } + steps = append(steps, &cloudbuildpb.BuildStep{ Name: "bash", Script: downloadGoScript, @@ -121,7 +125,17 @@ export PATH=/workspace/released_go/bin:$PATH Script: saveOutputsScript.String(), Dir: dir, }, - ), + ) + + return steps + } + + return c.RunCustomSteps(ctx, steps) +} + +func (c *RealCloudBuildClient) RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep) (CloudBuild, error) { + build := &cloudbuildpb.Build{ + Steps: steps(fmt.Sprintf("%v/script-build-%v", c.ScratchURL, rand.Int63())), Options: &cloudbuildpb.BuildOptions{ MachineType: cloudbuildpb.BuildOptions_E2_HIGHCPU_8, Logging: cloudbuildpb.BuildOptions_CLOUD_LOGGING_ONLY, @@ -142,8 +156,7 @@ export PATH=/workspace/released_go/bin:$PATH if err != nil { return CloudBuild{}, fmt.Errorf("reading metadata: %w", err) } - return CloudBuild{Project: c.ScriptProject, ID: meta.Build.Id, ResultURL: resultURL}, nil - + return CloudBuild{Project: c.ScriptProject, ID: meta.Build.Id}, nil } func (c *RealCloudBuildClient) Completed(ctx context.Context, build CloudBuild) (string, bool, error) { diff --git a/internal/task/fakes.go b/internal/task/fakes.go index 8f64368cb8..1f3bc18e18 100644 --- a/internal/task/fakes.go +++ b/internal/task/fakes.go @@ -30,6 +30,7 @@ import ( "testing" "time" + "cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb" "github.com/google/go-github/v48/github" "github.com/google/uuid" "github.com/shurcooL/githubv4" @@ -631,6 +632,12 @@ case "$1" in esac ` +const fakeChown = ` +#!/bin/bash -eux +echo "chown change owner successful" +exit 0 +` + func NewFakeCloudBuild(t *testing.T, gerrit *FakeGerrit, project string, allowedTriggers map[string]map[string]string, fakeGo string) *FakeCloudBuild { toolDir := t.TempDir() if err := os.WriteFile(filepath.Join(toolDir, "go"), []byte(fakeGo), 0777); err != nil { @@ -639,6 +646,9 @@ func NewFakeCloudBuild(t *testing.T, gerrit *FakeGerrit, project string, allowed if err := os.WriteFile(filepath.Join(toolDir, "gsutil"), []byte(fakeGsutil), 0777); err != nil { t.Fatal(err) } + if err := os.WriteFile(filepath.Join(toolDir, "chown"), []byte(fakeChown), 0777); err != nil { + t.Fatal(err) + } return &FakeCloudBuild{ t: t, gerrit: gerrit, @@ -742,6 +752,44 @@ func (cb *FakeCloudBuild) RunScript(ctx context.Context, script string, gerritPr return CloudBuild{Project: cb.project, ID: id, ResultURL: "file://" + resultDir}, nil } +func (cb *FakeCloudBuild) RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep) (CloudBuild, error) { + var gerritProject, fullScript string + resultURL := "file://" + cb.t.TempDir() + for _, step := range steps(resultURL) { + tool, found := strings.CutPrefix(step.Name, "gcr.io/cloud-builders/") + if !found { + return CloudBuild{}, fmt.Errorf("does not support custom image: %s", step.Name) + } + if tool == "git" && len(step.Args) > 0 && step.Args[0] == "clone" { + for _, arg := range step.Args { + project, found := strings.CutPrefix(arg, "https://go.googlesource.com/") + if found { + gerritProject = project + break + } + } + continue + } + if len(step.Args) > 0 { + fullScript += tool + " " + strings.Join(step.Args, " ") + "\n" + } + if step.Script != "" { + fullScript += step.Script + "\n" + } + } + // In real CloudBuild client, the RunScript calls this lower level method. + build, err := cb.RunScript(ctx, fullScript, gerritProject, nil) + if err != nil { + return CloudBuild{}, err + } + // Overwrites the ResultURL as the actual output is written to a unique result + // directory generated by this method. + // Unit tests should verify the contents of this directory. + // The ResultURL returned by RunScript is not used for output and will always + // point to a new, empty directory. + return CloudBuild{ID: build.ID, Project: build.Project, ResultURL: resultURL}, nil +} + type FakeSwarmingClient struct { t *testing.T toolDir string diff --git a/internal/task/releasegopls.go b/internal/task/releasegopls.go index c46ea1f0ea..f9751c866c 100644 --- a/internal/task/releasegopls.go +++ b/internal/task/releasegopls.go @@ -593,9 +593,9 @@ func (r *ReleaseGoplsTasks) possibleGoplsVersions(ctx *wf.TaskContext) ([]string var possible []string for _, v := range releaseVersions { for _, next := range []releaseVersion{ - {v.Major+1, 0, 0}, // next major - {v.Major, v.Minor+1, 0}, // next minor - {v.Major, v.Minor, v.Patch+1}, // next patch + {v.Major + 1, 0, 0}, // next major + {v.Major, v.Minor + 1, 0}, // next minor + {v.Major, v.Minor, v.Patch + 1}, // next patch } { if _, ok := seen[next]; !ok { possible = append(possible, next.String()) @@ -673,7 +673,7 @@ func (r *ReleaseGoplsTasks) updateVSCodeGoGoplsVersion(ctx *wf.TaskContext, revi } branches := []string{"master"} if prerelease == "" { - releaseBranch, err := vsCodeGoActiveReleaseBranch(ctx, r.Gerrit) + releaseBranch, err := vscodeGoActiveReleaseBranch(ctx, r.Gerrit) if err != nil { return nil, err } diff --git a/internal/task/releasevscodego.go b/internal/task/releasevscodego.go index 14df728704..4b1f656ddb 100644 --- a/internal/task/releasevscodego.go +++ b/internal/task/releasevscodego.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" + "cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb" "github.com/google/go-github/v48/github" "golang.org/x/build/gerrit" "golang.org/x/build/internal/relui/groups" @@ -52,6 +53,7 @@ import ( type ReleaseVSCodeGoTasks struct { Gerrit GerritClient GitHub GitHubClientInterface + CloudBuild CloudBuildClient ApproveAction func(*wf.TaskContext) error } @@ -68,11 +70,11 @@ var nextVersionParam = wf.ParamDef[string]{ } //go:embed template/vscode-go-release-issue.md -var vscodeGOReleaseIssueTmplStr string +var vscodeGoReleaseIssueTmplStr string -// vsCodeGoActiveReleaseBranch returns the current active release branch in +// vscodeGoActiveReleaseBranch returns the current active release branch in // vscode-go project. -func vsCodeGoActiveReleaseBranch(ctx *wf.TaskContext, gerrit GerritClient) (string, error) { +func vscodeGoActiveReleaseBranch(ctx *wf.TaskContext, gerrit GerritClient) (string, error) { branches, err := gerrit.ListBranches(ctx, "vscode-go") if err != nil { return "", err @@ -132,17 +134,78 @@ func (r *ReleaseVSCodeGoTasks) NewPrereleaseDefinition() *wf.Definition { release := wf.Task1(wd, "determine the release version", r.determineReleaseVersion, versionBumpStrategy) prerelease := wf.Task1(wd, "find the next pre-release version", r.nextPrereleaseVersion, release) + revision := wf.Task2(wd, "find the revision for the pre-release version", r.findRevision, release, prerelease) approved := wf.Action2(wd, "await release coordinator's approval", r.approvePrereleaseVersion, release, prerelease) - _ = wf.Task1(wd, "create release milestone and issue", r.createReleaseMilestoneAndIssue, release, wf.After(approved)) + verified := wf.Action1(wd, "verify the release candidate", r.verifyTestResults, revision, wf.After(approved)) - branched := wf.Action2(wd, "create release branch", r.createReleaseBranch, release, prerelease, wf.After(approved)) - // TODO(hxjiang): replace empty commit with the branch's head once verified. - _ = wf.Action3(wd, "tag release candidate", r.tag, wf.Const(""), release, prerelease, wf.After(branched)) + _ = wf.Task1(wd, "create release milestone and issue", r.createReleaseMilestoneAndIssue, release, wf.After(verified)) + branched := wf.Action2(wd, "create release branch", r.createReleaseBranch, release, prerelease, wf.After(verified)) + _ = wf.Action3(wd, "tag release candidate", r.tag, revision, release, prerelease, wf.After(branched)) return wd } +// findRevision determines the appropriate revision for the current release. +// Returns the head of the master branch if this is the first release candidate +// for a stable minor version (as no release branch exists yet). +// Returns the head of the corresponding release branch otherwise. +func (r *ReleaseVSCodeGoTasks) findRevision(ctx *wf.TaskContext, release releaseVersion, prerelease string) (string, error) { + branch := fmt.Sprintf("release-v%v.%v", release.Major, release.Minor) + if release.Patch == 0 && prerelease == "rc.1" { + branch = "master" + } + + return r.Gerrit.ReadBranchHead(ctx, "vscode-go", branch) +} + +func (r *ReleaseVSCodeGoTasks) verifyTestResults(ctx *wf.TaskContext, revision string) error { + // We are running all tests in a docker as a user 'node' (uid: 1000) + // Let the user own the directory. + testScript := `#!/usr/bin/env bash +set -eux +set -o pipefail + +chown -R 1000:1000 . +./build/all.bash testlocal &> output.log +` + + build, err := r.CloudBuild.RunCustomSteps(ctx, func(resultURL string) []*cloudbuildpb.BuildStep { + return []*cloudbuildpb.BuildStep{ + { + Name: "gcr.io/cloud-builders/git", + Args: []string{"clone", "https://go.googlesource.com/vscode-go", "vscode-go"}, + }, + { + Name: "gcr.io/cloud-builders/git", + Args: []string{"checkout", revision}, + Dir: "vscode-go", + }, + { + Name: "gcr.io/cloud-builders/docker", + Script: testScript, + Dir: "vscode-go", + }, + { + Name: "gcr.io/cloud-builders/gsutil", + Args: []string{"cp", "output.log", fmt.Sprintf("%s/output.log", resultURL)}, + Dir: "vscode-go", + }, + } + }) + if err != nil { + return err + } + + outputs, err := buildToOutputs(ctx, r.CloudBuild, build) + if err != nil { + return err + } + + ctx.Printf("the output from test run:\n%s\n", outputs["output.log"]) + return nil +} + func (r *ReleaseVSCodeGoTasks) createReleaseMilestoneAndIssue(ctx *wf.TaskContext, semv releaseVersion) (int, error) { version := fmt.Sprintf("v%v.%v.%v", semv.Major, semv.Minor, semv.Patch) @@ -168,7 +231,7 @@ func (r *ReleaseVSCodeGoTasks) createReleaseMilestoneAndIssue(ctx *wf.TaskContex } } - content := fmt.Sprintf(vscodeGOReleaseIssueTmplStr, version) + content := fmt.Sprintf(vscodeGoReleaseIssueTmplStr, version) issue, _, err := r.GitHub.CreateIssue(ctx, "golang", "vscode-go", &github.IssueRequest{ Title: &title, Body: &content, diff --git a/internal/task/releasevscodego_test.go b/internal/task/releasevscodego_test.go index cc8bf53884..83715b5f42 100644 --- a/internal/task/releasevscodego_test.go +++ b/internal/task/releasevscodego_test.go @@ -346,13 +346,80 @@ func TestVSCodeGoActiveReleaseBranch(t *testing.T) { Context: context.Background(), Logger: &testLogger{t, ""}, } - got, err := vsCodeGoActiveReleaseBranch(ctx, gerrit) + got, err := vscodeGoActiveReleaseBranch(ctx, gerrit) if err != nil { t.Fatal(err) } if tc.want != got { - t.Errorf("vsCodeGoActiveReleaseBranch() = %q, want %q", got, tc.want) + t.Errorf("vscodeGoActiveReleaseBranch() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestVerifyTestResults(t *testing.T) { + mustHaveShell(t) + fakeScriptFmt := `#!/bin/bash -exu + +case "$1" in +"testlocal") + echo "the testlocal return %v" + exit %v + ;; +*) + echo unexpected command $@ + exit 1 + ;; +esac +` + testcases := []struct { + name string + rc int + wantErr bool + }{ + { + name: "test failed, return error", + rc: 1, + wantErr: true, + }, + { + name: "test passed, return nil", + rc: 0, + wantErr: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + vscodego := NewFakeRepo(t, "vscode-go") + commit := vscodego.Commit(map[string]string{ + "go.mod": "module github.com/golang/vscode-go\n", + "go.sum": "\n", + "build/all.bash": fmt.Sprintf(fakeScriptFmt, tc.rc, tc.rc), + }) + // Overwrite the script to empty to make sure vscode-go flow will checkout + // the specific commit. + _ = vscodego.Commit(map[string]string{ + "build/all.bash": "", + }) + + gerrit := NewFakeGerrit(t, vscodego) + ctx := &workflow.TaskContext{ + Context: context.Background(), + Logger: &testLogger{t, ""}, + } + + tasks := &ReleaseVSCodeGoTasks{ + Gerrit: gerrit, + CloudBuild: NewFakeCloudBuild(t, gerrit, "vscode-go", nil, ""), + } + + err := tasks.verifyTestResults(ctx, commit) + if tc.wantErr && err == nil { + t.Errorf("verifyTestResult() should return error but return nil") + } else if !tc.wantErr && err != nil { + t.Errorf("verifyTestResult() should return nil but return err: %v", err) } }) }