diff --git a/rollout/bluegreen.go b/rollout/bluegreen.go index ea51c475ab..6aa2f75452 100644 --- a/rollout/bluegreen.go +++ b/rollout/bluegreen.go @@ -32,7 +32,7 @@ func (c *rolloutContext) rolloutBlueGreen() error { return err } - if replicasetutil.CheckPodSpecChange(c.rollout, c.newRS) { + if replicasetutil.CheckPodSpecChange(c.rollout, c.newRS) && replicasetutil.ShouldSkipBlueGreenReconciliation(c.rollout) { return c.syncRolloutStatusBlueGreen(previewSvc, activeSvc) } diff --git a/utils/replicaset/replicaset.go b/utils/replicaset/replicaset.go index 6f1f6dafb4..80449531e8 100644 --- a/utils/replicaset/replicaset.go +++ b/utils/replicaset/replicaset.go @@ -492,6 +492,25 @@ func CheckPodSpecChange(rollout *v1alpha1.Rollout, newRS *appsv1.ReplicaSet) boo return false } +// ShouldSkipBlueGreenReconciliation determines if Blue-Green rollout reconciliation should be skipped +// when a pod spec change is detected. Returns false (don't skip) for rollouts with analysis or manual promotion. +func ShouldSkipBlueGreenReconciliation(rollout *v1alpha1.Rollout) bool { + if rollout.Spec.Strategy.BlueGreen == nil { + return true // Not a Blue-Green rollout, use default behavior + } + + hasPrePromotionAnalysis := rollout.Spec.Strategy.BlueGreen.PrePromotionAnalysis != nil + hasPostPromotionAnalysis := rollout.Spec.Strategy.BlueGreen.PostPromotionAnalysis != nil + needsManualPromotion := rollout.Spec.Strategy.BlueGreen.AutoPromotionEnabled != nil && !*rollout.Spec.Strategy.BlueGreen.AutoPromotionEnabled + + // If the rollout has analysis or needs manual promotion, don't skip reconciliation + if hasPrePromotionAnalysis || hasPostPromotionAnalysis || needsManualPromotion { + return false + } + + return true // No special conditions, can skip reconciliation +} + // PodTemplateOrStepsChanged detects if there is a change in either the pod template, or canary steps func PodTemplateOrStepsChanged(rollout *v1alpha1.Rollout, newRS *appsv1.ReplicaSet) bool { if checkStepHashChange(rollout) { diff --git a/utils/replicaset/replicaset_test.go b/utils/replicaset/replicaset_test.go index 00dcbac8f1..c26a327960 100644 --- a/utils/replicaset/replicaset_test.go +++ b/utils/replicaset/replicaset_test.go @@ -22,7 +22,6 @@ import ( "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" "github.com/argoproj/argo-rollouts/utils/annotations" - "github.com/argoproj/argo-rollouts/utils/conditions" "github.com/argoproj/argo-rollouts/utils/hash" timeutil "github.com/argoproj/argo-rollouts/utils/time" ) @@ -677,15 +676,42 @@ func TestCheckPodSpecChange(t *testing.T) { assert.True(t, CheckPodSpecChange(&ro, &rs)) } -func TestCheckStepHashChange(t *testing.T) { - ro := generateRollout("nginx") - ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{} - assert.True(t, checkStepHashChange(&ro)) - ro.Status.CurrentStepHash = conditions.ComputeStepHash(&ro) - assert.False(t, checkStepHashChange(&ro)) +func TestShouldSkipBlueGreenReconciliation(t *testing.T) { + t.Run("Blue-Green with PrePromotionAnalysis should not skip", func(t *testing.T) { + ro := generateRollout("nginx") + ro.Spec.Strategy.BlueGreen = &v1alpha1.BlueGreenStrategy{ + PrePromotionAnalysis: &v1alpha1.RolloutAnalysis{}, + } + assert.False(t, ShouldSkipBlueGreenReconciliation(&ro)) + }) + + t.Run("Blue-Green with PostPromotionAnalysis should not skip", func(t *testing.T) { + ro := generateRollout("nginx") + ro.Spec.Strategy.BlueGreen = &v1alpha1.BlueGreenStrategy{ + PostPromotionAnalysis: &v1alpha1.RolloutAnalysis{}, + } + assert.False(t, ShouldSkipBlueGreenReconciliation(&ro)) + }) - ro.Status.CurrentStepHash = "different-hash" - assert.True(t, checkStepHashChange(&ro)) + t.Run("Blue-Green with manual promotion should not skip", func(t *testing.T) { + ro := generateRollout("nginx") + ro.Spec.Strategy.BlueGreen = &v1alpha1.BlueGreenStrategy{ + AutoPromotionEnabled: ptr.To[bool](false), + } + assert.False(t, ShouldSkipBlueGreenReconciliation(&ro)) + }) + + t.Run("Blue-Green without analysis or manual promotion should skip", func(t *testing.T) { + ro := generateRollout("nginx") + ro.Spec.Strategy.BlueGreen = &v1alpha1.BlueGreenStrategy{} + assert.True(t, ShouldSkipBlueGreenReconciliation(&ro)) + }) + + t.Run("Non-Blue-Green rollout should skip", func(t *testing.T) { + ro := generateRollout("nginx") + ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{} + assert.True(t, ShouldSkipBlueGreenReconciliation(&ro)) + }) } func TestResetCurrentStepIndex(t *testing.T) {