From 4504b5acabb5b28cba33a792d461d184a278688b Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Mon, 13 Jan 2020 17:02:34 -0800 Subject: [PATCH 1/8] Standardize True/False/Unknown status throughout knative-gcp --- pkg/apis/duck/v1alpha1/pubsub_lifecycle.go | 31 +- .../events/v1alpha1/auditlogs_lifecycle.go | 21 +- pkg/apis/events/v1alpha1/pubsub_lifecycle.go | 35 +- .../events/v1alpha1/scheduler_lifecycle.go | 23 +- pkg/apis/events/v1alpha1/storage_lifecycle.go | 21 +- .../messaging/v1alpha1/channel_lifecycle.go | 36 +- .../v1alpha1/channel_lifecycle_test.go | 50 +- .../v1alpha1/pull_subscription_lifecycle.go | 5 + pkg/apis/pubsub/v1alpha1/topic_lifecycle.go | 7 +- .../events/auditlogs/auditlogs_test.go | 150 ++- pkg/reconciler/events/pubsub/pubsub.go | 2 +- pkg/reconciler/events/pubsub/pubsub_test.go | 37 +- .../events/scheduler/scheduler_test.go | 1081 +++++++++-------- pkg/reconciler/events/storage/storage_test.go | 890 ++++++++------ pkg/reconciler/messaging/channel/channel.go | 2 +- .../messaging/channel/channel_test.go | 488 ++++---- pkg/reconciler/pubsub/reconciler.go | 97 +- pkg/reconciler/pubsub/reconciler_test.go | 155 ++- pkg/reconciler/testing/auditlogs.go | 20 +- pkg/reconciler/testing/channel.go | 10 +- pkg/reconciler/testing/pubsub.go | 16 +- pkg/reconciler/testing/pullsubscription.go | 15 + pkg/reconciler/testing/scheduler.go | 38 +- pkg/reconciler/testing/storage.go | 32 +- pkg/reconciler/testing/topic.go | 13 + 25 files changed, 2037 insertions(+), 1238 deletions(-) diff --git a/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go b/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go index 922ef7af92..d20f8c6235 100644 --- a/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go @@ -20,23 +20,44 @@ import ( "knative.dev/pkg/apis" ) -// MarkTopicNotReady sets the condition that the PubSub Topic is not ready and why. -func (s *PubSubStatus) MarkTopicNotReady(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFalse sets the condition that the PubSub Topic is not ready and why. +func (s *PubSubStatus) MarkTopicFalse(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { cs.Manage(s).MarkFalse(TopicReady, reason, messageFormat, messageA...) } +// MarkTopicUnknown sets the condition that the PubSub Topic is False and why. +func (s *PubSubStatus) MarkTopicUnknown(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { + cs.Manage(s).MarkUnknown(TopicReady, reason, messageFormat, messageA...) +} + // MarkTopicReady sets the condition that the PubSub Topic is ready. func (s *PubSubStatus) MarkTopicReady(cs *apis.ConditionSet) { cs.Manage(s).MarkTrue(TopicReady) } -// MarkPullSubscriptionNotReady sets the condition that the PubSub PullSUbscription is -// not ready and why. -func (s *PubSubStatus) MarkPullSubscriptionNotReady(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { +// MarkTopicNotConfigured sets the condition that the PubSub Topic has not yet been reconciled. +func (s *PubSubStatus) MarkTopicNotConfigured(cs *apis.ConditionSet) { + cs.Manage(s).MarkUnknown(TopicReady, "TopicNotConfigured", "Topic has not yet been reconciled") +} + +// MarkPullSubscriptionFalse sets the condition that the PubSub PullSubscription is +// False and why. +func (s *PubSubStatus) MarkPullSubscriptionFalse(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { cs.Manage(s).MarkFalse(PullSubscriptionReady, reason, messageFormat, messageA...) } +// MarkPullSubscriptionUnknown sets the condition that the PubSub PullSubscription is Unknown. +func (s *PubSubStatus) MarkPullSubscriptionUnknown(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { + cs.Manage(s).MarkUnknown(PullSubscriptionReady, reason, messageFormat, messageA...) +} + + // MarkPullSubscriptionReady sets the condition that the PubSub PullSubscription is ready. func (s *PubSubStatus) MarkPullSubscriptionReady(cs *apis.ConditionSet) { cs.Manage(s).MarkTrue(PullSubscriptionReady) } + +// MarkPullSubscriptionNotConfigured sets the condition that the PubSub PullSubscription has not yet been reconciled. +func (s *PubSubStatus) MarkPullSubscriptionNotConfigured(cs *apis.ConditionSet) { + cs.Manage(s).MarkUnknown(PullSubscriptionReady, "PullSubscriptionNotConfigured", "PullSubscription has not yet been reconciled") +} \ No newline at end of file diff --git a/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go b/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go index 8a91811e4b..bbce850706 100644 --- a/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go +++ b/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go @@ -36,22 +36,33 @@ func (s *AuditLogsSourceStatus) InitializeConditions() { auditLogsSourceCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionNotReady sets the condition that the underlying PullSubscription -// source is not ready and why. -func (s *AuditLogsSourceStatus) MarkPullSubscriptionNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkPullSubscriptionNotReady sets the condition that the status of underlying PullSubscription +// source is False and why. +func (s *AuditLogsSourceStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } +// MarkPullSubscriptionUnknown sets the condition that the status of underlying PullSubscription +// source is Unknown and why. +func (s *AuditLogsSourceStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { + auditLogsSourceCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) +} + // MarkPullSubscriptionReady sets the condition that the underlying PubSub source is ready. func (s *AuditLogsSourceStatus) MarkPullSubscriptionReady() { auditLogsSourceCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } -// MarkTopicNotReady sets the condition that the PubSub topic was not created and why. -func (s *AuditLogsSourceStatus) MarkTopicNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFalse sets the condition that the status of PubSub topic is False and why. +func (s *AuditLogsSourceStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) } +// MarkTopicUnknown sets the condition that the status of PubSub topic is Unknown and why. +func (s *AuditLogsSourceStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { + auditLogsSourceCondSet.Manage(s).MarkUnknown(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) +} + // MarkTopicReady sets the condition that the underlying PubSub topic was created successfully. func (s *AuditLogsSourceStatus) MarkTopicReady() { auditLogsSourceCondSet.Manage(s).MarkTrue(duckv1alpha1.TopicReady) diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go index 6774c18fd4..1ec1513529 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" @@ -37,24 +38,42 @@ func (ps *PubSubStatus) InitializeConditions() { pubSubCondSet.Manage(ps).InitializeConditions() } -// MarkPullSubscriptionNotReady sets the condition that the underlying PullSubscription -// source is not ready and why. -func (ps *PubSubStatus) MarkPullSubscriptionNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkPullSubscriptionFalse sets the condition that the underlying PullSubscription +// source is False and why. +func (ps *PubSubStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { pubSubCondSet.Manage(ps).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } +// MarkPullSubscriptionNotConfigured sets the condition that the underlying PullSubscription +// source has not been reconciled and why. +func (ps *PubSubStatus) MarkPullSubscriptionNotConfigured() { + pubSubCondSet.Manage(ps).MarkUnknown(duckv1alpha1.PullSubscriptionReady, "PullSubscriptionNotConfigured", "PullSubscription has not yet been reconciled") +} + // MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. func (ps *PubSubStatus) MarkPullSubscriptionReady() { pubSubCondSet.Manage(ps).MarkTrue(duckv1alpha1.PullSubscriptionReady) } +// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is Unknown and why. +func (ps *PubSubStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { + pubSubCondSet.Manage(ps).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) +} + func (ps *PubSubStatus) PropagatePullSubscriptionStatus(ready *apis.Condition) { + if ready == nil { + ps.MarkPullSubscriptionNotConfigured() + return + } + switch { - case ready == nil: - ps.MarkPullSubscriptionNotReady("PullSubscriptionNotReady", "PullSubscription has no Ready type status") - case ready.IsTrue(): + case ready.Status == corev1.ConditionUnknown: + ps.MarkPullSubscriptionUnknown(ready.Reason, ready.Message) + case ready.Status == corev1.ConditionTrue: ps.MarkPullSubscriptionReady() - case ready.IsFalse(): - ps.MarkPullSubscriptionNotReady(ready.Reason, ready.Message) + case ready.Status == corev1.ConditionFalse: + ps.MarkPullSubscriptionFalse(ready.Reason, ready.Message) + default: + ps.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "The status of PullSubscription is invalid: %v", ready.Status) } } diff --git a/pkg/apis/events/v1alpha1/scheduler_lifecycle.go b/pkg/apis/events/v1alpha1/scheduler_lifecycle.go index c176ec28ca..cbabcc041f 100644 --- a/pkg/apis/events/v1alpha1/scheduler_lifecycle.go +++ b/pkg/apis/events/v1alpha1/scheduler_lifecycle.go @@ -37,22 +37,33 @@ func (s *SchedulerStatus) InitializeConditions() { schedulerCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionNotReady sets the condition that the underlying PullSubscription -// is not ready and why -func (s *SchedulerStatus) MarkPullSubscriptionNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkPullSubscriptionFalse sets the condition that the underlying PullSubscription +// is False and why. +func (s *SchedulerStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { schedulerCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } -// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready +// MarkPullSubscriptionUnknown sets the condition that the underlying PullSubscription +// is Unknown and why. +func (s *SchedulerStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { + schedulerCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) +} + +// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. func (s *SchedulerStatus) MarkPullSubscriptionReady() { schedulerCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } -// MarkTopicNotReady sets the condition that the Topic was not created and why -func (s *SchedulerStatus) MarkTopicNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFalse sets the condition that the Topic was not created and why. +func (s *SchedulerStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { schedulerCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) } +// MarkTopicUnknown sets the condition that the status of Topic is Unknown and why. +func (s *SchedulerStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { + schedulerCondSet.Manage(s).MarkUnknown(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) +} + // MarkTopicReady sets the condition that the underlying Topic was created // successfully and sets the Status.TopicID to the specified topic // and Status.ProjectID to the specified project. diff --git a/pkg/apis/events/v1alpha1/storage_lifecycle.go b/pkg/apis/events/v1alpha1/storage_lifecycle.go index d7b891775f..127ddd9f7f 100644 --- a/pkg/apis/events/v1alpha1/storage_lifecycle.go +++ b/pkg/apis/events/v1alpha1/storage_lifecycle.go @@ -37,22 +37,33 @@ func (s *StorageStatus) InitializeConditions() { storageCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionNotReady sets the condition that the underlying PullSubscription -// source is not ready and why. -func (s *StorageStatus) MarkPullSubscriptionNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkPullSubscriptionFalse sets the condition that the status of underlying PullSubscription +// source is False and why. +func (s *StorageStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { storageCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } +// MarkPullSubscriptionUnknown sets the condition that the status of underlying PullSubscription +// source is Unknown and why. +func (s *StorageStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { + storageCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) +} + // MarkPullSubscriptionReady sets the condition that the underlying PubSub source is ready. func (s *StorageStatus) MarkPullSubscriptionReady() { storageCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } -// MarkTopicNotReady sets the condition that the PubSub topic was not created and why. -func (s *StorageStatus) MarkTopicNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFalse sets the condition that the status of PubSub topic is False why. +func (s *StorageStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { storageCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) } +// MarkTopicUnknown sets the condition that the status of PubSub topic is Unknown why. +func (s *StorageStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { + storageCondSet.Manage(s).MarkUnknown(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) +} + // MarkTopicReady sets the condition that the underlying PubSub topic was created successfully. func (s *StorageStatus) MarkTopicReady() { storageCondSet.Manage(s).MarkTrue(duckv1alpha1.TopicReady) diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index 53cf774931..cdd0b5ded9 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -17,6 +17,7 @@ package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" ) @@ -27,6 +28,11 @@ func (cs *ChannelStatus) GetCondition(t apis.ConditionType) *apis.Condition { return channelCondSet.Manage(cs).GetCondition(t) } +// GetTopLevelCondition returns the top level condition. +func (cs *ChannelStatus) GetTopLevelCondition() *apis.Condition { + return channelCondSet.Manage(cs).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (cs *ChannelStatus) IsReady() bool { return channelCondSet.Manage(cs).IsHappy() @@ -58,22 +64,38 @@ func (cs *ChannelStatus) MarkTopicReady() { } func (cs *ChannelStatus) PropagateTopicStatus(ready *apis.Condition) { + if ready == nil { + cs.MarkTopicNotConfigured() + return + } + switch { - case ready == nil: - cs.MarkNoTopic("TopicNotReady", "Topic has no Ready type status") - case ready.IsTrue(): + case ready.Status == corev1.ConditionUnknown: + cs.MarkTopicUnknown(ready.Reason, ready.Message) + case ready.Status == corev1.ConditionTrue: cs.MarkTopicReady() - case ready.IsFalse(): - cs.MarkNoTopic(ready.Reason, ready.Message) + case ready.Status == corev1.ConditionFalse: + cs.MarkTopicFalse(ready.Reason, ready.Message) + default: + cs.MarkTopicUnknown("TopicUnknown", "The status of Topic is invalid: %v", ready.Status) } } -// MarkNoTopic sets the condition that signals there is not a topic for this +// MarkTopicFalse sets the condition that signals there is not a topic for this // Channel. This could be because of an error or the Channel is being deleted. -func (cs *ChannelStatus) MarkNoTopic(reason, messageFormat string, messageA ...interface{}) { +func (cs *ChannelStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { channelCondSet.Manage(cs).MarkFalse(ChannelConditionTopicReady, reason, messageFormat, messageA...) } func (cs *ChannelStatus) MarkTopicNotOwned(messageFormat string, messageA ...interface{}) { channelCondSet.Manage(cs).MarkFalse(ChannelConditionTopicReady, "NotOwned", messageFormat, messageA...) } + +func (cs *ChannelStatus) MarkTopicNotConfigured() { + channelCondSet.Manage(cs).MarkUnknown(ChannelConditionTopicReady, + "TopicNotConfigured", "Topic has not yet been reconciled") +} + +func (cs *ChannelStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { + channelCondSet.Manage(cs).MarkUnknown(ChannelConditionTopicReady, reason, messageFormat, messageA...) +} diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index 09cf90151b..8d28e898f1 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -149,25 +149,30 @@ func TestChannelInitializeConditions(t *testing.T) { func TestChannelIsReady(t *testing.T) { tests := []struct { - name string - setAddress bool - markTopic bool - wantReady bool + name string + setAddress bool + topicStatus corev1.ConditionStatus + wantConditionStatus corev1.ConditionStatus }{{ - name: "all happy", - setAddress: true, - markTopic: true, - wantReady: true, + name: "all happy", + setAddress: true, + topicStatus: corev1.ConditionTrue, + wantConditionStatus: corev1.ConditionTrue, }, { - name: "address not set", - setAddress: false, - markTopic: true, - wantReady: false, + name: "address not set", + setAddress: false, + topicStatus: corev1.ConditionTrue, + wantConditionStatus: corev1.ConditionUnknown, }, { - name: "topic not ready", - setAddress: true, - markTopic: false, - wantReady: false, + name: "the status of topic is false", + setAddress: true, + topicStatus: corev1.ConditionFalse, + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "the status of topic is unknown", + setAddress: true, + topicStatus: corev1.ConditionUnknown, + wantConditionStatus: corev1.ConditionUnknown, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -176,14 +181,17 @@ func TestChannelIsReady(t *testing.T) { if test.setAddress { cs.SetAddress(&apis.URL{Scheme: "http", Host: "foo.bar"}) } - if test.markTopic { + if test.topicStatus == corev1.ConditionTrue { cs.MarkTopicReady() + } else if test.topicStatus == corev1.ConditionUnknown { + cs.MarkTopicUnknown("The status of topic is unknown", "The status of topic is unknown: nil") } else { - cs.MarkNoTopic("NoTopic", "UnitTest") + cs.MarkTopicFalse("The status of topic is false", "The status of topic is unknown: nil") } - got := cs.IsReady() - if test.wantReady != got { - t.Errorf("unexpected readiness: want %v, got %v", test.wantReady, got) + + got := cs.GetTopLevelCondition().Status + if test.wantConditionStatus != got { + t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) } }) } diff --git a/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle.go b/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle.go index be56de5a23..65b676a295 100644 --- a/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle.go +++ b/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle.go @@ -25,6 +25,11 @@ func (s *PullSubscriptionStatus) GetCondition(t apis.ConditionType) *apis.Condit return pullSubscriptionCondSet.Manage(s).GetCondition(t) } +// GetTopLevelCondition returns the top level Condition. +func (s *PullSubscriptionStatus) GetTopLevelCondition() *apis.Condition { + return pullSubscriptionCondSet.Manage(s).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (s *PullSubscriptionStatus) IsReady() bool { return pullSubscriptionCondSet.Manage(s).IsHappy() diff --git a/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go b/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go index 2a122ade97..092cb611a4 100644 --- a/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go +++ b/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go @@ -27,6 +27,11 @@ func (ts *TopicStatus) GetCondition(t apis.ConditionType) *apis.Condition { return topicCondSet.Manage(ts).GetCondition(t) } +// GetTopLevelCondition returns the top level condition +func (ts *TopicStatus) GetTopLevelCondition() *apis.Condition { + return topicCondSet.Manage(ts).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (ts *TopicStatus) IsReady() bool { return topicCondSet.Manage(ts).IsHappy() @@ -79,7 +84,7 @@ func (ts *TopicStatus) MarkTopicReady() { topicCondSet.Manage(ts).MarkTrue(TopicConditionTopicExists) } -// MarkNoTopic sets the condition that signals there is not a topic for this +// MarkTopicFalse sets the condition that signals there is not a topic for this // Topic. This could be because of an error or the Topic is being deleted. func (ts *TopicStatus) MarkNoTopic(reason, messageFormat string, messageA ...interface{}) { topicCondSet.Manage(ts).MarkFalse(TopicConditionTopicExists, reason, messageFormat, messageA...) diff --git a/pkg/reconciler/events/auditlogs/auditlogs_test.go b/pkg/reconciler/events/auditlogs/auditlogs_test.go index a4e8e31ee4..2b35d1a3e7 100644 --- a/pkg/reconciler/events/auditlogs/auditlogs_test.go +++ b/pkg/reconciler/events/auditlogs/auditlogs_test.go @@ -66,11 +66,13 @@ const ( sinkDNS = sinkName + ".mynamespace.svc.cluster.local" sinkURI = "http://" + sinkDNS + "/" - topicNotReadyMsg = `Topic "test-auditlogssource" not ready` - pullSubscriptionNotReadyMsg = `PullSubscription "test-auditlogssource" not ready` - failedToCreateSinkMsg = `failed to ensure creation of logging sink` - failedToSetPermissionsMsg = `failed to ensure sink has pubsub.publisher permission on source topic` - failedToDeleteSinkMsg = `Failed to delete Stackdriver sink` + topicNotReadyMsg = `Topic "test-auditlogssource" not ready` + pullSubscriptionNotReadyMsg = `PullSubscription "test-auditlogssource" not ready` + failedToReconcileTopicMsg = `Topic has not yet been reconciled` + failedToReconcilePullSubscriptionMsg = `PullSubscription has not yet been reconciled` + failedToCreateSinkMsg = `failed to ensure creation of logging sink` + failedToSetPermissionsMsg = `failed to ensure sink has pubsub.publisher permission on source topic` + failedToDeleteSinkMsg = `Failed to delete Stackdriver sink` ) var ( @@ -134,7 +136,7 @@ func TestAllCases(t *testing.T) { // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", }, { - Name: "topic created, not ready", + Name: "topic created, not yet been reconciled", Objects: []runtime.Object{ NewAuditLogsSource(sourceName, testNS), }, @@ -143,7 +145,7 @@ func TestAllCases(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: NewAuditLogsSource(sourceName, testNS, WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicNotReady("TopicNotReady", topicNotReadyMsg)), + WithAuditLogsSourceTopicUnknown("TopicNotConfigured", failedToReconcileTopicMsg)), }}, WantCreates: []runtime.Object{ NewTopic(sourceName, testNS, @@ -162,10 +164,10 @@ func TestAllCases(t *testing.T) { }, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "Updated", "Updated AuditLogsSource %q finalizers", sourceName), - Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q not ready", sourceName), + Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q has not yet been reconciled", sourceName), }, }, { - Name: "topic exists, topic not ready", + Name: "topic exists, topic has not yet been reconciled", Objects: []runtime.Object{ NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), @@ -180,10 +182,10 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicNotReady("TopicNotReady", topicNotReadyMsg)), + WithAuditLogsSourceTopicUnknown("TopicNotConfigured", failedToReconcileTopicMsg)), }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q not ready", sourceName), + Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q has not yet been reconciled", sourceName), }, }, { Name: "topic exists and is ready, no projectid", @@ -202,7 +204,7 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicNotReady("TopicNotReady", `Topic "test-auditlogssource" did not expose projectid`), + WithAuditLogsSourceTopicFalse("TopicNotReady", `Topic "test-auditlogssource" did not expose projectid`), ), }}, WantEvents: []string{ @@ -226,7 +228,7 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicNotReady("TopicNotReady", `Topic "test-auditlogssource" did not expose topicid`), + WithAuditLogsSourceTopicFalse("TopicNotReady", `Topic "test-auditlogssource" did not expose topicid`), ), }}, WantEvents: []string{ @@ -250,12 +252,56 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicNotReady("TopicNotReady", `Topic "test-auditlogssource" mismatch: expected "auditlogssource-test-auditlogssource-uid" got "garbaaaaage"`), + WithAuditLogsSourceTopicFalse("TopicNotReady", `Topic "test-auditlogssource" mismatch: expected "auditlogssource-test-auditlogssource-uid" got "garbaaaaage"`), ), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", `Topic %q mismatch: expected "auditlogssource-test-auditlogssource-uid" got "garbaaaaage"`, sourceName), }, + }, { + Name: "topic exists and the status of topic is false", + Objects: []runtime.Object{ + NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + ), + NewTopic(sourceName, testNS, + WithTopicFalse(), + WithTopicTopicID(testTopicID), + ), + }, + Key: testNS + "/" + sourceName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + WithInitAuditLogsSourceConditions, + WithAuditLogsSourceTopicFalse("PublisherStatus", "Publisher has no Ready type status")), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is False", sourceName), + }, + }, { + Name: "topic exists and the status of topic is unknown", + Objects: []runtime.Object{ + NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + ), + NewTopic(sourceName, testNS, + WithTopicUnknown(), + WithTopicTopicID(testTopicID), + ), + }, + Key: testNS + "/" + sourceName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + WithInitAuditLogsSourceConditions, + WithAuditLogsSourceTopicUnknown("", "")), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is Unknown", sourceName), + }, }, { Name: "topic exists and is ready, pullsubscription created", Objects: []runtime.Object{ @@ -278,7 +324,7 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceProjectID(testProject), WithInitAuditLogsSourceConditions, WithAuditLogsSourceTopicReady(testTopicID), - WithAuditLogsSourcePullSubscriptionNotReady("PullSubscriptionNotReady", pullSubscriptionNotReadyMsg), + WithAuditLogsSourcePullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilePullSubscriptionMsg), ), }}, WantCreates: []runtime.Object{ @@ -299,10 +345,10 @@ func TestAllCases(t *testing.T) { ), }, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q not ready", sourceName), + Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q has not yet been reconciled", sourceName), }, }, { - Name: "topic exists and ready, pullsubscription exists but is not ready", + Name: "topic exists and ready, pullsubscription exists but has not yet been reconciled", Objects: []runtime.Object{ NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), @@ -324,13 +370,71 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceProjectID(testProject), WithInitAuditLogsSourceConditions, WithAuditLogsSourceTopicReady(testTopicID), - WithAuditLogsSourcePullSubscriptionNotReady("PullSubscriptionNotReady", pullSubscriptionNotReadyMsg), + WithAuditLogsSourcePullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilePullSubscriptionMsg), ), }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q not ready", sourceName), + Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q has not yet been reconciled", sourceName), }, }, { + Name: "topic exists and ready, pullsubscription exists and the status of pullsubscription is false", + Objects: []runtime.Object{ + NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + WithAuditLogsSourceSink(sinkGVK, sinkName), + ), + NewTopic(sourceName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(sourceName, testNS, WithPullSubscriptionFalse()), + }, + Key: testNS + "/" + sourceName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + WithAuditLogsSourceSink(sinkGVK, sinkName), + WithAuditLogsSourceProjectID(testProject), + WithInitAuditLogsSourceConditions, + WithAuditLogsSourceTopicReady(testTopicID), + WithAuditLogsSourcePullSubscriptionFalse("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of PullSubscription %q is False", sourceName), + }, + }, { + Name: "topic exists and ready, pullsubscription exists and the status of pullsubscription is unknown", + Objects: []runtime.Object{ + NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + WithAuditLogsSourceSink(sinkGVK, sinkName), + ), + NewTopic(sourceName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(sourceName, testNS, WithPullSubscriptionUnknown()), + }, + Key: testNS + "/" + sourceName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewAuditLogsSource(sourceName, testNS, + WithAuditLogsSourceFinalizers(finalizerName), + WithAuditLogsSourceSink(sinkGVK, sinkName), + WithAuditLogsSourceProjectID(testProject), + WithInitAuditLogsSourceConditions, + WithAuditLogsSourceTopicReady(testTopicID), + WithAuditLogsSourcePullSubscriptionUnknown("", ""), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of PullSubscription %q is Unknown", sourceName), + }, + },{ Name: "logging client create fails", Objects: []runtime.Object{ NewAuditLogsSource(sourceName, testNS, @@ -776,8 +880,8 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceSink(sinkGVK, sinkName), WithInitAuditLogsSourceConditions, WithAuditLogsSourceSinkNotReady("SinkDeleted", "Successfully deleted Stackdriver sink: %s", testSinkID), - WithAuditLogsSourceTopicNotReady("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), - WithAuditLogsSourcePullSubscriptionNotReady("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), + WithAuditLogsSourceTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), + WithAuditLogsSourcePullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), WithAuditLogsSourceDeletionTimestamp, ), }}, @@ -834,8 +938,8 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceSink(sinkGVK, sinkName), WithInitAuditLogsSourceConditions, WithAuditLogsSourceSinkNotReady("SinkDeleted", "Successfully deleted Stackdriver sink: %s", testSinkID), - WithAuditLogsSourceTopicNotReady("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), - WithAuditLogsSourcePullSubscriptionNotReady("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), + WithAuditLogsSourceTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), + WithAuditLogsSourcePullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), WithAuditLogsSourceDeletionTimestamp, ), }}, diff --git a/pkg/reconciler/events/pubsub/pubsub.go b/pkg/reconciler/events/pubsub/pubsub.go index 206696d3d0..57ba8fa81d 100644 --- a/pkg/reconciler/events/pubsub/pubsub.go +++ b/pkg/reconciler/events/pubsub/pubsub.go @@ -124,7 +124,7 @@ func (r *Reconciler) reconcile(ctx context.Context, pubsub *v1alpha1.PubSub) err ps, err := r.reconcilePullSubscription(ctx, pubsub) if err != nil { - pubsub.Status.MarkPullSubscriptionNotReady("PullSubscriptionReconcileFailed", "Failed to reconcile PullSubscription: %s", err.Error()) + pubsub.Status.MarkPullSubscriptionFalse("PullSubscriptionReconcileFailed", "Failed to reconcile PullSubscription: %s", err.Error()) return err } pubsub.Status.PropagatePullSubscriptionStatus(ps.Status.GetCondition(apis.ConditionReady)) diff --git a/pkg/reconciler/events/pubsub/pubsub_test.go b/pkg/reconciler/events/pubsub/pubsub_test.go index 52c39f3bc3..540cced577 100644 --- a/pkg/reconciler/events/pubsub/pubsub_test.go +++ b/pkg/reconciler/events/pubsub/pubsub_test.go @@ -146,7 +146,7 @@ func TestAllCases(t *testing.T) { WithInitPubSubConditions, WithPubSubObjectMetaGeneration(generation), WithPubSubFinalizers(finalizerName), - WithPubSubPullSubscriptionNotReady("PullSubscriptionNotReady", "PullSubscription has no Ready type status"), + WithPubSubPullSubscriptionUnknown("PullSubscriptionNotConfigured", "PullSubscription has not yet been reconciled"), ), }}, WantCreates: []runtime.Object{ @@ -169,7 +169,7 @@ func TestAllCases(t *testing.T) { Eventf(corev1.EventTypeNormal, "Updated", "Updated PubSub %q", pubsubName), }, }, { - Name: "pullsubscription exists but is not ready", + Name: "pullsubscription exists and the status is false", Objects: []runtime.Object{ NewPubSub(pubsubName, testNS, WithPubSubObjectMetaGeneration(generation), @@ -178,7 +178,7 @@ func TestAllCases(t *testing.T) { WithPubSubFinalizers(finalizerName), ), NewPullSubscriptionWithNoDefaults(pubsubName, testNS, - WithPullSubscriptionReadyStatus(corev1.ConditionFalse, "PullSubscriptionNotReady", "no ready test message")), + WithPullSubscriptionReadyStatus(corev1.ConditionFalse, "PullSubscriptionFalse", "status false test message")), newSink(), }, Key: testNS + "/" + pubsubName, @@ -191,7 +191,36 @@ func TestAllCases(t *testing.T) { WithPubSubFinalizers(finalizerName), WithInitPubSubConditions, WithPubSubObjectMetaGeneration(generation), - WithPubSubPullSubscriptionNotReady("PullSubscriptionNotReady", "no ready test message"), + WithPubSubPullSubscriptionFalse("PullSubscriptionFalse", "status false test message"), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Updated", "Updated PubSub %q", pubsubName), + }, + }, { + Name: "pullsubscription exists and the status is unknown", + Objects: []runtime.Object{ + NewPubSub(pubsubName, testNS, + WithPubSubObjectMetaGeneration(generation), + WithPubSubTopic(testTopicID), + WithPubSubSink(sinkGVK, sinkName), + WithPubSubFinalizers(finalizerName), + ), + NewPullSubscriptionWithNoDefaults(pubsubName, testNS, + WithPullSubscriptionReadyStatus(corev1.ConditionUnknown, "PullSubscriptionUnknown", "status unknown test message")), + newSink(), + }, + Key: testNS + "/" + pubsubName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewPubSub(pubsubName, testNS, + WithPubSubObjectMetaGeneration(generation), + WithPubSubStatusObservedGeneration(generation), + WithPubSubTopic(testTopicID), + WithPubSubSink(sinkGVK, sinkName), + WithPubSubFinalizers(finalizerName), + WithInitPubSubConditions, + WithPubSubObjectMetaGeneration(generation), + WithPubSubPullSubscriptionUnknown("PullSubscriptionUnknown", "status unknown test message"), ), }}, WantEvents: []string{ diff --git a/pkg/reconciler/events/scheduler/scheduler_test.go b/pkg/reconciler/events/scheduler/scheduler_test.go index 1344f9fac7..6a9103e250 100644 --- a/pkg/reconciler/events/scheduler/scheduler_test.go +++ b/pkg/reconciler/events/scheduler/scheduler_test.go @@ -62,10 +62,10 @@ const ( onceAMinuteSchedule = "* * * * *" // Message for when the topic and pullsubscription with the above variables are not ready. - topicNotReadyMsg = `Topic "my-test-scheduler" not ready` - pullSubscriptionNotReadyMsg = `PullSubscription "my-test-scheduler" not ready` - failedToReconcileJobMsg = `Failed to reconcile Scheduler job` - failedToDeleteJobMsg = `Failed to delete Scheduler job` + failedToReconcileTopicMsg = `Topic has not yet been reconciled` + failedToReconcilePullSubscriptionMsg = `PullSubscription has not yet been reconciled` + failedToReconcileJobMsg = `Failed to reconcile Scheduler job` + failedToDeleteJobMsg = `Failed to delete Scheduler job` ) var ( @@ -176,7 +176,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicNotReady("TopicNotReady", topicNotReadyMsg), + WithSchedulerTopicUnknown("TopicNotConfigured", failedToReconcileTopicMsg), ), }}, WantCreates: []runtime.Object{ @@ -196,10 +196,10 @@ func TestAllCases(t *testing.T) { }, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q finalizers", schedulerName), - Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q not ready", schedulerName), + Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q has not yet been reconciled", schedulerName), }, }, { - Name: "topic exists, topic not ready", + Name: "topic exists, topic has not yet been reconciled", Objects: []runtime.Object{ NewScheduler(schedulerName, testNS, WithSchedulerSink(sinkGVK, sinkName), @@ -223,11 +223,11 @@ func TestAllCases(t *testing.T) { WithSchedulerSchedule(onceAMinuteSchedule), WithSchedulerFinalizers(finalizerName), WithInitSchedulerConditions, - WithSchedulerTopicNotReady("TopicNotReady", topicNotReadyMsg), + WithSchedulerTopicUnknown("TopicNotConfigured", failedToReconcileTopicMsg), ), }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q not ready", schedulerName), + Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q has not yet been reconciled", schedulerName), }, }, { Name: "topic exists and is ready, no projectid", @@ -254,7 +254,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicNotReady("TopicNotReady", `Topic "my-test-scheduler" did not expose projectid`), + WithSchedulerTopicFalse("TopicNotReady", `Topic "my-test-scheduler" did not expose projectid`), WithSchedulerFinalizers(finalizerName), ), }}, @@ -287,7 +287,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicNotReady("TopicNotReady", `Topic "my-test-scheduler" did not expose topicid`), + WithSchedulerTopicFalse("TopicNotReady", `Topic "my-test-scheduler" did not expose topicid`), WithSchedulerFinalizers(finalizerName), ), }}, @@ -320,7 +320,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicNotReady("TopicNotReady", `Topic "my-test-scheduler" mismatch: expected "scheduler-test-scheduler-uid" got "garbaaaaage"`), + WithSchedulerTopicFalse("TopicNotReady", `Topic "my-test-scheduler" mismatch: expected "scheduler-test-scheduler-uid" got "garbaaaaage"`), WithSchedulerFinalizers(finalizerName), ), }}, @@ -328,7 +328,7 @@ func TestAllCases(t *testing.T) { Eventf(corev1.EventTypeWarning, "InternalError", `Topic %q mismatch: expected "scheduler-test-scheduler-uid" got "garbaaaaage"`, schedulerName), }, }, { - Name: "topic exists and is ready, pullsubscription created", + Name: "topic exists and the status topic is false", Objects: []runtime.Object{ NewScheduler(schedulerName, testNS, WithSchedulerSink(sinkGVK, sinkName), @@ -338,8 +338,7 @@ func TestAllCases(t *testing.T) { WithSchedulerFinalizers(finalizerName), ), NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), + WithTopicFalse(), WithTopicProjectID(testProject), ), newSink(), @@ -352,33 +351,16 @@ func TestAllCases(t *testing.T) { WithSchedulerLocation(location), WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), WithSchedulerFinalizers(finalizerName), - WithSchedulerPullSubscriptionNotReady("PullSubscriptionNotReady", pullSubscriptionNotReadyMsg), + WithInitSchedulerConditions, + WithSchedulerTopicFalse("PublisherStatus", "Publisher has no Ready type status"), ), }}, - WantCreates: []runtime.Object{ - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionSpecWithNoDefaults(pubsubv1alpha1.PullSubscriptionSpec{ - Topic: testTopicID, - Secret: &secret, - }), - WithPullSubscriptionSink(sinkGVK, sinkName), - WithPullSubscriptionLabels(map[string]string{ - "receive-adapter": receiveAdapterName, - }), - WithPullSubscriptionAnnotations(map[string]string{ - "metrics-resource-group": resourceGroup, - }), - WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), - ), - }, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q not ready", schedulerName), + Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is False", schedulerName), }, }, { - Name: "topic exists and ready, pullsubscription exists but is not ready", + Name: "topic exists and the status topic is unknown", Objects: []runtime.Object{ NewScheduler(schedulerName, testNS, WithSchedulerSink(sinkGVK, sinkName), @@ -388,11 +370,9 @@ func TestAllCases(t *testing.T) { WithSchedulerFinalizers(finalizerName), ), NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), + WithTopicUnknown(), WithTopicProjectID(testProject), ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS), newSink(), }, Key: testNS + "/" + schedulerName, @@ -405,453 +385,608 @@ func TestAllCases(t *testing.T) { WithSchedulerSchedule(onceAMinuteSchedule), WithSchedulerFinalizers(finalizerName), WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionNotReady("PullSubscriptionNotReady", pullSubscriptionNotReadyMsg), + WithSchedulerTopicUnknown("", ""), ), }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q not ready", schedulerName), - }, - }, { - Name: "topic and pullsubscription exist and ready, create client fails", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - CreateClientErr: errors.New("create-client-induced-error"), + Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is Unknown", schedulerName), + }, + }, + { + Name: "topic exists and is ready, pullsubscription created", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + newSink(), }, - }, - Key: testNS + "/" + schedulerName, - WantErr: true, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileJobMsg, "create-client-induced-error")), - WithSchedulerSinkURI(schedulerSinkURL)), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "create-client-induced-error"), - }, - }, { - Name: "topic and pullsubscription exist and ready, get job fails with non-grpc", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - GetJobErr: errors.New("get-job-induced-error"), + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerFinalizers(finalizerName), + WithSchedulerPullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilePullSubscriptionMsg), + ), + }}, + WantCreates: []runtime.Object{ + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionSpecWithNoDefaults(pubsubv1alpha1.PullSubscriptionSpec{ + Topic: testTopicID, + Secret: &secret, + }), + WithPullSubscriptionSink(sinkGVK, sinkName), + WithPullSubscriptionLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + WithPullSubscriptionAnnotations(map[string]string{ + "metrics-resource-group": resourceGroup, + }), + WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), }, - }, - Key: testNS + "/" + schedulerName, - WantErr: true, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileJobMsg, "get-job-induced-error")), - WithSchedulerSinkURI(schedulerSinkURL)), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "get-job-induced-error"), - }, - }, { - Name: "topic and pullsubscription exist and ready, get job fails with grpc unknown error", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - GetJobErr: gstatus.Error(codes.Unknown, "get-job-induced-error"), + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q has not yet been reconciled", schedulerName), }, - }, - Key: testNS + "/" + schedulerName, - WantErr: true, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: rpc error: code = %s desc = %s", failedToReconcileJobMsg, codes.Unknown, "get-job-induced-error")), - WithSchedulerSinkURI(schedulerSinkURL)), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", fmt.Sprintf("rpc error: code = %s desc = %s", codes.Unknown, "get-job-induced-error")), - }, - }, { - Name: "topic and pullsubscription exist and ready, get job fails with grpc not found error, create job fails", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - GetJobErr: gstatus.Error(codes.NotFound, "get-job-induced-error"), - CreateJobErr: errors.New("create-job-induced-error"), + }, { + Name: "topic exists and ready, pullsubscription exists but has not yet been reconciled", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS), + newSink(), }, - }, - Key: testNS + "/" + schedulerName, - WantErr: true, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileJobMsg, "create-job-induced-error")), - WithSchedulerSinkURI(schedulerSinkURL)), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "create-job-induced-error"), - }, - }, { - Name: "topic and pullsubscription exist and ready, get job fails with grpc not found error, create job succeeds", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - GetJobErr: gstatus.Error(codes.NotFound, "get-job-induced-error"), + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilePullSubscriptionMsg), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q has not yet been reconciled", schedulerName), }, - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobReady(jobName), - WithSchedulerSinkURI(schedulerSinkURL)), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "ReadinessChanged", "Scheduler %q became ready", schedulerName), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q", schedulerName), - }, - }, { - Name: "topic and pullsubscription exist and ready, job exists", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobReady(jobName), - WithSchedulerSinkURI(schedulerSinkURL)), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "ReadinessChanged", "Scheduler %q became ready", schedulerName), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q", schedulerName), - }, - }, { - Name: "scheduler job fails to delete with no-grpc error", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobReady(jobName), - WithSchedulerSinkURI(schedulerSinkURL), - WithSchedulerDeletionTimestamp, - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobName(jobName), - WithSchedulerJobNotReady("JobDeleteFailed", fmt.Sprintf("%s: %s", failedToDeleteJobMsg, "delete-job-induced-error")), - WithSchedulerSinkURI(schedulerSinkURL), - WithSchedulerDeletionTimestamp, - ), - }}, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - DeleteJobErr: errors.New("delete-job-induced-error"), + }, { + Name: "topic exists and ready, pullsubscription exists and the status of pullsubscription is false", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, WithPullSubscriptionFalse()), + newSink(), }, - }, - WantErr: true, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "delete-job-induced-error"), - }, - }, { - Name: "scheduler job fails to delete with Unknown grpc error", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobReady(jobName), - WithSchedulerSinkURI(schedulerSinkURL), - WithSchedulerDeletionTimestamp, - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobName(jobName), - WithSchedulerJobNotReady("JobDeleteFailed", fmt.Sprintf("%s: rpc error: code = %s desc = %s", failedToDeleteJobMsg, codes.Unknown, "delete-job-induced-error")), - WithSchedulerSinkURI(schedulerSinkURL), - WithSchedulerDeletionTimestamp, - ), - }}, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - DeleteJobErr: gstatus.Error(codes.Unknown, "delete-job-induced-error"), + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionFalse("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of PullSubscription %q is False", schedulerName), }, - }, - WantErr: true, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", fmt.Sprintf("rpc error: code = %s desc = %s", codes.Unknown, "delete-job-induced-error")), - }, - }, { - Name: "scheduler successfully deleted with NotFound grpc error", - Objects: []runtime.Object{ - NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionReady(), - WithSchedulerJobReady(jobName), - WithSchedulerSinkURI(schedulerSinkURL), - WithSchedulerDeletionTimestamp, - ), - NewTopic(schedulerName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewScheduler(schedulerName, testNS, - WithSchedulerProject(testProject), - WithSchedulerSink(sinkGVK, sinkName), - WithSchedulerLocation(location), - WithSchedulerFinalizers(finalizerName), - WithSchedulerData(testData), - WithSchedulerSchedule(onceAMinuteSchedule), - WithInitSchedulerConditions, - WithSchedulerJobNotReady("JobDeleted", fmt.Sprintf("Successfully deleted Scheduler job: %s", jobName)), - WithSchedulerTopicNotReady("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", schedulerName)), - WithSchedulerPullSubscriptionNotReady("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", schedulerName)), - WithSchedulerDeletionTimestamp, - ), - }}, - WantDeletes: []clientgotesting.DeleteActionImpl{ - {ActionImpl: clientgotesting.ActionImpl{ - Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "topics"}}, - Name: schedulerName, + }, { + Name: "topic exists and ready, pullsubscription exists and the status of pullsubscription is unknown", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, WithPullSubscriptionUnknown()), + newSink(), }, - {ActionImpl: clientgotesting.ActionImpl{ - Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "pullsubscriptions"}}, - Name: schedulerName, + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithSchedulerFinalizers(finalizerName), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionUnknown("", ""), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of PullSubscription %q is Unknown", schedulerName), }, - }, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, schedulerName, false), - }, - OtherTestData: map[string]interface{}{ - "scheduler": gscheduler.TestClientData{ - DeleteJobErr: gstatus.Error(codes.NotFound, "delete-job-induced-error"), + }, { + Name: "topic and pullsubscription exist and ready, create client fails", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), }, - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q finalizers", schedulerName), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q", schedulerName), - }, - }} + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + CreateClientErr: errors.New("create-client-induced-error"), + }, + }, + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileJobMsg, "create-client-induced-error")), + WithSchedulerSinkURI(schedulerSinkURL)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "create-client-induced-error"), + }, + }, { + Name: "topic and pullsubscription exist and ready, get job fails with non-grpc", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + GetJobErr: errors.New("get-job-induced-error"), + }, + }, + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileJobMsg, "get-job-induced-error")), + WithSchedulerSinkURI(schedulerSinkURL)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "get-job-induced-error"), + }, + }, { + Name: "topic and pullsubscription exist and ready, get job fails with grpc unknown error", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + GetJobErr: gstatus.Error(codes.Unknown, "get-job-induced-error"), + }, + }, + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: rpc error: code = %s desc = %s", failedToReconcileJobMsg, codes.Unknown, "get-job-induced-error")), + WithSchedulerSinkURI(schedulerSinkURL)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", fmt.Sprintf("rpc error: code = %s desc = %s", codes.Unknown, "get-job-induced-error")), + }, + }, { + Name: "topic and pullsubscription exist and ready, get job fails with grpc not found error, create job fails", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + GetJobErr: gstatus.Error(codes.NotFound, "get-job-induced-error"), + CreateJobErr: errors.New("create-job-induced-error"), + }, + }, + Key: testNS + "/" + schedulerName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobNotReady("JobReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileJobMsg, "create-job-induced-error")), + WithSchedulerSinkURI(schedulerSinkURL)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "create-job-induced-error"), + }, + }, { + Name: "topic and pullsubscription exist and ready, get job fails with grpc not found error, create job succeeds", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + GetJobErr: gstatus.Error(codes.NotFound, "get-job-induced-error"), + }, + }, + Key: testNS + "/" + schedulerName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobReady(jobName), + WithSchedulerSinkURI(schedulerSinkURL)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "ReadinessChanged", "Scheduler %q became ready", schedulerName), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q", schedulerName), + }, + }, { + Name: "topic and pullsubscription exist and ready, job exists", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + schedulerName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobReady(jobName), + WithSchedulerSinkURI(schedulerSinkURL)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "ReadinessChanged", "Scheduler %q became ready", schedulerName), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q", schedulerName), + }, + }, { + Name: "scheduler job fails to delete with no-grpc error", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobReady(jobName), + WithSchedulerSinkURI(schedulerSinkURL), + WithSchedulerDeletionTimestamp, + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + schedulerName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobName(jobName), + WithSchedulerJobNotReady("JobDeleteFailed", fmt.Sprintf("%s: %s", failedToDeleteJobMsg, "delete-job-induced-error")), + WithSchedulerSinkURI(schedulerSinkURL), + WithSchedulerDeletionTimestamp, + ), + }}, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + DeleteJobErr: errors.New("delete-job-induced-error"), + }, + }, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "delete-job-induced-error"), + }, + }, { + Name: "scheduler job fails to delete with Unknown grpc error", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobReady(jobName), + WithSchedulerSinkURI(schedulerSinkURL), + WithSchedulerDeletionTimestamp, + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + schedulerName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobName(jobName), + WithSchedulerJobNotReady("JobDeleteFailed", fmt.Sprintf("%s: rpc error: code = %s desc = %s", failedToDeleteJobMsg, codes.Unknown, "delete-job-induced-error")), + WithSchedulerSinkURI(schedulerSinkURL), + WithSchedulerDeletionTimestamp, + ), + }}, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + DeleteJobErr: gstatus.Error(codes.Unknown, "delete-job-induced-error"), + }, + }, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", fmt.Sprintf("rpc error: code = %s desc = %s", codes.Unknown, "delete-job-induced-error")), + }, + }, { + Name: "scheduler successfully deleted with NotFound grpc error", + Objects: []runtime.Object{ + NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerTopicReady(testTopicID, testProject), + WithSchedulerPullSubscriptionReady(), + WithSchedulerJobReady(jobName), + WithSchedulerSinkURI(schedulerSinkURL), + WithSchedulerDeletionTimestamp, + ), + NewTopic(schedulerName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + schedulerName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewScheduler(schedulerName, testNS, + WithSchedulerProject(testProject), + WithSchedulerSink(sinkGVK, sinkName), + WithSchedulerLocation(location), + WithSchedulerFinalizers(finalizerName), + WithSchedulerData(testData), + WithSchedulerSchedule(onceAMinuteSchedule), + WithInitSchedulerConditions, + WithSchedulerJobNotReady("JobDeleted", fmt.Sprintf("Successfully deleted Scheduler job: %s", jobName)), + WithSchedulerTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", schedulerName)), + WithSchedulerPullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", schedulerName)), + WithSchedulerDeletionTimestamp, + ), + }}, + WantDeletes: []clientgotesting.DeleteActionImpl{ + {ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "topics"}}, + Name: schedulerName, + }, + {ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "pullsubscriptions"}}, + Name: schedulerName, + }, + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(testNS, schedulerName, false), + }, + OtherTestData: map[string]interface{}{ + "scheduler": gscheduler.TestClientData{ + DeleteJobErr: gstatus.Error(codes.NotFound, "delete-job-induced-error"), + }, + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q finalizers", schedulerName), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Scheduler %q", schedulerName), + }, + }} defer logtesting.ClearAll() table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, testData map[string]interface{}) controller.Reconciler { diff --git a/pkg/reconciler/events/storage/storage_test.go b/pkg/reconciler/events/storage/storage_test.go index f1023e35e9..67e943dacc 100644 --- a/pkg/reconciler/events/storage/storage_test.go +++ b/pkg/reconciler/events/storage/storage_test.go @@ -60,10 +60,10 @@ const ( generation = 1 // Message for when the topic and pullsubscription with the above variables are not ready. - topicNotReadyMsg = `Topic "my-test-storage" not ready` - pullSubscriptionNotReadyMsg = `PullSubscription "my-test-storage" not ready` - failedToReconcileNotificationMsg = `Failed to reconcile Storage notification` - failedToDeleteNotificationMsg = `Failed to delete Storage notification` + failedToReconcileTopicMsg = `Topic has not yet been reconciled` + failedToReconcilepullSubscriptionMsg = `PullSubscription has not yet been reconciled` + failedToReconcileNotificationMsg = `Failed to reconcile Storage notification` + failedToDeleteNotificationMsg = `Failed to delete Storage notification` ) var ( @@ -155,7 +155,7 @@ func TestAllCases(t *testing.T) { // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", }, { - Name: "topic created, not ready", + Name: "topic created, not yet been reconciled", Objects: []runtime.Object{ NewStorage(storageName, testNS, WithStorageObjectMetaGeneration(generation), @@ -172,7 +172,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicNotReady("TopicNotReady", topicNotReadyMsg), + WithStorageTopicUnknown("TopicNotConfigured", failedToReconcileTopicMsg), ), }}, WantCreates: []runtime.Object{ @@ -192,10 +192,10 @@ func TestAllCases(t *testing.T) { }, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q finalizers", storageName), - Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q not ready", storageName), + Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q has not yet been reconciled", storageName), }, }, { - Name: "topic exists, topic not ready", + Name: "topic exists, topic not yet been reconciled", Objects: []runtime.Object{ NewStorage(storageName, testNS, WithStorageObjectMetaGeneration(generation), @@ -217,11 +217,11 @@ func TestAllCases(t *testing.T) { WithStorageSink(sinkGVK, sinkName), WithStorageFinalizers(finalizerName), WithInitStorageConditions, - WithStorageTopicNotReady("TopicNotReady", topicNotReadyMsg), + WithStorageTopicUnknown("TopicNotConfigured", failedToReconcileTopicMsg), ), }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q not ready", storageName), + Eventf(corev1.EventTypeWarning, "InternalError", "Topic %q has not yet been reconciled", storageName), }, }, { Name: "topic exists and is ready, no projectid", @@ -246,7 +246,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicNotReady("TopicNotReady", `Topic "my-test-storage" did not expose projectid`), + WithStorageTopicFalse("TopicNotReady", `Topic "my-test-storage" did not expose projectid`), WithStorageFinalizers(finalizerName), ), }}, @@ -277,7 +277,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicNotReady("TopicNotReady", `Topic "my-test-storage" did not expose topicid`), + WithStorageTopicFalse("TopicNotReady", `Topic "my-test-storage" did not expose topicid`), WithStorageFinalizers(finalizerName), ), }}, @@ -308,7 +308,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicNotReady("TopicNotReady", `Topic "my-test-storage" mismatch: expected "storage-test-storage-uid" got "garbaaaaage"`), + WithStorageTopicFalse("TopicNotReady", `Topic "my-test-storage" mismatch: expected "storage-test-storage-uid" got "garbaaaaage"`), WithStorageFinalizers(finalizerName), ), }}, @@ -316,7 +316,7 @@ func TestAllCases(t *testing.T) { Eventf(corev1.EventTypeWarning, "InternalError", `Topic %q mismatch: expected "storage-test-storage-uid" got "garbaaaaage"`, storageName), }, }, { - Name: "topic exists and is ready, pullsubscription created", + Name: "topic exists and the status of topic is false", Objects: []runtime.Object{ NewStorage(storageName, testNS, WithStorageObjectMetaGeneration(generation), @@ -325,8 +325,7 @@ func TestAllCases(t *testing.T) { WithStorageFinalizers(finalizerName), ), NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), + WithTopicFalse(), WithTopicProjectID(testProject), ), newSink(), @@ -339,33 +338,15 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicReady(testTopicID), - WithStorageProjectID(testProject), + WithStorageTopicFalse("PublisherStatus", "Publisher has no Ready type status"), WithStorageFinalizers(finalizerName), - WithStoragePullSubscriptionNotReady("PullSubscriptionNotReady", pullSubscriptionNotReadyMsg), ), }}, - WantCreates: []runtime.Object{ - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionSpecWithNoDefaults(pubsubv1alpha1.PullSubscriptionSpec{ - Topic: testTopicID, - Secret: &secret, - }), - WithPullSubscriptionSink(sinkGVK, sinkName), - WithPullSubscriptionLabels(map[string]string{ - "receive-adapter": receiveAdapterName, - }), - WithPullSubscriptionAnnotations(map[string]string{ - "metrics-resource-group": resourceGroup, - }), - WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), - ), - }, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q not ready", storageName), + Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is False", storageName), }, }, { - Name: "topic exists and ready, pullsubscription exists but is not ready", + Name: "topic exists and the status of topic is unknown", Objects: []runtime.Object{ NewStorage(storageName, testNS, WithStorageObjectMetaGeneration(generation), @@ -374,11 +355,9 @@ func TestAllCases(t *testing.T) { WithStorageFinalizers(finalizerName), ), NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), + WithTopicUnknown(), WithTopicProjectID(testProject), ), - NewPullSubscriptionWithNoDefaults(storageName, testNS), newSink(), }, Key: testNS + "/" + storageName, @@ -388,25 +367,21 @@ func TestAllCases(t *testing.T) { WithStorageObjectMetaGeneration(generation), WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), - WithStorageFinalizers(finalizerName), WithInitStorageConditions, - WithStorageTopicReady(testTopicID), - WithStorageProjectID(testProject), - WithStoragePullSubscriptionNotReady("PullSubscriptionNotReady", pullSubscriptionNotReadyMsg), + WithStorageTopicUnknown("", ""), + WithStorageFinalizers(finalizerName), ), }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q not ready", storageName), + Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is Unknown", storageName), }, }, { - Name: "client create fails", + Name: "topic exists and is ready, pullsubscription created", Objects: []runtime.Object{ NewStorage(storageName, testNS, - WithStorageProject(testProject), WithStorageObjectMetaGeneration(generation), WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), WithStorageFinalizers(finalizerName), ), NewTopic(storageName, testNS, @@ -414,375 +389,528 @@ func TestAllCases(t *testing.T) { WithTopicAddress(testTopicURI), WithTopicProjectID(testProject), ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), - ), newSink(), }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - CreateClientErr: errors.New("create-client-induced-error"), - }, - }, + Key: testNS + "/" + storageName, WantErr: true, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "create-client-induced-error"), - }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), WithStorageObjectMetaGeneration(generation), WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), WithStorageTopicReady(testTopicID), WithStorageProjectID(testProject), - WithStoragePullSubscriptionReady(), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationNotReady("NotificationReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileNotificationMsg, "create-client-induced-error")), - ), - }}, - }, { - Name: "bucket notifications fails", - Objects: []runtime.Object{ - NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), WithStorageFinalizers(finalizerName), + WithStoragePullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilepullSubscriptionMsg), ), - NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), + }}, + WantCreates: []runtime.Object{ NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), + WithPullSubscriptionSpecWithNoDefaults(pubsubv1alpha1.PullSubscriptionSpec{ + Topic: testTopicID, + Secret: &secret, + }), + WithPullSubscriptionSink(sinkGVK, sinkName), + WithPullSubscriptionLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + WithPullSubscriptionAnnotations(map[string]string{ + "metrics-resource-group": resourceGroup, + }), + WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), ), - newSink(), - }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - BucketData: gstorage.TestBucketData{ - NotificationsErr: errors.New("bucket-notifications-induced-error"), - }, - }, }, - WantErr: true, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "bucket-notifications-induced-error"), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), - WithStorageTopicReady(testTopicID), - WithStorageProjectID(testProject), - WithStoragePullSubscriptionReady(), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationNotReady("NotificationReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileNotificationMsg, "bucket-notifications-induced-error")), - ), - }}, - }, { - Name: "bucket add notification fails", - Objects: []runtime.Object{ - NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - ), - NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), + Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q has not yet been reconciled", storageName), + }, + }, + { + Name: "topic exists and ready, pullsubscription exists but has not yet been reconciled", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS), + newSink(), + }, + Key: testNS + "/" + storageName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilepullSubscriptionMsg), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "PullSubscription %q has not yet been reconciled", storageName), + }, + }, { + Name: "topic exists and ready, pullsubscription exists and the status of pullsubscription is false", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, WithPullSubscriptionFalse()), + }, + Key: testNS + "/" + storageName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionFalse("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of PullSubscription %q is False", storageName), + }, + }, { + Name: "topic exists and ready, pullsubscription exists and the status of pullsubscription is unknown", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, WithPullSubscriptionUnknown()), + }, + Key: testNS + "/" + storageName, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionUnknown("", ""), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "the status of PullSubscription %q is Unknown", storageName), + }, }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - BucketData: gstorage.TestBucketData{ - AddNotificationErr: errors.New("bucket-add-notification-induced-error"), + { + Name: "client create fails", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + CreateClientErr: errors.New("create-client-induced-error"), }, }, - }, - WantErr: true, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "bucket-add-notification-induced-error"), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), - WithStorageTopicReady(testTopicID), - WithStorageProjectID(testProject), - WithStoragePullSubscriptionReady(), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationNotReady("NotificationReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileNotificationMsg, "bucket-add-notification-induced-error")), - ), - }}, - }, { - Name: "successfully created notification", - Objects: []runtime.Object{ - NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - ), - NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - BucketData: gstorage.TestBucketData{ - AddNotificationID: notificationId, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "create-client-induced-error"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionReady(), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationNotReady("NotificationReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileNotificationMsg, "create-client-induced-error")), + ), + }}, + }, { + Name: "bucket notifications fails", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + BucketData: gstorage.TestBucketData{ + NotificationsErr: errors.New("bucket-notifications-induced-error"), + }, }, }, - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "ReadinessChanged", "Storage %q became ready", storageName), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q", storageName), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageStatusObservedGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), - WithStorageTopicReady(testTopicID), - WithStorageProjectID(testProject), - WithStoragePullSubscriptionReady(), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationReady(notificationId)), - }}, - }, { - Name: "delete fails with non grpc error", - Objects: []runtime.Object{ - NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationReady(notificationId), - WithStorageTopicReady(testTopicID), - WithDeletionTimestamp(), - ), - NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - BucketData: gstorage.TestBucketData{ - Notifications: map[string]*storage.Notification{ - notificationId: { - ID: notificationId, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "bucket-notifications-induced-error"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionReady(), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationNotReady("NotificationReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileNotificationMsg, "bucket-notifications-induced-error")), + ), + }}, + }, { + Name: "bucket add notification fails", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + BucketData: gstorage.TestBucketData{ + AddNotificationErr: errors.New("bucket-add-notification-induced-error"), + }, + }, + }, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "bucket-add-notification-induced-error"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionReady(), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationNotReady("NotificationReconcileFailed", fmt.Sprintf("%s: %s", failedToReconcileNotificationMsg, "bucket-add-notification-induced-error")), + ), + }}, + }, { + Name: "successfully created notification", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + BucketData: gstorage.TestBucketData{ + AddNotificationID: notificationId, + }, + }, + }, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "ReadinessChanged", "Storage %q became ready", storageName), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q", storageName), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageStatusObservedGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageTopicReady(testTopicID), + WithStorageProjectID(testProject), + WithStoragePullSubscriptionReady(), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationReady(notificationId)), + }}, + }, { + Name: "delete fails with non grpc error", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationReady(notificationId), + WithStorageTopicReady(testTopicID), + WithDeletionTimestamp(), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + BucketData: gstorage.TestBucketData{ + Notifications: map[string]*storage.Notification{ + notificationId: { + ID: notificationId, + }, }, + DeleteErr: errors.New("delete-notification-induced-error"), }, - DeleteErr: errors.New("delete-notification-induced-error"), }, }, - }, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "delete-notification-induced-error"), - }, - WantErr: true, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationNotReady("NotificationDeleteFailed", fmt.Sprintf("%s: %s", failedToDeleteNotificationMsg, "delete-notification-induced-error")), - WithStorageNotificationID(notificationId), - WithStorageTopicReady(testTopicID), - WithDeletionTimestamp()), - }}, - }, { - Name: "delete fails with Unknown grpc error", - Objects: []runtime.Object{ - NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationReady(notificationId), - WithStorageTopicReady(testTopicID), - WithDeletionTimestamp(), - ), - NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - BucketData: gstorage.TestBucketData{ - Notifications: map[string]*storage.Notification{ - notificationId: { - ID: notificationId, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "delete-notification-induced-error"), + }, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationNotReady("NotificationDeleteFailed", fmt.Sprintf("%s: %s", failedToDeleteNotificationMsg, "delete-notification-induced-error")), + WithStorageNotificationID(notificationId), + WithStorageTopicReady(testTopicID), + WithDeletionTimestamp()), + }}, + }, { + Name: "delete fails with Unknown grpc error", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationReady(notificationId), + WithStorageTopicReady(testTopicID), + WithDeletionTimestamp(), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + BucketData: gstorage.TestBucketData{ + Notifications: map[string]*storage.Notification{ + notificationId: { + ID: notificationId, + }, }, + DeleteErr: gstatus.Error(codes.Unknown, "delete-notification-induced-error"), }, - DeleteErr: gstatus.Error(codes.Unknown, "delete-notification-induced-error"), }, }, - }, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "InternalError", "rpc error: code = %s desc = %s", codes.Unknown, "delete-notification-induced-error"), - }, - WantErr: true, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationNotReady("NotificationDeleteFailed", fmt.Sprintf("%s: rpc error: code = %s desc = %s", failedToDeleteNotificationMsg, codes.Unknown, "delete-notification-induced-error")), - WithStorageNotificationID(notificationId), - WithStorageTopicReady(testTopicID), - WithDeletionTimestamp()), - }}, - }, { - Name: "successfully deleted storage", - Objects: []runtime.Object{ - NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithStorageSinkURI(storageSinkURL), - WithStorageNotificationReady(notificationId), - WithStorageTopicReady(testTopicID), - WithDeletionTimestamp(), - ), - NewTopic(storageName, testNS, - WithTopicReady(testTopicID), - WithTopicAddress(testTopicURI), - WithTopicProjectID(testProject), - ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, - WithPullSubscriptionReady(sinkURI), - ), - newSink(), - }, - Key: testNS + "/" + storageName, - OtherTestData: map[string]interface{}{ - "storage": gstorage.TestClientData{ - BucketData: gstorage.TestBucketData{ - Notifications: map[string]*storage.Notification{ - notificationId: { - ID: notificationId, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", "rpc error: code = %s desc = %s", codes.Unknown, "delete-notification-induced-error"), + }, + WantErr: true, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationNotReady("NotificationDeleteFailed", fmt.Sprintf("%s: rpc error: code = %s desc = %s", failedToDeleteNotificationMsg, codes.Unknown, "delete-notification-induced-error")), + WithStorageNotificationID(notificationId), + WithStorageTopicReady(testTopicID), + WithDeletionTimestamp()), + }}, + }, { + Name: "successfully deleted storage", + Objects: []runtime.Object{ + NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithStorageSinkURI(storageSinkURL), + WithStorageNotificationReady(notificationId), + WithStorageTopicReady(testTopicID), + WithDeletionTimestamp(), + ), + NewTopic(storageName, testNS, + WithTopicReady(testTopicID), + WithTopicAddress(testTopicURI), + WithTopicProjectID(testProject), + ), + NewPullSubscriptionWithNoDefaults(storageName, testNS, + WithPullSubscriptionReady(sinkURI), + ), + newSink(), + }, + Key: testNS + "/" + storageName, + OtherTestData: map[string]interface{}{ + "storage": gstorage.TestClientData{ + BucketData: gstorage.TestBucketData{ + Notifications: map[string]*storage.Notification{ + notificationId: { + ID: notificationId, + }, }, }, }, }, - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q finalizers", storageName), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q", storageName), - }, - WantDeletes: []clientgotesting.DeleteActionImpl{ - {ActionImpl: clientgotesting.ActionImpl{ - Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "topics"}}, - Name: storageName, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q finalizers", storageName), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Storage %q", storageName), }, - {ActionImpl: clientgotesting.ActionImpl{ - Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "pullsubscriptions"}}, - Name: storageName, + WantDeletes: []clientgotesting.DeleteActionImpl{ + {ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "topics"}}, + Name: storageName, + }, + {ActionImpl: clientgotesting.ActionImpl{ + Namespace: testNS, Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "pullsubscriptions"}}, + Name: storageName, + }, }, - }, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, storageName, false), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewStorage(storageName, testNS, - WithStorageProject(testProject), - WithStorageObjectMetaGeneration(generation), - WithStorageStatusObservedGeneration(generation), - WithStorageBucket(bucket), - WithStorageSink(sinkGVK, sinkName), - WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), - WithStorageFinalizers(finalizerName), - WithInitStorageConditions, - WithStorageObjectMetaGeneration(generation), - WithStorageNotificationNotReady("NotificationDeleted", fmt.Sprintf("Successfully deleted Storage notification: %s", notificationId)), - WithStorageTopicNotReady("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", storageName)), - WithStoragePullSubscriptionNotReady("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", storageName)), - WithDeletionTimestamp()), - }}, - }} + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(testNS, storageName, false), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewStorage(storageName, testNS, + WithStorageProject(testProject), + WithStorageObjectMetaGeneration(generation), + WithStorageStatusObservedGeneration(generation), + WithStorageBucket(bucket), + WithStorageSink(sinkGVK, sinkName), + WithStorageEventTypes([]string{storagev1alpha1.StorageFinalize}), + WithStorageFinalizers(finalizerName), + WithInitStorageConditions, + WithStorageObjectMetaGeneration(generation), + WithStorageNotificationNotReady("NotificationDeleted", fmt.Sprintf("Successfully deleted Storage notification: %s", notificationId)), + WithStorageTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", storageName)), + WithStoragePullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", storageName)), + WithDeletionTimestamp()), + }}, + }} defer logtesting.ClearAll() table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, testData map[string]interface{}) controller.Reconciler { diff --git a/pkg/reconciler/messaging/channel/channel.go b/pkg/reconciler/messaging/channel/channel.go index bb54a2cd42..aca95514bd 100644 --- a/pkg/reconciler/messaging/channel/channel.go +++ b/pkg/reconciler/messaging/channel/channel.go @@ -121,7 +121,7 @@ func (r *Reconciler) reconcile(ctx context.Context, channel *v1alpha1.Channel) e // 1. Create the Topic. topic, err := r.reconcileTopic(ctx, channel) if err != nil { - channel.Status.MarkNoTopic("TopicReconcileFailed", "Failed to reconcile Topic: %s", err.Error()) + channel.Status.MarkTopicFalse("TopicReconcileFailed", "Failed to reconcile Topic: %s", err.Error()) return err } channel.Status.PropagateTopicStatus(topic.Status.GetCondition(pubsubv1alpha1.TopicConditionReady)) diff --git a/pkg/reconciler/messaging/channel/channel_test.go b/pkg/reconciler/messaging/channel/channel_test.go index b1ed678cfe..86fa0ea1bf 100644 --- a/pkg/reconciler/messaging/channel/channel_test.go +++ b/pkg/reconciler/messaging/channel/channel_test.go @@ -118,7 +118,7 @@ func TestAllCases(t *testing.T) { WithInitChannelConditions, WithChannelSubscribersStatus([]eventingduck.SubscriberStatus(nil)), WithChannelTopicID(testTopicID), - WithChannelNoTopic("TopicNotReady", "Topic has no Ready type status"), + WithChannelTopicUnknown("TopicNotConfigured", "Topic has not yet been reconciled"), ), }}, WantCreates: []runtime.Object{ @@ -134,7 +134,6 @@ func TestAllCases(t *testing.T) { }), WithInitChannelConditions, WithChannelDefaults, - WithChannelTopic(testTopicID), ), newReadyTopic(), }, @@ -158,7 +157,7 @@ func TestAllCases(t *testing.T) { ), }}, }, { - Name: "new subscriber", + Name: "the status of topic is false", Objects: []runtime.Object{ NewChannel(channelName, testNS, WithChannelUID(channelUID), @@ -167,17 +166,11 @@ func TestAllCases(t *testing.T) { }), WithInitChannelConditions, WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), ), - newReadyTopic(), + newFalseTopic(), }, Key: testNS + "/" + channelName, WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "SubscriberCreated", "Created Subscriber %q", "cre-sub-testsubscription-abc-123"), Eventf(corev1.EventTypeNormal, "Updated", "Updated Channel %q", channelName), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ @@ -189,225 +182,264 @@ func TestAllCases(t *testing.T) { WithInitChannelConditions, WithChannelDefaults, WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), // Updates - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ - {UID: subscriptionUID, Ready: corev1.ConditionFalse, Message: "PullSubscription cre-sub-testsubscription-abc-123 is not ready"}, - }), - ), - }}, - WantCreates: []runtime.Object{ - newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), - }, - }, { - Name: "update subscriber", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, Generation: 2, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ - {UID: subscriptionUID, ObservedGeneration: 1}, - }), - ), - newReadyTopic(), - newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: apis.HTTP("wrong"), ReplyURI: apis.HTTP("wrong")}), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "SubscriberUpdated", "Updated Subscriber %q", "cre-sub-testsubscription-abc-123"), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Channel %q", channelName), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, Generation: 2, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), - // Updates - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ - {UID: subscriptionUID, ObservedGeneration: 2, Ready: corev1.ConditionFalse, Message: "PullSubscription cre-sub-testsubscription-abc-123 is not ready"}, - }), - ), - }}, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), - }}, - }, { - Name: "subscriber already exists owned by other channel", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), - ), - newReadyTopic(), - newPullSubscriptionWithOwner( - eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - NewChannel("other-channel", testNS, WithChannelUID("other-id"), WithInitChannelConditions), - ), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "SubscriberNotOwned", "Subscriber %q is not owned by this channel", "cre-sub-testsubscription-abc-123"), - Eventf(corev1.EventTypeWarning, "InternalError", "channel %q does not own subscriber %q", channelName, "cre-sub-testsubscription-abc-123"), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), - // Updates - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{}), - ), - }}, - WantCreates: []runtime.Object{ - newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), - }, - WantErr: true, - }, { - Name: "subscriber already exists in status owned by other channel", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ - {UID: subscriptionUID, ObservedGeneration: 1}, - }), - ), - newReadyTopic(), - newPullSubscriptionWithOwner( - eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - NewChannel("other-channel", testNS, WithChannelUID("other-id"), WithInitChannelConditions), - ), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "SubscriberNotOwned", "Subscriber %q is not owned by this channel", "cre-sub-testsubscription-abc-123"), - Eventf(corev1.EventTypeWarning, "InternalError", "channel %q does not own subscriber %q", channelName, "cre-sub-testsubscription-abc-123"), - }, - WantCreates: []runtime.Object{ - newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), - }, - WantErr: true, - }, { - Name: "update - subscriber missing", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{ - {UID: subscriptionUID, Generation: 1, SubscriberURI: subscriberURI, ReplyURI: replyURI}, - }), - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ - {UID: subscriptionUID, ObservedGeneration: 1, Ready: corev1.ConditionFalse, Message: "PullSubscription cre-sub-testsubscription-abc-123 is not ready"}, - }), - ), - newReadyTopic(), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "SubscriberCreated", "Created Subscriber %q", "cre-sub-testsubscription-abc-123"), - }, - WantCreates: []runtime.Object{ - newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), - }, - }, { - Name: "delete subscriber", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{}), - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ - {UID: subscriptionUID}, - }), - ), - newReadyTopic(), - newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "SubscriberDeleted", "Deleted Subscriber %q", "cre-sub-testsubscription-abc-123"), - Eventf(corev1.EventTypeNormal, "Updated", "Updated Channel %q", channelName), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelDefaults, - WithChannelTopic(testTopicID), - WithChannelAddress(topicURI), - WithChannelSubscribers([]eventingduck.SubscriberSpec{}), - // Updates - WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{}), + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus(nil)), + WithChannelTopicFalse("PublisherStatus", "Publisher has no Ready type status"), ), }}, - WantDeletes: []clientgotesting.DeleteActionImpl{ - {ActionImpl: clientgotesting.ActionImpl{ - Namespace: "testnamespace", Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "pullsubscriptions"}}, - Name: "cre-sub-testsubscription-abc-123", + }, + { + Name: "new subscriber", + Objects: []runtime.Object{ + NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + ), + newReadyTopic(), }, - }, - }} + Key: testNS + "/" + channelName, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "SubscriberCreated", "Created Subscriber %q", "cre-sub-testsubscription-abc-123"), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Channel %q", channelName), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + // Updates + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ + {UID: subscriptionUID, Ready: corev1.ConditionFalse, Message: "PullSubscription cre-sub-testsubscription-abc-123 is not ready"}, + }), + ), + }}, + WantCreates: []runtime.Object{ + newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), + }, + }, { + Name: "update subscriber", + Objects: []runtime.Object{ + NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, Generation: 2, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ + {UID: subscriptionUID, ObservedGeneration: 1}, + }), + ), + newReadyTopic(), + newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: apis.HTTP("wrong"), ReplyURI: apis.HTTP("wrong")}), + }, + Key: testNS + "/" + channelName, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "SubscriberUpdated", "Updated Subscriber %q", "cre-sub-testsubscription-abc-123"), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Channel %q", channelName), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, Generation: 2, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + // Updates + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ + {UID: subscriptionUID, ObservedGeneration: 2, Ready: corev1.ConditionFalse, Message: "PullSubscription cre-sub-testsubscription-abc-123 is not ready"}, + }), + ), + }}, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), + }}, + }, { + Name: "subscriber already exists owned by other channel", + Objects: []runtime.Object{ + NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + ), + newReadyTopic(), + newPullSubscriptionWithOwner( + eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + NewChannel("other-channel", testNS, WithChannelUID("other-id"), WithInitChannelConditions), + ), + }, + Key: testNS + "/" + channelName, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "SubscriberNotOwned", "Subscriber %q is not owned by this channel", "cre-sub-testsubscription-abc-123"), + Eventf(corev1.EventTypeWarning, "InternalError", "channel %q does not own subscriber %q", channelName, "cre-sub-testsubscription-abc-123"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + // Updates + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{}), + ), + }}, + WantCreates: []runtime.Object{ + newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), + }, + WantErr: true, + }, { + Name: "subscriber already exists in status owned by other channel", + Objects: []runtime.Object{ + NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ + {UID: subscriptionUID, ObservedGeneration: 1}, + }), + ), + newReadyTopic(), + newPullSubscriptionWithOwner( + eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + NewChannel("other-channel", testNS, WithChannelUID("other-id"), WithInitChannelConditions), + ), + }, + Key: testNS + "/" + channelName, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "SubscriberNotOwned", "Subscriber %q is not owned by this channel", "cre-sub-testsubscription-abc-123"), + Eventf(corev1.EventTypeWarning, "InternalError", "channel %q does not own subscriber %q", channelName, "cre-sub-testsubscription-abc-123"), + }, + WantCreates: []runtime.Object{ + newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), + }, + WantErr: true, + }, { + Name: "update - subscriber missing", + Objects: []runtime.Object{ + NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{ + {UID: subscriptionUID, Generation: 1, SubscriberURI: subscriberURI, ReplyURI: replyURI}, + }), + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ + {UID: subscriptionUID, ObservedGeneration: 1, Ready: corev1.ConditionFalse, Message: "PullSubscription cre-sub-testsubscription-abc-123 is not ready"}, + }), + ), + newReadyTopic(), + }, + Key: testNS + "/" + channelName, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "SubscriberCreated", "Created Subscriber %q", "cre-sub-testsubscription-abc-123"), + }, + WantCreates: []runtime.Object{ + newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), + }, + }, { + Name: "delete subscriber", + Objects: []runtime.Object{ + NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{}), + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{ + {UID: subscriptionUID}, + }), + ), + newReadyTopic(), + newPullSubscription(eventingduck.SubscriberSpec{UID: subscriptionUID, SubscriberURI: subscriberURI, ReplyURI: replyURI}), + }, + Key: testNS + "/" + channelName, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "SubscriberDeleted", "Deleted Subscriber %q", "cre-sub-testsubscription-abc-123"), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Channel %q", channelName), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelDefaults, + WithChannelTopic(testTopicID), + WithChannelAddress(topicURI), + WithChannelSubscribers([]eventingduck.SubscriberSpec{}), + // Updates + WithChannelSubscribersStatus([]eventingduck.SubscriberStatus{}), + ), + }}, + WantDeletes: []clientgotesting.DeleteActionImpl{ + {ActionImpl: clientgotesting.ActionImpl{ + Namespace: "testnamespace", Verb: "delete", Resource: schema.GroupVersionResource{Group: "pubsub.cloud.google.com", Version: "v1alpha1", Resource: "pullsubscriptions"}}, + Name: "cre-sub-testsubscription-abc-123", + }, + }, + }} defer logtesting.ClearAll() table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher, _ map[string]interface{}) controller.Reconciler { @@ -450,6 +482,14 @@ func newReadyTopic() *pubsubv1alpha1.Topic { return topic } +func newFalseTopic() *pubsubv1alpha1.Topic { + topic := newTopic() + url, _ := apis.ParseURL(topicURI) + topic.Status.SetAddress(url) + topic.Status.MarkNotDeployed("PublisherStatus", "Publisher has no Ready type status") + return topic +} + func newPullSubscription(subscriber eventingduck.SubscriberSpec) *pubsubv1alpha1.PullSubscription { channel := NewChannel(channelName, testNS, WithChannelUID(channelUID), diff --git a/pkg/reconciler/pubsub/reconciler.go b/pkg/reconciler/pubsub/reconciler.go index e2fc9ed367..52d8adc95e 100644 --- a/pkg/reconciler/pubsub/reconciler.go +++ b/pkg/reconciler/pubsub/reconciler.go @@ -19,6 +19,7 @@ package pubsub import ( "context" "fmt" + "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" pubsubsourcev1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" pubsubsourceclientset "github.com/google/knative-gcp/pkg/client/clientset/versioned" @@ -26,6 +27,7 @@ import ( "github.com/google/knative-gcp/pkg/reconciler" "github.com/google/knative-gcp/pkg/reconciler/pubsub/resources" "go.uber.org/zap" + corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -76,29 +78,11 @@ func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubS } cs := pubsubable.ConditionSet() - if !t.Status.IsReady() { - status.MarkTopicNotReady(cs, "TopicNotReady", "Topic %q not ready", t.Name) - return t, nil, fmt.Errorf("Topic %q not ready", t.Name) - } - if t.Status.ProjectID == "" { - status.MarkTopicNotReady(cs, "TopicNotReady", "Topic %q did not expose projectid", t.Name) - return t, nil, fmt.Errorf("Topic %q did not expose projectid", t.Name) + if err := propagateTopicStatus(t, status, cs, topic); err != nil { + return t, nil, err } - if t.Status.TopicID == "" { - status.MarkTopicNotReady(cs, "TopicNotReady", "Topic %q did not expose topicid", t.Name) - return t, nil, fmt.Errorf("Topic %q did not expose topicid", t.Name) - } - - if t.Status.TopicID != topic { - status.MarkTopicNotReady(cs, "TopicNotReady", "Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) - return t, nil, fmt.Errorf("Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) - } - - status.TopicID = t.Status.TopicID - status.ProjectID = t.Status.ProjectID - status.MarkTopicReady(cs) // Ok, so the Topic is ready, let's reconcile PullSubscription. pullSubscriptions := psb.pubsubClient.PubsubV1alpha1().PullSubscriptions(namespace) @@ -116,12 +100,10 @@ func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubS } } - if !ps.Status.IsReady() { - status.MarkPullSubscriptionNotReady(cs, "PullSubscriptionNotReady", "PullSubscription %q not ready", ps.Name) - return t, ps, fmt.Errorf("PullSubscription %q not ready", ps.Name) - } else { - status.MarkPullSubscriptionReady(cs) + if err := propagatePullSubscriptionStatus(ps, status, cs); err != nil { + return t, ps, err } + uri, err := apis.ParseURL(ps.Status.SinkURI) if err != nil { return t, ps, fmt.Errorf("failed to parse url %q: %w", ps.Status.SinkURI, err) @@ -130,6 +112,63 @@ func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubS return t, ps, nil } +func propagatePullSubscriptionStatus(ps *pubsubsourcev1alpha1.PullSubscription, status *v1alpha1.PubSubStatus, cs *apis.ConditionSet) error { + pc := ps.Status.GetTopLevelCondition() + if pc == nil { + status.MarkPullSubscriptionNotConfigured(cs) + return fmt.Errorf("PullSubscription %q has not yet been reconciled", ps.Name) + } + switch { + case pc.Status == corev1.ConditionUnknown: + status.MarkPullSubscriptionUnknown(cs, pc.Reason, pc.Message) + return fmt.Errorf("the status of PullSubscription %q is Unknown", ps.Name) + case pc.Status == corev1.ConditionTrue: + status.MarkPullSubscriptionReady(cs) + case pc.Status == corev1.ConditionFalse: + status.MarkPullSubscriptionFalse(cs, pc.Reason, pc.Message) + return fmt.Errorf("the status of PullSubscription %q is False", ps.Name) + default: + status.MarkPullSubscriptionUnknown(cs, "PullSubscriptionUnknown", "The status of PullSubscription is invalid: %v", pc.Status) + return fmt.Errorf("the status of PullSubscription %q is invalid: %v", ps.Name, pc.Status) + } + return nil +} + +func propagateTopicStatus(t *pubsubsourcev1alpha1.Topic, status *v1alpha1.PubSubStatus, cs *apis.ConditionSet, topic string ) error { + tc := t.Status.GetTopLevelCondition() + if tc == nil { + status.MarkTopicNotConfigured(cs) + return fmt.Errorf("Topic %q has not yet been reconciled", t.Name) + } + if tc.Status == corev1.ConditionUnknown { + status.MarkTopicUnknown(cs, tc.Reason, tc.Message) + return fmt.Errorf("the status of Topic %q is Unknown", t.Name) + } else if tc.Status == corev1.ConditionFalse { + status.MarkTopicFalse(cs, tc.Reason, tc.Message) + return fmt.Errorf("the status of Topic %q is False", t.Name) + } else if tc.Status != corev1.ConditionTrue { + status.MarkTopicUnknown(cs, "TopicUnknown", "The status of Topic is invalid: %v", tc.Status) + return fmt.Errorf("the status of Topic %q is invalid: %v", t.Name, tc.Status) + } + + if t.Status.ProjectID == "" { + status.MarkTopicFalse(cs, "TopicNotReady", "Topic %q did not expose projectid", t.Name) + return fmt.Errorf("Topic %q did not expose projectid", t.Name) + } + if t.Status.TopicID == "" { + status.MarkTopicFalse(cs, "TopicNotReady", "Topic %q did not expose topicid", t.Name) + return fmt.Errorf("Topic %q did not expose topicid", t.Name) + } + if t.Status.TopicID != topic { + status.MarkTopicFalse(cs, "TopicNotReady", "Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) + return fmt.Errorf("Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) + } + status.TopicID = t.Status.TopicID + status.ProjectID = t.Status.ProjectID + status.MarkTopicReady(cs) + return nil +} + func (psb *PubSubBase) DeletePubSub(ctx context.Context, pubsubable duck.PubSubable) error { if pubsubable == nil { return fmt.Errorf("nil pubsubable passed in") @@ -143,10 +182,10 @@ func (psb *PubSubBase) DeletePubSub(ctx context.Context, pubsubable duck.PubSuba err := psb.pubsubClient.PubsubV1alpha1().Topics(namespace).Delete(name, nil) if err != nil && !apierrs.IsNotFound(err) { logging.FromContext(ctx).Desugar().Error("Failed to delete Topic", zap.String("name", name), zap.Error(err)) - status.MarkTopicNotReady(cs, "TopicDeleteFailed", "Failed to delete Topic: %s", err.Error()) + status.MarkTopicFalse(cs, "TopicDeleteFailed", "Failed to delete Topic: %s", err.Error()) return fmt.Errorf("failed to delete topic: %w", err) } - status.MarkTopicNotReady(cs, "TopicDeleted", "Successfully deleted Topic: %s", name) + status.MarkTopicFalse(cs, "TopicDeleted", "Successfully deleted Topic: %s", name) status.TopicID = "" status.ProjectID = "" @@ -154,10 +193,10 @@ func (psb *PubSubBase) DeletePubSub(ctx context.Context, pubsubable duck.PubSuba err = psb.pubsubClient.PubsubV1alpha1().PullSubscriptions(namespace).Delete(name, nil) if err != nil && !apierrs.IsNotFound(err) { logging.FromContext(ctx).Desugar().Error("Failed to delete PullSubscription", zap.String("name", name), zap.Error(err)) - status.MarkPullSubscriptionNotReady(cs, "PullSubscriptionDeleteFailed", "Failed to delete PullSubscription: %s", err.Error()) + status.MarkPullSubscriptionFalse(cs, "PullSubscriptionDeleteFailed", "Failed to delete PullSubscription: %s", err.Error()) return fmt.Errorf("failed to delete PullSubscription: %w", err) } - status.MarkPullSubscriptionNotReady(cs, "PullSubscriptionDeleted", "Successfully deleted PullSubscription: %s", name) + status.MarkPullSubscriptionFalse(cs, "PullSubscriptionDeleted", "Successfully deleted PullSubscription: %s", name) status.SinkURI = nil return nil } diff --git a/pkg/reconciler/pubsub/reconciler_test.go b/pkg/reconciler/pubsub/reconciler_test.go index 804be1504b..54578b9232 100644 --- a/pkg/reconciler/pubsub/reconciler_test.go +++ b/pkg/reconciler/pubsub/reconciler_test.go @@ -87,7 +87,7 @@ func TestCreates(t *testing.T) { expectedErr string wantCreates []runtime.Object }{{ - name: "topic does not exist, created, not ready", + name: "topic does not exist, created, not yet been reconciled", expectedTopic: rectesting.NewTopic(name, testNS, rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ Secret: &secret, @@ -100,7 +100,7 @@ func TestCreates(t *testing.T) { rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), ), expectedPS: nil, - expectedErr: fmt.Sprintf("Topic %q not ready", name), + expectedErr: fmt.Sprintf("Topic %q has not yet been reconciled", name), wantCreates: []runtime.Object{ rectesting.NewTopic(name, testNS, rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ @@ -114,7 +114,7 @@ func TestCreates(t *testing.T) { ), }, }, { - name: "topic exists but is not ready", + name: "topic exists but is not yet been reconciled", objects: []runtime.Object{ rectesting.NewTopic(name, testNS, rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ @@ -139,7 +139,7 @@ func TestCreates(t *testing.T) { rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), ), expectedPS: nil, - expectedErr: fmt.Sprintf("Topic %q not ready", name), + expectedErr: fmt.Sprintf("Topic %q has not yet been reconciled", name), }, { name: "topic exists and is ready but no projectid", objects: []runtime.Object{ @@ -173,6 +173,70 @@ func TestCreates(t *testing.T) { expectedPS: nil, expectedErr: fmt.Sprintf("Topic %q did not expose projectid", name), }, { + name: "topic exists and the status of topic is false", + objects: []runtime.Object{ + rectesting.NewTopic(name, testNS, + rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithTopicLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicFalse(), + ), + }, + expectedTopic: rectesting.NewTopic(name, testNS, + rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ + Secret: &secret, + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithTopicLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithTopicFalse(), + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + expectedPS: nil, + expectedErr: fmt.Sprintf("the status of Topic %q is False", name), + }, { + name: "topic exists and the status of topic is unknown", + objects: []runtime.Object{ + rectesting.NewTopic(name, testNS, + rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithTopicLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicUnknown(), + ), + }, + expectedTopic: rectesting.NewTopic(name, testNS, + rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ + Secret: &secret, + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithTopicLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithTopicUnknown(), + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + expectedPS: nil, + expectedErr: fmt.Sprintf("the status of Topic %q is Unknown", name), + },{ name: "topic exists and is ready but no topicid", objects: []runtime.Object{ rectesting.NewTopic(name, testNS, @@ -207,7 +271,7 @@ func TestCreates(t *testing.T) { expectedPS: nil, expectedErr: fmt.Sprintf("Topic %q did not expose topicid", name), }, { - name: "topic exists and is ready, pullsubscription created", + name: "topic exists and is ready, pullsubscription created, not yet been reconciled", objects: []runtime.Object{ rectesting.NewTopic(name, testNS, rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ @@ -251,7 +315,7 @@ func TestCreates(t *testing.T) { }), rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), ), - expectedErr: fmt.Sprintf("PullSubscription %q not ready", name), + expectedErr: fmt.Sprintf("PullSubscription %q has not yet been reconciled", name), wantCreates: []runtime.Object{ rectesting.NewPullSubscriptionWithNoDefaults(name, testNS, rectesting.WithPullSubscriptionSpecWithNoDefaults(pubsubsourcev1alpha1.PullSubscriptionSpec{ @@ -268,7 +332,7 @@ func TestCreates(t *testing.T) { ), }, }, { - name: "topic exists and is ready, pullsubscription exists, not ready", + name: "topic exists and is ready, pullsubscription exists, not yet been reconciled", objects: []runtime.Object{ rectesting.NewTopic(name, testNS, rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ @@ -325,9 +389,9 @@ func TestCreates(t *testing.T) { }), rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), ), - expectedErr: fmt.Sprintf("PullSubscription %q not ready", name), + expectedErr: fmt.Sprintf("PullSubscription %q has not yet been reconciled", name), }, { - name: "topic exists and is ready, pullsubscription exists and is ready", + name: "topic exists and is ready, pullsubscription exists and the status is false", objects: []runtime.Object{ rectesting.NewTopic(name, testNS, rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ @@ -354,7 +418,7 @@ func TestCreates(t *testing.T) { "metrics-resource-group": resourceGroup, }), rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), - rectesting.WithPullSubscriptionReady("http://example.com"), + rectesting.WithPullSubscriptionFalse(), ), }, expectedTopic: rectesting.NewTopic(name, testNS, @@ -384,9 +448,70 @@ func TestCreates(t *testing.T) { "metrics-resource-group": resourceGroup, }), rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), - rectesting.WithPullSubscriptionReady("http://example.com"), + rectesting.WithPullSubscriptionFalse(), ), - expectedErr: "", + expectedErr: fmt.Sprintf("the status of PullSubscription %q is False", name), + },{ + name: "topic exists and is ready, pullsubscription exists and the status is unknown", + objects: []runtime.Object{ + rectesting.NewTopic(name, testNS, + rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithTopicLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicReady(testTopicID), + rectesting.WithTopicAddress(testTopicURI), + ), + rectesting.NewPullSubscriptionWithNoDefaults(name, testNS, + rectesting.WithPullSubscriptionSpecWithNoDefaults(pubsubsourcev1alpha1.PullSubscriptionSpec{ + Topic: testTopicID, + Secret: &secret, + }), + rectesting.WithPullSubscriptionLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithPullSubscriptionAnnotations(map[string]string{ + "metrics-resource-group": resourceGroup, + }), + rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithPullSubscriptionUnknown(), + ), + }, + expectedTopic: rectesting.NewTopic(name, testNS, + rectesting.WithTopicSpec(pubsubsourcev1alpha1.TopicSpec{ + Secret: &secret, + Topic: testTopicID, + PropagationPolicy: "CreateDelete", + }), + rectesting.WithTopicLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithTopicReady(testTopicID), + rectesting.WithTopicProjectID(testProjectID), + rectesting.WithTopicAddress(testTopicURI), + rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), + ), + expectedPS: rectesting.NewPullSubscriptionWithNoDefaults(name, testNS, + rectesting.WithPullSubscriptionSpecWithNoDefaults(pubsubsourcev1alpha1.PullSubscriptionSpec{ + Topic: testTopicID, + Secret: &secret, + }), + rectesting.WithPullSubscriptionLabels(map[string]string{ + "receive-adapter": receiveAdapterName, + }), + rectesting.WithPullSubscriptionAnnotations(map[string]string{ + "metrics-resource-group": resourceGroup, + }), + rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), + rectesting.WithPullSubscriptionUnknown(), + ), + expectedErr: fmt.Sprintf("the status of PullSubscription %q is Unknown", name), }} defer logtesting.ClearAll() @@ -407,13 +532,13 @@ func TestCreates(t *testing.T) { if (tc.expectedErr != "" && err == nil) || (tc.expectedErr == "" && err != nil) || (tc.expectedErr != "" && err != nil && tc.expectedErr != err.Error()) { - t.Errorf("Error mismatch, want: %q got: %q", tc.expectedErr, err) + t.Errorf("Test case %q, Error mismatch, want: %q got: %q", tc.name, tc.expectedErr, err) } if diff := cmp.Diff(tc.expectedTopic, topic, ignoreLastTransitionTime); diff != "" { - t.Errorf("unexpected topic (-want, +got) = %v", diff) + t.Errorf("Test case %q, unexpected topic (-want, +got) = %v", tc.name, diff) } if diff := cmp.Diff(tc.expectedPS, ps, ignoreLastTransitionTime); diff != "" { - t.Errorf("unexpected pullsubscription (-want, +got) = %v", diff) + t.Errorf("Test case %q, unexpected pullsubscription (-want, +got) = %v", tc.name, diff) } // Validate creates. diff --git a/pkg/reconciler/testing/auditlogs.go b/pkg/reconciler/testing/auditlogs.go index d19a7c7f33..a329795591 100644 --- a/pkg/reconciler/testing/auditlogs.go +++ b/pkg/reconciler/testing/auditlogs.go @@ -48,9 +48,15 @@ func WithInitAuditLogsSourceConditions(s *v1alpha1.AuditLogsSource) { s.Status.InitializeConditions() } -func WithAuditLogsSourceTopicNotReady(reason, message string) AuditLogsSourceOption { +func WithAuditLogsSourceTopicFalse(reason, message string) AuditLogsSourceOption { return func(s *v1alpha1.AuditLogsSource) { - s.Status.MarkTopicNotReady(reason, message) + s.Status.MarkTopicFalse(reason, message) + } +} + +func WithAuditLogsSourceTopicUnknown(reason, message string) AuditLogsSourceOption { + return func(s *v1alpha1.AuditLogsSource) { + s.Status.MarkTopicUnknown(reason, message) } } @@ -61,9 +67,15 @@ func WithAuditLogsSourceTopicReady(topicID string) AuditLogsSourceOption { } } -func WithAuditLogsSourcePullSubscriptionNotReady(reason, message string) AuditLogsSourceOption { +func WithAuditLogsSourcePullSubscriptionFalse(reason, message string) AuditLogsSourceOption { + return func(s *v1alpha1.AuditLogsSource) { + s.Status.MarkPullSubscriptionFalse(reason, message) + } +} + +func WithAuditLogsSourcePullSubscriptionUnknown(reason, message string) AuditLogsSourceOption { return func(s *v1alpha1.AuditLogsSource) { - s.Status.MarkPullSubscriptionNotReady(reason, message) + s.Status.MarkPullSubscriptionUnknown(reason, message) } } diff --git a/pkg/reconciler/testing/channel.go b/pkg/reconciler/testing/channel.go index 683e78f1ae..84b012543b 100644 --- a/pkg/reconciler/testing/channel.go +++ b/pkg/reconciler/testing/channel.go @@ -92,9 +92,15 @@ func WithChannelTopicID(topicID string) ChannelOption { } } -func WithChannelNoTopic(reason, message string) ChannelOption { +func WithChannelTopicFalse(reason, message string) ChannelOption { return func(c *v1alpha1.Channel) { - c.Status.MarkNoTopic(reason, message) + c.Status.MarkTopicFalse(reason, message) + } +} + +func WithChannelTopicUnknown(reason, message string) ChannelOption { + return func(c *v1alpha1.Channel) { + c.Status.MarkTopicUnknown(reason, message) } } diff --git a/pkg/reconciler/testing/pubsub.go b/pkg/reconciler/testing/pubsub.go index 561d867a07..38e6c559af 100644 --- a/pkg/reconciler/testing/pubsub.go +++ b/pkg/reconciler/testing/pubsub.go @@ -70,11 +70,19 @@ func WithInitPubSubConditions(ps *v1alpha1.PubSub) { ps.Status.InitializeConditions() } -// WithPubSubPullSubscriptionNotReady marks the condition that the -// topic is not ready -func WithPubSubPullSubscriptionNotReady(reason, message string) PubSubOption { +// WithPubSubPullSubscriptionFalse marks the condition that the +// topic is False +func WithPubSubPullSubscriptionFalse(reason, message string) PubSubOption { + return func(ps *v1alpha1.PubSub) { + ps.Status.MarkPullSubscriptionFalse(reason, message) + } +} + +// WithPubSubPullSubscriptionUnknown marks the condition that the +// topic is Unknown +func WithPubSubPullSubscriptionUnknown(reason, message string) PubSubOption { return func(ps *v1alpha1.PubSub) { - ps.Status.MarkPullSubscriptionNotReady(reason, message) + ps.Status.MarkPullSubscriptionUnknown(reason, message) } } diff --git a/pkg/reconciler/testing/pullsubscription.go b/pkg/reconciler/testing/pullsubscription.go index e9cd39602f..7adc5b9bf5 100644 --- a/pkg/reconciler/testing/pullsubscription.go +++ b/pkg/reconciler/testing/pullsubscription.go @@ -193,6 +193,21 @@ func WithPullSubscriptionReady(sink string) PullSubscriptionOption { } } +func WithPullSubscriptionFalse() PullSubscriptionOption { + return func(s *v1alpha1.PullSubscription) { + s.Status.InitializeConditions() + s.Status.MarkNoSink("InvalidSink", + `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`) + + } +} + +func WithPullSubscriptionUnknown() PullSubscriptionOption { + return func(s *v1alpha1.PullSubscription) { + s.Status.InitializeConditions() + } +} + func WithPullSubscriptionJobFailure(subscriptionID, reason, message string) PullSubscriptionOption { return func(s *v1alpha1.PullSubscription) { s.Status.SubscriptionID = subscriptionID diff --git a/pkg/reconciler/testing/scheduler.go b/pkg/reconciler/testing/scheduler.go index fe4cc7c82b..143796803b 100644 --- a/pkg/reconciler/testing/scheduler.go +++ b/pkg/reconciler/testing/scheduler.go @@ -94,32 +94,48 @@ func WithInitSchedulerConditions(s *v1alpha1.Scheduler) { s.Status.InitializeConditions() } -// WithSchedulerTopicNotReady marks the condition that the -// topic is not ready -func WithSchedulerTopicNotReady(reason, message string) SchedulerOption { +// WithSchedulerTopicFalse marks the condition that the +// status of topic is False. +func WithSchedulerTopicFalse(reason, message string) SchedulerOption { + return func(s *v1alpha1.Scheduler) { + s.Status.MarkTopicFalse(reason, message) + } +} + +// WithSchedulerTopicUnknown marks the condition that the +// status of topic is Unknown. +func WithSchedulerTopicUnknown(reason, message string) SchedulerOption { return func(s *v1alpha1.Scheduler) { - s.Status.MarkTopicNotReady(reason, message) + s.Status.MarkTopicUnknown(reason, message) } } // WithSchedulerTopicNotReady marks the condition that the -// topic is not ready +// topic is not ready. func WithSchedulerTopicReady(topicID, projectID string) SchedulerOption { return func(s *v1alpha1.Scheduler) { s.Status.MarkTopicReady(topicID, projectID) } } -// WithSchedulerPullSubscriptionNotReady marks the condition that the -// topic is not ready -func WithSchedulerPullSubscriptionNotReady(reason, message string) SchedulerOption { +// WithSchedulerPullSubscriptionFalse marks the condition that the +// topic is False. +func WithSchedulerPullSubscriptionFalse(reason, message string) SchedulerOption { + return func(s *v1alpha1.Scheduler) { + s.Status.MarkPullSubscriptionFalse(reason, message) + } +} + +// WithSchedulerPullSubscriptionUnknown marks the condition that the +// topic is Unknown. +func WithSchedulerPullSubscriptionUnknown(reason, message string) SchedulerOption { return func(s *v1alpha1.Scheduler) { - s.Status.MarkPullSubscriptionNotReady(reason, message) + s.Status.MarkPullSubscriptionUnknown(reason, message) } } -// WithSchedulerPullSubscriptionNotReady marks the condition that the -// topic is not ready +// WithSchedulerPullSubscriptionReady marks the condition that the +// topic is ready. func WithSchedulerPullSubscriptionReady() SchedulerOption { return func(s *v1alpha1.Scheduler) { s.Status.MarkPullSubscriptionReady() diff --git a/pkg/reconciler/testing/storage.go b/pkg/reconciler/testing/storage.go index 043f8d5280..bd31913f4d 100644 --- a/pkg/reconciler/testing/storage.go +++ b/pkg/reconciler/testing/storage.go @@ -83,11 +83,19 @@ func WithInitStorageConditions(s *v1alpha1.Storage) { s.Status.InitializeConditions() } -// WithStorageTopicNotReady marks the condition that the -// topic is not ready -func WithStorageTopicNotReady(reason, message string) StorageOption { +// WithStorageTopicFalse marks the condition that the +// topic is False +func WithStorageTopicFalse(reason, message string) StorageOption { + return func(s *v1alpha1.Storage) { + s.Status.MarkTopicFalse(reason, message) + } +} + +// WithStorageTopicUnknown marks the condition that the +// topic is False +func WithStorageTopicUnknown(reason, message string) StorageOption { return func(s *v1alpha1.Storage) { - s.Status.MarkTopicNotReady(reason, message) + s.Status.MarkTopicUnknown(reason, message) } } @@ -106,11 +114,19 @@ func WithStorageTopicID(topicID string) StorageOption { } } -// WithStoragePullSubscriptionNotReady marks the condition that the -// topic is not ready -func WithStoragePullSubscriptionNotReady(reason, message string) StorageOption { +// WithStoragePullSubscriptionFalse marks the condition that the +// status of topic is False +func WithStoragePullSubscriptionFalse(reason, message string) StorageOption { + return func(s *v1alpha1.Storage) { + s.Status.MarkPullSubscriptionFalse(reason, message) + } +} + +// WithStoragePullSubscriptionUnknown marks the condition that the +// status of topic is Unknown +func WithStoragePullSubscriptionUnknown(reason, message string) StorageOption { return func(s *v1alpha1.Storage) { - s.Status.MarkPullSubscriptionNotReady(reason, message) + s.Status.MarkPullSubscriptionUnknown(reason, message) } } diff --git a/pkg/reconciler/testing/topic.go b/pkg/reconciler/testing/topic.go index f8f6da53b5..d9e7bbb7a3 100644 --- a/pkg/reconciler/testing/topic.go +++ b/pkg/reconciler/testing/topic.go @@ -126,6 +126,19 @@ func WithTopicReady(topicID string) TopicOption { } } +func WithTopicFalse() TopicOption { + return func(s *v1alpha1.Topic) { + s.Status.InitializeConditions() + s.Status.MarkNotDeployed("PublisherStatus", "Publisher has no Ready type status") + } +} + +func WithTopicUnknown() TopicOption { + return func(s *v1alpha1.Topic) { + s.Status.InitializeConditions() + } +} + func WithTopicDeleted(t *v1alpha1.Topic) { tt := metav1.NewTime(time.Unix(1e9, 0)) t.ObjectMeta.SetDeletionTimestamp(&tt) From 1eb06078f2550bc3f38133016cebbb111ff9180e Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Tue, 14 Jan 2020 17:11:45 -0800 Subject: [PATCH 2/8] renamed methods, added some unit tests --- pkg/apis/duck/v1alpha1/pubsub_lifecycle.go | 10 +- .../events/v1alpha1/auditlogs_lifecycle.go | 13 +- .../v1alpha1/auditlogs_lifecycle_test.go | 96 ++++++++----- pkg/apis/events/v1alpha1/pubsub_lifecycle.go | 11 +- .../events/v1alpha1/pubsub_lifecycle_test.go | 128 ++++++++++++++++++ .../events/v1alpha1/scheduler_lifecycle.go | 8 +- pkg/apis/events/v1alpha1/storage_lifecycle.go | 8 +- pkg/apis/events/v1alpha1/test_helper.go | 47 +++++++ .../messaging/v1alpha1/channel_lifecycle.go | 6 +- .../v1alpha1/channel_lifecycle_test.go | 2 +- pkg/apis/pubsub/v1alpha1/topic_lifecycle.go | 2 +- .../events/auditlogs/auditlogs_test.go | 22 +-- pkg/reconciler/events/pubsub/pubsub.go | 4 +- pkg/reconciler/events/pubsub/pubsub_test.go | 2 +- .../events/scheduler/scheduler_test.go | 18 +-- pkg/reconciler/events/storage/storage_test.go | 18 +-- pkg/reconciler/messaging/channel/channel.go | 2 +- .../messaging/channel/channel_test.go | 2 +- pkg/reconciler/pubsub/reconciler.go | 18 +-- pkg/reconciler/pubsub/reconciler_test.go | 8 +- pkg/reconciler/testing/auditlogs.go | 8 +- pkg/reconciler/testing/channel.go | 4 +- pkg/reconciler/testing/pubsub.go | 8 +- pkg/reconciler/testing/pullsubscription.go | 2 +- pkg/reconciler/testing/scheduler.go | 12 +- pkg/reconciler/testing/storage.go | 18 +-- pkg/reconciler/testing/topic.go | 2 +- 27 files changed, 349 insertions(+), 130 deletions(-) create mode 100644 pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go create mode 100644 pkg/apis/events/v1alpha1/test_helper.go diff --git a/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go b/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go index d20f8c6235..5e9b12adb6 100644 --- a/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go @@ -20,12 +20,12 @@ import ( "knative.dev/pkg/apis" ) -// MarkTopicFalse sets the condition that the PubSub Topic is not ready and why. -func (s *PubSubStatus) MarkTopicFalse(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFailed sets the condition that the PubSub Topic is False and why. +func (s *PubSubStatus) MarkTopicFailed(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { cs.Manage(s).MarkFalse(TopicReady, reason, messageFormat, messageA...) } -// MarkTopicUnknown sets the condition that the PubSub Topic is False and why. +// MarkTopicUnknown sets the condition that the PubSub Topic is Unknown and why. func (s *PubSubStatus) MarkTopicUnknown(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { cs.Manage(s).MarkUnknown(TopicReady, reason, messageFormat, messageA...) } @@ -40,9 +40,9 @@ func (s *PubSubStatus) MarkTopicNotConfigured(cs *apis.ConditionSet) { cs.Manage(s).MarkUnknown(TopicReady, "TopicNotConfigured", "Topic has not yet been reconciled") } -// MarkPullSubscriptionFalse sets the condition that the PubSub PullSubscription is +// MarkPullSubscriptionFailed sets the condition that the PubSub PullSubscription is // False and why. -func (s *PubSubStatus) MarkPullSubscriptionFalse(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { +func (s *PubSubStatus) MarkPullSubscriptionFailed(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { cs.Manage(s).MarkFalse(PullSubscriptionReady, reason, messageFormat, messageA...) } diff --git a/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go b/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go index bbce850706..b96e615db8 100644 --- a/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go +++ b/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go @@ -26,6 +26,11 @@ func (s *AuditLogsSourceStatus) GetCondition(t apis.ConditionType) *apis.Conditi return auditLogsSourceCondSet.Manage(s).GetCondition(t) } +// GetTopLevelCondition returns the top level condition. +func (s *AuditLogsSourceStatus) GetTopLevelCondition() *apis.Condition { + return auditLogsSourceCondSet.Manage(s).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (s *AuditLogsSourceStatus) IsReady() bool { return auditLogsSourceCondSet.Manage(s).IsHappy() @@ -36,9 +41,9 @@ func (s *AuditLogsSourceStatus) InitializeConditions() { auditLogsSourceCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionNotReady sets the condition that the status of underlying PullSubscription +// MarkPullSubscriptionFailed sets the condition that the status of underlying PullSubscription // source is False and why. -func (s *AuditLogsSourceStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { +func (s *AuditLogsSourceStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } @@ -53,8 +58,8 @@ func (s *AuditLogsSourceStatus) MarkPullSubscriptionReady() { auditLogsSourceCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } -// MarkTopicFalse sets the condition that the status of PubSub topic is False and why. -func (s *AuditLogsSourceStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFailed sets the condition that the status of PubSub topic is False and why. +func (s *AuditLogsSourceStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) } diff --git a/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go b/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go index 4e875b4dd1..eff6a9a9ef 100644 --- a/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go @@ -27,63 +27,97 @@ import ( func TestAuditLogsSourceStatusIsReady(t *testing.T) { tests := []struct { - name string - s *AuditLogsSourceStatus - want bool + name string + s *AuditLogsSourceStatus + wantConditionStatus corev1.ConditionStatus + want bool }{{ - name: "uninitialized", - s: &AuditLogsSourceStatus{}, - }, { name: "initialized", s: func() *AuditLogsSourceStatus { s := &AuditLogsSourceStatus{} s.InitializeConditions() return s }(), + wantConditionStatus: corev1.ConditionUnknown, }, { - name: "topic not ready", + name: "the status of topic is false", s: func() *AuditLogsSourceStatus { s := &AuditLogsSourceStatus{} s.InitializeConditions() s.MarkPullSubscriptionReady() s.MarkSinkReady() + s.MarkTopicFailed("test", "the status of topic is false") return s }(), + wantConditionStatus: corev1.ConditionFalse, }, { - name: "pullsubscription not ready", - s: func() *AuditLogsSourceStatus { - s := &AuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkSinkReady() - return s - }(), - }, { - name: "not ready", - s: func() *AuditLogsSourceStatus { - s := &AuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionReady() - return s - }(), - }, { - name: "ready", + name: "the status of topic is unknown", s: func() *AuditLogsSourceStatus { s := &AuditLogsSourceStatus{} s.InitializeConditions() - s.MarkTopicReady() s.MarkPullSubscriptionReady() s.MarkSinkReady() + s.MarkTopicUnknown("test", "the status of topic is unknown") return s }(), - want: true, - }} + wantConditionStatus: corev1.ConditionUnknown, + }, + { + name: "the status of pullsubscription is false", + s: func() *AuditLogsSourceStatus { + s := &AuditLogsSourceStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkSinkReady() + s.MarkPullSubscriptionFailed("test", "the status of pullsubscription is false") + return s + }(), + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "the status of pullsubscription is unknown", + s: func() *AuditLogsSourceStatus { + s := &AuditLogsSourceStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkSinkReady() + s.MarkPullSubscriptionUnknown("test", "the status of pullsubscription is unknown") + return s + }(), + wantConditionStatus: corev1.ConditionUnknown, + }, + { + name: "sink is not ready", + s: func() *AuditLogsSourceStatus { + s := &AuditLogsSourceStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkPullSubscriptionReady() + s.MarkSinkNotReady("test", "sink is not ready") + return s + }(), + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "ready", + s: func() *AuditLogsSourceStatus { + s := &AuditLogsSourceStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkPullSubscriptionReady() + s.MarkSinkReady() + return s + }(), + wantConditionStatus: corev1.ConditionTrue, + want:true, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + gotConditionStatus := test.s.GetTopLevelCondition().Status got := test.s.IsReady() - if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("%s: unexpected condition (-want, +got) = %v", test.name, diff) + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go index 1ec1513529..1a1d76ae89 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go @@ -28,6 +28,11 @@ func (ps *PubSubStatus) GetCondition(t apis.ConditionType) *apis.Condition { return pubSubCondSet.Manage(ps).GetCondition(t) } +// GetTopLevelCondition returns the top level condition. +func (ps *PubSubStatus) GetTopLevelCondition() *apis.Condition { + return pubSubCondSet.Manage(ps).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (ps *PubSubStatus) IsReady() bool { return pubSubCondSet.Manage(ps).IsHappy() @@ -38,9 +43,9 @@ func (ps *PubSubStatus) InitializeConditions() { pubSubCondSet.Manage(ps).InitializeConditions() } -// MarkPullSubscriptionFalse sets the condition that the underlying PullSubscription +// MarkPullSubscriptionFailed sets the condition that the underlying PullSubscription // source is False and why. -func (ps *PubSubStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { +func (ps *PubSubStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { pubSubCondSet.Manage(ps).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } @@ -72,7 +77,7 @@ func (ps *PubSubStatus) PropagatePullSubscriptionStatus(ready *apis.Condition) { case ready.Status == corev1.ConditionTrue: ps.MarkPullSubscriptionReady() case ready.Status == corev1.ConditionFalse: - ps.MarkPullSubscriptionFalse(ready.Reason, ready.Message) + ps.MarkPullSubscriptionFailed(ready.Reason, ready.Message) default: ps.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "The status of PullSubscription is invalid: %v", ready.Status) } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go new file mode 100644 index 0000000000..f84fca1c1c --- /dev/null +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -0,0 +1,128 @@ +/* +Copyright 2019 Google LLC. + +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 + + http://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 v1alpha1 + +import ( + "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" + pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/apis" +) + +func TestPubSubSourceStatusIsReady(t *testing.T) { + tests := []struct { + name string + pullsubscriptionStatus *pubsubv1alpha1.PullSubscriptionStatus + wantConditionStatus corev1.ConditionStatus + want bool + }{{ + name: "the status of pullsubscription is false", + pullsubscriptionStatus: TestHelper.FalsePullSubscriptionStatus(), + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "the status of pullsubscription is unknown", + pullsubscriptionStatus: TestHelper.UnknownPullSubscriptionStatus(), + wantConditionStatus: corev1.ConditionUnknown, + }, + { + name: "ready", + pullsubscriptionStatus: TestHelper.ReadyPullSubscriptionStatus(), + wantConditionStatus: corev1.ConditionTrue, + want: true, + }} + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ps := &PubSubStatus{} + ps.PropagatePullSubscriptionStatus(test.pullsubscriptionStatus.GetTopLevelCondition()) + gotConditionStatus := ps.GetTopLevelCondition().Status + got := ps.IsReady() + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + } + }) + } +} +func TestPubSubSourceGetCondition(t *testing.T) { + tests := []struct { + name string + s *PubSubStatus + condQuery apis.ConditionType + want *apis.Condition + }{{ + name: "uninitialized", + s: &PubSubStatus{}, + condQuery: v1alpha1.PullSubscriptionReady, + want: nil, + }, { + name: "initialized", + s: func() *PubSubStatus { + s := &PubSubStatus{} + s.InitializeConditions() + return s + }(), + condQuery: v1alpha1.PullSubscriptionReady, + want: &apis.Condition{ + Type: v1alpha1.PullSubscriptionReady, + Status: corev1.ConditionUnknown, + }, + }, { + name: "not ready", + s: func() *PubSubStatus { + s := &PubSubStatus{} + s.InitializeConditions() + s.MarkPullSubscriptionFailed("NotReady", "test message") + return s + }(), + condQuery: v1alpha1.PullSubscriptionReady, + want: &apis.Condition{ + Type: v1alpha1.PullSubscriptionReady, + Status: corev1.ConditionFalse, + Reason: "NotReady", + Message: "test message", + }, + }, { + name: "ready", + s: func() *PubSubStatus { + s := &PubSubStatus{} + s.InitializeConditions() + s.MarkPullSubscriptionReady() + return s + }(), + condQuery: v1alpha1.PullSubscriptionReady, + want: &apis.Condition{ + Type: v1alpha1.PullSubscriptionReady, + Status: corev1.ConditionTrue, + }, + }} + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := test.s.GetCondition(test.condQuery) + ignoreTime := cmpopts.IgnoreFields(apis.Condition{}, + "LastTransitionTime", "Severity") + if diff := cmp.Diff(test.want, got, ignoreTime); diff != "" { + t.Errorf("unexpected condition (-want, +got) = %v", diff) + } + }) + } +} diff --git a/pkg/apis/events/v1alpha1/scheduler_lifecycle.go b/pkg/apis/events/v1alpha1/scheduler_lifecycle.go index cbabcc041f..a575634b9e 100644 --- a/pkg/apis/events/v1alpha1/scheduler_lifecycle.go +++ b/pkg/apis/events/v1alpha1/scheduler_lifecycle.go @@ -37,9 +37,9 @@ func (s *SchedulerStatus) InitializeConditions() { schedulerCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionFalse sets the condition that the underlying PullSubscription +// MarkPullSubscriptionFailed sets the condition that the underlying PullSubscription // is False and why. -func (s *SchedulerStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { +func (s *SchedulerStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { schedulerCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } @@ -54,8 +54,8 @@ func (s *SchedulerStatus) MarkPullSubscriptionReady() { schedulerCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } -// MarkTopicFalse sets the condition that the Topic was not created and why. -func (s *SchedulerStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFailed sets the condition that the Topic was not created and why. +func (s *SchedulerStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { schedulerCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) } diff --git a/pkg/apis/events/v1alpha1/storage_lifecycle.go b/pkg/apis/events/v1alpha1/storage_lifecycle.go index 127ddd9f7f..a82c77da80 100644 --- a/pkg/apis/events/v1alpha1/storage_lifecycle.go +++ b/pkg/apis/events/v1alpha1/storage_lifecycle.go @@ -37,9 +37,9 @@ func (s *StorageStatus) InitializeConditions() { storageCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionFalse sets the condition that the status of underlying PullSubscription +// MarkPullSubscriptionFailed sets the condition that the status of underlying PullSubscription // source is False and why. -func (s *StorageStatus) MarkPullSubscriptionFalse(reason, messageFormat string, messageA ...interface{}) { +func (s *StorageStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { storageCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } @@ -54,8 +54,8 @@ func (s *StorageStatus) MarkPullSubscriptionReady() { storageCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } -// MarkTopicFalse sets the condition that the status of PubSub topic is False why. -func (s *StorageStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { +// MarkTopicFailed sets the condition that the status of PubSub topic is False why. +func (s *StorageStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { storageCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) } diff --git a/pkg/apis/events/v1alpha1/test_helper.go b/pkg/apis/events/v1alpha1/test_helper.go new file mode 100644 index 0000000000..0201506cd4 --- /dev/null +++ b/pkg/apis/events/v1alpha1/test_helper.go @@ -0,0 +1,47 @@ +/* +Copyright 2019 Google LLC + +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 + + http://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 v1alpha1 + +import ( + "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" +) + +type testHelper struct{} + +// TestHelper contains helpers for unit tests. +var TestHelper = testHelper{} + +func (testHelper) ReadyPullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { + pss := &v1alpha1.PullSubscriptionStatus{} + pss.MarkSink("http://test.mynamespace.svc.cluster.local") + pss.MarkDeployed() + pss.MarkSubscribed("subID") + return pss +} + +func (testHelper) FalsePullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { + pss := &v1alpha1.PullSubscriptionStatus{} + pss.MarkNotDeployed("not deployed", "not deployed") + return pss +} + +func (testHelper) UnknownPullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { + pss := &v1alpha1.PullSubscriptionStatus{} + pss.InitializeConditions() + return pss +} + diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index cdd0b5ded9..8b4a9b07ca 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -75,15 +75,15 @@ func (cs *ChannelStatus) PropagateTopicStatus(ready *apis.Condition) { case ready.Status == corev1.ConditionTrue: cs.MarkTopicReady() case ready.Status == corev1.ConditionFalse: - cs.MarkTopicFalse(ready.Reason, ready.Message) + cs.MarkTopicFailed(ready.Reason, ready.Message) default: cs.MarkTopicUnknown("TopicUnknown", "The status of Topic is invalid: %v", ready.Status) } } -// MarkTopicFalse sets the condition that signals there is not a topic for this +// MarkTopicFailed sets the condition that signals there is not a topic for this // Channel. This could be because of an error or the Channel is being deleted. -func (cs *ChannelStatus) MarkTopicFalse(reason, messageFormat string, messageA ...interface{}) { +func (cs *ChannelStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { channelCondSet.Manage(cs).MarkFalse(ChannelConditionTopicReady, reason, messageFormat, messageA...) } diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index 8d28e898f1..b8b6f2ca0e 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -186,7 +186,7 @@ func TestChannelIsReady(t *testing.T) { } else if test.topicStatus == corev1.ConditionUnknown { cs.MarkTopicUnknown("The status of topic is unknown", "The status of topic is unknown: nil") } else { - cs.MarkTopicFalse("The status of topic is false", "The status of topic is unknown: nil") + cs.MarkTopicFailed("The status of topic is false", "The status of topic is unknown: nil") } got := cs.GetTopLevelCondition().Status diff --git a/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go b/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go index 092cb611a4..0ec0ee30c3 100644 --- a/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go +++ b/pkg/apis/pubsub/v1alpha1/topic_lifecycle.go @@ -84,7 +84,7 @@ func (ts *TopicStatus) MarkTopicReady() { topicCondSet.Manage(ts).MarkTrue(TopicConditionTopicExists) } -// MarkTopicFalse sets the condition that signals there is not a topic for this +// MarkNoTopic sets the condition that signals there is not a topic for this // Topic. This could be because of an error or the Topic is being deleted. func (ts *TopicStatus) MarkNoTopic(reason, messageFormat string, messageA ...interface{}) { topicCondSet.Manage(ts).MarkFalse(TopicConditionTopicExists, reason, messageFormat, messageA...) diff --git a/pkg/reconciler/events/auditlogs/auditlogs_test.go b/pkg/reconciler/events/auditlogs/auditlogs_test.go index 2b35d1a3e7..cec5e96ab8 100644 --- a/pkg/reconciler/events/auditlogs/auditlogs_test.go +++ b/pkg/reconciler/events/auditlogs/auditlogs_test.go @@ -204,7 +204,7 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicFalse("TopicNotReady", `Topic "test-auditlogssource" did not expose projectid`), + WithAuditLogsSourceTopicFailed("TopicNotReady", `Topic "test-auditlogssource" did not expose projectid`), ), }}, WantEvents: []string{ @@ -228,7 +228,7 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicFalse("TopicNotReady", `Topic "test-auditlogssource" did not expose topicid`), + WithAuditLogsSourceTopicFailed("TopicNotReady", `Topic "test-auditlogssource" did not expose topicid`), ), }}, WantEvents: []string{ @@ -252,7 +252,7 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicFalse("TopicNotReady", `Topic "test-auditlogssource" mismatch: expected "auditlogssource-test-auditlogssource-uid" got "garbaaaaage"`), + WithAuditLogsSourceTopicFailed("TopicNotReady", `Topic "test-auditlogssource" mismatch: expected "auditlogssource-test-auditlogssource-uid" got "garbaaaaage"`), ), }}, WantEvents: []string{ @@ -265,7 +265,7 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceFinalizers(finalizerName), ), NewTopic(sourceName, testNS, - WithTopicFalse(), + WithTopicFailed(), WithTopicTopicID(testTopicID), ), }, @@ -275,7 +275,7 @@ func TestAllCases(t *testing.T) { Object: NewAuditLogsSource(sourceName, testNS, WithAuditLogsSourceFinalizers(finalizerName), WithInitAuditLogsSourceConditions, - WithAuditLogsSourceTopicFalse("PublisherStatus", "Publisher has no Ready type status")), + WithAuditLogsSourceTopicFailed("PublisherStatus", "Publisher has no Ready type status")), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", "the status of Topic %q is False", sourceName), @@ -388,7 +388,7 @@ func TestAllCases(t *testing.T) { WithTopicAddress(testTopicURI), WithTopicProjectID(testProject), ), - NewPullSubscriptionWithNoDefaults(sourceName, testNS, WithPullSubscriptionFalse()), + NewPullSubscriptionWithNoDefaults(sourceName, testNS, WithPullSubscriptionFailed()), }, Key: testNS + "/" + sourceName, WantErr: true, @@ -399,7 +399,7 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceProjectID(testProject), WithInitAuditLogsSourceConditions, WithAuditLogsSourceTopicReady(testTopicID), - WithAuditLogsSourcePullSubscriptionFalse("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), + WithAuditLogsSourcePullSubscriptionFailed("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), ), }}, WantEvents: []string{ @@ -880,8 +880,8 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceSink(sinkGVK, sinkName), WithInitAuditLogsSourceConditions, WithAuditLogsSourceSinkNotReady("SinkDeleted", "Successfully deleted Stackdriver sink: %s", testSinkID), - WithAuditLogsSourceTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), - WithAuditLogsSourcePullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), + WithAuditLogsSourceTopicFailed("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), + WithAuditLogsSourcePullSubscriptionFailed("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), WithAuditLogsSourceDeletionTimestamp, ), }}, @@ -938,8 +938,8 @@ func TestAllCases(t *testing.T) { WithAuditLogsSourceSink(sinkGVK, sinkName), WithInitAuditLogsSourceConditions, WithAuditLogsSourceSinkNotReady("SinkDeleted", "Successfully deleted Stackdriver sink: %s", testSinkID), - WithAuditLogsSourceTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), - WithAuditLogsSourcePullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), + WithAuditLogsSourceTopicFailed("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", sourceName)), + WithAuditLogsSourcePullSubscriptionFailed("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", sourceName)), WithAuditLogsSourceDeletionTimestamp, ), }}, diff --git a/pkg/reconciler/events/pubsub/pubsub.go b/pkg/reconciler/events/pubsub/pubsub.go index 57ba8fa81d..40870d759f 100644 --- a/pkg/reconciler/events/pubsub/pubsub.go +++ b/pkg/reconciler/events/pubsub/pubsub.go @@ -124,10 +124,10 @@ func (r *Reconciler) reconcile(ctx context.Context, pubsub *v1alpha1.PubSub) err ps, err := r.reconcilePullSubscription(ctx, pubsub) if err != nil { - pubsub.Status.MarkPullSubscriptionFalse("PullSubscriptionReconcileFailed", "Failed to reconcile PullSubscription: %s", err.Error()) + pubsub.Status.MarkPullSubscriptionFailed("PullSubscriptionReconcileFailed", "Failed to reconcile PullSubscription: %s", err.Error()) return err } - pubsub.Status.PropagatePullSubscriptionStatus(ps.Status.GetCondition(apis.ConditionReady)) + pubsub.Status.PropagatePullSubscriptionStatus(ps.Status.GetTopLevelCondition()) // Sink has been resolved from the underlying PullSubscription, set it here. sinkURI, err := apis.ParseURL(ps.Status.SinkURI) diff --git a/pkg/reconciler/events/pubsub/pubsub_test.go b/pkg/reconciler/events/pubsub/pubsub_test.go index 540cced577..a3d1525c71 100644 --- a/pkg/reconciler/events/pubsub/pubsub_test.go +++ b/pkg/reconciler/events/pubsub/pubsub_test.go @@ -191,7 +191,7 @@ func TestAllCases(t *testing.T) { WithPubSubFinalizers(finalizerName), WithInitPubSubConditions, WithPubSubObjectMetaGeneration(generation), - WithPubSubPullSubscriptionFalse("PullSubscriptionFalse", "status false test message"), + WithPubSubPullSubscriptionFailed("PullSubscriptionFalse", "status false test message"), ), }}, WantEvents: []string{ diff --git a/pkg/reconciler/events/scheduler/scheduler_test.go b/pkg/reconciler/events/scheduler/scheduler_test.go index 6a9103e250..0890f2e565 100644 --- a/pkg/reconciler/events/scheduler/scheduler_test.go +++ b/pkg/reconciler/events/scheduler/scheduler_test.go @@ -254,7 +254,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicFalse("TopicNotReady", `Topic "my-test-scheduler" did not expose projectid`), + WithSchedulerTopicFailed("TopicNotReady", `Topic "my-test-scheduler" did not expose projectid`), WithSchedulerFinalizers(finalizerName), ), }}, @@ -287,7 +287,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicFalse("TopicNotReady", `Topic "my-test-scheduler" did not expose topicid`), + WithSchedulerTopicFailed("TopicNotReady", `Topic "my-test-scheduler" did not expose topicid`), WithSchedulerFinalizers(finalizerName), ), }}, @@ -320,7 +320,7 @@ func TestAllCases(t *testing.T) { WithSchedulerData(testData), WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, - WithSchedulerTopicFalse("TopicNotReady", `Topic "my-test-scheduler" mismatch: expected "scheduler-test-scheduler-uid" got "garbaaaaage"`), + WithSchedulerTopicFailed("TopicNotReady", `Topic "my-test-scheduler" mismatch: expected "scheduler-test-scheduler-uid" got "garbaaaaage"`), WithSchedulerFinalizers(finalizerName), ), }}, @@ -338,7 +338,7 @@ func TestAllCases(t *testing.T) { WithSchedulerFinalizers(finalizerName), ), NewTopic(schedulerName, testNS, - WithTopicFalse(), + WithTopicFailed(), WithTopicProjectID(testProject), ), newSink(), @@ -353,7 +353,7 @@ func TestAllCases(t *testing.T) { WithSchedulerSchedule(onceAMinuteSchedule), WithSchedulerFinalizers(finalizerName), WithInitSchedulerConditions, - WithSchedulerTopicFalse("PublisherStatus", "Publisher has no Ready type status"), + WithSchedulerTopicFailed("PublisherStatus", "Publisher has no Ready type status"), ), }}, WantEvents: []string{ @@ -492,7 +492,7 @@ func TestAllCases(t *testing.T) { WithTopicAddress(testTopicURI), WithTopicProjectID(testProject), ), - NewPullSubscriptionWithNoDefaults(schedulerName, testNS, WithPullSubscriptionFalse()), + NewPullSubscriptionWithNoDefaults(schedulerName, testNS, WithPullSubscriptionFailed()), newSink(), }, Key: testNS + "/" + schedulerName, @@ -506,7 +506,7 @@ func TestAllCases(t *testing.T) { WithSchedulerFinalizers(finalizerName), WithInitSchedulerConditions, WithSchedulerTopicReady(testTopicID, testProject), - WithSchedulerPullSubscriptionFalse("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), + WithSchedulerPullSubscriptionFailed("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), ), }}, WantEvents: []string{ @@ -959,8 +959,8 @@ func TestAllCases(t *testing.T) { WithSchedulerSchedule(onceAMinuteSchedule), WithInitSchedulerConditions, WithSchedulerJobNotReady("JobDeleted", fmt.Sprintf("Successfully deleted Scheduler job: %s", jobName)), - WithSchedulerTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", schedulerName)), - WithSchedulerPullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", schedulerName)), + WithSchedulerTopicFailed("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", schedulerName)), + WithSchedulerPullSubscriptionFailed("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", schedulerName)), WithSchedulerDeletionTimestamp, ), }}, diff --git a/pkg/reconciler/events/storage/storage_test.go b/pkg/reconciler/events/storage/storage_test.go index 67e943dacc..673094afef 100644 --- a/pkg/reconciler/events/storage/storage_test.go +++ b/pkg/reconciler/events/storage/storage_test.go @@ -246,7 +246,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicFalse("TopicNotReady", `Topic "my-test-storage" did not expose projectid`), + WithStorageTopicFailed("TopicNotReady", `Topic "my-test-storage" did not expose projectid`), WithStorageFinalizers(finalizerName), ), }}, @@ -277,7 +277,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicFalse("TopicNotReady", `Topic "my-test-storage" did not expose topicid`), + WithStorageTopicFailed("TopicNotReady", `Topic "my-test-storage" did not expose topicid`), WithStorageFinalizers(finalizerName), ), }}, @@ -308,7 +308,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicFalse("TopicNotReady", `Topic "my-test-storage" mismatch: expected "storage-test-storage-uid" got "garbaaaaage"`), + WithStorageTopicFailed("TopicNotReady", `Topic "my-test-storage" mismatch: expected "storage-test-storage-uid" got "garbaaaaage"`), WithStorageFinalizers(finalizerName), ), }}, @@ -325,7 +325,7 @@ func TestAllCases(t *testing.T) { WithStorageFinalizers(finalizerName), ), NewTopic(storageName, testNS, - WithTopicFalse(), + WithTopicFailed(), WithTopicProjectID(testProject), ), newSink(), @@ -338,7 +338,7 @@ func TestAllCases(t *testing.T) { WithStorageBucket(bucket), WithStorageSink(sinkGVK, sinkName), WithInitStorageConditions, - WithStorageTopicFalse("PublisherStatus", "Publisher has no Ready type status"), + WithStorageTopicFailed("PublisherStatus", "Publisher has no Ready type status"), WithStorageFinalizers(finalizerName), ), }}, @@ -473,7 +473,7 @@ func TestAllCases(t *testing.T) { WithTopicAddress(testTopicURI), WithTopicProjectID(testProject), ), - NewPullSubscriptionWithNoDefaults(storageName, testNS, WithPullSubscriptionFalse()), + NewPullSubscriptionWithNoDefaults(storageName, testNS, WithPullSubscriptionFailed()), }, Key: testNS + "/" + storageName, WantErr: true, @@ -486,7 +486,7 @@ func TestAllCases(t *testing.T) { WithInitStorageConditions, WithStorageTopicReady(testTopicID), WithStorageProjectID(testProject), - WithStoragePullSubscriptionFalse("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), + WithStoragePullSubscriptionFailed("InvalidSink", `failed to get ref &ObjectReference{Kind:Sink,Namespace:testnamespace,Name:sink,UID:,APIVersion:testing.cloud.google.com/v1alpha1,ResourceVersion:,FieldPath:,}: sinks.testing.cloud.google.com "sink" not found`), ), }}, WantEvents: []string{ @@ -906,8 +906,8 @@ func TestAllCases(t *testing.T) { WithInitStorageConditions, WithStorageObjectMetaGeneration(generation), WithStorageNotificationNotReady("NotificationDeleted", fmt.Sprintf("Successfully deleted Storage notification: %s", notificationId)), - WithStorageTopicFalse("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", storageName)), - WithStoragePullSubscriptionFalse("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", storageName)), + WithStorageTopicFailed("TopicDeleted", fmt.Sprintf("Successfully deleted Topic: %s", storageName)), + WithStoragePullSubscriptionFailed("PullSubscriptionDeleted", fmt.Sprintf("Successfully deleted PullSubscription: %s", storageName)), WithDeletionTimestamp()), }}, }} diff --git a/pkg/reconciler/messaging/channel/channel.go b/pkg/reconciler/messaging/channel/channel.go index aca95514bd..8127f6a4e1 100644 --- a/pkg/reconciler/messaging/channel/channel.go +++ b/pkg/reconciler/messaging/channel/channel.go @@ -121,7 +121,7 @@ func (r *Reconciler) reconcile(ctx context.Context, channel *v1alpha1.Channel) e // 1. Create the Topic. topic, err := r.reconcileTopic(ctx, channel) if err != nil { - channel.Status.MarkTopicFalse("TopicReconcileFailed", "Failed to reconcile Topic: %s", err.Error()) + channel.Status.MarkTopicFailed("TopicReconcileFailed", "Failed to reconcile Topic: %s", err.Error()) return err } channel.Status.PropagateTopicStatus(topic.Status.GetCondition(pubsubv1alpha1.TopicConditionReady)) diff --git a/pkg/reconciler/messaging/channel/channel_test.go b/pkg/reconciler/messaging/channel/channel_test.go index 86fa0ea1bf..65c518d83c 100644 --- a/pkg/reconciler/messaging/channel/channel_test.go +++ b/pkg/reconciler/messaging/channel/channel_test.go @@ -185,7 +185,7 @@ func TestAllCases(t *testing.T) { // Updates WithChannelAddress(topicURI), WithChannelSubscribersStatus([]eventingduck.SubscriberStatus(nil)), - WithChannelTopicFalse("PublisherStatus", "Publisher has no Ready type status"), + WithChannelTopicFailed("PublisherStatus", "Publisher has no Ready type status"), ), }}, }, diff --git a/pkg/reconciler/pubsub/reconciler.go b/pkg/reconciler/pubsub/reconciler.go index 52d8adc95e..77f3bb9f9f 100644 --- a/pkg/reconciler/pubsub/reconciler.go +++ b/pkg/reconciler/pubsub/reconciler.go @@ -125,7 +125,7 @@ func propagatePullSubscriptionStatus(ps *pubsubsourcev1alpha1.PullSubscription, case pc.Status == corev1.ConditionTrue: status.MarkPullSubscriptionReady(cs) case pc.Status == corev1.ConditionFalse: - status.MarkPullSubscriptionFalse(cs, pc.Reason, pc.Message) + status.MarkPullSubscriptionFailed(cs, pc.Reason, pc.Message) return fmt.Errorf("the status of PullSubscription %q is False", ps.Name) default: status.MarkPullSubscriptionUnknown(cs, "PullSubscriptionUnknown", "The status of PullSubscription is invalid: %v", pc.Status) @@ -144,7 +144,7 @@ func propagateTopicStatus(t *pubsubsourcev1alpha1.Topic, status *v1alpha1.PubSub status.MarkTopicUnknown(cs, tc.Reason, tc.Message) return fmt.Errorf("the status of Topic %q is Unknown", t.Name) } else if tc.Status == corev1.ConditionFalse { - status.MarkTopicFalse(cs, tc.Reason, tc.Message) + status.MarkTopicFailed(cs, tc.Reason, tc.Message) return fmt.Errorf("the status of Topic %q is False", t.Name) } else if tc.Status != corev1.ConditionTrue { status.MarkTopicUnknown(cs, "TopicUnknown", "The status of Topic is invalid: %v", tc.Status) @@ -152,15 +152,15 @@ func propagateTopicStatus(t *pubsubsourcev1alpha1.Topic, status *v1alpha1.PubSub } if t.Status.ProjectID == "" { - status.MarkTopicFalse(cs, "TopicNotReady", "Topic %q did not expose projectid", t.Name) + status.MarkTopicFailed(cs, "TopicNotReady", "Topic %q did not expose projectid", t.Name) return fmt.Errorf("Topic %q did not expose projectid", t.Name) } if t.Status.TopicID == "" { - status.MarkTopicFalse(cs, "TopicNotReady", "Topic %q did not expose topicid", t.Name) + status.MarkTopicFailed(cs, "TopicNotReady", "Topic %q did not expose topicid", t.Name) return fmt.Errorf("Topic %q did not expose topicid", t.Name) } if t.Status.TopicID != topic { - status.MarkTopicFalse(cs, "TopicNotReady", "Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) + status.MarkTopicFailed(cs, "TopicNotReady", "Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) return fmt.Errorf("Topic %q mismatch: expected %q got %q", t.Name, topic, t.Status.TopicID) } status.TopicID = t.Status.TopicID @@ -182,10 +182,10 @@ func (psb *PubSubBase) DeletePubSub(ctx context.Context, pubsubable duck.PubSuba err := psb.pubsubClient.PubsubV1alpha1().Topics(namespace).Delete(name, nil) if err != nil && !apierrs.IsNotFound(err) { logging.FromContext(ctx).Desugar().Error("Failed to delete Topic", zap.String("name", name), zap.Error(err)) - status.MarkTopicFalse(cs, "TopicDeleteFailed", "Failed to delete Topic: %s", err.Error()) + status.MarkTopicFailed(cs, "TopicDeleteFailed", "Failed to delete Topic: %s", err.Error()) return fmt.Errorf("failed to delete topic: %w", err) } - status.MarkTopicFalse(cs, "TopicDeleted", "Successfully deleted Topic: %s", name) + status.MarkTopicFailed(cs, "TopicDeleted", "Successfully deleted Topic: %s", name) status.TopicID = "" status.ProjectID = "" @@ -193,10 +193,10 @@ func (psb *PubSubBase) DeletePubSub(ctx context.Context, pubsubable duck.PubSuba err = psb.pubsubClient.PubsubV1alpha1().PullSubscriptions(namespace).Delete(name, nil) if err != nil && !apierrs.IsNotFound(err) { logging.FromContext(ctx).Desugar().Error("Failed to delete PullSubscription", zap.String("name", name), zap.Error(err)) - status.MarkPullSubscriptionFalse(cs, "PullSubscriptionDeleteFailed", "Failed to delete PullSubscription: %s", err.Error()) + status.MarkPullSubscriptionFailed(cs, "PullSubscriptionDeleteFailed", "Failed to delete PullSubscription: %s", err.Error()) return fmt.Errorf("failed to delete PullSubscription: %w", err) } - status.MarkPullSubscriptionFalse(cs, "PullSubscriptionDeleted", "Successfully deleted PullSubscription: %s", name) + status.MarkPullSubscriptionFailed(cs, "PullSubscriptionDeleted", "Successfully deleted PullSubscription: %s", name) status.SinkURI = nil return nil } diff --git a/pkg/reconciler/pubsub/reconciler_test.go b/pkg/reconciler/pubsub/reconciler_test.go index 54578b9232..50d6f12b43 100644 --- a/pkg/reconciler/pubsub/reconciler_test.go +++ b/pkg/reconciler/pubsub/reconciler_test.go @@ -185,7 +185,7 @@ func TestCreates(t *testing.T) { }), rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), rectesting.WithTopicProjectID(testProjectID), - rectesting.WithTopicFalse(), + rectesting.WithTopicFailed(), ), }, expectedTopic: rectesting.NewTopic(name, testNS, @@ -198,7 +198,7 @@ func TestCreates(t *testing.T) { "receive-adapter": receiveAdapterName, }), rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), - rectesting.WithTopicFalse(), + rectesting.WithTopicFailed(), rectesting.WithTopicProjectID(testProjectID), rectesting.WithTopicOwnerReferences([]metav1.OwnerReference{ownerRef()}), ), @@ -418,7 +418,7 @@ func TestCreates(t *testing.T) { "metrics-resource-group": resourceGroup, }), rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), - rectesting.WithPullSubscriptionFalse(), + rectesting.WithPullSubscriptionFailed(), ), }, expectedTopic: rectesting.NewTopic(name, testNS, @@ -448,7 +448,7 @@ func TestCreates(t *testing.T) { "metrics-resource-group": resourceGroup, }), rectesting.WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}), - rectesting.WithPullSubscriptionFalse(), + rectesting.WithPullSubscriptionFailed(), ), expectedErr: fmt.Sprintf("the status of PullSubscription %q is False", name), },{ diff --git a/pkg/reconciler/testing/auditlogs.go b/pkg/reconciler/testing/auditlogs.go index a329795591..8b478a0026 100644 --- a/pkg/reconciler/testing/auditlogs.go +++ b/pkg/reconciler/testing/auditlogs.go @@ -48,9 +48,9 @@ func WithInitAuditLogsSourceConditions(s *v1alpha1.AuditLogsSource) { s.Status.InitializeConditions() } -func WithAuditLogsSourceTopicFalse(reason, message string) AuditLogsSourceOption { +func WithAuditLogsSourceTopicFailed(reason, message string) AuditLogsSourceOption { return func(s *v1alpha1.AuditLogsSource) { - s.Status.MarkTopicFalse(reason, message) + s.Status.MarkTopicFailed(reason, message) } } @@ -67,9 +67,9 @@ func WithAuditLogsSourceTopicReady(topicID string) AuditLogsSourceOption { } } -func WithAuditLogsSourcePullSubscriptionFalse(reason, message string) AuditLogsSourceOption { +func WithAuditLogsSourcePullSubscriptionFailed(reason, message string) AuditLogsSourceOption { return func(s *v1alpha1.AuditLogsSource) { - s.Status.MarkPullSubscriptionFalse(reason, message) + s.Status.MarkPullSubscriptionFailed(reason, message) } } diff --git a/pkg/reconciler/testing/channel.go b/pkg/reconciler/testing/channel.go index 84b012543b..1d5b4be2a2 100644 --- a/pkg/reconciler/testing/channel.go +++ b/pkg/reconciler/testing/channel.go @@ -92,9 +92,9 @@ func WithChannelTopicID(topicID string) ChannelOption { } } -func WithChannelTopicFalse(reason, message string) ChannelOption { +func WithChannelTopicFailed(reason, message string) ChannelOption { return func(c *v1alpha1.Channel) { - c.Status.MarkTopicFalse(reason, message) + c.Status.MarkTopicFailed(reason, message) } } diff --git a/pkg/reconciler/testing/pubsub.go b/pkg/reconciler/testing/pubsub.go index 38e6c559af..26a1d69b94 100644 --- a/pkg/reconciler/testing/pubsub.go +++ b/pkg/reconciler/testing/pubsub.go @@ -70,11 +70,11 @@ func WithInitPubSubConditions(ps *v1alpha1.PubSub) { ps.Status.InitializeConditions() } -// WithPubSubPullSubscriptionFalse marks the condition that the -// topic is False -func WithPubSubPullSubscriptionFalse(reason, message string) PubSubOption { +// WithPubSubPullSubscriptionFailed marks the condition that the +// status of PullSubscription is False +func WithPubSubPullSubscriptionFailed(reason, message string) PubSubOption { return func(ps *v1alpha1.PubSub) { - ps.Status.MarkPullSubscriptionFalse(reason, message) + ps.Status.MarkPullSubscriptionFailed(reason, message) } } diff --git a/pkg/reconciler/testing/pullsubscription.go b/pkg/reconciler/testing/pullsubscription.go index 7adc5b9bf5..f8a0171206 100644 --- a/pkg/reconciler/testing/pullsubscription.go +++ b/pkg/reconciler/testing/pullsubscription.go @@ -193,7 +193,7 @@ func WithPullSubscriptionReady(sink string) PullSubscriptionOption { } } -func WithPullSubscriptionFalse() PullSubscriptionOption { +func WithPullSubscriptionFailed() PullSubscriptionOption { return func(s *v1alpha1.PullSubscription) { s.Status.InitializeConditions() s.Status.MarkNoSink("InvalidSink", diff --git a/pkg/reconciler/testing/scheduler.go b/pkg/reconciler/testing/scheduler.go index 143796803b..422798a929 100644 --- a/pkg/reconciler/testing/scheduler.go +++ b/pkg/reconciler/testing/scheduler.go @@ -94,11 +94,11 @@ func WithInitSchedulerConditions(s *v1alpha1.Scheduler) { s.Status.InitializeConditions() } -// WithSchedulerTopicFalse marks the condition that the +// WithSchedulerTopicFailed marks the condition that the // status of topic is False. -func WithSchedulerTopicFalse(reason, message string) SchedulerOption { +func WithSchedulerTopicFailed(reason, message string) SchedulerOption { return func(s *v1alpha1.Scheduler) { - s.Status.MarkTopicFalse(reason, message) + s.Status.MarkTopicFailed(reason, message) } } @@ -118,11 +118,11 @@ func WithSchedulerTopicReady(topicID, projectID string) SchedulerOption { } } -// WithSchedulerPullSubscriptionFalse marks the condition that the +// WithSchedulerPullSubscriptionFailed marks the condition that the // topic is False. -func WithSchedulerPullSubscriptionFalse(reason, message string) SchedulerOption { +func WithSchedulerPullSubscriptionFailed(reason, message string) SchedulerOption { return func(s *v1alpha1.Scheduler) { - s.Status.MarkPullSubscriptionFalse(reason, message) + s.Status.MarkPullSubscriptionFailed(reason, message) } } diff --git a/pkg/reconciler/testing/storage.go b/pkg/reconciler/testing/storage.go index bd31913f4d..806f2b4889 100644 --- a/pkg/reconciler/testing/storage.go +++ b/pkg/reconciler/testing/storage.go @@ -83,11 +83,11 @@ func WithInitStorageConditions(s *v1alpha1.Storage) { s.Status.InitializeConditions() } -// WithStorageTopicFalse marks the condition that the +// WithStorageTopicFailed marks the condition that the // topic is False -func WithStorageTopicFalse(reason, message string) StorageOption { +func WithStorageTopicFailed(reason, message string) StorageOption { return func(s *v1alpha1.Storage) { - s.Status.MarkTopicFalse(reason, message) + s.Status.MarkTopicFailed(reason, message) } } @@ -114,24 +114,24 @@ func WithStorageTopicID(topicID string) StorageOption { } } -// WithStoragePullSubscriptionFalse marks the condition that the +// WithStoragePullSubscriptionFailed marks the condition that the // status of topic is False -func WithStoragePullSubscriptionFalse(reason, message string) StorageOption { +func WithStoragePullSubscriptionFailed(reason, message string) StorageOption { return func(s *v1alpha1.Storage) { - s.Status.MarkPullSubscriptionFalse(reason, message) + s.Status.MarkPullSubscriptionFailed(reason, message) } } // WithStoragePullSubscriptionUnknown marks the condition that the -// status of topic is Unknown +// status of topic is Unknown. func WithStoragePullSubscriptionUnknown(reason, message string) StorageOption { return func(s *v1alpha1.Storage) { s.Status.MarkPullSubscriptionUnknown(reason, message) } } -// WithStoragePullSubscriptionNotReady marks the condition that the -// topic is not ready +// WithStoragePullSubscriptionReady marks the condition that the +// topic is ready. func WithStoragePullSubscriptionReady() StorageOption { return func(s *v1alpha1.Storage) { s.Status.MarkPullSubscriptionReady() diff --git a/pkg/reconciler/testing/topic.go b/pkg/reconciler/testing/topic.go index d9e7bbb7a3..f63679c938 100644 --- a/pkg/reconciler/testing/topic.go +++ b/pkg/reconciler/testing/topic.go @@ -126,7 +126,7 @@ func WithTopicReady(topicID string) TopicOption { } } -func WithTopicFalse() TopicOption { +func WithTopicFailed() TopicOption { return func(s *v1alpha1.Topic) { s.Status.InitializeConditions() s.Status.MarkNotDeployed("PublisherStatus", "Publisher has no Ready type status") From 5db090c4b10a0c8755126cb14058f7e8e2d9493c Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Tue, 14 Jan 2020 19:55:34 -0800 Subject: [PATCH 3/8] added unit tests --- .../v1alpha1/auditlogs_lifecycle_test.go | 21 +++- .../events/v1alpha1/pubsub_lifecycle_test.go | 19 ++-- .../events/v1alpha1/scheduler_lifecycle.go | 5 + .../v1alpha1/scheduler_lifecycle_test.go | 101 ++++++++++++------ pkg/apis/events/v1alpha1/storage_lifecycle.go | 5 + .../events/v1alpha1/storage_lifecycle_test.go | 87 ++++++++++----- pkg/apis/events/v1alpha1/test_helper.go | 3 +- .../v1alpha1/channel_lifecycle_test.go | 57 +++++++--- .../pull_subscription_lifecycle_test.go | 50 ++++++--- .../pubsub/v1alpha1/topic_lifecycle_test.go | 32 ++++-- 10 files changed, 269 insertions(+), 111 deletions(-) diff --git a/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go b/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go index eff6a9a9ef..4618264e11 100644 --- a/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go @@ -32,6 +32,10 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { wantConditionStatus corev1.ConditionStatus want bool }{{ + name: "uninitialized", + s: &AuditLogsSourceStatus{}, + want: false, + }, { name: "initialized", s: func() *AuditLogsSourceStatus { s := &AuditLogsSourceStatus{} @@ -39,6 +43,7 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { return s }(), wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "the status of topic is false", s: func() *AuditLogsSourceStatus { @@ -50,6 +55,7 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { return s }(), wantConditionStatus: corev1.ConditionFalse, + want: false, }, { name: "the status of topic is unknown", s: func() *AuditLogsSourceStatus { @@ -61,6 +67,7 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { return s }(), wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "the status of pullsubscription is false", @@ -84,6 +91,7 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { return s }(), wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "sink is not ready", @@ -96,6 +104,7 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { return s }(), wantConditionStatus: corev1.ConditionFalse, + want: false, }, { name: "ready", s: func() *AuditLogsSourceStatus { @@ -107,15 +116,17 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { return s }(), wantConditionStatus: corev1.ConditionTrue, - want:true, + want: true, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { - gotConditionStatus := test.s.GetTopLevelCondition().Status - got := test.s.IsReady() - if gotConditionStatus != test.wantConditionStatus { - t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + if test.wantConditionStatus != "" { + gotConditionStatus := test.s.GetTopLevelCondition().Status + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } } + got := test.s.IsReady() if got != test.want { t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go index 8085dc4b23..f9467fc45c 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -33,15 +33,16 @@ func TestPubSubStatusIsReady(t *testing.T) { pullsubscriptionStatus *pubsubv1alpha1.PullSubscriptionStatus wantConditionStatus corev1.ConditionStatus want bool - }{{ - name: "the status of pullsubscription is false", - pullsubscriptionStatus: TestHelper.FalsePullSubscriptionStatus(), - wantConditionStatus: corev1.ConditionFalse, - }, { - name: "the status of pullsubscription is unknown", - pullsubscriptionStatus: TestHelper.UnknownPullSubscriptionStatus(), - wantConditionStatus: corev1.ConditionUnknown, - }, + }{ + { + name: "the status of pullsubscription is false", + pullsubscriptionStatus: TestHelper.FalsePullSubscriptionStatus(), + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "the status of pullsubscription is unknown", + pullsubscriptionStatus: TestHelper.UnknownPullSubscriptionStatus(), + wantConditionStatus: corev1.ConditionUnknown, + }, { name: "ready", pullsubscriptionStatus: TestHelper.ReadyPullSubscriptionStatus(), diff --git a/pkg/apis/events/v1alpha1/scheduler_lifecycle.go b/pkg/apis/events/v1alpha1/scheduler_lifecycle.go index a575634b9e..e09743bc9c 100644 --- a/pkg/apis/events/v1alpha1/scheduler_lifecycle.go +++ b/pkg/apis/events/v1alpha1/scheduler_lifecycle.go @@ -27,6 +27,11 @@ func (s *SchedulerStatus) GetCondition(t apis.ConditionType) *apis.Condition { return schedulerCondSet.Manage(s).GetCondition(t) } +// GetTopLevelCondition returns the top level condition. +func (s *SchedulerStatus) GetTopLevelCondition() *apis.Condition { + return schedulerCondSet.Manage(s).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (s *SchedulerStatus) IsReady() bool { return schedulerCondSet.Manage(s).IsHappy() diff --git a/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go b/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go index 1387e1f995..1844c0d673 100644 --- a/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go @@ -28,9 +28,10 @@ import ( func TestSchedulerStatusIsReady(t *testing.T) { tests := []struct { - name string - s *SchedulerStatus - want bool + name string + s *SchedulerStatus + wantConditionStatus corev1.ConditionStatus + want bool }{{ name: "uninitialized", s: &SchedulerStatus{}, @@ -42,54 +43,92 @@ func TestSchedulerStatusIsReady(t *testing.T) { s.InitializeConditions() return s }(), + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { - name: "topic not ready", + name: "the status of topic is false", s: func() *SchedulerStatus { s := &SchedulerStatus{} s.InitializeConditions() s.MarkPullSubscriptionReady() s.MarkJobReady("jobName") - s.MarkTopicFailed("NotReady", "topic not ready") + s.MarkTopicFailed("TopicFailed", "the status of topic is false") return s }(), + wantConditionStatus: corev1.ConditionFalse, + want: false, }, { - name: "pullsubscription not ready", + name: "the status of topic is unknown", s: func() *SchedulerStatus { s := &SchedulerStatus{} s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") - s.MarkPullSubscriptionFailed("NotReady", "ps not ready") - s.MarkJobReady("jobName") - return s - }(), - }, { - name: "job not ready", - s: func() *SchedulerStatus { - s := &SchedulerStatus{} - s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") - s.MarkPullSubscriptionReady() - s.MarkJobNotReady("NotReady", "ps not ready") - return s - }(), - }, { - name: "ready", - s: func() *SchedulerStatus { - s := &SchedulerStatus{} - s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") s.MarkPullSubscriptionReady() s.MarkJobReady("jobName") + s.MarkTopicUnknown("TopicUnknown", "the status of topic is unknown") return s }(), - want: true, - }} + wantConditionStatus: corev1.ConditionUnknown, + want: false, + }, + { + name: "the status pullsubscription is false", + s: func() *SchedulerStatus { + s := &SchedulerStatus{} + s.InitializeConditions() + s.MarkTopicReady("topicID", "projectID") + s.MarkPullSubscriptionFailed("PullSubscriptionFailed", "the status of pullsubscription is false") + s.MarkJobReady("jobName") + return s + }(), + wantConditionStatus: corev1.ConditionFalse, + want: false, + }, { + name: "the status pullsubscription is unknown", + s: func() *SchedulerStatus { + s := &SchedulerStatus{} + s.InitializeConditions() + s.MarkTopicReady("topicID", "projectID") + s.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "the status of pullsubscription is unknown") + s.MarkJobReady("jobName") + return s + }(), + wantConditionStatus: corev1.ConditionUnknown, + want: false, + }, + { + name: "job not ready", + s: func() *SchedulerStatus { + s := &SchedulerStatus{} + s.InitializeConditions() + s.MarkTopicReady("topicID", "projectID") + s.MarkPullSubscriptionReady() + s.MarkJobNotReady("NotReady", "ps not ready") + return s + }(), + }, { + name: "ready", + s: func() *SchedulerStatus { + s := &SchedulerStatus{} + s.InitializeConditions() + s.MarkTopicReady("topicID", "projectID") + s.MarkPullSubscriptionReady() + s.MarkJobReady("jobName") + return s + }(), + want: true, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + if test.wantConditionStatus != "" { + gotConditionStatus := test.s.GetTopLevelCondition().Status + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } + } got := test.s.IsReady() - if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("%s: unexpected condition (-want, +got) = %v", test.name, diff) + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/events/v1alpha1/storage_lifecycle.go b/pkg/apis/events/v1alpha1/storage_lifecycle.go index a82c77da80..a5e83653a7 100644 --- a/pkg/apis/events/v1alpha1/storage_lifecycle.go +++ b/pkg/apis/events/v1alpha1/storage_lifecycle.go @@ -27,6 +27,11 @@ func (s *StorageStatus) GetCondition(t apis.ConditionType) *apis.Condition { return storageCondSet.Manage(s).GetCondition(t) } +// GetTopLevelCondition returns the top level condition. +func (s *StorageStatus) GetTopLevelCondition() *apis.Condition { + return storageCondSet.Manage(s).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (s *StorageStatus) IsReady() bool { return storageCondSet.Manage(s).IsHappy() diff --git a/pkg/apis/events/v1alpha1/storage_lifecycle_test.go b/pkg/apis/events/v1alpha1/storage_lifecycle_test.go index d265aefa0a..689aaf3d15 100644 --- a/pkg/apis/events/v1alpha1/storage_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/storage_lifecycle_test.go @@ -28,9 +28,10 @@ import ( func TestStorageStatusIsReady(t *testing.T) { tests := []struct { - name string - s *StorageStatus - want bool + name string + s *StorageStatus + wantConditionStatus corev1.ConditionStatus + want bool }{{ name: "uninitialized", s: &StorageStatus{}, @@ -42,54 +43,92 @@ func TestStorageStatusIsReady(t *testing.T) { s.InitializeConditions() return s }(), + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { - name: "topic not ready", + name: "the status of topic is false", s: func() *StorageStatus { s := &StorageStatus{} s.InitializeConditions() s.MarkPullSubscriptionReady() s.MarkNotificationReady("notificationID") - s.MarkTopicFailed("NotReady", "topic not ready") + s.MarkTopicFailed("TopicFailed", "the status of topic is false") return s }(), + wantConditionStatus: corev1.ConditionFalse, + want: false, }, { - name: "pullsubscription not ready", + name: "the status of topic is unknown", s: func() *StorageStatus { s := &StorageStatus{} s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionFailed("NotReady", "ps not ready") - s.MarkNotificationReady("notificationID") - return s - }(), - }, { - name: "notification not ready", - s: func() *StorageStatus { - s := &StorageStatus{} - s.InitializeConditions() - s.MarkTopicReady() s.MarkPullSubscriptionReady() - s.MarkNotificationNotReady("NotReady", "notification not ready") + s.MarkNotificationReady("notificationID") + s.MarkTopicUnknown("TopicUnknown", "the status of topic is unknown") return s }(), + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { - name: "ready", + name: "the status of pullsubscription is false", s: func() *StorageStatus { s := &StorageStatus{} s.InitializeConditions() s.MarkTopicReady() - s.MarkPullSubscriptionReady() + s.MarkPullSubscriptionFailed("PullSubscriptionFailed", "the status of pullsubscription is false") s.MarkNotificationReady("notificationID") return s }(), - want: true, - }} + wantConditionStatus: corev1.ConditionFalse, + want: false, + }, + { + name: "the status of pullsubscription is unknown", + s: func() *StorageStatus { + s := &StorageStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "the status of pullsubscription is unknown") + s.MarkNotificationReady("notificationID") + return s + }(), + wantConditionStatus: corev1.ConditionUnknown, + want: false, + }, { + name: "notification not ready", + s: func() *StorageStatus { + s := &StorageStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkPullSubscriptionReady() + s.MarkNotificationNotReady("NotReady", "notification not ready") + return s + }(), + }, { + name: "ready", + s: func() *StorageStatus { + s := &StorageStatus{} + s.InitializeConditions() + s.MarkTopicReady() + s.MarkPullSubscriptionReady() + s.MarkNotificationReady("notificationID") + return s + }(), + wantConditionStatus: corev1.ConditionTrue, + want: true, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + if test.wantConditionStatus != "" { + gotConditionStatus := test.s.GetTopLevelCondition().Status + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } + } got := test.s.IsReady() - if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("%s: unexpected condition (-want, +got) = %v", test.name, diff) + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/events/v1alpha1/test_helper.go b/pkg/apis/events/v1alpha1/test_helper.go index 0201506cd4..4a439e3a89 100644 --- a/pkg/apis/events/v1alpha1/test_helper.go +++ b/pkg/apis/events/v1alpha1/test_helper.go @@ -27,6 +27,7 @@ var TestHelper = testHelper{} func (testHelper) ReadyPullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { pss := &v1alpha1.PullSubscriptionStatus{} + pss.InitializeConditions() pss.MarkSink("http://test.mynamespace.svc.cluster.local") pss.MarkDeployed() pss.MarkSubscribed("subID") @@ -35,6 +36,7 @@ func (testHelper) ReadyPullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus func (testHelper) FalsePullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { pss := &v1alpha1.PullSubscriptionStatus{} + pss.InitializeConditions() pss.MarkNotDeployed("not deployed", "not deployed") return pss } @@ -44,4 +46,3 @@ func (testHelper) UnknownPullSubscriptionStatus() *v1alpha1.PullSubscriptionStat pss.InitializeConditions() return pss } - diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index b8b6f2ca0e..ad0b2465bf 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" "sort" "testing" @@ -151,28 +152,33 @@ func TestChannelIsReady(t *testing.T) { tests := []struct { name string setAddress bool - topicStatus corev1.ConditionStatus + topicStatus *v1alpha1.TopicStatus wantConditionStatus corev1.ConditionStatus + want bool }{{ name: "all happy", setAddress: true, - topicStatus: corev1.ConditionTrue, + topicStatus: ReadyTopicStatus(), wantConditionStatus: corev1.ConditionTrue, + want: true, }, { name: "address not set", setAddress: false, - topicStatus: corev1.ConditionTrue, + topicStatus: ReadyTopicStatus(), wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "the status of topic is false", setAddress: true, - topicStatus: corev1.ConditionFalse, + topicStatus: FalseTopicStatus(), wantConditionStatus: corev1.ConditionFalse, + want: false, }, { name: "the status of topic is unknown", setAddress: true, - topicStatus: corev1.ConditionUnknown, + topicStatus: UnknownTopicStatus(), wantConditionStatus: corev1.ConditionUnknown, + want: false, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -181,17 +187,14 @@ func TestChannelIsReady(t *testing.T) { if test.setAddress { cs.SetAddress(&apis.URL{Scheme: "http", Host: "foo.bar"}) } - if test.topicStatus == corev1.ConditionTrue { - cs.MarkTopicReady() - } else if test.topicStatus == corev1.ConditionUnknown { - cs.MarkTopicUnknown("The status of topic is unknown", "The status of topic is unknown: nil") - } else { - cs.MarkTopicFailed("The status of topic is false", "The status of topic is unknown: nil") + cs.PropagateTopicStatus(test.topicStatus.GetTopLevelCondition()) + gotConditionStatus := cs.GetTopLevelCondition().Status + if test.wantConditionStatus != gotConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) } - - got := cs.GetTopLevelCondition().Status - if test.wantConditionStatus != got { - t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) + got := cs.IsReady() + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } }) } @@ -251,3 +254,27 @@ func TestPubSubChannelStatus_SetAddressable(t *testing.T) { }) } } + +func ReadyTopicStatus() *v1alpha1.TopicStatus { + ts := &v1alpha1.TopicStatus{} + ts.InitializeConditions() + ts.MarkDeployed() + ts.MarkTopicReady() + url, _ := apis.ParseURL("http://testchannel.mynamespace.svc.cluster.local/") + ts.SetAddress(url) + ts.TopicID = "test" + return ts +} + +func FalseTopicStatus() *v1alpha1.TopicStatus { + ts := &v1alpha1.TopicStatus{} + ts.InitializeConditions() + ts.MarkNoTopic("TopicNotFound", "topic not found") + return ts +} + +func UnknownTopicStatus() *v1alpha1.TopicStatus { + ts := &v1alpha1.TopicStatus{} + ts.InitializeConditions() + return ts +} diff --git a/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go b/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go index 75b266e8f8..0a3e4b7520 100644 --- a/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go +++ b/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go @@ -28,9 +28,10 @@ import ( func TestPubSubStatusIsReady(t *testing.T) { tests := []struct { - name string - s *PullSubscriptionStatus - want bool + name string + s *PullSubscriptionStatus + wantConditionStatus corev1.ConditionStatus + want bool }{{ name: "uninitialized", s: &PullSubscriptionStatus{}, @@ -42,7 +43,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.InitializeConditions() return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark deployed", s: func() *PullSubscriptionStatus { @@ -51,7 +53,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkDeployed() return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark sink", s: func() *PullSubscriptionStatus { @@ -60,7 +63,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkSink("uri://example") return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark subscribed", s: func() *PullSubscriptionStatus { @@ -69,7 +73,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkSubscribed("subID") return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark sink and deployed", s: func() *PullSubscriptionStatus { @@ -79,7 +84,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkDeployed() return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark sink and deployed and subscribed", s: func() *PullSubscriptionStatus { @@ -90,7 +96,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkSubscribed("subID") return s }(), - want: true, + wantConditionStatus: corev1.ConditionTrue, + want: true, }, { name: "mark sink and deployed and subscribed, then no sink", s: func() *PullSubscriptionStatus { @@ -102,7 +109,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkNoSink("Testing", "") return s }(), - want: false, + wantConditionStatus: corev1.ConditionFalse, + want: false, }, { name: "mark sink and deployed and subscribed then not deployed", s: func() *PullSubscriptionStatus { @@ -114,7 +122,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkNotDeployed("Testing", "") return s }(), - want: false, + wantConditionStatus: corev1.ConditionFalse, + want: false, }, { name: "mark sink and subscribed and not deployed then deployed", s: func() *PullSubscriptionStatus { @@ -126,7 +135,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkDeployed() return s }(), - want: true, + wantConditionStatus: corev1.ConditionTrue, + want: true, }, { name: "mark sink empty and deployed and subscribed", s: func() *PullSubscriptionStatus { @@ -137,7 +147,8 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkSubscribed("subID") return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark sink empty and deployed and subscribed then sink", s: func() *PullSubscriptionStatus { @@ -149,14 +160,21 @@ func TestPubSubStatusIsReady(t *testing.T) { s.MarkSink("uri://example") return s }(), - want: true, + wantConditionStatus: corev1.ConditionTrue, + want: true, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + if test.wantConditionStatus != "" { + gotConditionStatus := test.s.GetTopLevelCondition().Status + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } + } got := test.s.IsReady() - if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("%s: unexpected condition (-want, +got) = %v", test.name, diff) + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go b/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go index 406232e6ca..fc3518b174 100644 --- a/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go +++ b/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go @@ -27,9 +27,10 @@ import ( func TestTopicStatusIsReady(t *testing.T) { tests := []struct { - name string - s *TopicStatus - want bool + name string + s *TopicStatus + wantConditionStatus corev1.ConditionStatus + want bool }{{ name: "uninitialized", s: &TopicStatus{}, @@ -41,7 +42,8 @@ func TestTopicStatusIsReady(t *testing.T) { s.InitializeConditions() return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark deployed", s: func() *TopicStatus { @@ -50,7 +52,8 @@ func TestTopicStatusIsReady(t *testing.T) { s.MarkDeployed() return s }(), - want: false, + wantConditionStatus: corev1.ConditionUnknown, + want: false, }, { name: "mark addressable", s: func() *TopicStatus { @@ -61,7 +64,8 @@ func TestTopicStatusIsReady(t *testing.T) { s.SetAddress(&apis.URL{}) return s }(), - want: true, + wantConditionStatus: corev1.ConditionTrue, + want: true, }, { name: "mark nil addressable", s: func() *TopicStatus { @@ -72,7 +76,8 @@ func TestTopicStatusIsReady(t *testing.T) { s.SetAddress(nil) return s }(), - want: false, + wantConditionStatus: corev1.ConditionFalse, + want: false, }, { name: "mark not deployed then deployed", s: func() *TopicStatus { @@ -84,14 +89,21 @@ func TestTopicStatusIsReady(t *testing.T) { s.MarkDeployed() return s }(), - want: true, + wantConditionStatus: corev1.ConditionTrue, + want: true, }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + if test.wantConditionStatus != "" { + gotConditionStatus := test.s.GetTopLevelCondition().Status + if gotConditionStatus != test.wantConditionStatus { + t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) + } + } got := test.s.IsReady() - if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("%s: unexpected condition (-want, +got) = %v", test.name, diff) + if got != test.want { + t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) } }) } From 748881fe427ab29fb8e233ca900c89784342d089 Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Wed, 15 Jan 2020 14:02:46 -0800 Subject: [PATCH 4/8] modified comments --- pkg/apis/duck/v1alpha1/pubsub_lifecycle.go | 6 ++++-- pkg/apis/events/v1alpha1/auditlogs_lifecycle.go | 6 +++--- pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go | 2 +- pkg/apis/events/v1alpha1/pubsub_lifecycle.go | 4 ++-- pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go | 6 +++--- pkg/apis/events/v1alpha1/storage_lifecycle.go | 6 +++--- 6 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go b/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go index 5e9b12adb6..184babdbcf 100644 --- a/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/duck/v1alpha1/pubsub_lifecycle.go @@ -35,7 +35,8 @@ func (s *PubSubStatus) MarkTopicReady(cs *apis.ConditionSet) { cs.Manage(s).MarkTrue(TopicReady) } -// MarkTopicNotConfigured sets the condition that the PubSub Topic has not yet been reconciled. +// MarkTopicNotConfigured changes the TopicReady condition to be unknown to reflect +// that the Topic does not yet have a Status. func (s *PubSubStatus) MarkTopicNotConfigured(cs *apis.ConditionSet) { cs.Manage(s).MarkUnknown(TopicReady, "TopicNotConfigured", "Topic has not yet been reconciled") } @@ -57,7 +58,8 @@ func (s *PubSubStatus) MarkPullSubscriptionReady(cs *apis.ConditionSet) { cs.Manage(s).MarkTrue(PullSubscriptionReady) } -// MarkPullSubscriptionNotConfigured sets the condition that the PubSub PullSubscription has not yet been reconciled. +// MarkPullSubscriptionNotConfigured changes the PullSubscriptionReady condition to be unknown to reflect +// that the PullSubscription does not yet have a Status. func (s *PubSubStatus) MarkPullSubscriptionNotConfigured(cs *apis.ConditionSet) { cs.Manage(s).MarkUnknown(PullSubscriptionReady, "PullSubscriptionNotConfigured", "PullSubscription has not yet been reconciled") } \ No newline at end of file diff --git a/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go b/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go index b96e615db8..ed8ec40a49 100644 --- a/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go +++ b/pkg/apis/events/v1alpha1/auditlogs_lifecycle.go @@ -42,18 +42,18 @@ func (s *AuditLogsSourceStatus) InitializeConditions() { } // MarkPullSubscriptionFailed sets the condition that the status of underlying PullSubscription -// source is False and why. +// is False and why. func (s *AuditLogsSourceStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } // MarkPullSubscriptionUnknown sets the condition that the status of underlying PullSubscription -// source is Unknown and why. +// is Unknown and why. func (s *AuditLogsSourceStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { auditLogsSourceCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } -// MarkPullSubscriptionReady sets the condition that the underlying PubSub source is ready. +// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. func (s *AuditLogsSourceStatus) MarkPullSubscriptionReady() { auditLogsSourceCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } diff --git a/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go b/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go index 4618264e11..4c1074b48a 100644 --- a/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/auditlogs_lifecycle_test.go @@ -128,7 +128,7 @@ func TestAuditLogsSourceStatusIsReady(t *testing.T) { } got := test.s.IsReady() if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go index 1a1d76ae89..5825d4ef99 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go @@ -49,8 +49,8 @@ func (ps *PubSubStatus) MarkPullSubscriptionFailed(reason, messageFormat string, pubSubCondSet.Manage(ps).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } -// MarkPullSubscriptionNotConfigured sets the condition that the underlying PullSubscription -// source has not been reconciled and why. +// MarkPullSubscriptionNotConfigured changes the PullSubscriptionReady condition to be unknown to reflect +// that the PullSubscription does not yet have a Status. func (ps *PubSubStatus) MarkPullSubscriptionNotConfigured() { pubSubCondSet.Manage(ps).MarkUnknown(duckv1alpha1.PullSubscriptionReady, "PullSubscriptionNotConfigured", "PullSubscription has not yet been reconciled") } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go index f9467fc45c..cd875b4b3a 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -17,14 +17,14 @@ limitations under the License. package v1alpha1 import ( - pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" - corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" + + pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" + corev1 "k8s.io/api/core/v1" ) func TestPubSubStatusIsReady(t *testing.T) { diff --git a/pkg/apis/events/v1alpha1/storage_lifecycle.go b/pkg/apis/events/v1alpha1/storage_lifecycle.go index a5e83653a7..bc22b699b9 100644 --- a/pkg/apis/events/v1alpha1/storage_lifecycle.go +++ b/pkg/apis/events/v1alpha1/storage_lifecycle.go @@ -43,18 +43,18 @@ func (s *StorageStatus) InitializeConditions() { } // MarkPullSubscriptionFailed sets the condition that the status of underlying PullSubscription -// source is False and why. +// is False and why. func (s *StorageStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { storageCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } // MarkPullSubscriptionUnknown sets the condition that the status of underlying PullSubscription -// source is Unknown and why. +// is Unknown and why. func (s *StorageStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { storageCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } -// MarkPullSubscriptionReady sets the condition that the underlying PubSub source is ready. +// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. func (s *StorageStatus) MarkPullSubscriptionReady() { storageCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) } From 3fad33e6c6105a7bdbb7f69541d99659d0a43eba Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Wed, 15 Jan 2020 14:15:05 -0800 Subject: [PATCH 5/8] fixed unit tests --- pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go index cd875b4b3a..5942bb88ac 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -21,10 +21,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "knative.dev/pkg/apis" - + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/apis" ) func TestPubSubStatusIsReady(t *testing.T) { From fd695f3c328cb55d4675c1fd9bd4ed34adf3215c Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Wed, 15 Jan 2020 15:07:41 -0800 Subject: [PATCH 6/8] modified propagateXXXStatus methods --- pkg/apis/events/v1alpha1/pubsub_lifecycle.go | 21 ++++++++++--------- .../events/v1alpha1/pubsub_lifecycle_test.go | 2 +- .../messaging/v1alpha1/channel_lifecycle.go | 18 +++++++++------- .../v1alpha1/channel_lifecycle_test.go | 2 +- pkg/reconciler/events/pubsub/pubsub.go | 2 +- pkg/reconciler/messaging/channel/channel.go | 2 +- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go index 5825d4ef99..382b96ef55 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go @@ -17,10 +17,10 @@ limitations under the License. package v1alpha1 import ( + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" + "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" - - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" ) // GetCondition returns the condition currently associated with the given type, or nil. @@ -65,20 +65,21 @@ func (ps *PubSubStatus) MarkPullSubscriptionUnknown(reason, messageFormat string pubSubCondSet.Manage(ps).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } -func (ps *PubSubStatus) PropagatePullSubscriptionStatus(ready *apis.Condition) { - if ready == nil { +func (ps *PubSubStatus) PropagatePullSubscriptionStatus(pss *v1alpha1.PullSubscriptionStatus) { + psc := pss.GetTopLevelCondition() + if psc == nil { ps.MarkPullSubscriptionNotConfigured() return } switch { - case ready.Status == corev1.ConditionUnknown: - ps.MarkPullSubscriptionUnknown(ready.Reason, ready.Message) - case ready.Status == corev1.ConditionTrue: + case psc.Status == corev1.ConditionUnknown: + ps.MarkPullSubscriptionUnknown(psc.Reason, psc.Message) + case psc.Status == corev1.ConditionTrue: ps.MarkPullSubscriptionReady() - case ready.Status == corev1.ConditionFalse: - ps.MarkPullSubscriptionFailed(ready.Reason, ready.Message) + case psc.Status == corev1.ConditionFalse: + ps.MarkPullSubscriptionFailed(psc.Reason, psc.Message) default: - ps.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "The status of PullSubscription is invalid: %v", ready.Status) + ps.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "The status of PullSubscription is invalid: %v", psc.Status) } } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go index 5942bb88ac..73a2c8288d 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -52,7 +52,7 @@ func TestPubSubStatusIsReady(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { ps := &PubSubStatus{} - ps.PropagatePullSubscriptionStatus(test.pullsubscriptionStatus.GetTopLevelCondition()) + ps.PropagatePullSubscriptionStatus(test.pullsubscriptionStatus) gotConditionStatus := ps.GetTopLevelCondition().Status got := ps.IsReady() if gotConditionStatus != test.wantConditionStatus { diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index 8b4a9b07ca..963c9f0527 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -17,6 +17,7 @@ package v1alpha1 import ( + "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -63,21 +64,22 @@ func (cs *ChannelStatus) MarkTopicReady() { channelCondSet.Manage(cs).MarkTrue(ChannelConditionTopicReady) } -func (cs *ChannelStatus) PropagateTopicStatus(ready *apis.Condition) { - if ready == nil { +func (cs *ChannelStatus) PropagateTopicStatus(ts *v1alpha1.TopicStatus) { + tc := ts.GetTopLevelCondition() + if tc == nil { cs.MarkTopicNotConfigured() return } switch { - case ready.Status == corev1.ConditionUnknown: - cs.MarkTopicUnknown(ready.Reason, ready.Message) - case ready.Status == corev1.ConditionTrue: + case tc.Status == corev1.ConditionUnknown: + cs.MarkTopicUnknown(tc.Reason, tc.Message) + case tc.Status == corev1.ConditionTrue: cs.MarkTopicReady() - case ready.Status == corev1.ConditionFalse: - cs.MarkTopicFailed(ready.Reason, ready.Message) + case tc.Status == corev1.ConditionFalse: + cs.MarkTopicFailed(tc.Reason, tc.Message) default: - cs.MarkTopicUnknown("TopicUnknown", "The status of Topic is invalid: %v", ready.Status) + cs.MarkTopicUnknown("TopicUnknown", "The status of Topic is invalid: %v", tc.Status) } } diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index ad0b2465bf..a1e623e49c 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -187,7 +187,7 @@ func TestChannelIsReady(t *testing.T) { if test.setAddress { cs.SetAddress(&apis.URL{Scheme: "http", Host: "foo.bar"}) } - cs.PropagateTopicStatus(test.topicStatus.GetTopLevelCondition()) + cs.PropagateTopicStatus(test.topicStatus) gotConditionStatus := cs.GetTopLevelCondition().Status if test.wantConditionStatus != gotConditionStatus { t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) diff --git a/pkg/reconciler/events/pubsub/pubsub.go b/pkg/reconciler/events/pubsub/pubsub.go index 40870d759f..2ddab8d6d4 100644 --- a/pkg/reconciler/events/pubsub/pubsub.go +++ b/pkg/reconciler/events/pubsub/pubsub.go @@ -127,7 +127,7 @@ func (r *Reconciler) reconcile(ctx context.Context, pubsub *v1alpha1.PubSub) err pubsub.Status.MarkPullSubscriptionFailed("PullSubscriptionReconcileFailed", "Failed to reconcile PullSubscription: %s", err.Error()) return err } - pubsub.Status.PropagatePullSubscriptionStatus(ps.Status.GetTopLevelCondition()) + pubsub.Status.PropagatePullSubscriptionStatus(&ps.Status) // Sink has been resolved from the underlying PullSubscription, set it here. sinkURI, err := apis.ParseURL(ps.Status.SinkURI) diff --git a/pkg/reconciler/messaging/channel/channel.go b/pkg/reconciler/messaging/channel/channel.go index 8127f6a4e1..8bb7b59558 100644 --- a/pkg/reconciler/messaging/channel/channel.go +++ b/pkg/reconciler/messaging/channel/channel.go @@ -124,7 +124,7 @@ func (r *Reconciler) reconcile(ctx context.Context, channel *v1alpha1.Channel) e channel.Status.MarkTopicFailed("TopicReconcileFailed", "Failed to reconcile Topic: %s", err.Error()) return err } - channel.Status.PropagateTopicStatus(topic.Status.GetCondition(pubsubv1alpha1.TopicConditionReady)) + channel.Status.PropagateTopicStatus(&topic.Status) channel.Status.TopicID = topic.Spec.Topic // 2. Sync all subscriptions. From d0df2cd9fd77883e112dc68e62c5052426501480 Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Thu, 16 Jan 2020 11:17:39 -0800 Subject: [PATCH 7/8] modified comments and the code based on code review --- pkg/apis/events/v1alpha1/pubsub_lifecycle.go | 2 +- .../events/v1alpha1/pubsub_lifecycle_test.go | 30 ++++++++++-- .../v1alpha1/scheduler_lifecycle_test.go | 2 +- .../events/v1alpha1/storage_lifecycle_test.go | 2 +- pkg/apis/events/v1alpha1/test_helper.go | 48 ------------------- .../v1alpha1/channel_lifecycle_test.go | 4 +- .../pull_subscription_lifecycle_test.go | 2 +- .../pubsub/v1alpha1/topic_lifecycle_test.go | 2 +- pkg/reconciler/pubsub/reconciler.go | 26 +++++----- 9 files changed, 47 insertions(+), 71 deletions(-) delete mode 100644 pkg/apis/events/v1alpha1/test_helper.go diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go index 382b96ef55..4985bf9b1e 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle.go @@ -44,7 +44,7 @@ func (ps *PubSubStatus) InitializeConditions() { } // MarkPullSubscriptionFailed sets the condition that the underlying PullSubscription -// source is False and why. +// is False and why. func (ps *PubSubStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { pubSubCondSet.Manage(ps).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) } diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go index 73a2c8288d..c2aa28eced 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -36,16 +36,16 @@ func TestPubSubStatusIsReady(t *testing.T) { }{ { name: "the status of pullsubscription is false", - pullsubscriptionStatus: TestHelper.FalsePullSubscriptionStatus(), + pullsubscriptionStatus: FalsePullSubscriptionStatus(), wantConditionStatus: corev1.ConditionFalse, }, { name: "the status of pullsubscription is unknown", - pullsubscriptionStatus: TestHelper.UnknownPullSubscriptionStatus(), + pullsubscriptionStatus: UnknownPullSubscriptionStatus(), wantConditionStatus: corev1.ConditionUnknown, }, { name: "ready", - pullsubscriptionStatus: TestHelper.ReadyPullSubscriptionStatus(), + pullsubscriptionStatus: ReadyPullSubscriptionStatus(), wantConditionStatus: corev1.ConditionTrue, want: true, }} @@ -59,7 +59,7 @@ func TestPubSubStatusIsReady(t *testing.T) { t.Errorf("unexpected condition status: want %v, got %v", test.wantConditionStatus, gotConditionStatus) } if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } @@ -127,3 +127,25 @@ func TestPubSubStatusGetCondition(t *testing.T) { }) } } + +func ReadyPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { + pss := &pubsubv1alpha1.PullSubscriptionStatus{} + pss.InitializeConditions() + pss.MarkSink("http://test.mynamespace.svc.cluster.local") + pss.MarkDeployed() + pss.MarkSubscribed("subID") + return pss +} + +func FalsePullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { + pss := &pubsubv1alpha1.PullSubscriptionStatus{} + pss.InitializeConditions() + pss.MarkNotDeployed("not deployed", "not deployed") + return pss +} + +func UnknownPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { + pss := &pubsubv1alpha1.PullSubscriptionStatus{} + pss.InitializeConditions() + return pss +} diff --git a/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go b/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go index 1844c0d673..8f24eb381a 100644 --- a/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/scheduler_lifecycle_test.go @@ -128,7 +128,7 @@ func TestSchedulerStatusIsReady(t *testing.T) { } got := test.s.IsReady() if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/events/v1alpha1/storage_lifecycle_test.go b/pkg/apis/events/v1alpha1/storage_lifecycle_test.go index 689aaf3d15..9ae2de00ba 100644 --- a/pkg/apis/events/v1alpha1/storage_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/storage_lifecycle_test.go @@ -128,7 +128,7 @@ func TestStorageStatusIsReady(t *testing.T) { } got := test.s.IsReady() if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/events/v1alpha1/test_helper.go b/pkg/apis/events/v1alpha1/test_helper.go deleted file mode 100644 index 4a439e3a89..0000000000 --- a/pkg/apis/events/v1alpha1/test_helper.go +++ /dev/null @@ -1,48 +0,0 @@ -/* -Copyright 2019 Google LLC - -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 - - http://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 v1alpha1 - -import ( - "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" -) - -type testHelper struct{} - -// TestHelper contains helpers for unit tests. -var TestHelper = testHelper{} - -func (testHelper) ReadyPullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { - pss := &v1alpha1.PullSubscriptionStatus{} - pss.InitializeConditions() - pss.MarkSink("http://test.mynamespace.svc.cluster.local") - pss.MarkDeployed() - pss.MarkSubscribed("subID") - return pss -} - -func (testHelper) FalsePullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { - pss := &v1alpha1.PullSubscriptionStatus{} - pss.InitializeConditions() - pss.MarkNotDeployed("not deployed", "not deployed") - return pss -} - -func (testHelper) UnknownPullSubscriptionStatus() *v1alpha1.PullSubscriptionStatus { - pss := &v1alpha1.PullSubscriptionStatus{} - pss.InitializeConditions() - return pss -} diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index a1e623e49c..e3fe2dc4f0 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -17,12 +17,12 @@ limitations under the License. package v1alpha1 import ( - "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" "sort" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -194,7 +194,7 @@ func TestChannelIsReady(t *testing.T) { } got := cs.IsReady() if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go b/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go index 0a3e4b7520..022a6f16bc 100644 --- a/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go +++ b/pkg/apis/pubsub/v1alpha1/pull_subscription_lifecycle_test.go @@ -174,7 +174,7 @@ func TestPubSubStatusIsReady(t *testing.T) { } got := test.s.IsReady() if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } diff --git a/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go b/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go index fc3518b174..ffcfedbc2b 100644 --- a/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go +++ b/pkg/apis/pubsub/v1alpha1/topic_lifecycle_test.go @@ -103,7 +103,7 @@ func TestTopicStatusIsReady(t *testing.T) { } got := test.s.IsReady() if got != test.want { - t.Errorf("unexpected readiniess: want %v, got %v", test.want, got) + t.Errorf("unexpected readiness: want %v, got %v", test.want, got) } }) } diff --git a/pkg/reconciler/pubsub/reconciler.go b/pkg/reconciler/pubsub/reconciler.go index 77f3bb9f9f..9feea1b29e 100644 --- a/pkg/reconciler/pubsub/reconciler.go +++ b/pkg/reconciler/pubsub/reconciler.go @@ -19,10 +19,10 @@ package pubsub import ( "context" "fmt" - "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" - pubsubsourcev1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" - pubsubsourceclientset "github.com/google/knative-gcp/pkg/client/clientset/versioned" + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" + pubsubv1alpha1 "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" + clientset "github.com/google/knative-gcp/pkg/client/clientset/versioned" "github.com/google/knative-gcp/pkg/duck" "github.com/google/knative-gcp/pkg/reconciler" "github.com/google/knative-gcp/pkg/reconciler/pubsub/resources" @@ -38,7 +38,7 @@ type PubSubBase struct { *reconciler.Base // For dealing with Topics and Pullsubscriptions - pubsubClient pubsubsourceclientset.Interface + pubsubClient clientset.Interface // What do we tag receive adapter as. receiveAdapterName string @@ -52,7 +52,7 @@ type PubSubBase struct { // "TopicReady", and "PullSubscriptionReady" // Also sets the following fields in the pubsubable.Status upon success // TopicID, ProjectID, and SinkURI -func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubSubable, topic, resourceGroup string) (*pubsubsourcev1alpha1.Topic, *pubsubsourcev1alpha1.PullSubscription, error) { +func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubSubable, topic, resourceGroup string) (*pubsubv1alpha1.Topic, *pubsubv1alpha1.PullSubscription, error) { if pubsubable == nil { return nil, nil, fmt.Errorf("nil pubsubable passed in") } @@ -83,7 +83,6 @@ func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubS return t, nil, err } - // Ok, so the Topic is ready, let's reconcile PullSubscription. pullSubscriptions := psb.pubsubClient.PubsubV1alpha1().PullSubscriptions(namespace) ps, err := pullSubscriptions.Get(name, v1.GetOptions{}) @@ -112,7 +111,7 @@ func (psb *PubSubBase) ReconcilePubSub(ctx context.Context, pubsubable duck.PubS return t, ps, nil } -func propagatePullSubscriptionStatus(ps *pubsubsourcev1alpha1.PullSubscription, status *v1alpha1.PubSubStatus, cs *apis.ConditionSet) error { +func propagatePullSubscriptionStatus(ps *pubsubv1alpha1.PullSubscription, status *duckv1alpha1.PubSubStatus, cs *apis.ConditionSet) error { pc := ps.Status.GetTopLevelCondition() if pc == nil { status.MarkPullSubscriptionNotConfigured(cs) @@ -134,23 +133,26 @@ func propagatePullSubscriptionStatus(ps *pubsubsourcev1alpha1.PullSubscription, return nil } -func propagateTopicStatus(t *pubsubsourcev1alpha1.Topic, status *v1alpha1.PubSubStatus, cs *apis.ConditionSet, topic string ) error { +func propagateTopicStatus(t *pubsubv1alpha1.Topic, status *duckv1alpha1.PubSubStatus, cs *apis.ConditionSet, topic string) error { tc := t.Status.GetTopLevelCondition() if tc == nil { status.MarkTopicNotConfigured(cs) return fmt.Errorf("Topic %q has not yet been reconciled", t.Name) } - if tc.Status == corev1.ConditionUnknown { + + switch { + case tc.Status == corev1.ConditionUnknown: status.MarkTopicUnknown(cs, tc.Reason, tc.Message) return fmt.Errorf("the status of Topic %q is Unknown", t.Name) - } else if tc.Status == corev1.ConditionFalse { + case tc.Status == corev1.ConditionTrue: + break + case tc.Status == corev1.ConditionFalse: status.MarkTopicFailed(cs, tc.Reason, tc.Message) return fmt.Errorf("the status of Topic %q is False", t.Name) - } else if tc.Status != corev1.ConditionTrue { + default: status.MarkTopicUnknown(cs, "TopicUnknown", "The status of Topic is invalid: %v", tc.Status) return fmt.Errorf("the status of Topic %q is invalid: %v", t.Name, tc.Status) } - if t.Status.ProjectID == "" { status.MarkTopicFailed(cs, "TopicNotReady", "Topic %q did not expose projectid", t.Name) return fmt.Errorf("Topic %q did not expose projectid", t.Name) From f846f3919f5ccadd0130015f9c5cba85cc1897bb Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Thu, 16 Jan 2020 11:26:54 -0800 Subject: [PATCH 8/8] added comments and renamed methods --- pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go | 12 ++++++------ pkg/reconciler/pubsub/reconciler.go | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go index c2aa28eced..604324fbdb 100644 --- a/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/pubsub_lifecycle_test.go @@ -36,16 +36,16 @@ func TestPubSubStatusIsReady(t *testing.T) { }{ { name: "the status of pullsubscription is false", - pullsubscriptionStatus: FalsePullSubscriptionStatus(), + pullsubscriptionStatus: falsePullSubscriptionStatus(), wantConditionStatus: corev1.ConditionFalse, }, { name: "the status of pullsubscription is unknown", - pullsubscriptionStatus: UnknownPullSubscriptionStatus(), + pullsubscriptionStatus: unknownPullSubscriptionStatus(), wantConditionStatus: corev1.ConditionUnknown, }, { name: "ready", - pullsubscriptionStatus: ReadyPullSubscriptionStatus(), + pullsubscriptionStatus: readyPullSubscriptionStatus(), wantConditionStatus: corev1.ConditionTrue, want: true, }} @@ -128,7 +128,7 @@ func TestPubSubStatusGetCondition(t *testing.T) { } } -func ReadyPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { +func readyPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { pss := &pubsubv1alpha1.PullSubscriptionStatus{} pss.InitializeConditions() pss.MarkSink("http://test.mynamespace.svc.cluster.local") @@ -137,14 +137,14 @@ func ReadyPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { return pss } -func FalsePullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { +func falsePullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { pss := &pubsubv1alpha1.PullSubscriptionStatus{} pss.InitializeConditions() pss.MarkNotDeployed("not deployed", "not deployed") return pss } -func UnknownPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { +func unknownPullSubscriptionStatus() *pubsubv1alpha1.PullSubscriptionStatus { pss := &pubsubv1alpha1.PullSubscriptionStatus{} pss.InitializeConditions() return pss diff --git a/pkg/reconciler/pubsub/reconciler.go b/pkg/reconciler/pubsub/reconciler.go index 9feea1b29e..e735e2b6a1 100644 --- a/pkg/reconciler/pubsub/reconciler.go +++ b/pkg/reconciler/pubsub/reconciler.go @@ -145,6 +145,7 @@ func propagateTopicStatus(t *pubsubv1alpha1.Topic, status *duckv1alpha1.PubSubSt status.MarkTopicUnknown(cs, tc.Reason, tc.Message) return fmt.Errorf("the status of Topic %q is Unknown", t.Name) case tc.Status == corev1.ConditionTrue: + // When the status of Topic is ConditionTrue, break here since we also need to check the ProjectID and TopicID before we make the Topic to be Ready. break case tc.Status == corev1.ConditionFalse: status.MarkTopicFailed(cs, tc.Reason, tc.Message)