diff --git a/pkg/reconciler/configuration/configuration.go b/pkg/reconciler/configuration/configuration.go index eea2a1356d7e..3e5a8f64aa3c 100644 --- a/pkg/reconciler/configuration/configuration.go +++ b/pkg/reconciler/configuration/configuration.go @@ -20,6 +20,8 @@ import ( "context" "fmt" "reflect" + "sort" + "strconv" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" @@ -27,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "k8s.io/client-go/tools/cache" "knative.dev/pkg/controller" "knative.dev/pkg/logging" @@ -167,23 +170,14 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati case rc.Status == corev1.ConditionTrue: logger.Infof("Revision %q of configuration is ready", revName) - - created, ready := config.Status.LatestCreatedRevisionName, config.Status.LatestReadyRevisionName - if ready == "" { + if config.Status.LatestReadyRevisionName == "" { // Surface an event for the first revision becoming ready. c.Recorder.Event(config, corev1.EventTypeNormal, "ConfigurationReady", "Configuration becomes ready") } - // Update the LatestReadyRevisionName and surface an event for the transition. - config.Status.SetLatestReadyRevisionName(lcr.Name) - if created != ready { - c.Recorder.Eventf(config, corev1.EventTypeNormal, "LatestReadyUpdate", - "LatestReadyRevisionName updated to %q", lcr.Name) - } case rc.Status == corev1.ConditionFalse: logger.Infof("Revision %q of configuration has failed", revName) - // TODO(mattmoor): Only emit the event the first time we see this. config.Status.MarkLatestCreatedFailed(lcr.Name, rc.Message) c.Recorder.Eventf(config, corev1.EventTypeWarning, "LatestCreatedFailed", @@ -193,9 +187,88 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati return fmt.Errorf("unrecognized condition status: %v on revision %q", rc.Status, revName) } + if err = c.findAndSetLatestReadyRevision(config); err != nil { + return fmt.Errorf("failed to find and set latest ready revision: %w", err) + } + return nil +} + +// findAndSetLatestReadyRevision finds the last ready revision and sets LatestReadyRevisionName to it. +func (c *Reconciler) findAndSetLatestReadyRevision(config *v1alpha1.Configuration) error { + sortedRevisions, err := c.getSortedCreatedRevisions(config) + if err != nil { + return err + } + for _, rev := range sortedRevisions { + if rev.Status.IsReady() { + old, new := config.Status.LatestReadyRevisionName, rev.Name + config.Status.SetLatestReadyRevisionName(rev.Name) + if old != new { + c.Recorder.Eventf(config, corev1.EventTypeNormal, "LatestReadyUpdate", + "LatestReadyRevisionName updated to %q", rev.Name) + } + return nil + } + } return nil } +// getSortedCreatedRevisions returns the list of created revisions sorted in descending +// generation order between the generation of the latest ready revision and config's generation (both inclusive). +func (c *Reconciler) getSortedCreatedRevisions(config *v1alpha1.Configuration) ([]*v1alpha1.Revision, error) { + lister := c.revisionLister.Revisions(config.Namespace) + configSelector := labels.SelectorFromSet(map[string]string{ + serving.ConfigurationLabelKey: config.Name, + }) + if config.Status.LatestReadyRevisionName != "" { + lrr, err := lister.Get(config.Status.LatestReadyRevisionName) + if err != nil { + return nil, err + } + start := lrr.Generation + var generations []string + for i := start; i <= int64(config.Generation); i++ { + generations = append(generations, strconv.FormatInt(i, 10)) + } + + // Add an "In" filter so that the configurations we get back from List have generation + // in range (config's latest ready generation, config's generation] + generationKey := serving.ConfigurationGenerationLabelKey + inReq, err := labels.NewRequirement(generationKey, + selection.In, + generations, + ) + if err != nil { + return nil, err + } + configSelector = configSelector.Add(*inReq) + } + + list, err := lister.List(configSelector) + if err != nil { + return nil, err + } + // Return a sorted list with Generation in descending order + if len(list) > 1 { + sort.Slice(list, func(i, j int) bool { + // BYO name always be the first + if config.Spec.Template.Name == list[i].Name { + return true + } + if config.Spec.Template.Name == list[j].Name { + return false + } + intI, errI := strconv.Atoi(list[i].Labels[serving.ConfigurationGenerationLabelKey]) + intJ, errJ := strconv.Atoi(list[j].Labels[serving.ConfigurationGenerationLabelKey]) + if errI != nil || errJ != nil { + return true + } + return intI > intJ + }) + } + return list, nil +} + // CheckNameAvailability checks that if the named Revision specified by the Configuration // is available (not found), exists (but matches), or exists with conflict (doesn't match). func CheckNameAvailability(config *v1alpha1.Configuration, lister listers.RevisionLister) (*v1alpha1.Revision, error) { diff --git a/pkg/reconciler/configuration/configuration_test.go b/pkg/reconciler/configuration/configuration_test.go index 1cf3bd1576eb..a0a1cd4f0ddd 100644 --- a/pkg/reconciler/configuration/configuration_test.go +++ b/pkg/reconciler/configuration/configuration_test.go @@ -387,6 +387,38 @@ 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, func(cfg *v1alpha1.Configuration) { + cfg.Spec.GetTemplate().Name = "threerevs-00003" + }, + ), + 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), MarkInactive("", "")), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: cfg("threerevs", "foo", 3, + WithLatestCreated("threerevs-00003"), + WithLatestReady("threerevs-00002"), + WithObservedGen, func(cfg *v1alpha1.Configuration) { + cfg.Spec.GetTemplate().Name = "threerevs-00003" + }, + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "LatestReadyUpdate", "LatestReadyRevisionName updated to %q", "threerevs-00002"), + }, + Key: "foo/threerevs", }} table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {