Skip to content

Commit ce44d5b

Browse files
taraspostaraspos
committed
PR feedback, part 2
1 parent 42c033c commit ce44d5b

File tree

3 files changed

+81
-43
lines changed

3 files changed

+81
-43
lines changed

‎tools/amplify-preview/amplify.go

+39-18
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"log/slog"
2324
"strconv"
2425
"strings"
2526
"sync"
@@ -32,13 +33,9 @@ import (
3233
)
3334

3435
var (
35-
errBranchNotFound = errors.New("branch not found")
36-
errNoJobForBranch = errors.New("current branch has no jobs")
37-
amplifyJobCompletedStatuses = map[types.JobStatus]struct{}{
38-
types.JobStatusFailed: {},
39-
types.JobStatusCancelled: {},
40-
types.JobStatusSucceed: {},
41-
}
36+
errBranchNotFound = errors.New("branch not found")
37+
errNoJobForBranch = errors.New("current branch has no jobs")
38+
errNilBranch = errors.New("branch is nil")
4239
)
4340

4441
const (
@@ -50,6 +47,10 @@ const (
5047
amplifyDefaultDomain = "amplifyapp.com"
5148
)
5249

50+
// AmplifyPreview is used to get/create amplify preview
51+
// deployments on AWS Amplify across multiple apps
52+
// to work around hard limit 50 branches per app
53+
// https://docs.aws.amazon.com/amplify/latest/userguide/quotas-chapter.html
5354
type AmplifyPreview struct {
5455
appIDs []string
5556
branchName string
@@ -97,7 +98,7 @@ func (amp *AmplifyPreview) FindExistingBranch(ctx context.Context) (*types.Branc
9798
for resp := range resultCh {
9899
var errNotFound *types.NotFoundException
99100
if errors.As(resp.err, &errNotFound) {
100-
logger.Debug("Branch not found", logKeyAppID, resp.appID, logKeyBranchName, amp.branchName)
101+
slog.Debug("Branch not found", logKeyAppID, resp.appID, logKeyBranchName, amp.branchName)
101102
continue
102103
} else if resp.err != nil {
103104
failedResp.perAppErr[resp.appID] = resp.err
@@ -126,20 +127,20 @@ func (amp *AmplifyPreview) CreateBranch(ctx context.Context) (*types.Branch, err
126127
resp, err := amp.client.CreateBranch(ctx, &amplify.CreateBranchInput{
127128
AppId: aws.String(appID),
128129
BranchName: aws.String(amp.branchName),
129-
Description: aws.String("Branch generated for PR TODO"),
130+
Description: aws.String("Branch created from 'amplify-preview' GHA action"),
130131
Stage: types.StagePullRequest,
131132
EnableAutoBuild: aws.Bool(true),
132133
})
133134

134135
var errLimitExceeded *types.LimitExceededException
135136
if errors.As(err, &errLimitExceeded) {
136-
logger.Debug("Reached branches limit", logKeyAppID, appID)
137+
slog.Debug("Reached branches limit", logKeyAppID, appID)
137138
} else if err != nil {
138139
failedResp.perAppErr[appID] = err
139140
}
140141

141142
if resp != nil {
142-
logger.Info("Successfully created branch", logKeyAppID, appID, logKeyBranchName, *resp.Branch.BranchName, logKeyJobID, resp.Branch.ActiveJobId)
143+
slog.Info("Successfully created branch", logKeyAppID, appID, logKeyBranchName, *resp.Branch.BranchName, logKeyJobID, resp.Branch.ActiveJobId)
143144
return resp.Branch, nil
144145
}
145146
}
@@ -148,6 +149,9 @@ func (amp *AmplifyPreview) CreateBranch(ctx context.Context) (*types.Branch, err
148149
}
149150

150151
func (amp *AmplifyPreview) StartJob(ctx context.Context, branch *types.Branch) (*types.JobSummary, error) {
152+
if branch == nil {
153+
return nil, errNilBranch
154+
}
151155
appID, err := appIDFromBranchARN(*branch.BranchArn)
152156
if err != nil {
153157
return nil, err
@@ -164,13 +168,16 @@ func (amp *AmplifyPreview) StartJob(ctx context.Context, branch *types.Branch) (
164168
return nil, err
165169
}
166170

167-
logger.Info("Successfully started job", logKeyAppID, appID, logKeyBranchName, *branch.BranchName, logKeyJobID, *resp.JobSummary.JobId)
171+
slog.Info("Successfully started job", logKeyAppID, appID, logKeyBranchName, *branch.BranchName, logKeyJobID, *resp.JobSummary.JobId)
168172

169173
return resp.JobSummary, nil
170174

171175
}
172176

173177
func (amp *AmplifyPreview) findJobsByID(ctx context.Context, branch *types.Branch, includeLatest bool, ids ...string) (jobSummaries []types.JobSummary, err error) {
178+
if branch == nil {
179+
return nil, errNilBranch
180+
}
174181
appID, err := appIDFromBranchARN(*branch.BranchArn)
175182
if err != nil {
176183
return nil, err
@@ -193,9 +200,15 @@ func (amp *AmplifyPreview) findJobsByID(ctx context.Context, branch *types.Branc
193200
}
194201

195202
for _, id := range ids {
196-
wantJobID, _ := strconv.Atoi(id)
203+
wantJobID, err := strconv.Atoi(id)
204+
if err != nil {
205+
slog.Debug("unexpected Job ID value", logKeyJobID, wantJobID)
206+
}
197207
for _, j := range resp.JobSummaries {
198-
jobID, _ := strconv.Atoi(*j.JobId)
208+
jobID, err := strconv.Atoi(*j.JobId)
209+
if err != nil {
210+
slog.Debug("unexpected Job ID value", logKeyJobID, jobID)
211+
}
199212
if jobID == wantJobID {
200213
jobSummaries = append(jobSummaries, j)
201214
break
@@ -208,6 +221,10 @@ func (amp *AmplifyPreview) findJobsByID(ctx context.Context, branch *types.Branc
208221
}
209222

210223
func (amp *AmplifyPreview) GetLatestAndActiveJobs(ctx context.Context, branch *types.Branch) (latestJob, activeJob *types.JobSummary, err error) {
224+
if branch == nil {
225+
return nil, nil, errNilBranch
226+
}
227+
211228
var jobIDs []string
212229
if branch.ActiveJobId != nil {
213230
jobIDs = append(jobIDs, *branch.ActiveJobId)
@@ -228,7 +245,7 @@ func (amp *AmplifyPreview) GetLatestAndActiveJobs(ctx context.Context, branch *t
228245
}
229246

230247
func (amp *AmplifyPreview) WaitForJobCompletion(ctx context.Context, branch *types.Branch, job *types.JobSummary) (currentJob, activeJob *types.JobSummary, err error) {
231-
for i := 0; i < jobWaitTimeAttempts; i++ {
248+
for i := range jobWaitTimeAttempts {
232249
jobSummaries, err := amp.findJobsByID(ctx, branch, true, *job.JobId)
233250
if err != nil {
234251
return nil, nil, err
@@ -243,7 +260,7 @@ func (amp *AmplifyPreview) WaitForJobCompletion(ctx context.Context, branch *typ
243260
break
244261
}
245262

246-
logger.Info("Job is not in a completed state yet. Sleeping...",
263+
slog.Info(fmt.Sprintf("Job is not in a completed state yet. Sleeping for %s", jobWaitSleepTime.String()),
247264
logKeyBranchName, amp.branchName, "job_status", currentJob.Status, "job_id", *currentJob.JobId, "attempts_left", jobWaitTimeAttempts-i)
248265
time.Sleep(jobWaitSleepTime)
249266
}
@@ -265,8 +282,12 @@ func appIDFromBranchARN(branchArn string) (string, error) {
265282
}
266283

267284
func isAmplifyJobCompleted(job *types.JobSummary) bool {
268-
_, ok := amplifyJobCompletedStatuses[job.Status]
269-
return ok
285+
switch job.Status {
286+
case types.JobStatusFailed, types.JobStatusCancelled, types.JobStatusSucceed:
287+
return true
288+
default:
289+
return false
290+
}
270291
}
271292

272293
func (err aggregatedError) Error() error {

‎tools/amplify-preview/github.go

+8-9
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,18 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23-
"log"
2423
"os"
2524
"strconv"
2625
"strings"
2726

2827
"github.com/gravitational/shared-workflows/libs/github"
2928
)
3029

31-
const (
32-
prRefNameSuffix = "/merge"
33-
)
34-
3530
func postPreviewURL(ctx context.Context, commentBody string) error {
31+
const prRefNameSuffix = "/merge"
3632
refName := os.Getenv("GITHUB_REF_NAME")
3733
githubRepository := os.Getenv("GITHUB_REPOSITORY")
3834
if !strings.HasSuffix(refName, prRefNameSuffix) {
39-
return nil
4035
}
4136

4237
gh, err := github.NewClientFromGHAuth(ctx)
@@ -46,17 +41,21 @@ func postPreviewURL(ctx context.Context, commentBody string) error {
4641

4742
prID, err := strconv.Atoi(strings.TrimSuffix(refName, "/merge"))
4843
if err != nil {
49-
log.Fatalf("Failed to extract PR ID from GITHUB_REF_NAME=%s: %s", refName, err)
44+
return fmt.Errorf("Failed to extract PR ID from GITHUB_REF_NAME=%s: %s", refName, err)
5045
}
5146

5247
targetComment := github.CommentTraits{
5348
BodyContains: amplifyMarkdownHeader,
5449
}
5550

51+
githubRepoParts := strings.Split(githubRepository, "/")
52+
if len(githubRepoParts) < 2 {
53+
return fmt.Errorf("Couldn't extract repo and owner from %q", githubRepository)
54+
}
5655
currentPR := github.IssueIdentifier{
5756
Number: prID,
58-
Owner: strings.Split(githubRepository, "/")[0],
59-
Repo: strings.Split(githubRepository, "/")[1],
57+
Owner: githubRepoParts[0],
58+
Repo: githubRepoParts[1],
6059
}
6160

6261
comment, err := gh.FindCommentByTraits(ctx, currentPR, targetComment)

‎tools/amplify-preview/main.go

+34-16
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
"log/slog"
2424
"os"
25+
"os/signal"
2526
"strings"
2627
"time"
2728

@@ -34,8 +35,6 @@ import (
3435
)
3536

3637
var (
37-
logger = slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug}))
38-
3938
amplifyAppIDs = kingpin.Flag("amplify-app-ids", "List of Amplify App IDs").Envar("AMPLIFY_APP_IDS").Required().Strings()
4039
gitBranchName = kingpin.Flag("git-branch-name", "Git branch name").Envar("GIT_BRANCH_NAME").Required().String()
4140
createBranches = kingpin.Flag("create-branches",
@@ -51,19 +50,26 @@ const (
5150

5251
func main() {
5352
kingpin.Parse()
54-
ctx := context.Background()
53+
slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug})))
54+
ctx, cancel := handleInterruption(context.Background())
55+
defer cancel()
56+
57+
if err := run(ctx); err != nil {
58+
slog.Error("run failed", "error", err)
59+
os.Exit(1)
60+
}
61+
}
5562

63+
func run(ctx context.Context) error {
5664
cfg, err := config.LoadDefaultConfig(ctx, config.WithRetryer(func() aws.Retryer {
5765
return retry.AddWithMaxAttempts(retry.NewStandard(), 10)
5866
}))
5967
if err != nil {
60-
logger.Error("failed to load configuration", "error", err)
61-
os.Exit(1)
68+
return fmt.Errorf("failed to load AWS configuration: %w", err)
6269
}
6370

6471
if amplifyAppIDs == nil || len(*amplifyAppIDs) == 0 {
65-
logger.Error("expected one more more Amplify App IDs")
66-
os.Exit(1)
72+
return errors.New("expected one more more Amplify App IDs")
6773
}
6874

6975
amp := AmplifyPreview{
@@ -78,13 +84,6 @@ func main() {
7884
amp.appIDs = strings.Split(amp.appIDs[0], ",")
7985
}
8086

81-
if err := execute(ctx, amp); err != nil {
82-
logger.Error("execution failed", "error", err)
83-
os.Exit(1)
84-
}
85-
}
86-
87-
func execute(ctx context.Context, amp AmplifyPreview) error {
8887
branch, err := ensureAmplifyBranch(ctx, amp)
8988
if err != nil {
9089
return err
@@ -99,7 +98,7 @@ func execute(ctx context.Context, amp AmplifyPreview) error {
9998
return fmt.Errorf("failed to post preview URL: %w", err)
10099
}
101100

102-
logger.Info("Successfully posted PR comment")
101+
slog.Info("Successfully posted PR comment")
103102

104103
if *wait {
105104
currentJob, activeJob, err = amp.WaitForJobCompletion(ctx, branch, currentJob)
@@ -114,7 +113,7 @@ func execute(ctx context.Context, amp AmplifyPreview) error {
114113
}
115114

116115
if currentJob.Status == types.JobStatusFailed {
117-
logger.Error("amplify job is in failed state", logKeyBranchName, amp.branchName, "job_status", currentJob.Status, "job_id", *currentJob.JobId)
116+
slog.Error("amplify job is in failed state", logKeyBranchName, amp.branchName, "job_status", currentJob.Status, "job_id", *currentJob.JobId)
118117
return fmt.Errorf("amplify job is in %q state", currentJob.Status)
119118
}
120119

@@ -148,3 +147,22 @@ func ensureAmplifyDeployment(ctx context.Context, amp AmplifyPreview, branch *ty
148147
return nil, nil, fmt.Errorf("failed to lookup amplify job for branch %q: %w", amp.branchName, err)
149148
}
150149
}
150+
151+
func handleInterruption(ctx context.Context) (context.Context, context.CancelFunc) {
152+
ctx, cancel := context.WithCancel(ctx)
153+
c := make(chan os.Signal, 1)
154+
signal.Notify(c, os.Interrupt)
155+
156+
go func() {
157+
select {
158+
case <-c:
159+
cancel()
160+
case <-ctx.Done():
161+
}
162+
}()
163+
164+
return ctx, func() {
165+
signal.Stop(c)
166+
cancel()
167+
}
168+
}

0 commit comments

Comments
 (0)