Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow configuration of retry attempts for Promotion steps #2940

Merged
merged 8 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
709 changes: 467 additions & 242 deletions api/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions api/v1alpha1/generated.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions api/v1alpha1/promotion_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,27 @@ type PromotionVariable struct {
Value string `json:"value" protobuf:"bytes,2,opt,name=value"`
}

// PromotionStepRetry describes the retry policy for a PromotionStep.
type PromotionStepRetry struct {
// Attempts is the number of times the step can be attempted before the
// PromotionStep is marked as failed.
//
// If this field is set to 1, the step will not be retried. If this
// field is set to -1, the step will be retried indefinitely.
//
// The default of this field depends on the step being executed. Refer to
// the documentation for the specific step for more information.
Attempts int64 `json:"attempts,omitempty" protobuf:"varint,1,opt,name=attempts"`
}

// GetAttempts returns the Attempts field with the given fallback value.
func (r *PromotionStepRetry) GetAttempts(fallback int64) int64 {
if r == nil || r.Attempts == 0 {
return fallback
}
return r.Attempts
}

// PromotionStep describes a directive to be executed as part of a Promotion.
type PromotionStep struct {
// Uses identifies a runner that can execute this step.
Expand All @@ -117,6 +138,8 @@ type PromotionStep struct {
Uses string `json:"uses" protobuf:"bytes,1,opt,name=uses"`
// As is the alias this step can be referred to as.
As string `json:"as,omitempty" protobuf:"bytes,2,opt,name=as"`
// Retry is the retry policy for this step.
Retry *PromotionStepRetry `json:"retry,omitempty" protobuf:"bytes,4,opt,name=retry"`
// Config is opaque configuration for the PromotionStep that is understood
// only by each PromotionStep's implementation. It is legal to utilize
// expressions in defining values at any level of this block.
Expand Down Expand Up @@ -154,6 +177,9 @@ type PromotionStatus struct {
// permits steps that have already run successfully to be skipped on
// subsequent reconciliations attempts.
CurrentStep int64 `json:"currentStep,omitempty" protobuf:"varint,9,opt,name=currentStep"`
// CurrentStepAttempt is the number of times the current step has been
// attempted.
CurrentStepAttempt int64 `json:"currentStepAttempt,omitempty" protobuf:"varint,11,opt,name=currentStepAttempt"`
// State stores the state of the promotion process between reconciliation
// attempts.
State *apiextensionsv1.JSON `json:"state,omitempty" protobuf:"bytes,10,opt,name=state"`
Expand Down
39 changes: 39 additions & 0 deletions api/v1alpha1/promotion_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package v1alpha1

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestPromotionRetry_GetAttempts(t *testing.T) {
tests := []struct {
name string
retry *PromotionStepRetry
fallback int64
want int64
}{
{
name: "retry is nil",
retry: nil,
fallback: 1,
want: 1,
},
{
name: "attempts is not set",
retry: &PromotionStepRetry{},
fallback: -1,
want: -1,
},
{
name: "attempts is set",
retry: &PromotionStepRetry{
Attempts: 3,
},
want: 3,
},
}
for _, tt := range tests {
require.Equal(t, tt.want, tt.retry.GetAttempts(tt.fallback))
}
}
20 changes: 20 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions charts/kargo/resources/crds/kargo.akuity.io_promotions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,22 @@ spec:
expressions in defining values at any level of this block.
See https://docs.kargo.io/references/expression-language for details.
x-kubernetes-preserve-unknown-fields: true
retry:
description: Retry is the retry policy for this step.
properties:
attempts:
description: |-
Attempts is the number of times the step can be attempted before the
PromotionStep is marked as failed.

If this field is set to 1, the step will not be retried. If this
field is set to -1, the step will be retried indefinitely.

The default of this field depends on the step being executed. Refer to
the documentation for the specific step for more information.
format: int64
type: integer
type: object
uses:
description: Uses identifies a runner that can execute this
step.
Expand Down Expand Up @@ -145,6 +161,12 @@ spec:
subsequent reconciliations attempts.
format: int64
type: integer
currentStepAttempt:
description: |-
CurrentStepAttempt is the number of times the current step has been
attempted.
format: int64
type: integer
finishedAt:
description: FinishedAt is the time when the promotion was completed.
format: date-time
Expand Down
28 changes: 28 additions & 0 deletions charts/kargo/resources/crds/kargo.akuity.io_stages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ spec:
expressions in defining values at any level of this block.
See https://docs.kargo.io/references/expression-language for details.
x-kubernetes-preserve-unknown-fields: true
retry:
description: Retry is the retry policy for this step.
properties:
attempts:
description: |-
Attempts is the number of times the step can be attempted before the
PromotionStep is marked as failed.

If this field is set to 1, the step will not be retried. If this
field is set to -1, the step will be retried indefinitely.

The default of this field depends on the step being executed. Refer to
the documentation for the specific step for more information.
format: int64
type: integer
type: object
uses:
description: Uses identifies a runner that can execute
this step.
Expand Down Expand Up @@ -469,6 +485,12 @@ spec:
subsequent reconciliations attempts.
format: int64
type: integer
currentStepAttempt:
description: |-
CurrentStepAttempt is the number of times the current step has been
attempted.
format: int64
type: integer
finishedAt:
description: FinishedAt is the time when the promotion was
completed.
Expand Down Expand Up @@ -1290,6 +1312,12 @@ spec:
subsequent reconciliations attempts.
format: int64
type: integer
currentStepAttempt:
description: |-
CurrentStepAttempt is the number of times the current step has been
attempted.
format: int64
type: integer
finishedAt:
description: FinishedAt is the time when the promotion was
completed.
Expand Down
4 changes: 4 additions & 0 deletions internal/controller/promotions/promotions.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@
steps[i] = directives.PromotionStep{
Kind: step.Uses,
Alias: step.As,
Retry: step.Retry,

Check warning on line 471 in internal/controller/promotions/promotions.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/promotions/promotions.go#L471

Added line #L471 was not covered by tests
Config: step.Config.Raw,
}
}
Expand All @@ -481,6 +482,7 @@
FreightRequests: stage.Spec.RequestedFreight,
Freight: *workingPromo.Status.FreightCollection.DeepCopy(),
StartFromStep: promo.Status.CurrentStep,
Attempts: promo.Status.CurrentStepAttempt,

Check warning on line 485 in internal/controller/promotions/promotions.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/promotions/promotions.go#L485

Added line #L485 was not covered by tests
State: directives.State(workingPromo.Status.GetState()),
Vars: workingPromo.Spec.Vars,
}
Expand All @@ -490,6 +492,7 @@
// allows individual steps to self-discover that they've run before and
// examine the results of their own previous execution.
promoCtx.StartFromStep = 0
promoCtx.Attempts = 0

Check warning on line 495 in internal/controller/promotions/promotions.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/promotions/promotions.go#L495

Added line #L495 was not covered by tests
} else if !os.IsExist(err) {
return nil, fmt.Errorf("error creating working directory: %w", err)
}
Expand All @@ -505,6 +508,7 @@
workingPromo.Status.Phase = res.Status
workingPromo.Status.Message = res.Message
workingPromo.Status.CurrentStep = res.CurrentStep
workingPromo.Status.CurrentStepAttempt = res.Attempt

Check warning on line 511 in internal/controller/promotions/promotions.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/promotions/promotions.go#L511

Added line #L511 was not covered by tests
workingPromo.Status.State = &apiextensionsv1.JSON{Raw: res.State.ToJSON()}
if res.Status == kargoapi.PromotionPhaseSucceeded {
var healthChecks []kargoapi.HealthCheckStep
Expand Down
2 changes: 1 addition & 1 deletion internal/directives/argocd_revisions.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
return "",
fmt.Errorf("output from step with alias %q is not a map[string]any", stepAlias)
}
commitAny, exists := stepOutputMap[commitKey]
commitAny, exists := stepOutputMap[stateKeyCommit]

Check warning on line 180 in internal/directives/argocd_revisions.go

View check run for this annotation

Codecov / codecov/patch

internal/directives/argocd_revisions.go#L180

Added line #L180 was not covered by tests
if !exists {
return "",
fmt.Errorf("no commit found in output from step with alias %q", stepAlias)
Expand Down
5 changes: 5 additions & 0 deletions internal/directives/argocd_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@
return "argocd-update"
}

// DefaultAttempts implements the RetryableStepRunner interface.
func (a *argocdUpdater) DefaultAttempts() int64 {
return -1

Check warning on line 132 in internal/directives/argocd_updater.go

View check run for this annotation

Codecov / codecov/patch

internal/directives/argocd_updater.go#L131-L132

Added lines #L131 - L132 were not covered by tests
}

// RunPromotionStep implements the PromotionStepRunner interface.
func (a *argocdUpdater) RunPromotionStep(
ctx context.Context,
Expand Down
5 changes: 3 additions & 2 deletions internal/directives/git_commiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (
"github.com/akuity/kargo/internal/controller/git"
)

const commitKey = "commit"
// stateKeyCommit is the key used to store the commit ID in the shared State.
const stateKeyCommit = "commit"

func init() {
builtins.RegisterPromotionStepRunner(newGitCommitter(), nil)
Expand Down Expand Up @@ -111,7 +112,7 @@ func (g *gitCommitter) runPromotionStep(
}
return PromotionStepResult{
Status: kargoapi.PromotionPhaseSucceeded,
Output: map[string]any{commitKey: commitID},
Output: map[string]any{stateKeyCommit: commitID},
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion internal/directives/git_commiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func Test_gitCommitter_runPromotionStep(t *testing.T) {
require.Equal(t, kargoapi.PromotionPhaseSucceeded, res.Status)
expectedCommit, err := workTree.LastCommitID()
require.NoError(t, err)
actualCommit, ok := res.Output[commitKey]
actualCommit, ok := res.Output[stateKeyCommit]
require.True(t, ok)
require.Equal(t, expectedCommit, actualCommit)
lastCommitMsg, err := workTree.CommitMessage("HEAD")
Expand Down
13 changes: 7 additions & 6 deletions internal/directives/git_pr_opener.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
_ "github.com/akuity/kargo/internal/gitprovider/gitlab" // GitLab provider registration
)

const prNumberKey = "prNumber"
// stateKeyPRNumber is the key used to store the PR number in the shared State.
const stateKeyPRNumber = "prNumber"

func init() {
builtins.RegisterPromotionStepRunner(
Expand Down Expand Up @@ -83,7 +84,7 @@
return PromotionStepResult{
Status: kargoapi.PromotionPhaseSucceeded,
Output: map[string]any{
prNumberKey: prNumber,
stateKeyPRNumber: prNumber,

Check warning on line 87 in internal/directives/git_pr_opener.go

View check run for this annotation

Codecov / codecov/patch

internal/directives/git_pr_opener.go#L87

Added line #L87 was not covered by tests
},
}, nil
}
Expand Down Expand Up @@ -161,7 +162,7 @@
return PromotionStepResult{
Status: kargoapi.PromotionPhaseSucceeded,
Output: map[string]any{
prNumberKey: pr.Number,
stateKeyPRNumber: pr.Number,

Check warning on line 165 in internal/directives/git_pr_opener.go

View check run for this annotation

Codecov / codecov/patch

internal/directives/git_pr_opener.go#L165

Added line #L165 was not covered by tests
},
}, nil
}
Expand Down Expand Up @@ -219,7 +220,7 @@
return PromotionStepResult{
Status: kargoapi.PromotionPhaseSucceeded,
Output: map[string]any{
prNumberKey: pr.Number,
stateKeyPRNumber: pr.Number,
},
}, nil
}
Expand All @@ -243,7 +244,7 @@
stepCtx.Alias,
)
}
prNumberAny, exists := stepOutputMap[prNumberKey]
prNumberAny, exists := stepOutputMap[stateKeyPRNumber]

Check warning on line 247 in internal/directives/git_pr_opener.go

View check run for this annotation

Codecov / codecov/patch

internal/directives/git_pr_opener.go#L247

Added line #L247 was not covered by tests
if !exists {
return -1, nil
}
Expand Down Expand Up @@ -283,7 +284,7 @@
cfg.SourceBranchFromStep,
)
}
sourceBranchAny, exists := stepOutputMap[branchKey]
sourceBranchAny, exists := stepOutputMap[stateKeyBranch]
if !exists {
return "", fmt.Errorf(
"no branch found in output from step with alias %q",
Expand Down
Loading