update traffic strategy for canary

Signed-off-by: yunbo <[email protected]>
openkruise · Jun 24, 2024 · f3ccd17 · f3ccd17
1 parent c48c476
commit f3ccd17
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 26 deletions.
diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go
@@ -570,8 +570,6 @@ const (
 	FinalisingStepTypeStableService FinalisingStepType = "RestoreStableService"
 	// Restore the GatewayAPI/Ingress/Istio
 	FinalisingStepTypeGateway FinalisingStepType = "RestoreGateway"
-	// Delete Canary Service
-	FinalisingStepTypeCanaryService FinalisingStepType = "DeleteCanayService"
 	// Delete Batch Release
 	FinalisingStepTypeDeleteBR FinalisingStepType = "DeleteBatchRelease"
 )

diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go
@@ -355,44 +355,23 @@ func (m *canaryReleaseManager) doCanaryFinalising(c *RolloutContext) (bool, erro
 	default:
 		canaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeStableService
 		fallthrough
-
+	// restore stable service selector to select all pods [with grace time]
 	case v1beta1.FinalisingStepTypeStableService:
-		// restore stable service selector to select all pods [with grace time]
 		done, err := m.trafficRoutingManager.RestoreStableService(tr)
 		if err != nil || !done {
 			canaryStatus.LastUpdateTime = tr.LastUpdateTime
 			return done, err
 		}
 		canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()}
 		canaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeGateway
-
+	// modify network api(ingress or gateway api) configuration; delete canary service
 	case v1beta1.FinalisingStepTypeGateway:
-		// modify network api(ingress or gateway api) configuration
 		done, err := m.trafficRoutingManager.FinalisingTrafficRouting(tr)
 		if err != nil || !done {
 			canaryStatus.LastUpdateTime = tr.LastUpdateTime
 			return done, err
 		}
 		canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()}
-		canaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeCanaryService
-
-	/*
-		//TODO - As mentioned in FinalisingTrafficRouting function,
-		we should wait grace time between FinalisingStepTypeGateway and FinalisingStepTypeCanaryService
-		to avoid a very rare case which could cause minor traffic loss (espically, Istio), but it's difficult
-		to implement now.
-		However, we still reserve the FinalisingStepTypeCanaryService step here, but instead of removing the
-		canary Service as expected (which has been done in FinalisingStepTypeGateway), FinalisingStepTypeCanaryService
-		simply wait a gracetime between FinalisingStepTypeCanaryService and FinalisingStepTypeDeleteBR now
-	*/
-	case v1beta1.FinalisingStepTypeCanaryService:
-		// wait a gracetime for safety
-		if canaryStatus.LastUpdateTime != nil {
-			if verifyTime := canaryStatus.LastUpdateTime.Add(time.Second * time.Duration(3)); verifyTime.After(time.Now()) {
-				klog.Infof("restoring network configuration, but we need to wait %d seconds", 3)
-				return false, nil
-			}
-		}
 		canaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeBatchRelease
 
 	case v1beta1.FinalisingStepTypeBatchRelease:

diff --git a/pkg/controller/rollout/rollout_progressing_test.go b/pkg/controller/rollout/rollout_progressing_test.go
@@ -417,7 +417,7 @@ func TestReconcileRolloutProgressing(t *testing.T) {
 				s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2"
 				s.CanaryStatus.CurrentStepIndex = 4
 				s.CanaryStatus.NextStepIndex = 0
-				s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeCanaryService
+				s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeBatchRelease
 				s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted
 				cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing)
 				cond.Reason = v1alpha1.ProgressingReasonFinalising