Fix for latestRevision routes may point to older revisions if latestC…

…reated is not ready

pkg/reconciler/configuration/configuration.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"sort"
 
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
@@ -127,7 +128,8 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati
 	}
 
 	// First, fetch the revision that should exist for the current generation.
-	lcr, err := c.latestCreatedRevision(config)
+	sortedRevisions, err := c.getSortedCreatedRevisions(config)
+	var lcr *v1alpha1.Revision
 	if errors.IsNotFound(err) {
 		lcr, err = c.createRevision(ctx, config)
 		if err != nil {
@@ -151,8 +153,14 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati
 	} else if err != nil {
 		logger.Errorf("Failed to reconcile Configuration %q - failed to get Revision: %v", config.Name, err)
 		return err
+	} else if len(sortedRevisions) == 0 {
+		logger.Errorf("Failed to reconcile Configuration %q - created revision list is empty", config.Name)
+		return err
 	}
 
+	if lcr == nil {
+		lcr = sortedRevisions[0]
+	}
 	revName := lcr.Name
 
 	// Second, set this to be the latest revision that we have created.
@@ -196,6 +204,16 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati
 		return err
 	}
 
+	if (rc == nil || rc.Status == corev1.ConditionUnknown || rc.Status == corev1.ConditionFalse) && sortedRevisions != nil && len(sortedRevisions) > 1 {
+		// Update the LatestReadyRevisionName to the last ready revision
+		for _, rev := range sortedRevisions {
+			if rev.Status.IsReady() {
+				config.Status.SetLatestReadyRevisionName(rev.Name)
+				break
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -237,21 +255,23 @@ func CheckNameAvailability(config *v1alpha1.Configuration, lister listers.Revisi
 	return rev, nil
 }
 
-func (c *Reconciler) latestCreatedRevision(config *v1alpha1.Configuration) (*v1alpha1.Revision, error) {
+func (c *Reconciler) getSortedCreatedRevisions(config *v1alpha1.Configuration) ([]*v1alpha1.Revision, error) {
 	if rev, err := CheckNameAvailability(config, c.revisionLister); rev != nil || err != nil {
-		return rev, err
+		return []*v1alpha1.Revision{rev}, err
 	}
 
 	lister := c.revisionLister.Revisions(config.Namespace)
-	generationKey := serving.ConfigurationGenerationLabelKey
 
 	list, err := lister.List(labels.SelectorFromSet(map[string]string{
-		generationKey:                 resources.RevisionLabelValueForKey(generationKey, config),
 		serving.ConfigurationLabelKey: config.Name,
 	}))
 
 	if err == nil && len(list) > 0 {
-		return list[0], nil
+		// Return a sorted list with Generation in descending order
+		sort.Slice(list[:], func(i, j int) bool {
+			return list[i].Labels[serving.ConfigurationGenerationLabelKey] > list[j].Labels[serving.ConfigurationGenerationLabelKey]
+		})
+		return list, nil
 	}
 
 	return nil, errors.NewNotFound(v1alpha1.Resource("revisions"), fmt.Sprintf("revision for %s", config.Name))

pkg/reconciler/configuration/configuration_test.go

@@ -392,6 +392,30 @@ func TestReconcile(t *testing.T) {
 				WithCreationTimestamp(now), MarkRevisionReady),
 		},
 		Key: "foo/double-trouble",
+	}, {
+		Name: "three revisions with the latest revision failed, the latest ready should be updated to the last ready revision",
+		Objects: []runtime.Object{
+			cfg("threerevs", "foo", 3,
+				WithLatestCreated("threerevs-00002"),
+				WithLatestReady("threerevs-00001"), WithObservedGen),
+			rev("threerevs", "foo", 1,
+				WithRevName("threerevs-00001"),
+				WithCreationTimestamp(now), MarkRevisionReady),
+			rev("threerevs", "foo", 2,
+				WithRevName("threerevs-00002"),
+				WithCreationTimestamp(now), MarkRevisionReady),
+			rev("threerevs", "foo", 3,
+				WithRevName("threerevs-00003"),
+				WithCreationTimestamp(now), MarkRevisionFailed),
+		},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: cfg("threerevs", "foo", 3,
+				WithLatestCreated("threerevs-00003"),
+				WithLatestReady("threerevs-00002"),
+				WithObservedGen,
+			),
+		}},
+		Key: "foo/threerevs",
 	}}
 
 	defer logtesting.ClearAll()

pkg/testing/v1alpha1/revision.go

@@ -155,6 +155,13 @@ func MarkRevisionReady(r *v1alpha1.Revision) {
 	r.Status.MarkContainerHealthy()
 }
 
+// MarkRevisionFailed calls the necessary helpers to make the Revision Ready=False.
+func MarkRevisionFailed(r *v1alpha1.Revision) {
+	WithInitRevConditions(r)
+	MarkInactive("", "")
+	r.Status.MarkInactive("", "")
+}
+
 // WithRevisionLabel attaches a particular label to the revision.
 func WithRevisionLabel(key, value string) RevisionOption {
 	return func(config *v1alpha1.Revision) {