diff --git a/pkg/reconciler/configuration/configuration.go b/pkg/reconciler/configuration/configuration.go index 4b25a6bb7ef4..c8a7adfb3a84 100644 --- a/pkg/reconciler/configuration/configuration.go +++ b/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)) diff --git a/pkg/reconciler/configuration/configuration_test.go b/pkg/reconciler/configuration/configuration_test.go index c4126026c3ba..b39fbed87172 100644 --- a/pkg/reconciler/configuration/configuration_test.go +++ b/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() diff --git a/pkg/testing/v1alpha1/revision.go b/pkg/testing/v1alpha1/revision.go index c8b8ed292762..b082e333c016 100644 --- a/pkg/testing/v1alpha1/revision.go +++ b/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) {