stop disqualifying auto-promotion for Stages with multiple upstreams (#…

…1169) Signed-off-by: Kent <[email protected]>
akuity · Nov 22, 2023 · bce112d · bce112d
1 parent aeb15d8
commit bce112d
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 53 deletions.
diff --git a/internal/controller/stages/stages.go b/internal/controller/stages/stages.go
@@ -525,20 +525,17 @@ func (r *reconciler) syncNormalStage(
 		}
 	}
 
-	// All of these conditions disqualify auto-promotion
-	if stage.Spec.Subscriptions == nil || // No subs at all
-		(stage.Spec.Subscriptions.Warehouse == "" && len(stage.Spec.Subscriptions.UpstreamStages) == 0) || // No subs at all
-		(stage.Spec.Subscriptions.Warehouse != "" && len(stage.Spec.Subscriptions.UpstreamStages) > 0) || // Ambiguous
-		len(stage.Spec.Subscriptions.UpstreamStages) > 1 { // Ambiguous
-		logger.Debug("Stage is not eligible for auto-promotion")
+	// Stop here if we have no chance of finding any Freight to promote.
+	if stage.Spec.Subscriptions == nil ||
+		(stage.Spec.Subscriptions.Warehouse == "" && len(stage.Spec.Subscriptions.UpstreamStages) == 0) {
+		logger.Warn(
+			"Stage has no subscriptions. This may indicate an issue with resource" +
+				"validation logic.",
+		)
 		return status, nil
 	}
 
-	// If we get to here, we've determined that auto-promotion is possible.
-	// Now see if it's permitted...
-	logger.Debug(
-		"Stage is eligible for auto-promotion; checking if it is permitted...",
-	)
+	logger.Debug("checking if auto-promotion is permitted...")
 	if permitted, err :=
 		r.isAutoPromotionPermittedFn(ctx, stage.Namespace, stage.Name); err != nil {
 		return status, errors.Wrapf(
@@ -553,8 +550,8 @@ func (r *reconciler) syncNormalStage(
 		return status, nil
 	}
 
-	// If we get to here, we've determined that auto-promotion is both possible
-	// and permitted. Time to go looking for new Freight...
+	// If we get to here, auto-promotion is permitted. Time to go looking for new
+	// Freight...
 
 	latestFreight, err :=
 		r.getLatestAvailableFreightFn(ctx, stage.Namespace, stage)

diff --git a/internal/controller/stages/stages_test.go b/internal/controller/stages/stages_test.go
@@ -324,46 +324,6 @@ func TestSyncNormalStage(t *testing.T) {
 			},
 		},
 
-		{
-			name: "auto-promotion not possible",
-			stage: &kargoapi.Stage{
-				Spec: &kargoapi.StageSpec{
-					Subscriptions: &kargoapi.Subscriptions{
-						UpstreamStages: []kargoapi.StageSubscription{
-							{Name: "fake-stage"},
-							{Name: "another-fake-stage"},
-						},
-					},
-					PromotionMechanisms: &kargoapi.PromotionMechanisms{},
-				},
-				Status: kargoapi.StageStatus{
-					CurrentFreight: &kargoapi.SimpleFreight{},
-				},
-			},
-			reconciler: &reconciler{
-				hasNonTerminalPromotionsFn: noNonTerminalPromotionsFn,
-				checkHealthFn: func(
-					context.Context,
-					kargoapi.SimpleFreight,
-					[]kargoapi.ArgoCDAppUpdate,
-				) *kargoapi.Health {
-					return nil
-				},
-				verifyFreightInStageFn: func(context.Context, string, string, string) error {
-					return nil
-				},
-			},
-			assertions: func(
-				initialStatus kargoapi.StageStatus,
-				newStatus kargoapi.StageStatus,
-				err error,
-			) {
-				require.NoError(t, err)
-				// Status should be returned unchanged
-				require.Equal(t, initialStatus, newStatus)
-			},
-		},
-
 		{
 			name: "error checking if auto-promotion is permitted",
 			stage: &kargoapi.Stage{