[RELEASE 0.13] Backport fixes to minScale behavior.

pkg/reconciler/labeler/controller.go

@@ -19,16 +19,20 @@ package labeler
 import (
 	"context"
 
+	"k8s.io/client-go/tools/cache"
+
+	"knative.dev/serving/pkg/apis/serving"
 	servingclient "knative.dev/serving/pkg/client/injection/client"
 	configurationinformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/configuration"
 	revisioninformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/revision"
 	routeinformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/route"
 	routereconciler "knative.dev/serving/pkg/client/injection/reconciler/serving/v1/route"
+	servingreconciler "knative.dev/serving/pkg/reconciler"
 
 	"knative.dev/pkg/configmap"
 	"knative.dev/pkg/controller"
 	"knative.dev/pkg/logging"
-	servingreconciler "knative.dev/serving/pkg/reconciler"
+	pkgreconciler "knative.dev/pkg/reconciler"
 )
 
 const controllerAgentName = "labeler-controller"
@@ -56,5 +60,10 @@ func NewController(
 	logger.Info("Setting up event handlers")
 	routeInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))
 
+	configInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
+		FilterFunc: pkgreconciler.LabelExistsFilterFunc(serving.RouteLabelKey),
+		Handler:    controller.HandleAll(impl.EnqueueLabelOfNamespaceScopedResource("", serving.RouteLabelKey)),
+	})
+
 	return impl
 }

pkg/reconciler/labeler/labeler_test.go

@@ -74,6 +74,25 @@ func TestReconcile(t *testing.T) {
 			Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "first-reconcile"),
 		},
 		Key: "default/first-reconcile",
+	}, {
+		Name: "label pinned revision",
+		Objects: []runtime.Object{
+			pinnedRoute("default", "pinned-revision", "the-revision"),
+			simpleConfig("default", "the-config"),
+			rev("default", "the-config"),
+			rev("default", "the-config", WithRevName("the-revision")),
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			patchAddFinalizerAction("default", "pinned-revision"),
+			patchAddLabel("default", "the-revision",
+				"serving.knative.dev/route", "pinned-revision"),
+			patchAddLabel("default", "the-config",
+				"serving.knative.dev/route", "pinned-revision"),
+		},
+		WantEvents: []string{
+			Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "pinned-revision"),
+		},
+		Key: "default/pinned-revision",
 	}, {
 		Name: "steady state",
 		Objects: []runtime.Object{
@@ -84,6 +103,43 @@ func TestReconcile(t *testing.T) {
 				WithRevisionLabel("serving.knative.dev/route", "steady-state")),
 		},
 		Key: "default/steady-state",
+	}, {
+		Name: "no ready revision",
+		Objects: []runtime.Object{
+			simpleRunLatest("default", "no-ready-revision", "the-config", WithStatusTraffic()),
+			simpleConfig("default", "the-config", WithLatestReady("")),
+			rev("default", "the-config"),
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			patchAddFinalizerAction("default", "no-ready-revision"),
+			patchAddLabel("default", rev("default", "the-config").Name,
+				"serving.knative.dev/route", "no-ready-revision"),
+			patchAddLabel("default", "the-config",
+				"serving.knative.dev/route", "no-ready-revision"),
+		},
+		WantEvents: []string{
+			Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", "no-ready-revision"),
+		},
+		Key: "default/no-ready-revision",
+	}, {
+		Name: "transitioning route",
+		Objects: []runtime.Object{
+			simpleRunLatest("default", "transitioning-route", "old", WithRouteFinalizer,
+				WithSpecTraffic(configTraffic("new"))),
+			simpleConfig("default", "old",
+				WithConfigLabel("serving.knative.dev/route", "transitioning-route")),
+			rev("default", "old",
+				WithRevisionLabel("serving.knative.dev/route", "transitioning-route")),
+			simpleConfig("default", "new"),
+			rev("default", "new"),
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			patchAddLabel("default", rev("default", "new").Name,
+				"serving.knative.dev/route", "transitioning-route"),
+			patchAddLabel("default", "new",
+				"serving.knative.dev/route", "transitioning-route"),
+		},
+		Key: "default/transitioning-route",
 	}, {
 		Name: "failure adding label (revision)",
 		// Induce a failure during patching
@@ -161,6 +217,23 @@ func TestReconcile(t *testing.T) {
 			patchAddLabel("default", "new-config", "serving.knative.dev/route", "config-change"),
 		},
 		Key: "default/config-change",
+	}, {
+		Name: "update configuration",
+		Objects: []runtime.Object{
+			simpleRunLatest("default", "config-update", "the-config", WithRouteFinalizer),
+			simpleConfig("default", "the-config",
+				WithLatestCreated("the-config-ecoge"),
+				WithConfigLabel("serving.knative.dev/route", "config-update")),
+			rev("default", "the-config",
+				WithRevisionLabel("serving.knative.dev/route", "config-update")),
+			rev("default", "the-config",
+				WithRevName("the-config-ecoge")),
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			patchAddLabel("default", "the-config-ecoge",
+				"serving.knative.dev/route", "config-update"),
+		},
+		Key: "default/config-update",
 	}, {
 		Name: "delete route",
 		Objects: []runtime.Object{
@@ -242,15 +315,38 @@ func TestReconcile(t *testing.T) {
 	}))
 }
 
-func routeWithTraffic(namespace, name string, traffic v1.TrafficTarget, opts ...RouteOption) *v1.Route {
-	return Route(namespace, name, append(opts, WithStatusTraffic(traffic))...)
+func configTraffic(name string) v1.TrafficTarget {
+	return v1.TrafficTarget{
+		ConfigurationName: name,
+		Percent:           ptr.Int64(100),
+		LatestRevision:    ptr.Bool(true),
+	}
+}
+
+func revTraffic(name string, latest bool) v1.TrafficTarget {
+	return v1.TrafficTarget{
+		RevisionName:   name,
+		Percent:        ptr.Int64(100),
+		LatestRevision: ptr.Bool(latest),
+	}
+}
+
+func routeWithTraffic(namespace, name string, spec, status v1.TrafficTarget, opts ...RouteOption) *v1.Route {
+	return Route(namespace, name,
+		append([]RouteOption{WithSpecTraffic(spec), WithStatusTraffic(status)}, opts...)...)
 }
 
 func simpleRunLatest(namespace, name, config string, opts ...RouteOption) *v1.Route {
-	return routeWithTraffic(namespace, name, v1.TrafficTarget{
-		RevisionName: config + "-dbnfd",
-		Percent:      ptr.Int64(100),
-	}, opts...)
+	return routeWithTraffic(namespace, name,
+		configTraffic(config),
+		revTraffic(config+"-dbnfd", true),
+		opts...)
+}
+
+func pinnedRoute(namespace, name, revision string, opts ...RouteOption) *v1.Route {
+	traffic := revTraffic(revision, false)
+
+	return routeWithTraffic(namespace, name, traffic, traffic, opts...)
 }
 
 func simpleConfig(namespace, name string, opts ...ConfigOption) *v1.Configuration {
@@ -276,7 +372,7 @@ func rev(namespace, name string, opts ...RevisionOption) *v1.Revision {
 	rev := &v1.Revision{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:       namespace,
-			Name:            cfg.Status.LatestCreatedRevisionName,
+			Name:            cfg.Status.LatestReadyRevisionName,
 			ResourceVersion: "v1",
 			OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(cfg)},
 		},

pkg/reconciler/labeler/labels.go

@@ -45,17 +45,39 @@ func (c *Reconciler) syncLabels(ctx context.Context, r *v1.Route) error {
 	revisions := sets.NewString()
 	configs := sets.NewString()
 
-	// Walk the revisions in Route's .status.traffic and build a list
-	// of Configurations to label from their OwnerReferences.
-	for _, tt := range r.Status.Traffic {
-		rev, err := c.revisionLister.Revisions(r.Namespace).Get(tt.RevisionName)
-		if err != nil {
-			return err
+	// Walk the Route's .status.traffic and .spec.traffic and build a list
+	// of revisions and configurations to label
+	for _, tt := range append(r.Status.Traffic, r.Spec.Traffic...) {
+		revName := tt.RevisionName
+		configName := tt.ConfigurationName
+
+		if revName != "" {
+			rev, err := c.revisionLister.Revisions(r.Namespace).Get(revName)
+			if err != nil {
+				return err
+			}
+
+			revisions.Insert(revName)
+
+			// If the owner reference is a configuration, treat it like a configuration target
+			if owner := metav1.GetControllerOf(rev); owner != nil && owner.Kind == "Configuration" {
+				configName = owner.Name
+			}
 		}
-		revisions.Insert(tt.RevisionName)
-		owner := metav1.GetControllerOf(rev)
-		if owner != nil && owner.Kind == "Configuration" {
-			configs.Insert(owner.Name)
+
+		if configName != "" {
+			config, err := c.configurationLister.Configurations(r.Namespace).Get(configName)
+			if err != nil {
+				return err
+			}
+
+			configs.Insert(configName)
+
+			// If the target is for the latest revision, add the latest created revision to the list
+			// so that there is a smooth transition when the new revision becomes ready.
+			if config.Status.LatestCreatedRevisionName != "" && tt.LatestRevision != nil && *tt.LatestRevision {
+				revisions.Insert(config.Status.LatestCreatedRevisionName)
+			}
 		}
 	}