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

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

api/v1alpha1/promotion_types.go

@@ -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.
@@ -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.
@@ -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"`

api/v1alpha1/promotion_types_test.go

@@ -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))
+	}
+}

charts/kargo/resources/crds/kargo.akuity.io_promotions.yaml

@@ -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.
@@ -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

charts/kargo/resources/crds/kargo.akuity.io_stages.yaml

@@ -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.
@@ -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.
@@ -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.

internal/controller/promotions/promotions.go

@@ -468,6 +468,7 @@ func (r *reconciler) promote(
 		steps[i] = directives.PromotionStep{
 			Kind:   step.Uses,
 			Alias:  step.As,
+			Retry:  step.Retry,
 			Config: step.Config.Raw,
 		}
 	}
@@ -481,6 +482,7 @@ func (r *reconciler) promote(
 		FreightRequests: stage.Spec.RequestedFreight,
 		Freight:         *workingPromo.Status.FreightCollection.DeepCopy(),
 		StartFromStep:   promo.Status.CurrentStep,
+		Attempts:        promo.Status.CurrentStepAttempt,
 		State:           directives.State(workingPromo.Status.GetState()),
 		Vars:            workingPromo.Spec.Vars,
 	}
@@ -490,6 +492,7 @@ func (r *reconciler) promote(
 		// 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
 	} else if !os.IsExist(err) {
 		return nil, fmt.Errorf("error creating working directory: %w", err)
 	}
@@ -505,6 +508,7 @@ func (r *reconciler) promote(
 	workingPromo.Status.Phase = res.Status
 	workingPromo.Status.Message = res.Message
 	workingPromo.Status.CurrentStep = res.CurrentStep
+	workingPromo.Status.CurrentStepAttempt = res.Attempt
 	workingPromo.Status.State = &apiextensionsv1.JSON{Raw: res.State.ToJSON()}
 	if res.Status == kargoapi.PromotionPhaseSucceeded {
 		var healthChecks []kargoapi.HealthCheckStep

internal/directives/argocd_revisions.go

@@ -177,7 +177,7 @@ func getCommitFromStep(sharedState State, stepAlias string) (string, error) {
 		return "",
 			fmt.Errorf("output from step with alias %q is not a map[string]any", stepAlias)
 	}
-	commitAny, exists := stepOutputMap[commitKey]
+	commitAny, exists := stepOutputMap[stateKeyCommit]
 	if !exists {
 		return "",
 			fmt.Errorf("no commit found in output from step with alias %q", stepAlias)

internal/directives/argocd_updater.go

@@ -127,6 +127,11 @@ func (a *argocdUpdater) Name() string {
 	return "argocd-update"
 }
 
+// DefaultAttempts implements the RetryableStepRunner interface.
+func (a *argocdUpdater) DefaultAttempts() int64 {
+	return -1
+}
+
 // RunPromotionStep implements the PromotionStepRunner interface.
 func (a *argocdUpdater) RunPromotionStep(
 	ctx context.Context,

internal/directives/git_commiter.go

@@ -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)
@@ -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
 }
 

internal/directives/git_commiter_test.go

@@ -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")

internal/directives/git_pr_opener.go

@@ -18,7 +18,8 @@ import (
 	_ "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(
@@ -83,7 +84,7 @@ func (g *gitPROpener) runPromotionStep(
 		return PromotionStepResult{
 			Status: kargoapi.PromotionPhaseSucceeded,
 			Output: map[string]any{
-				prNumberKey: prNumber,
+				stateKeyPRNumber: prNumber,
 			},
 		}, nil
 	}
@@ -161,7 +162,7 @@ func (g *gitPROpener) runPromotionStep(
 		return PromotionStepResult{
 			Status: kargoapi.PromotionPhaseSucceeded,
 			Output: map[string]any{
-				prNumberKey: pr.Number,
+				stateKeyPRNumber: pr.Number,
 			},
 		}, nil
 	}
@@ -219,7 +220,7 @@ func (g *gitPROpener) runPromotionStep(
 	return PromotionStepResult{
 		Status: kargoapi.PromotionPhaseSucceeded,
 		Output: map[string]any{
-			prNumberKey: pr.Number,
+			stateKeyPRNumber: pr.Number,
 		},
 	}, nil
 }
@@ -243,7 +244,7 @@ func (g *gitPROpener) getPRNumber(
 			stepCtx.Alias,
 		)
 	}
-	prNumberAny, exists := stepOutputMap[prNumberKey]
+	prNumberAny, exists := stepOutputMap[stateKeyPRNumber]
 	if !exists {
 		return -1, nil
 	}
@@ -283,7 +284,7 @@ func (g *gitPROpener) getSourceBranch(
 				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",