Skip to content

Commit 8a3ba38

Browse files
houshengboknative-prow-robot
authored andcommitted
Shift traffic when the minimum number of replicas are ready during progressive rollout
Fixes: knative-extensions#302
1 parent b8d639d commit 8a3ba38

File tree

1 file changed

+56
-4
lines changed

1 file changed

+56
-4
lines changed

Diff for: pkg/reconciler/service/service.go

+56-4
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, service *servingv1.Servi
162162
}
163163

164164
// After the RolloutOrchestrator is created or updated, call the base reconciliation loop of the service.
165-
err = c.baseReconciler.ReconcileKind(ctx, TransformService(service, rolloutOrchestrator, c.rolloutConfig))
165+
err = c.baseReconciler.ReconcileKind(ctx, TransformService(service, rolloutOrchestrator, c.rolloutConfig, c.spaLister,
166+
c.routeLister))
166167
if err != nil {
167168
return err
168169
}
@@ -909,19 +910,70 @@ func calculateStageTargetRevisions(replicasMap map[string]int32, startRevisions
909910
return stageRevisionTarget
910911
}
911912

912-
func TransformService(service *servingv1.Service, ro *v1.RolloutOrchestrator, rc *RolloutConfig) *servingv1.Service {
913+
func TransformService(service *servingv1.Service, ro *v1.RolloutOrchestrator, rc *RolloutConfig,
914+
spaLister listers.StagePodAutoscalerLister, routeLister servinglisters.RouteLister) *servingv1.Service {
913915
// If Knative Service defines more than one traffic, this feature tentatively does not cover this case.
914916
if len(service.Spec.Traffic) > 1 {
915917
return service
916918
}
917919
service.Spec.RouteSpec = servingv1.RouteSpec{
918-
Traffic: convertIntoTrafficTarget(service.GetName(), ro, rc),
920+
Traffic: convertIntoTrafficTarget(service.GetName(), ro, rc, spaLister, routeLister),
919921
}
920922
return service
921923
}
922924

923-
func convertIntoTrafficTarget(name string, ro *v1.RolloutOrchestrator, rc *RolloutConfig) []servingv1.TrafficTarget {
925+
func convertIntoTrafficTarget(name string, ro *v1.RolloutOrchestrator, rc *RolloutConfig,
926+
spaLister listers.StagePodAutoscalerLister, routeLister servinglisters.RouteLister) []servingv1.TrafficTarget {
924927
revisionTarget := ro.Spec.StageTargetRevisions
928+
if !ro.IsNotConvertToOneUpgrade() && rc.ProgressiveRolloutEnabled {
929+
// The revisionTarget is set directly to ro.Spec.StageTargetRevisions, because this is not a
930+
// one-to-one revision upgrade or the rollout feature is disabled. We do not cover this use case
931+
// in the implementation. The traffic target is set directly configured in ro.Spec.StageTargetRevisions.
932+
// However, if this is one-to-one or many-to-one rollout, we need to make sure the minimum number of
933+
// replicas are met before shifting the traffic percentage over.
934+
// Find out the name of the revision scaling up. Get the name of the spa for the revision scaling up.
935+
// The name is the same as the revision name.
936+
finalTargetRevs := ro.Spec.TargetRevisions
937+
spaTargetRevName := finalTargetRevs[0].RevisionName
938+
targetNumberReplicas := finalTargetRevs[0].MinScale
939+
940+
// Find the target number of replicas for the current stage.
941+
for _, revision := range ro.Spec.StageTargetRevisions {
942+
if revision.RevisionName == spaTargetRevName {
943+
targetNumberReplicas = revision.TargetReplicas
944+
}
945+
}
946+
spa, err := spaLister.StagePodAutoscalers(ro.Namespace).Get(spaTargetRevName)
947+
// Check the number of replicas has reached the target number of replicas for the revision scaling up
948+
if err == nil && *spa.Status.ActualScale < *targetNumberReplicas {
949+
// If we have issues getting the spa, or the number of the replicas has reached the target number of
950+
// the revision to scale up, we set the revisionTarget to ro.Spec.StageTargetRevisions.
951+
952+
// However, if there is no issue getting yhe spa, and the actual number of replicas is less than
953+
// the target number of replicas, we need to use ro.Status.StageRevisionStatus or route.Status.Traffic
954+
// as the traffic information for the route.
955+
if len(ro.Status.StageRevisionStatus) > 0 {
956+
// If the ro has the StageRevisionStatus in the status, use it.
957+
revisionTarget = ro.Status.StageRevisionStatus
958+
for index := 0; index < len(revisionTarget); index++ {
959+
revisionTarget[index].LatestRevision = ptr.Bool(false)
960+
}
961+
} else {
962+
// If the ro does not have the StageRevisionStatus in the status, use the existing one in route.
963+
route, errRoute := routeLister.Routes(ro.Namespace).Get(ro.Name)
964+
if errRoute != nil && !apierrs.IsNotFound(errRoute) {
965+
if len(route.Status.Traffic) > 0 {
966+
traffics := route.Status.Traffic
967+
for index := 0; index < len(traffics); index++ {
968+
traffics[index].LatestRevision = ptr.Bool(false)
969+
}
970+
return traffics
971+
}
972+
}
973+
}
974+
}
975+
}
976+
925977
trafficTarget := make([]servingv1.TrafficTarget, 0, len(revisionTarget))
926978
for _, revision := range revisionTarget {
927979
target := servingv1.TrafficTarget{}

0 commit comments

Comments
 (0)