[release-1.17] Assign 0% when there is at least one pod running (#358)

* [release-1.17] Assign 0% when there is at least one pod running

* Removed reading the traffic from route

* Skip the case for RolloutDuration != 0

Author: houshengbo

pkg/reconciler/service/service.go

@@ -975,15 +975,15 @@ func convertIntoTrafficTarget(name string, ro *v1.RolloutOrchestrator, rc *Rollo
 
 		spa, err := spaLister.Get(spaTargetRevName)
 		// Check the number of replicas has reached the target number of replicas for the revision scaling up
-		if apierrs.IsNotFound(err) || spa.Status.ActualScale == nil || (err == nil && targetNumberReplicas != nil && spa.Status.ActualScale != nil &&
+		if err != nil || spa.Status.ActualScale == nil || (err == nil && targetNumberReplicas != nil && spa.Status.ActualScale != nil &&
 			minScale != nil && *targetNumberReplicas <= *minScale && *spa.Status.ActualScale < *targetNumberReplicas) {
 			// If we have issues getting the spa, or the number of the replicas has reached the target number of
 			// the revision to scale up, we set the revisionTarget to ro.Spec.StageTargetRevisions.
 
 			// However, if there is no issue getting yhe spa, and the actual number of replicas is less than
 			// the target number of replicas, we need to use ro.Status.StageRevisionStatus or route.Status.Traffic
 			// as the traffic information for the route.
-			if strings.EqualFold(rc.ProgressiveRolloutStrategy, strategies.AvailabilityStrategy) {
+			if strings.EqualFold(rc.ProgressiveRolloutStrategy, strategies.AvailabilityStrategy) && rc.RolloutDuration == "0" {
 				// Comment out the following lines for further consideration with resourceUtil mode
 				// || (strings.EqualFold(rc.ProgressiveRolloutStrategy, strategies.ResourceUtilStrategy) && !trafficDriven && !lastStage) {
 				if len(ro.Status.StageRevisionStatus) > 0 {
@@ -997,47 +997,17 @@ func convertIntoTrafficTarget(name string, ro *v1.RolloutOrchestrator, rc *Rollo
 						}
 						revisionTarget[index].LatestRevision = ptr.Bool(false)
 					}
-					if !found {
+
+					if !found && spa != nil && spa.Status.ActualScale != nil && *spa.Status.ActualScale > 0 {
 						// We must assign the traffic to the new revision, even of it is 0%. Otherwise, the PA will
 						// sometimes report the error of "No traffic. The target is not receiving traffic", which
 						// will kill the pods of the new revision during the progressive rollout.
 						// The last one of ro.Spec.StageTargetRevisions is certainly the RevisionTarget for the
-						// new revision,
+						// new revision.
 						newRevisionTarget := ro.Spec.StageTargetRevisions[len(ro.Spec.StageTargetRevisions)-1]
 						newRevisionTarget.Percent = ptr.Int64(int64(0))
 						revisionTarget = append(revisionTarget, newRevisionTarget)
 					}
-				} else {
-					// If the ro does not have the StageRevisionStatus in the status, use the existing one in route.
-					route, errRoute := routeLister.Get(ro.Name)
-					if errRoute == nil && len(route.Status.Traffic) > 0 {
-						traffics := route.Status.Traffic
-						//found := false
-						for index := range traffics {
-							if traffics[index].RevisionName == spaTargetRevName {
-								//found = true
-								continue
-							}
-							traffics[index].LatestRevision = ptr.Bool(false)
-						}
-						//if !found {
-						//	// We must assign the traffic to the new revision, even of it is 0%. Otherwise, the PA will
-						//	// sometimes report the error of "No traffic. The target is not receiving traffic", which
-						//	// will kill the pods of the new revision during the progressive rollout.
-						//	newRevisionTarget := ro.Spec.StageTargetRevisions[len(ro.Spec.StageTargetRevisions)-1]
-						//	newRevisionTarget.Percent = ptr.Int64(int64(0))
-						//
-						//	newRevisionTraffic := servingv1.TrafficTarget{
-						//		ConfigurationName: name,
-						//		LatestRevision:    newRevisionTarget.LatestRevision,
-						//		Percent:           newRevisionTarget.Percent,
-						//		Tag:               newRevisionTarget.Tag,
-						//		URL:               newRevisionTarget.URL,
-						//	}
-						//	traffics = append(traffics, newRevisionTraffic)
-						//}
-						return traffics
-					}
 				}
 			}
 		}

pkg/reconciler/service/service_test.go

@@ -956,6 +956,7 @@ func TestTransformService(t *testing.T) {
 		rc: &RolloutConfig{
 			ProgressiveRolloutEnabled:  true,
 			ProgressiveRolloutStrategy: strategies.AvailabilityStrategy,
+			RolloutDuration:            "0",
 		},
 		service: &servingv1.Service{
 			ObjectMeta: metav1.ObjectMeta{
@@ -1016,7 +1017,12 @@ func TestTransformService(t *testing.T) {
 						{
 							RevisionName:   "rev-001",
 							LatestRevision: ptr.Bool(false),
-							Percent:        ptr.Int64(100),
+							Percent:        ptr.Int64(80),
+						},
+						{
+							ConfigurationName: "test-name",
+							Percent:           ptr.Int64(20),
+							LatestRevision:    ptr.Bool(true),
 						},
 					},
 				},
@@ -1051,13 +1057,13 @@ func TestTransformService(t *testing.T) {
 					Traffic: []servingv1.TrafficTarget{
 						{
 							RevisionName:   "rev-001",
-							Percent:        ptr.Int64(90),
+							Percent:        ptr.Int64(80),
 							LatestRevision: ptr.Bool(false),
 						},
 						{
-							RevisionName:   "rev-002",
-							Percent:        ptr.Int64(10),
-							LatestRevision: ptr.Bool(true),
+							ConfigurationName: "test-name",
+							Percent:           ptr.Int64(20),
+							LatestRevision:    ptr.Bool(true),
 						},
 					},
 				},