diff --git a/apis/condition_set.go b/apis/condition_set.go index 49f9d2e9b6..8d02fec485 100644 --- a/apis/condition_set.go +++ b/apis/condition_set.go @@ -142,6 +142,11 @@ type conditionsImpl struct { accessor ConditionsAccessor } +// GetTopLevelConditionType is an accessor for the top-level happy condition. +func (r ConditionSet) GetTopLevelConditionType() ConditionType { + return r.happy +} + // Manage creates a ConditionManager from an accessor object using the original // ConditionSet as a reference. Status must be a pointer to a struct. func (r ConditionSet) Manage(status ConditionsAccessor) ConditionManager { diff --git a/reconciler/reconcile_common.go b/reconciler/reconcile_common.go new file mode 100644 index 0000000000..38e3bbe383 --- /dev/null +++ b/reconciler/reconcile_common.go @@ -0,0 +1,55 @@ +/* +Copyright 2020 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconciler + +import ( + "context" + + duckv1 "knative.dev/pkg/apis/duck/v1" + "knative.dev/pkg/logging" +) + +const failedGenerationBump = "NewObservedGenFailure" + +// PreProcessReconcile contains logic to apply before reconciliation of a resource. +func PreProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { + newStatus := resource.GetStatus() + + if newStatus.ObservedGeneration != resource.GetObjectMeta().GetGeneration() { + condSet := resource.GetConditionSet() + manager := condSet.Manage(newStatus) + + // Reset Ready/Successful to unknown. The reconciler is expected to overwrite this. + manager.MarkUnknown(condSet.GetTopLevelConditionType(), failedGenerationBump, "unsuccessfully observed a new generation") + } +} + +// PostProcessReconcile contains logic to apply after reconciliation of a resource. +func PostProcessReconcile(ctx context.Context, resource duckv1.KRShaped) { + logger := logging.FromContext(ctx) + newStatus := resource.GetStatus() + mgr := resource.GetConditionSet().Manage(newStatus) + + // Bump observed generation to denote that we have processed this + // generation regardless of success or failure. + newStatus.ObservedGeneration = resource.GetObjectMeta().GetGeneration() + + rc := mgr.GetTopLevelCondition() + if rc.Reason == failedGenerationBump { + logger.Warn("A reconciler observed a new generation without updating the resource status") + } +} diff --git a/reconciler/reconcile_common_test.go b/reconciler/reconcile_common_test.go new file mode 100644 index 0000000000..b61cc03996 --- /dev/null +++ b/reconciler/reconcile_common_test.go @@ -0,0 +1,96 @@ +/* +Copyright 2020 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconciler + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" +) + +func makeResource(topLevelCond string) *duckv1.KResource { + fooCond := apis.Condition{ + Type: "Foo", + Status: corev1.ConditionTrue, + Message: "Something something foo", + } + readyCond := apis.Condition{ + Type: apis.ConditionType(topLevelCond), + Status: corev1.ConditionTrue, + Message: "Something something bar", + } + + return &duckv1.KResource{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 42, + }, + + Status: duckv1.Status{ + ObservedGeneration: 0, + Conditions: duckv1.Conditions{fooCond, readyCond}, + }, + } +} + +func TestPreProcessResetsReady(t *testing.T) { + testCases := []struct { + name string + initTopLevelCond string + expectedTopLevelCondition apis.ConditionType + }{{ + name: "top level Ready", + initTopLevelCond: "Ready", + expectedTopLevelCondition: apis.ConditionReady, + }, { + name: "top level Succeeded", + initTopLevelCond: "Succeeded", + expectedTopLevelCondition: apis.ConditionSucceeded, + }} + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + resource := makeResource(test.initTopLevelCond) + krShape := duckv1.KRShaped(resource) + + PreProcessReconcile(context.Background(), krShape) + + if rc := resource.Status.GetCondition(test.expectedTopLevelCondition); rc.Status != "Unknown" { + t.Errorf("Expected unchanged ready status got=%s want=Unknown", rc.Status) + } + }) + } +} + +func TestPostProcessReconcileBumpsGeneration(t *testing.T) { + resource := makeResource("Ready") + + krShape := duckv1.KRShaped(resource) + PostProcessReconcile(context.Background(), krShape) + + if 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.GetObjectMeta().GetGeneration() { + t.Errorf("Expected observed generation bump got=%d want=%d", resource.Status.ObservedGeneration, resource.Generation) + } +}