From 7a4b38409139d9806df7dfaa9f29012b17c461ca Mon Sep 17 00:00:00 2001 From: Iris Chen <10179943+iyabchen@users.noreply.github.com> Date: Thu, 6 Jun 2024 15:28:28 -0700 Subject: [PATCH] Make magician subcommands use RunE and return errors instead of calling os.Exit directly (#10891) --- .ci/magician/cmd/check_cassettes.go | 31 +++----- .ci/magician/cmd/community_checker.go | 14 ++-- .ci/magician/cmd/generate_comment.go | 25 +++---- .ci/magician/cmd/generate_downstream.go | 50 +++++-------- .ci/magician/cmd/membership_checker.go | 17 ++--- .ci/magician/cmd/request_reviewer.go | 25 +++---- .ci/magician/cmd/request_service_reviewers.go | 27 +++---- .ci/magician/cmd/test_terraform_vcr.go | 75 +++++++------------ .ci/magician/cmd/test_tgc.go | 13 ++-- .ci/magician/cmd/test_tgc_integration.go | 32 ++++---- .ci/magician/cmd/test_tpg.go | 16 ++-- 11 files changed, 128 insertions(+), 197 deletions(-) diff --git a/.ci/magician/cmd/check_cassettes.go b/.ci/magician/cmd/check_cassettes.go index e19f3f7c9ef3..87e8cf746765 100644 --- a/.ci/magician/cmd/check_cassettes.go +++ b/.ci/magician/cmd/check_cassettes.go @@ -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) }, } @@ -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{ @@ -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) @@ -115,8 +109,7 @@ 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 @@ -124,9 +117,9 @@ func execCheckCassettes(commit string, vt *vcr.Tester, ctlr *source.Controller) 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() { diff --git a/.ci/magician/cmd/community_checker.go b/.ci/magician/cmd/community_checker.go index 2834009a5ebb..f86aabdfba58 100644 --- a/.ci/magician/cmd/community_checker.go +++ b/.ci/magician/cmd/community_checker.go @@ -19,7 +19,6 @@ import ( "fmt" "magician/cloudbuild" "magician/github" - "os" "github.com/spf13/cobra" ) @@ -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) @@ -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, @@ -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() { diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go index 1422e582d17f..68ef979a882e 100644 --- a/.ci/magician/cmd/generate_comment.go +++ b/.ci/magician/cmd/generate_comment.go @@ -102,13 +102,12 @@ 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 } @@ -116,24 +115,21 @@ var generateCommentCmd = &cobra.Command{ 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"], @@ -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)) @@ -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 diff --git a/.ci/magician/cmd/generate_downstream.go b/.ci/magician/cmd/generate_downstream.go index 79201986aba0..6d620ee6e3b4 100644 --- a/.ci/magician/cmd/generate_downstream.go +++ b/.ci/magician/cmd/generate_downstream.go @@ -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 } @@ -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) }, } @@ -98,7 +93,7 @@ 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" } @@ -106,8 +101,7 @@ func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh G 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", @@ -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) } } } @@ -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) { @@ -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) diff --git a/.ci/magician/cmd/membership_checker.go b/.ci/magician/cmd/membership_checker.go index 12a7a020ff02..7e149a122dad 100644 --- a/.ci/magician/cmd/membership_checker.go +++ b/.ci/magician/cmd/membership_checker.go @@ -19,7 +19,6 @@ import ( "fmt" "magician/cloudbuild" "magician/github" - "os" "github.com/spf13/cobra" ) @@ -45,7 +44,7 @@ var membershipCheckerCmd = &cobra.Command{ `, // This can change to cobra.ExactArgs(2) after at least a 2-week soak Args: cobra.RangeArgs(2, 6), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { prNumber := args[0] fmt.Println("PR Number: ", prNumber) @@ -54,20 +53,18 @@ var membershipCheckerCmd = &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() - execMembershipChecker(prNumber, commitSha, gh, cb) + return execMembershipChecker(prNumber, commitSha, gh, cb) }, } -func execMembershipChecker(prNumber, commitSha string, gh GithubClient, cb CloudbuildClient) { +func execMembershipChecker(prNumber, commitSha string, gh GithubClient, cb CloudbuildClient) error { pullRequest, err := gh.GetPullRequest(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } author := pullRequest.User.Login @@ -79,12 +76,12 @@ func execMembershipChecker(prNumber, commitSha string, gh GithubClient, cb Cloud if trusted { err = cb.ApproveCommunityChecker(prNumber, commitSha) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } } else { gh.AddLabels(prNumber, []string{"awaiting-approval"}) } + return nil } func init() { diff --git a/.ci/magician/cmd/request_reviewer.go b/.ci/magician/cmd/request_reviewer.go index f4d2122a6424..e3981467535c 100644 --- a/.ci/magician/cmd/request_reviewer.go +++ b/.ci/magician/cmd/request_reviewer.go @@ -45,24 +45,22 @@ var requestReviewerCmd = &cobra.Command{ c. As appropriate, posts a welcome comment on the PR. `, Args: cobra.ExactArgs(1), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { prNumber := args[0] fmt.Println("PR Number: ", prNumber) githubToken, ok := os.LookupEnv("GITHUB_TOKEN") if !ok { - fmt.Println("Did not provide GITHUB_TOKEN environment variable") - os.Exit(1) + return fmt.Errorf("did not provide GITHUB_TOKEN environment variable") } gh := github.NewClient(githubToken) - execRequestReviewer(prNumber, gh) + return execRequestReviewer(prNumber, gh) }, } -func execRequestReviewer(prNumber string, gh GithubClient) { +func execRequestReviewer(prNumber string, gh GithubClient) error { pullRequest, err := gh.GetPullRequest(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } author := pullRequest.User.Login @@ -71,14 +69,12 @@ func execRequestReviewer(prNumber string, gh GithubClient) { requestedReviewers, err := gh.GetPullRequestRequestedReviewers(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } previousReviewers, err := gh.GetPullRequestPreviousReviewers(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } reviewersToRequest, newPrimaryReviewer := github.ChooseCoreReviewers(requestedReviewers, previousReviewers) @@ -86,8 +82,7 @@ func execRequestReviewer(prNumber string, gh GithubClient) { if len(reviewersToRequest) > 0 { err = gh.RequestPullRequestReviewers(prNumber, reviewersToRequest) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } } @@ -95,11 +90,11 @@ func execRequestReviewer(prNumber string, gh GithubClient) { comment := github.FormatReviewerComment(newPrimaryReviewer) err = gh.PostComment(prNumber, comment) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } } } + return nil } func init() { diff --git a/.ci/magician/cmd/request_service_reviewers.go b/.ci/magician/cmd/request_service_reviewers.go index ed7a713b296c..267d182debd5 100644 --- a/.ci/magician/cmd/request_service_reviewers.go +++ b/.ci/magician/cmd/request_service_reviewers.go @@ -19,7 +19,6 @@ import ( "fmt" "magician/github" "math/rand" - "os" "strings" "github.com/GoogleCloudPlatform/magic-modules/tools/issue-labeler/labeler" @@ -36,17 +35,16 @@ var requestServiceReviewersCmd = &cobra.Command{ If a PR has more than 3 service labels, the command will not do anything. `, Args: cobra.ExactArgs(1), - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { prNumber := args[0] fmt.Println("PR Number: ", prNumber) githubToken, ok := lookupGithubTokenOrFallback("GITHUB_TOKEN_MAGIC_MODULES") if !ok { - fmt.Println("Did not provide GITHUB_TOKEN_MAGIC_MODULES or GITHUB_TOKEN environment variable") - os.Exit(1) + return fmt.Errorf("did not provide GITHUB_TOKEN_MAGIC_MODULES or GITHUB_TOKEN environment variable") } gh := github.NewClient(githubToken) - execRequestServiceReviewers(prNumber, gh, labeler.EnrolledTeamsYaml) + return execRequestServiceReviewers(prNumber, gh, labeler.EnrolledTeamsYaml) }, } @@ -55,29 +53,25 @@ type LabelData struct { Team string `yaml:"team,omitempty"` } -func execRequestServiceReviewers(prNumber string, gh GithubClient, enrolledTeamsYaml []byte) { +func execRequestServiceReviewers(prNumber string, gh GithubClient, enrolledTeamsYaml []byte) error { pullRequest, err := gh.GetPullRequest(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } enrolledTeams := make(map[string]LabelData) if err := yaml.Unmarshal(enrolledTeamsYaml, &enrolledTeams); err != nil { - fmt.Printf("Error unmarshalling enrolled teams yaml: %s", err) - os.Exit(1) + return fmt.Errorf("error unmarshalling enrolled teams yaml: %w", err) } requestedReviewers, err := gh.GetPullRequestRequestedReviewers(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } previousReviewers, err := gh.GetPullRequestPreviousReviewers(prNumber) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } // If more than three service labels are impacted, don't request reviews. @@ -96,7 +90,7 @@ func execRequestServiceReviewers(prNumber string, gh GithubClient, enrolledTeams if teamCount > 3 { fmt.Println("Provider-wide change (>3 services impacted); not requesting service team reviews") - return + return nil } // For each service team, check if one of the team members is already a reviewer. Rerequest @@ -150,8 +144,9 @@ func execRequestServiceReviewers(prNumber string, gh GithubClient, enrolledTeams exitCode = 1 } if exitCode != 0 { - os.Exit(1) + return fmt.Errorf("exit code = %d", exitCode) } + return nil } func init() { diff --git a/.ci/magician/cmd/test_terraform_vcr.go b/.ci/magician/cmd/test_terraform_vcr.go index 46fe634139f8..9bc4d850e996 100644 --- a/.ci/magician/cmd/test_terraform_vcr.go +++ b/.ci/magician/cmd/test_terraform_vcr.go @@ -44,13 +44,12 @@ var testTerraformVCRCmd = &cobra.Command{ Use: "test-terraform-vcr", Short: "Run vcr tests for affected packages", Long: `This command runs on new pull requests to replay VCR cassettes and re-record failing cassettes.`, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { env := make(map[string]string, len(ttvEnvironmentVariables)) for _, ev := range ttvEnvironmentVariables { 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 } @@ -58,8 +57,7 @@ var testTerraformVCRCmd = &cobra.Command{ 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 } @@ -72,26 +70,24 @@ var testTerraformVCRCmd = &cobra.Command{ 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(env["GOPATH"], "modular-magician", env["GITHUB_TOKEN_DOWNSTREAMS"], rnr) vt, err := vcr.NewTester(env, rnr) if err != nil { - fmt.Println("Error creating VCR tester: ", err) + return fmt.Errorf("error creating VCR tester: %w", err) } if len(args) != 5 { - fmt.Printf("Wrong number of arguments %d, expected 5\n", len(args)) - os.Exit(1) + return fmt.Errorf("wrong number of arguments %d, expected 5", len(args)) } - execTestTerraformVCR(args[0], args[1], args[2], args[3], args[4], baseBranch, gh, rnr, ctlr, vt) + return execTestTerraformVCR(args[0], args[1], args[2], args[3], args[4], baseBranch, gh, rnr, ctlr, vt) }, } -func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, baseBranch string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller, vt *vcr.Tester) { +func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, baseBranch string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller, vt *vcr.Tester) error { newBranch := "auto-pr-" + prNumber oldBranch := newBranch + "-old" @@ -109,49 +105,42 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, for _, repo := range []*source.Repo{tpgRepo, tpgbRepo} { ctlr.SetPath(repo) if err := ctlr.Clone(repo); err != nil { - fmt.Println("Error cloning repo: ", err) - os.Exit(1) + return fmt.Errorf("error cloning repo: %w", err) } if err := ctlr.Fetch(repo, oldBranch); err != nil { - fmt.Println("Failed to fetch old branch: ", err) - os.Exit(1) + return fmt.Errorf("failed to fetch old branch: %w", err) } changedFiles, err := ctlr.DiffNameOnly(repo, oldBranch, newBranch) if err != nil { - fmt.Println("Failed to compute name-only diff: ", err) - os.Exit(1) + return fmt.Errorf("failed to compute name-only diff: %w", err) } repo.ChangedFiles = changedFiles repo.UnifiedZeroDiff, err = ctlr.DiffUnifiedZero(repo, oldBranch, newBranch) if err != nil { - fmt.Println("Failed to compute unified=0 diff: ", err) - os.Exit(1) + return fmt.Errorf("failed to compute unified=0 diff: %w", err) } } vt.SetRepoPath(provider.Beta, tpgbRepo.Path) if err := rnr.PushDir(tpgbRepo.Path); err != nil { - fmt.Println("Error changing to tpgbRepo dir: ", err) - os.Exit(1) + return fmt.Errorf("error changing to tpgbRepo dir: %w", err) } services, runFullVCR := modifiedPackages(tpgbRepo.ChangedFiles) if len(services) == 0 && !runFullVCR { fmt.Println("Skipping tests: No go files or test fixtures changed") - os.Exit(0) + return nil } fmt.Println("Running tests: Go files or test fixtures changed") if err := vt.FetchCassettes(provider.Beta, baseBranch, prNumber); err != nil { - fmt.Println("Error fetching cassettes: ", err) - os.Exit(1) + return fmt.Errorf("error fetching cassettes: %w", err) } buildStatusTargetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", buildID, buildStep, projectID) if err := gh.PostBuildStatus(prNumber, "VCR-test", "pending", buildStatusTargetURL, mmCommitSha); err != nil { - fmt.Println("Error posting pending status: ", err) - os.Exit(1) + return fmt.Errorf("error posting pending status: %w", err) } replayingResult, affectedServicesComment, testDirs, replayingErr := runReplaying(runFullVCR, services, vt) @@ -161,15 +150,13 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, } if err := vt.UploadLogs("ci-vcr-logs", prNumber, buildID, false, false, vcr.Replaying, provider.Beta); err != nil { - fmt.Println("Error uploading replaying logs: ", err) - os.Exit(1) + return fmt.Errorf("error uploading replaying logs: %w", err) } if hasPanics, err := handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha, replayingResult, vcr.Replaying, gh); err != nil { - fmt.Println("Error handling panics: ", err) - os.Exit(1) + return fmt.Errorf("error handling panics: %w", err) } else if hasPanics { - os.Exit(0) + return nil } failedTestsPattern := strings.Join(replayingResult.FailedTests, "|") @@ -222,8 +209,7 @@ Tests were added that are GA-only additions and require manual runs: [Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/docs/getting-started/contributing/#general-contributing-steps)`, len(replayingResult.FailedTests), failedTestsPattern) if err := gh.PostComment(prNumber, comment); err != nil { - fmt.Println("Error posting comment: ", err) - os.Exit(1) + return fmt.Errorf("error posting comment: %w", err) } recordingResult, recordingErr := vt.RunParallel(vcr.Recording, provider.Beta, testDirs, replayingResult.FailedTests) @@ -234,20 +220,17 @@ Tests were added that are GA-only additions and require manual runs: } if err := vt.UploadCassettes("ci-vcr-cassettes", prNumber, provider.Beta); err != nil { - fmt.Println("Error uploading cassettes: ", err) - os.Exit(1) + return fmt.Errorf("error uploading cassettes: %w", err) } if err := vt.UploadLogs("ci-vcr-logs", prNumber, buildID, true, false, vcr.Recording, provider.Beta); err != nil { - fmt.Println("Error uploading recording logs: ", err) - os.Exit(1) + return fmt.Errorf("error uploading recording logs: %w", err) } if hasPanics, err := handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha, recordingResult, vcr.Recording, gh); err != nil { - fmt.Println("Error handling panics: ", err) - os.Exit(1) + return fmt.Errorf("error handling panics: %w", err) } else if hasPanics { - os.Exit(0) + return nil } comment = "" @@ -264,8 +247,7 @@ Tests were added that are GA-only additions and require manual runs: } if err := vt.UploadLogs("ci-vcr-logs", prNumber, buildID, true, true, vcr.Replaying, provider.Beta); err != nil { - fmt.Println("Error uploading recording logs: ", err) - os.Exit(1) + return fmt.Errorf("error uploading recording logs: %w", err) } if len(replayingAfterRecordingResult.FailedTests) > 0 { @@ -321,14 +303,13 @@ Please fix these to complete your PR. If you believe these test failures to be i comment += fmt.Sprintf("View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/build-log/replaying_test.log)", prNumber, buildID) } if err := gh.PostComment(prNumber, comment); err != nil { - fmt.Println("Error posting comment: ", err) - os.Exit(1) + return fmt.Errorf("error posting comment: %w", err) } if err := gh.PostBuildStatus(prNumber, "VCR-test", testState, buildStatusTargetURL, mmCommitSha); err != nil { - fmt.Println("Error posting build status: ", err) - os.Exit(1) + return fmt.Errorf("error posting build status: %w", err) } + return nil } var addedTestsRegexp = regexp.MustCompile(`(?m)^\+func (Test\w+)\(t \*testing.T\) {`) diff --git a/.ci/magician/cmd/test_tgc.go b/.ci/magician/cmd/test_tgc.go index 5f000d731b15..bbf746a975a2 100644 --- a/.ci/magician/cmd/test_tgc.go +++ b/.ci/magician/cmd/test_tgc.go @@ -32,31 +32,30 @@ var testTGCCmd = &cobra.Command{ 1. COMMIT_SHA 2. PR_NUMBER `, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { commit := os.Getenv("COMMIT_SHA") pr := os.Getenv("PR_NUMBER") 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) - execTestTGC(commit, pr, gh) + return execTestTGC(commit, pr, gh) }, } -func execTestTGC(commit, pr string, gh ttGithub) { +func execTestTGC(commit, pr string, gh ttGithub) error { if err := gh.CreateWorkflowDispatchEvent("test-tgc.yml", map[string]any{ "owner": "modular-magician", "repo": "terraform-google-conversion", "branch": "auto-pr-" + pr, "sha": commit, }); err != nil { - fmt.Printf("Error creating workflow dispatch event: %v\n", err) - os.Exit(1) + return fmt.Errorf("error creating workflow dispatch event: %w", err) } + return nil } func init() { diff --git a/.ci/magician/cmd/test_tgc_integration.go b/.ci/magician/cmd/test_tgc_integration.go index 04881ff62d23..f2d1cd334b34 100644 --- a/.ci/magician/cmd/test_tgc_integration.go +++ b/.ci/magician/cmd/test_tgc_integration.go @@ -20,22 +20,20 @@ var testTGCIntegrationCmd = &cobra.Command{ 1. GOPATH 2. GITHUB_TOKEN_MAGIC_MODULES `, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { goPath, ok := os.LookupEnv("GOPATH") if !ok { - fmt.Println("Did not provide GOPATH environment variable") - os.Exit(1) + return fmt.Errorf("did not provide GOPATH environment variable") } 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") } rnr, err := exec.NewRunner() if err != nil { - fmt.Println("Error creating runner: ", err) + return fmt.Errorf("error creating runner: %w", err) os.Exit(1) } @@ -43,11 +41,11 @@ var testTGCIntegrationCmd = &cobra.Command{ gh := github.NewClient(githubToken) - execTestTGCIntegration(args[0], args[1], args[2], args[3], args[4], args[5], "modular-magician", rnr, ctlr, gh) + return execTestTGCIntegration(args[0], args[1], args[2], args[3], args[4], args[5], "modular-magician", rnr, ctlr, gh) }, } -func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, ghRepo, githubUsername string, rnr ExecRunner, ctlr *source.Controller, gh GithubClient) { +func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, ghRepo, githubUsername string, rnr ExecRunner, ctlr *source.Controller, gh GithubClient) error { newBranch := "auto-pr-" + prNumber repo := &source.Repo{ Name: ghRepo, @@ -55,17 +53,14 @@ func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, g } ctlr.SetPath(repo) if err := ctlr.Clone(repo); err != nil { - fmt.Println("Error cloning repo: ", err) - os.Exit(1) + return fmt.Errorf("error cloning repo: %w", err) } if err := rnr.PushDir(repo.Path); err != nil { - fmt.Println("Error changing to repo dir: ", err) - os.Exit(1) + return fmt.Errorf("error changing to repo dir: %w", err) } diffs, err := rnr.Run("git", []string{"diff", "--name-only", "HEAD~1"}, nil) if err != nil { - fmt.Println("Error diffing repo: ", err) - os.Exit(1) + return fmt.Errorf("error diffing repo: %w", err) } hasGoFiles := false for _, diff := range strings.Split(diffs, "\n") { @@ -76,15 +71,14 @@ func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, g } if !hasGoFiles { fmt.Println("Skipping tests: No go files changed") - os.Exit(0) + return nil } fmt.Println("Running tests: Go files changed") targetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", buildID, buildStep, projectID) if err := gh.PostBuildStatus(prNumber, ghRepo+"-test-integration", "pending", targetURL, mmCommit); err != nil { - fmt.Println("Error posting build status: ", err) - os.Exit(1) + return fmt.Errorf("error posting build status: %w", err) } if _, err := rnr.Run("go", []string{"mod", "edit", "-replace", fmt.Sprintf("github.com/hashicorp/terraform-provider-google-beta=github.com/%s/terraform-provider-google-beta@%s", githubUsername, newBranch)}, nil); err != nil { @@ -104,9 +98,9 @@ func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, g } if err := gh.PostBuildStatus(prNumber, ghRepo+"-test-integration", state, targetURL, mmCommit); err != nil { - fmt.Println("Error posting build status: ", err) - os.Exit(1) + return fmt.Errorf("error posting build status: %w", err) } + return nil } func init() { diff --git a/.ci/magician/cmd/test_tpg.go b/.ci/magician/cmd/test_tpg.go index 260b5b2a7466..181644dbfcf9 100644 --- a/.ci/magician/cmd/test_tpg.go +++ b/.ci/magician/cmd/test_tpg.go @@ -37,31 +37,29 @@ var testTPGCmd = &cobra.Command{ 2. COMMIT_SHA 3. PR_NUMBER `, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { version := os.Getenv("VERSION") commit := os.Getenv("COMMIT_SHA") pr := os.Getenv("PR_NUMBER") 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) - execTestTPG(version, commit, pr, gh) + return execTestTPG(version, commit, pr, gh) }, } -func execTestTPG(version, commit, pr string, gh ttGithub) { +func execTestTPG(version, commit, pr string, gh ttGithub) error { var repo string if version == "ga" { repo = "terraform-provider-google" } else if version == "beta" { repo = "terraform-provider-google-beta" } else { - fmt.Println("invalid version specified") - os.Exit(1) + return fmt.Errorf("invalid version specified") } if err := gh.CreateWorkflowDispatchEvent("test-tpg.yml", map[string]any{ @@ -70,9 +68,9 @@ func execTestTPG(version, commit, pr string, gh ttGithub) { "branch": "auto-pr-" + pr, "sha": commit, }); err != nil { - fmt.Printf("Error creating workflow dispatch event: %v\n", err) - os.Exit(1) + return fmt.Errorf("error creating workflow dispatch event: %w", err) } + return nil } func init() {