From f993106e496a2c8531448016600e15a7dc306109 Mon Sep 17 00:00:00 2001 From: mrinalirao Date: Thu, 1 Dec 2022 08:42:25 +1100 Subject: [PATCH 1/4] fix logic in MultiErrors append func --- internal/cloud/errors.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/cloud/errors.go b/internal/cloud/errors.go index 339a6ea01f..193db30261 100644 --- a/internal/cloud/errors.go +++ b/internal/cloud/errors.go @@ -71,8 +71,9 @@ func (e *multiErrors) Append(errs ...error) { } if errs, ok := err.(multiErrors); ok { *e = append(*e, errs...) + } else { + *e = append(*e, err) } - *e = append(*e, err) } } From 5c5b1099c870d9ce5b5531e03eb4a31dd34ff26b Mon Sep 17 00:00:00 2001 From: mrinalirao Date: Fri, 2 Dec 2022 09:09:51 +1100 Subject: [PATCH 2/4] refactor runTaskStage --- internal/cloud/backend_taskStages.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/internal/cloud/backend_taskStages.go b/internal/cloud/backend_taskStages.go index b465af4113..eba4f15de3 100644 --- a/internal/cloud/backend_taskStages.go +++ b/internal/cloud/backend_taskStages.go @@ -84,21 +84,23 @@ func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWr case tfe.TaskStageRunning, tfe.TaskStagePassed, tfe.TaskStageCanceled, tfe.TaskStageErrored, tfe.TaskStageFailed: for _, s := range summarizers { cont, msg, err := s.Summarize(ctx, output, stage) - if cont { - if msg != nil { - if i%4 == 0 { - if i > 0 { - output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation - } - } - } - return true, nil - } if err != nil { errs.Append(err) + break } + if !cont { + continue + } + // cont is true and we must continue to poll + if msg != nil { + // print msg every 4 seconds + if i%4 == 0 && i > 0 { + output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation + } + } + return true, nil } - case "unreachable": + case tfe.TaskStageUnreachable: return false, nil default: return false, fmt.Errorf("Invalid Task stage status: %s ", stage.Status) From 15288caf644d2c95a401cb5008edc583ffaca98b Mon Sep 17 00:00:00 2001 From: mrinalirao Date: Fri, 2 Dec 2022 10:15:51 +1100 Subject: [PATCH 3/4] Code Improvements: - Use tfe consts instead of hardcoded values - fix logic when polling taskStage - remove inaccurate comment --- internal/cloud/backend_taskStage_taskResults.go | 8 ++++---- internal/cloud/backend_taskStages.go | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/cloud/backend_taskStage_taskResults.go b/internal/cloud/backend_taskStage_taskResults.go index 96d085df5a..0f96889b69 100644 --- a/internal/cloud/backend_taskStage_taskResults.go +++ b/internal/cloud/backend_taskStage_taskResults.go @@ -62,18 +62,18 @@ func (trs *taskResultSummarizer) Summarize(context *IntegrationContext, output I func summarizeTaskResults(taskResults []*tfe.TaskResult) *taskResultSummary { var pendingCount, errCount, errMandatoryCount, passedCount int for _, task := range taskResults { - if task.Status == "unreachable" { + if task.Status == tfe.TaskUnreachable { return &taskResultSummary{ unreachable: true, } - } else if task.Status == "running" || task.Status == "pending" { + } else if task.Status == tfe.TaskRunning || task.Status == tfe.TaskPending { pendingCount++ - } else if task.Status == "passed" { + } else if task.Status == tfe.TaskPassed { passedCount++ } else { // Everything else is a failure errCount++ - if task.WorkspaceTaskEnforcementLevel == "mandatory" { + if task.WorkspaceTaskEnforcementLevel == tfe.Mandatory { errMandatoryCount++ } } diff --git a/internal/cloud/backend_taskStages.go b/internal/cloud/backend_taskStages.go index eba4f15de3..c623d72bb5 100644 --- a/internal/cloud/backend_taskStages.go +++ b/internal/cloud/backend_taskStages.go @@ -89,11 +89,10 @@ func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWr break } if !cont { - continue + return false, nil } // cont is true and we must continue to poll if msg != nil { - // print msg every 4 seconds if i%4 == 0 && i > 0 { output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation } From a5add7e361fda5019f9ec8dcc1ddb3952e313256 Mon Sep 17 00:00:00 2001 From: mrinalirao Date: Fri, 2 Dec 2022 12:38:55 +1100 Subject: [PATCH 4/4] modify Poll func to pass in backoff interval --- .../cloud/backend_taskStage_taskResults_test.go | 2 +- internal/cloud/backend_taskStages.go | 15 ++++++++++----- internal/cloud/cloud_integration.go | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/internal/cloud/backend_taskStage_taskResults_test.go b/internal/cloud/backend_taskStage_taskResults_test.go index 7e14a15282..d6bc731b34 100644 --- a/internal/cloud/backend_taskStage_taskResults_test.go +++ b/internal/cloud/backend_taskStage_taskResults_test.go @@ -151,7 +151,7 @@ func TestCloud_runTasksWithTaskResults(t *testing.T) { trs := taskResultSummarizer{ cloud: b, } - c.context.Poll(func(i int) (bool, error) { + c.context.Poll(taskStageBackoffMin, taskStageBackoffMax, func(i int) (bool, error) { cont, _, _ := trs.Summarize(c.context, c.writer, c.taskStage()) if cont { return true, nil diff --git a/internal/cloud/backend_taskStages.go b/internal/cloud/backend_taskStages.go index c623d72bb5..719975d7c7 100644 --- a/internal/cloud/backend_taskStages.go +++ b/internal/cloud/backend_taskStages.go @@ -8,6 +8,11 @@ import ( tfe "github.com/hashicorp/go-tfe" ) +const ( + taskStageBackoffMin = 4000.0 + taskStageBackoffMax = 12000.0 +) + type taskStages map[tfe.Stage]*tfe.TaskStage type taskStageSummarizer interface { @@ -67,7 +72,7 @@ func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWr summarizers = append(summarizers, s) } - return ctx.Poll(func(i int) (bool, error) { + return ctx.Poll(taskStageBackoffMin, taskStageBackoffMax, func(i int) (bool, error) { options := tfe.TaskStageReadOptions{ Include: []tfe.TaskStageIncludeOpt{tfe.TaskStageTaskResults}, } @@ -88,14 +93,14 @@ func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWr errs.Append(err) break } + if !cont { - return false, nil + continue } + // cont is true and we must continue to poll if msg != nil { - if i%4 == 0 && i > 0 { - output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation - } + output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation } return true, nil } diff --git a/internal/cloud/cloud_integration.go b/internal/cloud/cloud_integration.go index 047fc57093..cd1c6be96a 100644 --- a/internal/cloud/cloud_integration.go +++ b/internal/cloud/cloud_integration.go @@ -38,14 +38,14 @@ type integrationCLIOutput struct { var _ IntegrationOutputWriter = (*integrationCLIOutput)(nil) // Compile time check -func (s *IntegrationContext) Poll(every func(i int) (bool, error)) error { +func (s *IntegrationContext) Poll(backoffMinInterval float64, backoffMaxInterval float64, every func(i int) (bool, error)) error { for i := 0; ; i++ { select { case <-s.StopContext.Done(): return s.StopContext.Err() case <-s.CancelContext.Done(): return s.CancelContext.Err() - case <-time.After(backoff(backoffMin, backoffMax, i)): + case <-time.After(backoff(backoffMinInterval, backoffMaxInterval, i)): // blocks for a time between min and max }