Fixed historical release awakening due to targetStep resolve inconsis…

…tency Due to a change introduced in #285, shipper strarted re-activating historical releases due to the way targetStep is resolved. The aforementioned step changed the composition of the pipeline and caused historical releases to be re-scheduled. This caused some historical items to re-activate their capacity and target targets. This change mainly reverts the re-composition of the pipeline and ensures the historical releases never re-activate their predecessors. Signed-off-by: Oleg Sidorov <oleg.sidorov@booking.com>
bookingcom · Feb 20, 2020 · f4cada9 · f4cada9
1 parent 18e1839
commit f4cada9
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 43 deletions.
diff --git a/pkg/controller/release/release_controller_test.go b/pkg/controller/release/release_controller_test.go
@@ -2842,10 +2842,42 @@ func TestUnhealthyTrafficAndCapacityIncumbentConvergesConsistently(t *testing.T)
 	// Mark contender as fully healthy
 	var step int32 = 2
 	contender.release.Spec.TargetStep = step
+	contender.release.Status.AchievedStep = &shipper.AchievedStep{Step: 2}
+	contender.release.Status = shipper.ReleaseStatus{
+		Conditions: []shipper.ReleaseCondition{
+			{Type: shipper.ReleaseConditionTypeBlocked, Status: corev1.ConditionFalse},
+			{Type: shipper.ReleaseConditionTypeScheduled, Status: corev1.ConditionTrue},
+		},
+		Strategy: &shipper.ReleaseStrategyStatus{
+			State: shipper.ReleaseStrategyState{
+				WaitingForInstallation: shipper.StrategyStateFalse,
+				WaitingForCommand:      shipper.StrategyStateFalse,
+				WaitingForTraffic:      shipper.StrategyStateFalse,
+				WaitingForCapacity:     shipper.StrategyStateFalse,
+			},
+			Conditions: []shipper.ReleaseStrategyCondition{
+				shipper.ReleaseStrategyCondition{
+					Type:   shipper.StrategyConditionContenderAchievedCapacity,
+					Status: corev1.ConditionTrue,
+					Step:   step,
+				},
+				shipper.ReleaseStrategyCondition{
+					Type:   shipper.StrategyConditionContenderAchievedInstallation,
+					Status: corev1.ConditionTrue,
+					Step:   step,
+				},
+				shipper.ReleaseStrategyCondition{
+					Type:   shipper.StrategyConditionContenderAchievedTraffic,
+					Status: corev1.ConditionTrue,
+					Step:   step,
+				},
+			},
+		},
+	}
+
 	contender.capacityTarget.Spec.Clusters[0].Percent = 100
 	contender.capacityTarget.Spec.Clusters[0].TotalReplicaCount = replicaCount
 	contender.trafficTarget.Spec.Clusters[0].Weight = 100
-	contender.release.Status.AchievedStep = &shipper.AchievedStep{Step: 2}
 
 	incumbent.trafficTarget.Spec.Clusters[0].Weight = 0
 	incumbent.trafficTarget.Spec.Clusters[1].Weight = 0
@@ -2960,44 +2992,8 @@ func TestUnhealthyTrafficAndCapacityIncumbentConvergesConsistently(t *testing.T)
 		patch,
 	))
 
-	newIncumbentStatus := map[string]interface{}{
-		"status": shipper.ReleaseStatus{
-			Strategy: &shipper.ReleaseStrategyStatus{
-				State: shipper.ReleaseStrategyState{
-					WaitingForInstallation: shipper.StrategyStateFalse,
-					WaitingForCommand:      shipper.StrategyStateFalse,
-					WaitingForTraffic:      shipper.StrategyStateFalse,
-					WaitingForCapacity:     shipper.StrategyStateTrue,
-				},
-				Conditions: []shipper.ReleaseStrategyCondition{
-					shipper.ReleaseStrategyCondition{
-						Type:    shipper.StrategyConditionContenderAchievedCapacity,
-						Status:  corev1.ConditionFalse,
-						Step:    step,
-						Reason:  ClustersNotReady,
-						Message: fmt.Sprintf("release \"test-incumbent\" hasn't achieved capacity in clusters: [broken-cluster]. for more details try `kubectl describe ct test-incumbent`"),
-					},
-					shipper.ReleaseStrategyCondition{
-						Type:   shipper.StrategyConditionContenderAchievedInstallation,
-						Status: corev1.ConditionTrue,
-						Step:   step,
-					},
-				},
-			},
-		},
-	}
-	patch, _ = json.Marshal(newIncumbentStatus)
-
-	f.actions = append(f.actions, kubetesting.NewPatchAction(
-		shipper.SchemeGroupVersion.WithResource("releases"),
-		incumbent.release.GetNamespace(),
-		incumbent.release.GetName(),
-		types.MergePatchType,
-		patch,
-	))
-
 	f.expectedEvents = append(f.expectedEvents,
-		`Normal ReleaseConditionChanged [] -> [Scheduled True], [] -> [StrategyExecuted True]`)
+		`Normal ReleaseConditionChanged [] -> [StrategyExecuted True]`)
 
 	f.run()
 }
diff --git a/pkg/controller/release/strategy_executor.go b/pkg/controller/release/strategy_executor.go
@@ -127,14 +127,17 @@ func (e *StrategyExecutor) Execute(prev, curr, succ *releaseInfo) (bool, []Strat
 	}
 
 	pipeline := NewPipeline()
-	pipeline.Enqueue(genInstallationEnforcer(curr, nil))
+	if isHead {
+		pipeline.Enqueue(genInstallationEnforcer(curr, nil))
+	}
 	pipeline.Enqueue(genCapacityEnforcer(curr, succ))
 	pipeline.Enqueue(genTrafficEnforcer(curr, succ))
-	if hasTail {
-		pipeline.Enqueue(genTrafficEnforcer(prev, curr))
-		pipeline.Enqueue(genCapacityEnforcer(prev, curr))
-	}
+
 	if isHead {
+		if hasTail {
+			pipeline.Enqueue(genTrafficEnforcer(prev, curr))
+			pipeline.Enqueue(genCapacityEnforcer(prev, curr))
+		}
 		pipeline.Enqueue(genReleaseStrategyStateEnforcer(curr, nil))
 	}