From ffb40faa090d31e2b51949c202ce0383a80310dd Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 18 Sep 2019 14:36:35 -0700 Subject: [PATCH 1/3] =?UTF-8?q?Mark=20Dependency=20status=20as=20Unknown?= =?UTF-8?q?=20when=20the=20dependency=20object=E2=80=99s=20ObjectMetadata.?= =?UTF-8?q?Generation=20and=20Status.ObservedGeneration=20are=20not=20equa?= =?UTF-8?q?l=20during=20the=20trigger=20reconcile=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/reconciler/trigger/trigger.go | 19 ++++++------ pkg/reconciler/trigger/trigger_test.go | 41 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/pkg/reconciler/trigger/trigger.go b/pkg/reconciler/trigger/trigger.go index 53399319c97..1eac0af4605 100644 --- a/pkg/reconciler/trigger/trigger.go +++ b/pkg/reconciler/trigger/trigger.go @@ -260,15 +260,16 @@ func (r *Reconciler) propagateDependencyReadiness(dependencyObjRef corev1.Object return fmt.Errorf("getting the dependency: %v", err) } dependency := dependencyObj.(*duckv1alpha1.KResource) - // Temporarily comment it until we figure out whether we update Status.ObservedGeneration when KResource changes - // From manual testing, it looks like we never update Status.ObservedGeneration - //if dependency.GetGeneration() != dependency.Status.ObservedGeneration { - // logging.FromContext(ctx).Error("The ObjectMeta Generation of dependency is not equal to the observedGeneration of status", - // zap.Any("ObjectMeta Generation of dependency", dependency.GetGeneration()), - // zap.Any("ObservedGeneration of status", dependency.Status.ObservedGeneration)) - // t.Status.MarkDependencyUnknown("GenerationNotEqual", "The ObjectMeta Generation of dependency %d is not equal to the ObservedGeneration of status %d", dependency.GetGeneration(), dependency.Status.ObservedGeneration) - // return nil - //} + + // The dependency hasn't yet reconciled our latest changes to + // its desired state, so its conditions are outdated. + if dependency.GetGeneration() != dependency.Status.ObservedGeneration { + logging.FromContext(ctx).Error("The ObjectMeta Generation of dependency is not equal to the observedGeneration of status", + zap.Any("ObjectMeta Generation of dependency", dependency.GetGeneration()), + zap.Any("ObservedGeneration of status", dependency.Status.ObservedGeneration)) + t.Status.MarkDependencyUnknown("GenerationNotEqual", "The ObjectMeta Generation of dependency %q is not equal to the ObservedGeneration of status %q", dependency.GetGeneration(), dependency.Status.ObservedGeneration) + return nil + } t.Status.PropagateDependencyStatus(dependency) return nil } diff --git a/pkg/reconciler/trigger/trigger_test.go b/pkg/reconciler/trigger/trigger_test.go index 49ce62d4346..a2ddbb12db8 100644 --- a/pkg/reconciler/trigger/trigger_test.go +++ b/pkg/reconciler/trigger/trigger_test.go @@ -79,6 +79,9 @@ const ( testSchedule = "*/2 * * * *" testData = "data" sinkName = "testsink" + + currentGeneration = 1 + outdatedGeneration = 0 ) var ( @@ -568,6 +571,37 @@ func TestAllCases(t *testing.T) { ), }}, }, { + Name: "Dependency generation not equal", + Key: triggerKey, + Objects: []runtime.Object{ + makeReadyBroker(), + makeBrokerFilterService(), + makeReadySubscription(), + makeGenerationNotEqualCronJobSource(), + reconciletesting.NewTrigger(triggerName, testNS, brokerName, + reconciletesting.WithTriggerUID(triggerUID), + reconciletesting.WithTriggerSubscriberURI(subscriberURI), + reconciletesting.WithInitTriggerConditions, + reconciletesting.WithDependencyAnnotation(dependencyAnnotation), + ), + }, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "TriggerReconciled", "Trigger reconciled")}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: reconciletesting.NewTrigger(triggerName, testNS, brokerName, + reconciletesting.WithTriggerUID(triggerUID), + reconciletesting.WithTriggerSubscriberURI(subscriberURI), + // The first reconciliation will initialize the status conditions. + reconciletesting.WithInitTriggerConditions, + reconciletesting.WithDependencyAnnotation(dependencyAnnotation), + reconciletesting.WithTriggerBrokerReady(), + reconciletesting.WithTriggerSubscribed(), + reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), + reconciletesting.WithTriggerDependencyUnknown("GenerationNotEqual", fmt.Sprintf("The ObjectMeta Generation of dependency %q is not equal to the ObservedGeneration of status %q", currentGeneration, outdatedGeneration))), + }}, + }, + { Name: "Dependency ready", Key: triggerKey, Objects: []runtime.Object{ @@ -761,6 +795,13 @@ func makeNotReadyCronJobSource() *sourcesv1alpha1.CronJobSource { return NewCronJobSource(cronJobSourceName, testNS, WithCronJobApiVersion(cronJobSourceAPIVersion), WithCronJobSourceSinkNotFound) } +func makeGenerationNotEqualCronJobSource() *sourcesv1alpha1.CronJobSource { + c := makeNotReadyCronJobSource() + c.Generation = currentGeneration + c.Status.ObservedGeneration = outdatedGeneration + return c +} + func makeReadyCronJobSource() *sourcesv1alpha1.CronJobSource { return NewCronJobSource(cronJobSourceName, testNS, WithCronJobApiVersion(cronJobSourceAPIVersion), From cd093a4e7b90dc35c58106bbbebcc2473e0b57d1 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Thu, 19 Sep 2019 13:06:32 -0700 Subject: [PATCH 2/3] changed naming --- pkg/reconciler/trigger/trigger.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/reconciler/trigger/trigger.go b/pkg/reconciler/trigger/trigger.go index 1eac0af4605..e1a020970cc 100644 --- a/pkg/reconciler/trigger/trigger.go +++ b/pkg/reconciler/trigger/trigger.go @@ -264,10 +264,10 @@ func (r *Reconciler) propagateDependencyReadiness(dependencyObjRef corev1.Object // The dependency hasn't yet reconciled our latest changes to // its desired state, so its conditions are outdated. if dependency.GetGeneration() != dependency.Status.ObservedGeneration { - logging.FromContext(ctx).Error("The ObjectMeta Generation of dependency is not equal to the observedGeneration of status", - zap.Any("ObjectMeta Generation of dependency", dependency.GetGeneration()), - zap.Any("ObservedGeneration of status", dependency.Status.ObservedGeneration)) - t.Status.MarkDependencyUnknown("GenerationNotEqual", "The ObjectMeta Generation of dependency %q is not equal to the ObservedGeneration of status %q", dependency.GetGeneration(), dependency.Status.ObservedGeneration) + logging.FromContext(ctx).Info("The ObjectMeta Generation of dependency is not equal to the observedGeneration of status", + zap.Any("objectMetaGeneration", dependency.GetGeneration()), + zap.Any("statusObservedGeneration", dependency.Status.ObservedGeneration)) + t.Status.MarkDependencyUnknown("GenerationNotEqual", "The dependency's metadata.generation, %q, is not equal to its status.observedGeneration, %q.", dependency.GetGeneration(), dependency.Status.ObservedGeneration) return nil } t.Status.PropagateDependencyStatus(dependency) From d321bb3b358015f7090489da375c0b3ce6cb6295 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Thu, 19 Sep 2019 13:08:15 -0700 Subject: [PATCH 3/3] fixed unit tests --- pkg/reconciler/trigger/trigger_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/trigger/trigger_test.go b/pkg/reconciler/trigger/trigger_test.go index a2ddbb12db8..782aba403b5 100644 --- a/pkg/reconciler/trigger/trigger_test.go +++ b/pkg/reconciler/trigger/trigger_test.go @@ -598,7 +598,7 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerBrokerReady(), reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), - reconciletesting.WithTriggerDependencyUnknown("GenerationNotEqual", fmt.Sprintf("The ObjectMeta Generation of dependency %q is not equal to the ObservedGeneration of status %q", currentGeneration, outdatedGeneration))), + reconciletesting.WithTriggerDependencyUnknown("GenerationNotEqual", fmt.Sprintf("The dependency's metadata.generation, %q, is not equal to its status.observedGeneration, %q.", currentGeneration, outdatedGeneration))), }}, }, {