diff --git a/codegen/cmd/injection-gen/generators/reconciler_reconciler.go b/codegen/cmd/injection-gen/generators/reconciler_reconciler.go index 647b82fd0e..8b4d0c7331 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_reconciler.go +++ b/codegen/cmd/injection-gen/generators/reconciler_reconciler.go @@ -320,7 +320,7 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) {{if .isKRShaped}} - reconciler.PostProcessReconcile(ctx, resource) + reconciler.PostProcessReconcile(ctx, resource, original) {{end}} } else if fin, ok := r.reconciler.(Finalizer); ok { // Append the target method to the logger. diff --git a/injection/README.md b/injection/README.md index d30b090bc7..e23ff32f52 100644 --- a/injection/README.md +++ b/injection/README.md @@ -471,7 +471,7 @@ reconciler.PreProcessReconcile(ctx, resource) reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) -reconciler.PostProcessReconcile(ctx, resource) +reconciler.PostProcessReconcile(ctx, resource, oldResource) ``` #### Stubs diff --git a/reconciler/reconcile_common.go b/reconciler/reconcile_common.go index c9f8b5cb84..a807edfb68 100644 --- a/reconciler/reconcile_common.go +++ b/reconciler/reconcile_common.go @@ -18,6 +18,10 @@ package reconciler import ( "context" + "reflect" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -50,7 +54,7 @@ func PreProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { } // PostProcessReconcile contains logic to apply after reconciliation of a resource. -func PostProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { +func PostProcessReconcile(ctx context.Context, resource, oldResource duckv1.KRShaped) { logger := logging.FromContext(ctx) status := resource.GetStatus() mgr := resource.GetConditionSet().Manage(status) @@ -64,4 +68,26 @@ func PostProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { } else if rc.Reason == failedGenerationBump { logger.Warn("A reconciler observed a new generation without updating the resource status") } + + groomConditionsTransitionTime(resource, oldResource) +} + +// groomConditionsTransitionTime ensures that the LastTransitionTime only advances for resources +// where the condition has changed during reconciliation. This also ensures that all advanced +// conditions share the same timestamp. +func groomConditionsTransitionTime(resource, oldResource duckv1.KRShaped) { + now := apis.VolatileTime{Inner: metav1.NewTime(time.Now())} + sts := resource.GetStatus() + for i := range sts.Conditions { + cond := &sts.Conditions[i] + + if oldCond := oldResource.GetStatus().GetCondition(cond.Type); oldCond != nil { + cond.LastTransitionTime = oldCond.LastTransitionTime + if reflect.DeepEqual(cond, oldCond) { + continue + } + } + + cond.LastTransitionTime = now + } } diff --git a/reconciler/reconcile_common_test.go b/reconciler/reconcile_common_test.go index b1956f94e8..bda8540ac0 100644 --- a/reconciler/reconcile_common_test.go +++ b/reconciler/reconcile_common_test.go @@ -19,6 +19,7 @@ package reconciler import ( "context" "testing" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -99,13 +100,43 @@ func TestPostProcessReconcileBumpsGeneration(t *testing.T) { resource := makeResource() krShape := duckv1.KRShaped(resource) - PostProcessReconcile(context.Background(), krShape) + PostProcessReconcile(context.Background(), krShape, krShape) if resource.Status.ObservedGeneration != resource.Generation { - t.Errorf("Expected observed generation bump got=%d want=%d", resource.Status.ObservedGeneration, resource.Generation) + t.Errorf("Expected observed generation bump got=%d want=%d", + resource.Status.ObservedGeneration, resource.Generation) } if krShape.GetStatus().ObservedGeneration != krShape.GetGeneration() { - t.Errorf("Expected observed generation bump got=%d want=%d", resource.Status.ObservedGeneration, resource.Generation) + t.Errorf("Expected observed generation bump got=%d want=%d", + resource.Status.ObservedGeneration, resource.Generation) + } +} + +func TestPostProcessReconcileUpdatesTransitionTimes(t *testing.T) { + oldNow := apis.VolatileTime{Inner: metav1.NewTime(time.Now())} + resource := makeResource() + oldResource := makeResource() + // initialize old conditions with oldNow + oldResource.Status.Conditions[0].LastTransitionTime = oldNow + oldResource.Status.Conditions[1].LastTransitionTime = oldNow + // change the second condition, but keep the old timestamp. + resource.Status.Conditions[1].LastTransitionTime = oldNow + resource.Status.Conditions[1].Status = corev1.ConditionFalse + + new := duckv1.KRShaped(resource) + old := duckv1.KRShaped(oldResource) + PostProcessReconcile(context.Background(), new, old) + + unchangedCond := resource.Status.Conditions[0] + if unchangedCond.LastTransitionTime != oldNow { + t.Errorf("Expected unchanged condition to keep old timestamp. Got=%v Want=%v", + unchangedCond.LastTransitionTime, oldNow) + } + + changedCond := resource.Status.Conditions[1] + if changedCond.LastTransitionTime == oldNow { + t.Errorf("Expected changed condition to get a new timestamp. Got=%v Want=%v", + changedCond.LastTransitionTime, oldNow) } }