Skip to content

Commit

Permalink
Make magician subcommands use RunE and return errors instead of calli…
Browse files Browse the repository at this point in the history 
…ng os.Exit directly (GoogleCloudPlatform#10891)
  • Loading branch information
@iyabchen
iyabchen authored Jun 6, 2024
1 parent 04e7ef6 commit 7a4b384
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 197 deletions.
31 changes: 12 additions & 19 deletions .ci/magician/cmd/check_cassettes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,37 +43,33 @@ var checkCassettesCmd = &cobra.Command{
` + listCCEnvironmentVariables() + `
It prints a list of tests that failed in replaying mode along with all test output.`,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
env := make(map[string]string, len(ccEnvironmentVariables))
for _, ev := range ccEnvironmentVariables {
val, ok := os.LookupEnv(ev)
if !ok {
fmt.Printf("Did not provide %s environment variable\n", ev)
os.Exit(1)
return fmt.Errorf("did not provide %s environment variable", ev)
}
env[ev] = val
}

githubToken, ok := lookupGithubTokenOrFallback("GITHUB_TOKEN_DOWNSTREAMS")
if !ok {
fmt.Println("Did not provide GITHUB_TOKEN_DOWNSTREAMS or GITHUB_TOKEN environment variables")
os.Exit(1)
return fmt.Errorf("did not provide GITHUB_TOKEN_DOWNSTREAMS or GITHUB_TOKEN environment variables")
}

rnr, err := exec.NewRunner()
if err != nil {
fmt.Println("Error creating Runner: ", err)
os.Exit(1)
return fmt.Errorf("error creating Runner: %w", err)
}

ctlr := source.NewController(env["GOPATH"], "modular-magician", githubToken, rnr)

vt, err := vcr.NewTester(env, rnr)
if err != nil {
fmt.Println("Error creating VCR tester: ", err)
os.Exit(1)
return fmt.Errorf("error creating VCR tester: %w", err)
}
execCheckCassettes(env["COMMIT_SHA"], vt, ctlr)
return execCheckCassettes(env["COMMIT_SHA"], vt, ctlr)
},
}

Expand All @@ -93,10 +89,9 @@ func listCCEnvironmentVariables() string {
return result
}

func execCheckCassettes(commit string, vt *vcr.Tester, ctlr *source.Controller) {
func execCheckCassettes(commit string, vt *vcr.Tester, ctlr *source.Controller) error {
if err := vt.FetchCassettes(provider.Beta, "main", ""); err != nil {
fmt.Println("Error fetching cassettes: ", err)
os.Exit(1)
return fmt.Errorf("error fetching cassettes: %w", err)
}

providerRepo := &source.Repo{
Expand All @@ -105,8 +100,7 @@ func execCheckCassettes(commit string, vt *vcr.Tester, ctlr *source.Controller)
}
ctlr.SetPath(providerRepo)
if err := ctlr.Clone(providerRepo); err != nil {
fmt.Println("Error cloning provider: ", err)
os.Exit(1)
return fmt.Errorf("error cloning provider: %w", err)
}
vt.SetRepoPath(provider.Beta, providerRepo.Path)

Expand All @@ -115,18 +109,17 @@ func execCheckCassettes(commit string, vt *vcr.Tester, ctlr *source.Controller)
fmt.Println("Error running VCR: ", err)
}
if err := vt.UploadLogs("vcr-check-cassettes", "", "", false, false, vcr.Replaying, provider.Beta); err != nil {
fmt.Println("Error uploading logs: ", err)
os.Exit(1)
return fmt.Errorf("error uploading logs: %w", err)
}
fmt.Println(len(result.FailedTests), " failed tests: ", result.FailedTests)
// TODO(trodge) report these failures to bigquery
fmt.Println(len(result.PassedTests), " passed tests: ", result.PassedTests)
fmt.Println(len(result.SkippedTests), " skipped tests: ", result.SkippedTests)

if err := vt.Cleanup(); err != nil {
fmt.Println("Error cleaning up vcr tester: ", err)
os.Exit(1)
return fmt.Errorf("error cleaning up vcr tester: %w", err)
}
return nil
}

func init() {
Expand Down
14 changes: 6 additions & 8 deletions .ci/magician/cmd/community_checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"magician/cloudbuild"
"magician/github"
"os"

"github.com/spf13/cobra"
)
Expand All @@ -42,7 +41,7 @@ var communityApprovalCmd = &cobra.Command{
1. Trigger cloud presubmits with specific substitutions for the PR.
2. Remove the 'awaiting-approval' label from the PR.
`,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
prNumber := args[0]
fmt.Println("PR Number: ", prNumber)

Expand All @@ -63,16 +62,15 @@ var communityApprovalCmd = &cobra.Command{

githubToken, ok := lookupGithubTokenOrFallback("GITHUB_TOKEN_MAGIC_MODULES")
if !ok {
fmt.Println("Did not provide GITHUB_TOKEN_MAGIC_MODULES or GITHUB_TOKEN environment variables")
os.Exit(1)
return fmt.Errorf("did not provide GITHUB_TOKEN_MAGIC_MODULES or GITHUB_TOKEN environment variables")
}
gh := github.NewClient(githubToken)
cb := cloudbuild.NewClient()
execCommunityChecker(prNumber, commitSha, branchName, headRepoUrl, headBranch, baseBranch, gh, cb)
return execCommunityChecker(prNumber, commitSha, branchName, headRepoUrl, headBranch, baseBranch, gh, cb)
},
}

func execCommunityChecker(prNumber, commitSha, branchName, headRepoUrl, headBranch, baseBranch string, gh GithubClient, cb CloudbuildClient) {
func execCommunityChecker(prNumber, commitSha, branchName, headRepoUrl, headBranch, baseBranch string, gh GithubClient, cb CloudbuildClient) error {
substitutions := map[string]string{
"BRANCH_NAME": branchName,
"_PR_NUMBER": prNumber,
Expand All @@ -85,13 +83,13 @@ func execCommunityChecker(prNumber, commitSha, branchName, headRepoUrl, headBran
// (explicitly or via membership-checker)
err := cb.TriggerMMPresubmitRuns(commitSha, substitutions)
if err != nil {
fmt.Println(err)
os.Exit(1)
return err
}

// in community-checker job:
// remove awaiting-approval label from external contributor PRs
gh.RemoveLabel(prNumber, "awaiting-approval")
return nil
}

func init() {
Expand Down
25 changes: 10 additions & 15 deletions .ci/magician/cmd/generate_comment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,38 +102,34 @@ var generateCommentCmd = &cobra.Command{
5. Report the results in a PR comment.
6. Run unit tests for the missing test detector.
`,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
env := make(map[string]string, len(gcEnvironmentVariables))
for _, ev := range gcEnvironmentVariables {
val, ok := os.LookupEnv(ev)
if !ok {
fmt.Printf("Did not provide %s environment variable\n", ev)
os.Exit(1)
return fmt.Errorf("did not provide %s environment variable", ev)
}
env[ev] = val
}

for _, tokenName := range []string{"GITHUB_TOKEN_DOWNSTREAMS", "GITHUB_TOKEN_MAGIC_MODULES"} {
val, ok := lookupGithubTokenOrFallback(tokenName)
if !ok {
fmt.Printf("Did not provide %s or GITHUB_TOKEN environment variable\n", tokenName)
os.Exit(1)
return fmt.Errorf("did not provide %s or GITHUB_TOKEN environment variable", tokenName)
}
env[tokenName] = val
}
gh := github.NewClient(env["GITHUB_TOKEN_MAGIC_MODULES"])
rnr, err := exec.NewRunner()
if err != nil {
fmt.Println("Error creating a runner: ", err)
os.Exit(1)
return fmt.Errorf("error creating a runner: %w", err)
}
ctlr := source.NewController(filepath.Join("workspace", "go"), "modular-magician", env["GITHUB_TOKEN_DOWNSTREAMS"], rnr)
prNumber, err := strconv.Atoi(env["PR_NUMBER"])
if err != nil {
fmt.Println("Error parsing PR_NUMBER: ", err)
os.Exit(1)
return fmt.Errorf("error parsing PR_NUMBER: %w", err)
}
execGenerateComment(
return execGenerateComment(
prNumber,
env["GITHUB_TOKEN_MAGIC_MODULES"],
env["BUILD_ID"],
Expand All @@ -155,7 +151,7 @@ func listGCEnvironmentVariables() string {
return result
}

func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep, projectId, commitSha string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) {
func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep, projectId, commitSha string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) error {
errors := map[string][]string{"Other": []string{}}

pullRequest, err := gh.GetPullRequest(strconv.Itoa(prNumber))
Expand Down Expand Up @@ -392,15 +388,14 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
// Post diff comment
message, err := formatDiffComment(data)
if err != nil {
fmt.Println("Error formatting message: ", err)
fmt.Printf("Data: %v\n", data)
os.Exit(1)
return fmt.Errorf("error formatting message: %w", err)
}
if err := gh.PostComment(strconv.Itoa(prNumber), message); err != nil {
fmt.Printf("Error posting comment to PR %d: %v\n", prNumber, err)
fmt.Println("Comment: ", message)
os.Exit(1)
return fmt.Errorf("error posting comment to PR %d: %w", prNumber, err)
}
return nil
}

// Build the diff processor for tpg or tpgb
Expand Down
50 changes: 18 additions & 32 deletions .ci/magician/cmd/generate_downstream.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,12 @@ var generateDownstreamCmd = &cobra.Command{
The following environment variables should be set:
` + listGDEnvironmentVariables(),
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
env := make(map[string]string, len(gdEnvironmentVariables))
for _, ev := range gdEnvironmentVariables {
val, ok := os.LookupEnv(ev)
if !ok {
fmt.Printf("Did not provide %s environment variable\n", ev)
os.Exit(1)
return fmt.Errorf("did not provide %s environment variable", ev)
}
env[ev] = val
}
Expand All @@ -65,28 +64,24 @@ var generateDownstreamCmd = &cobra.Command{
gh := github.NewClient(githubToken)
rnr, err := exec.NewRunner()
if err != nil {
fmt.Println("Error creating a runner: ", err)
os.Exit(1)
return fmt.Errorf("error creating a runner: %w", err)
}
ctlr := source.NewController(env["GOPATH"], "modular-magician", githubToken, rnr)
oldToken := os.Getenv("GITHUB_TOKEN")
if err := os.Setenv("GITHUB_TOKEN", githubToken); err != nil {
fmt.Println("Error setting GITHUB_TOKEN environment variable: ", err)
os.Exit(1)
return fmt.Errorf("error setting GITHUB_TOKEN environment variable: %w", err)
}
defer func() {
if err := os.Setenv("GITHUB_TOKEN", oldToken); err != nil {
fmt.Println("Error setting GITHUB_TOKEN environment variable: ", err)
os.Exit(1)
}
}()

if len(args) != 4 {
fmt.Printf("Wrong number of arguments %d, expected 4\n", len(args))
os.Exit(1)
return fmt.Errorf("wrong number of arguments %d, expected 4", len(args))
}

execGenerateDownstream(env["BASE_BRANCH"], args[0], args[1], args[2], args[3], gh, rnr, ctlr)
return execGenerateDownstream(env["BASE_BRANCH"], args[0], args[1], args[2], args[3], gh, rnr, ctlr)
},
}

Expand All @@ -98,16 +93,15 @@ func listGDEnvironmentVariables() string {
return result
}

func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) {
func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) error {
if baseBranch == "" {
baseBranch = "main"
}

mmLocalPath := filepath.Join(rnr.GetCWD(), "..", "..")
mmCopyPath := filepath.Join(mmLocalPath, "..", fmt.Sprintf("mm-%s-%s-%s", repo, version, command))
if _, err := rnr.Run("cp", []string{"-rp", mmLocalPath, mmCopyPath}, nil); err != nil {
fmt.Println("Error copying magic modules: ", err)
os.Exit(1)
return fmt.Errorf("error copying magic modules: %w", err)
}
mmRepo := &source.Repo{
Name: "magic-modules",
Expand All @@ -116,36 +110,30 @@ func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh G

downstreamRepo, scratchRepo, commitMessage, err := cloneRepo(mmRepo, baseBranch, repo, version, command, ref, rnr, ctlr)
if err != nil {
fmt.Println("Error cloning repo: ", err)
os.Exit(1)
return fmt.Errorf("error cloning repo: %w", err)
}

if err := rnr.PushDir(mmCopyPath); err != nil {
fmt.Println("Error changing directory to copied magic modules: ", err)
os.Exit(1)
return fmt.Errorf("error changing directory to copied magic modules: %w", err)
}

if err := setGitConfig(rnr); err != nil {
fmt.Println("Error setting config: ", err)
os.Exit(1)
return fmt.Errorf("error setting config: %w", err)
}

if err := runMake(downstreamRepo, command, rnr); err != nil {
fmt.Println("Error running make: ", err)
os.Exit(1)
return fmt.Errorf("error running make: %w", err)
}

var pullRequest *github.PullRequest
if command == "downstream" {
pullRequest, err = getPullRequest(baseBranch, ref, gh)
if err != nil {
fmt.Println("Error getting pull request: ", err)
os.Exit(1)
return fmt.Errorf("error getting pull request: %w", err)
}
if repo == "terraform" {
if err := addChangelogEntry(pullRequest, rnr); err != nil {
fmt.Println("Error adding changelog entry: ", err)
os.Exit(1)
return fmt.Errorf("error adding changelog entry: %w", err)
}
}
}
Expand All @@ -159,16 +147,15 @@ func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh G
}

if _, err := rnr.Run("git", []string{"push", ctlr.URL(scratchRepo), scratchRepo.Branch, "-f"}, nil); err != nil {
fmt.Println("Error pushing commit: ", err)
os.Exit(1)
return fmt.Errorf("error pushing commit: %w", err)
}

if commitErr == nil && command == "downstream" {
if err := mergePullRequest(downstreamRepo, scratchRepo, scratchCommitSha, pullRequest, rnr, gh); err != nil {
fmt.Println("Error merging pull request: ", err)
os.Exit(1)
return fmt.Errorf("error merging pull request: %w", err)
}
}
return nil
}

func cloneRepo(mmRepo *source.Repo, baseBranch, repo, version, command, ref string, rnr ExecRunner, ctlr *source.Controller) (*source.Repo, *source.Repo, string, error) {
Expand Down Expand Up @@ -323,8 +310,7 @@ func createCommit(scratchRepo *source.Repo, commitMessage string, rnr ExecRunner

commitSha, err := rnr.Run("git", []string{"rev-parse", "HEAD"}, nil)
if err != nil {
fmt.Println("Error retrieving commit sha: ", err)
os.Exit(1)
return "", fmt.Errorf("error retrieving commit sha: %w", err)
}

commitSha = strings.TrimSpace(commitSha)
Expand Down
Loading

0 comments on commit 7a4b384

Please sign in to comment.