From 3f0c03caac6b2626ada5124bf55d749e84ebb711 Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Thu, 26 Dec 2019 13:57:33 -0800 Subject: [PATCH 1/5] Standardize True/False/Unknown status throughout eventing --- Gopkg.lock | 7 +- Gopkg.toml | 3 +- .../eventing/v1alpha1/broker_lifecycle.go | 5 + .../eventing/v1alpha1/eventtype_lifecycle.go | 28 ++- .../v1alpha1/eventtype_lifecycle_test.go | 2 +- pkg/apis/eventing/v1alpha1/test_helper.go | 23 ++- .../eventing/v1alpha1/trigger_lifecycle.go | 36 +++- .../v1alpha1/trigger_lifecycle_test.go | 94 +++++++---- .../flows/v1alpha1/sequence_lifecycle_test.go | 2 +- .../messaging/v1alpha1/channel_lifecycle.go | 32 +++- .../v1alpha1/channel_lifecycle_test.go | 159 ++++++++---------- .../v1alpha1/in_memory_channel_lifecycle.go | 24 ++- .../v1alpha1/sequence_lifecycle_test.go | 2 +- .../v1alpha1/subscription_lifecycle.go | 19 ++- .../sources/v1alpha1/apiserver_lifecycle.go | 2 +- .../v1alpha1/apiserver_lifecycle_test.go | 18 ++ .../v1alpha1/containersource_lifecycle.go | 2 +- .../containersource_lifecycle_test.go | 2 +- .../sources/v1alpha1/cron_job_lifecycle.go | 2 +- .../v1alpha1/cron_job_lifecycle_test.go | 2 +- pkg/reconciler/eventtype/eventtype.go | 17 +- pkg/reconciler/eventtype/eventtype_test.go | 28 ++- .../controller/inmemorychannel.go | 14 +- .../controller/inmemorychannel_test.go | 64 ++++++- pkg/reconciler/subscription/subscription.go | 32 ++-- .../subscription/subscription_test.go | 4 +- pkg/reconciler/testing/eventtype.go | 13 +- pkg/reconciler/testing/inmemorychannel.go | 8 +- pkg/reconciler/testing/subscription.go | 6 + pkg/reconciler/testing/trigger.go | 13 ++ pkg/reconciler/trigger/trigger.go | 4 +- pkg/reconciler/trigger/trigger_test.go | 104 ++++++++++-- vendor/knative.dev/pkg/apis/condition_set.go | 34 ++++ 33 files changed, 598 insertions(+), 207 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index eda6bd237fa..81b303f5acb 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1253,8 +1253,8 @@ revision = "8d271d903fe4c290aa361acfb242cff7bcee96f1" [[projects]] - branch = "master" - digest = "1:2a1f47dba842efe06d9fa10c0323f7b8f39a6b6e86e944a40668679f102f413e" + branch = "add_isunknown" + digest = "1:110a4a57c9443afbeca13831ca57bb3c5ddd98f4fb40b99a6adf21442fb90560" name = "knative.dev/pkg" packages = [ "apis", @@ -1355,7 +1355,8 @@ "webhook/resourcesemantics/validation", ] pruneopts = "T" - revision = "0094d3a89242cddcd8532df4632fb619b2d7da09" + revision = "f3014f066bdf0857ad691a4b2a77106be21fb054" + source = "github.com/capri-xiyue/pkg" [[projects]] branch = "master" diff --git a/Gopkg.toml b/Gopkg.toml index 6f655904087..f5c02ff55e2 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -36,7 +36,8 @@ required = [ # Our master branch tracks knative/pkg master or a release. [[override]] name = "knative.dev/pkg" - branch = "master" + branch = "add_isunknown" + source = "github.com/capri-xiyue/pkg" # TODO why is this overridden? [[override]] diff --git a/pkg/apis/eventing/v1alpha1/broker_lifecycle.go b/pkg/apis/eventing/v1alpha1/broker_lifecycle.go index 0b564ca1a93..1bef94fc496 100644 --- a/pkg/apis/eventing/v1alpha1/broker_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/broker_lifecycle.go @@ -52,6 +52,11 @@ func (bs *BrokerStatus) IsReady() bool { return brokerCondSet.Manage(bs).IsHappy() } +// IsUnknown returns true if the resource is unknown overall. +func (bs *BrokerStatus) IsUnknown() bool { + return brokerCondSet.Manage(bs).IsUnknown() +} + // InitializeConditions sets relevant unset conditions to Unknown state. func (bs *BrokerStatus) InitializeConditions() { brokerCondSet.Manage(bs).InitializeConditions() diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go index 6783fb36811..4e956c3d470 100644 --- a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go @@ -51,10 +51,34 @@ func (et *EventTypeStatus) MarkBrokerDoesNotExist() { eventTypeCondSet.Manage(et).MarkFalse(EventTypeConditionBrokerExists, "BrokerDoesNotExist", "Broker does not exist") } +func (et *EventTypeStatus) MarkBrokerExistsUnknown(reason, messageFormat string, messageA ...interface{}) { + eventTypeCondSet.Manage(et).MarkUnknown(EventTypeConditionBrokerExists, reason, messageFormat, messageA...) +} + func (et *EventTypeStatus) MarkBrokerReady() { eventTypeCondSet.Manage(et).MarkTrue(EventTypeConditionBrokerReady) } -func (et *EventTypeStatus) MarkBrokerNotReady() { - eventTypeCondSet.Manage(et).MarkFalse(EventTypeConditionBrokerReady, "BrokerNotReady", "Broker is not ready") +func (et *EventTypeStatus) MarkBrokerFailed(reason, messageFormat string, messageA ...interface{}) { + eventTypeCondSet.Manage(et).MarkFalse(EventTypeConditionBrokerReady, reason, messageFormat, messageA...) +} + +func (et *EventTypeStatus) MarkBrokerUnknown(reason, messageFormat string, messageA ...interface{}) { + eventTypeCondSet.Manage(et).MarkUnknown(EventTypeConditionBrokerReady, reason, messageFormat, messageA...) +} + +func (et *EventTypeStatus) PropagateBrokerStatus(bs *BrokerStatus) { + if bs.IsReady() { + eventTypeCondSet.Manage(et).MarkTrue(EventTypeConditionBrokerReady) + } else { + msg := "nil" + if bc := brokerCondSet.Manage(bs).GetCondition(BrokerConditionReady); bc != nil { + msg = bc.Message + } + if bs.IsUnknown() { + et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: %s", msg) + } else { + et.MarkBrokerFailed("BrokerFalse", "The status of Broker is False: %s", msg) + } + } } diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go index 31d29983df3..29106828973 100644 --- a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go +++ b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go @@ -235,7 +235,7 @@ func TestEventTypeIsReady(t *testing.T) { if *test.markBrokerReady { ets.MarkBrokerReady() } else { - ets.MarkBrokerNotReady() + ets.MarkBrokerFailed("BrokerFalse", "the status of Broker is False") } } diff --git a/pkg/apis/eventing/v1alpha1/test_helper.go b/pkg/apis/eventing/v1alpha1/test_helper.go index 582480e83e4..be68c9da676 100644 --- a/pkg/apis/eventing/v1alpha1/test_helper.go +++ b/pkg/apis/eventing/v1alpha1/test_helper.go @@ -18,6 +18,7 @@ package v1alpha1 import ( v1 "k8s.io/api/apps/v1" + "knative.dev/eventing/pkg/apis/sources/v1alpha1" "knative.dev/pkg/apis" pkgduckv1alpha1 "knative.dev/pkg/apis/duck/v1alpha1" @@ -59,10 +60,10 @@ func (testHelper) ReadySubscriptionStatus() *messagingv1alpha1.SubscriptionStatu return ss } -func (testHelper) NotReadySubscriptionStatus() *messagingv1alpha1.SubscriptionStatus { +func (testHelper) FalseSubscriptionStatus() *messagingv1alpha1.SubscriptionStatus { ss := &messagingv1alpha1.SubscriptionStatus{} ss.MarkReferencesResolved() - ss.MarkChannelNotReady("testInducedError", "test induced %s", "error") + ss.MarkChannelFailed("testInducedError", "test induced %s", "error") return ss } @@ -75,17 +76,26 @@ func (t testHelper) ReadyBrokerStatus() *BrokerStatus { return bs } -func (t testHelper) NotReadyBrokerStatus() *BrokerStatus { +func (t testHelper) UnknownBrokerStatus() *BrokerStatus { bs := &BrokerStatus{} return bs } +func (t testHelper) FalseBrokerStatus() *BrokerStatus { + bs := &BrokerStatus{} + bs.MarkIngressFailed("DeploymentUnavailable", "The Deployment is unavailable.") + bs.MarkTriggerChannelFailed("ChannelNotReady", "trigger Channel is not ready: not addressalbe") + bs.MarkFilterFailed("DeploymentUnavailable", "The Deployment is unavailable.") + bs.SetAddress(nil) + return bs +} + func (t testHelper) ReadyTriggerStatus() *TriggerStatus { ts := &TriggerStatus{} ts.InitializeConditions() ts.SubscriberURI = &apis.URL{Scheme: "http", Host: "foo"} ts.PropagateBrokerStatus(t.ReadyBrokerStatus()) - ts.PropagateSubscriptionStatus(t.ReadySubscriptionStatus()) + ts.PropagateSubscriptionStatus(t.FalseSubscriptionStatus()) return ts } @@ -112,3 +122,8 @@ func (t testHelper) AvailableDeployment() *v1.Deployment { } return d } + +func (t testHelper) UnknownCronJobSourceStatus() *v1alpha1.CronJobSourceStatus { + cjss := &v1alpha1.CronJobSourceStatus{} + return cjss +} diff --git a/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go index 093f5894126..24630a8aeff 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go @@ -45,11 +45,21 @@ func (ts *TriggerStatus) GetCondition(t apis.ConditionType) *apis.Condition { return triggerCondSet.Manage(ts).GetCondition(t) } +// GetCondition returns the condition currently associated with the given ConditionType happy, or nil. +func (ts *TriggerStatus) GetHappyCondition() *apis.Condition { + return triggerCondSet.Manage(ts).GetHappyCondition() +} + // IsReady returns true if the resource is ready overall. func (ts *TriggerStatus) IsReady() bool { return triggerCondSet.Manage(ts).IsHappy() } +// IsReady returns true if the resource is unknown overall. +func (ts *TriggerStatus) IsUnknown() bool { + return triggerCondSet.Manage(ts).IsUnknown() +} + // InitializeConditions sets relevant unset conditions to Unknown state. func (ts *TriggerStatus) InitializeConditions() { triggerCondSet.Manage(ts).InitializeConditions() @@ -63,7 +73,11 @@ func (ts *TriggerStatus) PropagateBrokerStatus(bs *BrokerStatus) { if bc := brokerCondSet.Manage(bs).GetCondition(BrokerConditionReady); bc != nil { msg = bc.Message } - ts.MarkBrokerFailed("BrokerNotReady", "Broker is not ready: %s", msg) + if bs.IsUnknown() { + ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: %s", msg) + } else { + ts.MarkBrokerFailed("BrokerFalse", "The status of Broker is False: %s", msg) + } } } @@ -71,6 +85,10 @@ func (ts *TriggerStatus) MarkBrokerFailed(reason, messageFormat string, messageA triggerCondSet.Manage(ts).MarkFalse(TriggerConditionBroker, reason, messageFormat, messageA...) } +func (ts *TriggerStatus) MarkBrokerUnknown(reason, messageFormat string, messageA ...interface{}) { + triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionBroker, reason, messageFormat, messageA...) +} + func (ts *TriggerStatus) PropagateSubscriptionStatus(ss *messagingv1alpha1.SubscriptionStatus) { if ss.IsReady() { triggerCondSet.Manage(ts).MarkTrue(TriggerConditionSubscribed) @@ -79,7 +97,11 @@ func (ts *TriggerStatus) PropagateSubscriptionStatus(ss *messagingv1alpha1.Subsc if sc := ss.Status.GetCondition(messagingv1alpha1.SubscriptionConditionReady); sc != nil { msg = sc.Message } - ts.MarkNotSubscribed("SubscriptionNotReady", "Subscription is not ready: %s", msg) + if ss.IsUnknown() { + ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: %s", msg) + } else { + ts.MarkNotSubscribed("SubscriptionFalse", "The status of Subscription is False: %s", msg) + } } } @@ -87,6 +109,10 @@ func (ts *TriggerStatus) MarkNotSubscribed(reason, messageFormat string, message triggerCondSet.Manage(ts).MarkFalse(TriggerConditionSubscribed, reason, messageFormat, messageA...) } +func (ts *TriggerStatus) MarkSubscribedUnknown(reason, messageFormat string, messageA ...interface{}) { + triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionSubscribed, reason, messageFormat, messageA...) +} + func (ts *TriggerStatus) MarkSubscriptionNotOwned(sub *messagingv1alpha1.Subscription) { triggerCondSet.Manage(ts).MarkFalse(TriggerConditionSubscribed, "SubscriptionNotOwned", "Subscription %q is not owned by this Trigger.", sub.Name) } @@ -124,6 +150,10 @@ func (ts *TriggerStatus) PropagateDependencyStatus(ks *duckv1.KResource) { if kc != nil { msg = kc.Message } - ts.MarkDependencyFailed("DependencyNotReady", "Dependency is not ready: %s", msg) + if kc.IsUnknown() { + ts.MarkDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: %s", msg) + } else { + ts.MarkDependencyFailed("DependencyFalse", "The status of Dependency is False: %s", msg) + } } } diff --git a/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go index 6bcb567c0f8..0ea4891367a 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go +++ b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go @@ -226,7 +226,7 @@ func TestTriggerInitializeConditions(t *testing.T) { } } -func TestTriggerIsReady(t *testing.T) { +func TestTriggerConditionStatus(t *testing.T) { tests := []struct { name string brokerStatus *BrokerStatus @@ -236,8 +236,8 @@ func TestTriggerIsReady(t *testing.T) { subscriptionStatus *messagingv1alpha1.SubscriptionStatus subscriberResolvedStatus bool dependencyAnnotationExists bool - dependencyStatusReady bool - wantReady bool + dependencyStatus corev1.ConditionStatus + wantConditionStatus corev1.ConditionStatus }{{ name: "all happy", brokerStatus: TestHelper.ReadyBrokerStatus(), @@ -247,27 +247,37 @@ func TestTriggerIsReady(t *testing.T) { subscriptionStatus: TestHelper.ReadySubscriptionStatus(), subscriberResolvedStatus: true, dependencyAnnotationExists: false, - wantReady: true, + wantConditionStatus: corev1.ConditionTrue, }, { - name: "broker sad", - brokerStatus: TestHelper.NotReadyBrokerStatus(), + name: "broker status unknown", + brokerStatus: TestHelper.UnknownBrokerStatus(), markKubernetesServiceExists: true, markVirtualServiceExists: true, subscriptionOwned: true, subscriptionStatus: TestHelper.ReadySubscriptionStatus(), subscriberResolvedStatus: true, dependencyAnnotationExists: false, - wantReady: false, + wantConditionStatus: corev1.ConditionUnknown, + }, { + name: "broker status false", + brokerStatus: TestHelper.FalseBrokerStatus(), + markKubernetesServiceExists: true, + markVirtualServiceExists: true, + subscriptionOwned: true, + subscriptionStatus: TestHelper.ReadySubscriptionStatus(), + subscriberResolvedStatus: true, + dependencyAnnotationExists: false, + wantConditionStatus: corev1.ConditionFalse, }, { name: "subscribed sad", brokerStatus: TestHelper.ReadyBrokerStatus(), markKubernetesServiceExists: true, markVirtualServiceExists: true, subscriptionOwned: true, - subscriptionStatus: TestHelper.NotReadySubscriptionStatus(), + subscriptionStatus: TestHelper.FalseSubscriptionStatus(), subscriberResolvedStatus: true, dependencyAnnotationExists: false, - wantReady: false, + wantConditionStatus: corev1.ConditionFalse, }, { name: "subscription not owned", brokerStatus: TestHelper.ReadyBrokerStatus(), @@ -277,7 +287,7 @@ func TestTriggerIsReady(t *testing.T) { subscriptionStatus: TestHelper.ReadySubscriptionStatus(), subscriberResolvedStatus: true, dependencyAnnotationExists: false, - wantReady: false, + wantConditionStatus: corev1.ConditionFalse, }, { name: "failed to resolve subscriber", brokerStatus: TestHelper.ReadyBrokerStatus(), @@ -287,10 +297,21 @@ func TestTriggerIsReady(t *testing.T) { subscriptionStatus: TestHelper.ReadySubscriptionStatus(), subscriberResolvedStatus: false, dependencyAnnotationExists: true, - dependencyStatusReady: true, - wantReady: false, + dependencyStatus: corev1.ConditionTrue, + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "dependency unknown", + brokerStatus: TestHelper.ReadyBrokerStatus(), + markKubernetesServiceExists: true, + markVirtualServiceExists: true, + subscriptionOwned: true, + subscriptionStatus: TestHelper.ReadySubscriptionStatus(), + subscriberResolvedStatus: true, + dependencyAnnotationExists: true, + dependencyStatus: corev1.ConditionUnknown, + wantConditionStatus: corev1.ConditionUnknown, }, { - name: "dependency not ready", + name: "dependency false", brokerStatus: TestHelper.ReadyBrokerStatus(), markKubernetesServiceExists: true, markVirtualServiceExists: true, @@ -298,21 +319,20 @@ func TestTriggerIsReady(t *testing.T) { subscriptionStatus: TestHelper.ReadySubscriptionStatus(), subscriberResolvedStatus: true, dependencyAnnotationExists: true, - dependencyStatusReady: false, - wantReady: false, - }, - { - name: "all sad", - brokerStatus: TestHelper.NotReadyBrokerStatus(), - markKubernetesServiceExists: false, - markVirtualServiceExists: false, - subscriptionOwned: false, - subscriptionStatus: TestHelper.NotReadySubscriptionStatus(), - subscriberResolvedStatus: false, - dependencyAnnotationExists: true, - dependencyStatusReady: false, - wantReady: false, - }} + dependencyStatus: corev1.ConditionFalse, + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "all sad", + brokerStatus: TestHelper.FalseBrokerStatus(), + markKubernetesServiceExists: false, + markVirtualServiceExists: false, + subscriptionOwned: false, + subscriptionStatus: TestHelper.FalseSubscriptionStatus(), + subscriberResolvedStatus: false, + dependencyAnnotationExists: true, + dependencyStatus: corev1.ConditionFalse, + wantConditionStatus: corev1.ConditionFalse, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { ts := &TriggerStatus{} @@ -329,14 +349,20 @@ func TestTriggerIsReady(t *testing.T) { } else { ts.MarkSubscriberResolvedFailed("Unable to get the Subscriber's URI", "subscriber not found") } - if test.dependencyAnnotationExists && !test.dependencyStatusReady { - ts.MarkDependencyFailed("Dependency is not ready", "Dependency is not ready") - } else { + if !test.dependencyAnnotationExists { ts.MarkDependencySucceeded() + } else { + if test.dependencyStatus == corev1.ConditionTrue { + ts.MarkDependencySucceeded() + } else if test.dependencyStatus == corev1.ConditionUnknown { + ts.MarkDependencyUnknown("The status of dependency is unknown", "The status of dependency is unknown: nil") + } else { + ts.MarkDependencyFailed("The status of dependency is false", "The status of dependency is unknown: nil") + } } - got := ts.IsReady() - if test.wantReady != got { - t.Errorf("unexpected readiness: want %v, got %v", test.wantReady, got) + got := ts.GetHappyCondition().Status + if test.wantConditionStatus != got { + t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) } }) } diff --git a/pkg/apis/flows/v1alpha1/sequence_lifecycle_test.go b/pkg/apis/flows/v1alpha1/sequence_lifecycle_test.go index 40af7df6622..07a276b2bfa 100644 --- a/pkg/apis/flows/v1alpha1/sequence_lifecycle_test.go +++ b/pkg/apis/flows/v1alpha1/sequence_lifecycle_test.go @@ -65,7 +65,7 @@ func getSubscription(name string, ready bool) *messagingv1alpha1.Subscription { s.Status.MarkReferencesResolved() s.Status.MarkAddedToChannel() } else { - s.Status.MarkChannelNotReady("testInducedFailure", "Test Induced failure") + s.Status.MarkChannelFailed("testInducedFailure", "Test Induced failure") s.Status.MarkReferencesNotResolved("testInducedFailure", "Test Induced failure") s.Status.MarkNotAddedToChannel("testInducedfailure", "Test Induced failure") } diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index 1d97922ea12..8b6a49c3135 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -42,11 +42,26 @@ func (cs *ChannelStatus) GetCondition(t apis.ConditionType) *apis.Condition { return chCondSet.Manage(cs).GetCondition(t) } +// GetHappyCondition returns the condition currently associated with the ConditionType happy. +func (cs *ChannelStatus) GetHappyCondition() *apis.Condition { + return chCondSet.Manage(cs).GetHappyCondition() +} + // IsReady returns true if the resource is ready overall. func (cs *ChannelStatus) IsReady() bool { return chCondSet.Manage(cs).IsHappy() } +// IsUnknown returns true if the resource is unknown overall. +func (cs *ChannelStatus) IsUnknown() bool { + return chCondSet.Manage(cs).IsUnknown() +} + +// IsFalse returns true if the resource is false overall. +func (cs *ChannelStatus) IsFalse() bool { + return chCondSet.Manage(cs).IsFalse() +} + // InitializeConditions sets relevant unset conditions to Unknown state. func (cs *ChannelStatus) InitializeConditions() { chCondSet.Manage(cs).InitializeConditions() @@ -71,18 +86,27 @@ func (cs *ChannelStatus) MarkBackingChannelFailed(reason, messageFormat string, chCondSet.Manage(cs).MarkFalse(ChannelConditionBackingChannelReady, reason, messageFormat, messageA...) } +func (cs *ChannelStatus) MarkBackingChannelUnknown(reason, messageFormat string, messageA ...interface{}) { + chCondSet.Manage(cs).MarkUnknown(ChannelConditionBackingChannelReady, reason, messageFormat, messageA...) +} + func (cs *ChannelStatus) MarkBackingChannelReady() { chCondSet.Manage(cs).MarkTrue(ChannelConditionBackingChannelReady) } func (cs *ChannelStatus) PropagateStatuses(chs *eventingduck.ChannelableStatus) { // TODO: Once you can get a Ready status from Channelable in a generic way, use it here. + readyCondition := chs.Status.GetCondition(apis.ConditionReady) - if readyCondition != nil { - if readyCondition.Status != corev1.ConditionTrue { - cs.MarkBackingChannelFailed(readyCondition.Reason, readyCondition.Message) - } else { + if readyCondition == nil { + cs.MarkBackingChannelUnknown("readyCondition is: nil", "readyCondition is: nil") + } else { + if readyCondition.Status == corev1.ConditionTrue { cs.MarkBackingChannelReady() + } else if readyCondition.Status == corev1.ConditionUnknown { + cs.MarkBackingChannelUnknown(readyCondition.Reason, readyCondition.Message) + } else { + cs.MarkBackingChannelFailed(readyCondition.Reason, readyCondition.Message) } } // Set the address and update the Addressable conditions. diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index 6bad722a94d..48103eec211 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -30,6 +30,22 @@ import ( duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) +var ( + validAddress = &duckv1alpha1.Addressable{ + Addressable: duckv1beta1.Addressable{ + URL: &apis.URL{ + Scheme: "http", + Host: "test-domain", + }, + }, + Hostname: "test-domain", + } + urlNotSetAddress = &duckv1alpha1.Addressable{ + Addressable: duckv1beta1.Addressable{}, + Hostname: "test-domain", + } +) + func TestChannelGetCondition(t *testing.T) { tests := []struct { name string @@ -154,50 +170,54 @@ func TestChannelInitializeConditions(t *testing.T) { } } -func TestChannelIsReady(t *testing.T) { +func TestChannelConditionStatus(t *testing.T) { tests := []struct { - name string - setAddress bool - markBackingChannelReady bool - wantReady bool + name string + address *duckv1alpha1.Addressable + backingChannelStatus corev1.ConditionStatus + wantConditionStatus corev1.ConditionStatus }{{ - name: "all happy", - setAddress: true, - markBackingChannelReady: true, - wantReady: true, - }, { - name: "address not set", - setAddress: false, - markBackingChannelReady: true, - wantReady: false, + name: "all happy", + address: validAddress, + backingChannelStatus: corev1.ConditionTrue, + wantConditionStatus: corev1.ConditionTrue, }, { - name: "backing channel not ready", - setAddress: true, - markBackingChannelReady: false, - wantReady: false, - }} + name: "address not set", + address: &duckv1alpha1.Addressable{}, + backingChannelStatus: corev1.ConditionTrue, + wantConditionStatus: corev1.ConditionFalse, + }, + { + name: "address without url", + address: urlNotSetAddress, + backingChannelStatus: corev1.ConditionTrue, + wantConditionStatus: corev1.ConditionFalse, + }, { + name: "backing channel with unknown status", + address: validAddress, + backingChannelStatus: corev1.ConditionUnknown, + wantConditionStatus: corev1.ConditionUnknown, + }, { + name: "backing channel with false status", + address: validAddress, + backingChannelStatus: corev1.ConditionFalse, + wantConditionStatus: corev1.ConditionFalse, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { cs := &ChannelStatus{} cs.InitializeConditions() - if test.setAddress { - cs.SetAddress(&duckv1alpha1.Addressable{ - Addressable: duckv1beta1.Addressable{ - URL: &apis.URL{ - Scheme: "http", - Host: "test-domain", - }, - }, - }) - } - if test.markBackingChannelReady { + cs.SetAddress(test.address) + if test.backingChannelStatus == corev1.ConditionTrue { cs.MarkBackingChannelReady() - } else { + } else if test.backingChannelStatus == corev1.ConditionFalse { cs.MarkBackingChannelFailed("ChannelFailure", "testing") + } else { + cs.MarkBackingChannelUnknown("ChannelUnknown", "testing") } - got := cs.IsReady() - if test.wantReady != got { - t.Errorf("unexpected readiness: want %v, got %v", test.wantReady, got) + got := cs.GetHappyCondition().Status + if test.wantConditionStatus != got { + t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) } }) } @@ -275,24 +295,16 @@ func TestChannelSetAddressable(t *testing.T) { func TestChannelPropagateStatuses(t *testing.T) { testCases := map[string]struct { - channelableStatus *v1alpha1.ChannelableStatus - wantReady bool + channelableStatus *v1alpha1.ChannelableStatus + wantConditionStatus corev1.ConditionStatus }{ "address set": { channelableStatus: &v1alpha1.ChannelableStatus{ AddressStatus: duckv1alpha1.AddressStatus{ - Address: &duckv1alpha1.Addressable{ - Addressable: duckv1beta1.Addressable{ - URL: &apis.URL{ - Scheme: "http", - Host: "test-domain", - }, - }, - Hostname: "test-domain", - }, + Address: validAddress, }, }, - wantReady: false, + wantConditionStatus: corev1.ConditionUnknown, }, "address not set": { channelableStatus: &v1alpha1.ChannelableStatus{ @@ -300,31 +312,20 @@ func TestChannelPropagateStatuses(t *testing.T) { Address: &duckv1alpha1.Addressable{}, }, }, - wantReady: false, + wantConditionStatus: corev1.ConditionFalse, }, "url not set": { channelableStatus: &v1alpha1.ChannelableStatus{ AddressStatus: duckv1alpha1.AddressStatus{ - Address: &duckv1alpha1.Addressable{ - Addressable: duckv1beta1.Addressable{}, - Hostname: "test-domain", - }, + Address: urlNotSetAddress, }, }, - wantReady: false, + wantConditionStatus: corev1.ConditionFalse, }, "all set": { channelableStatus: &v1alpha1.ChannelableStatus{ AddressStatus: duckv1alpha1.AddressStatus{ - Address: &duckv1alpha1.Addressable{ - Addressable: duckv1beta1.Addressable{ - URL: &apis.URL{ - Scheme: "http", - Host: "test-domain", - }, - }, - Hostname: "test-domain", - }, + Address: validAddress, }, Status: duckv1.Status{ Conditions: []apis.Condition{{ @@ -333,20 +334,12 @@ func TestChannelPropagateStatuses(t *testing.T) { }}, }, }, - wantReady: true, + wantConditionStatus: corev1.ConditionTrue, }, - "backing channel not ready": { + "backing channel with unknown status": { channelableStatus: &v1alpha1.ChannelableStatus{ AddressStatus: duckv1alpha1.AddressStatus{ - Address: &duckv1alpha1.Addressable{ - Addressable: duckv1beta1.Addressable{ - URL: &apis.URL{ - Scheme: "http", - Host: "test-domain", - }, - }, - Hostname: "test-domain", - }, + Address: validAddress, }, Status: duckv1.Status{ Conditions: []apis.Condition{{ @@ -355,20 +348,12 @@ func TestChannelPropagateStatuses(t *testing.T) { }}, }, }, - wantReady: false, + wantConditionStatus: corev1.ConditionUnknown, }, "no condition ready in backing channel": { channelableStatus: &v1alpha1.ChannelableStatus{ AddressStatus: duckv1alpha1.AddressStatus{ - Address: &duckv1alpha1.Addressable{ - Addressable: duckv1beta1.Addressable{ - URL: &apis.URL{ - Scheme: "http", - Host: "test-domain", - }, - }, - Hostname: "test-domain", - }, + Address: validAddress, }, Status: duckv1.Status{ Conditions: []apis.Condition{{ @@ -377,7 +362,7 @@ func TestChannelPropagateStatuses(t *testing.T) { }}, }, }, - wantReady: false, + wantConditionStatus: corev1.ConditionUnknown, }, "test subscribableTypeStatus is set": { channelableStatus: &v1alpha1.ChannelableStatus{ @@ -398,16 +383,16 @@ func TestChannelPropagateStatuses(t *testing.T) { }, }, }, - wantReady: false, + wantConditionStatus: corev1.ConditionFalse, }, } for n, tc := range testCases { t.Run(n, func(t *testing.T) { cs := &ChannelStatus{} cs.PropagateStatuses(tc.channelableStatus) - got := cs.IsReady() - if tc.wantReady != got { - t.Errorf("unexpected readiness: want %v, got %v", tc.wantReady, got) + got := cs.GetHappyCondition().Status + if tc.wantConditionStatus != got { + t.Errorf("unexpected readiness: want %v, got %v", tc.wantConditionStatus, got) } }) } diff --git a/pkg/apis/messaging/v1alpha1/in_memory_channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/in_memory_channel_lifecycle.go index 15f55e20e0e..a9f1259e84a 100644 --- a/pkg/apis/messaging/v1alpha1/in_memory_channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/in_memory_channel_lifecycle.go @@ -87,14 +87,20 @@ func (imcs *InMemoryChannelStatus) MarkDispatcherFailed(reason, messageFormat st imcCondSet.Manage(imcs).MarkFalse(InMemoryChannelConditionDispatcherReady, reason, messageFormat, messageA...) } +func (imcs *InMemoryChannelStatus) MarkDispatcherUnknown(reason, messageFormat string, messageA ...interface{}) { + imcCondSet.Manage(imcs).MarkUnknown(InMemoryChannelConditionDispatcherReady, reason, messageFormat, messageA...) +} + // TODO: Unify this with the ones from Eventing. Say: Broker, Trigger. func (imcs *InMemoryChannelStatus) PropagateDispatcherStatus(ds *appsv1.DeploymentStatus) { for _, cond := range ds.Conditions { if cond.Type == appsv1.DeploymentAvailable { - if cond.Status != corev1.ConditionTrue { - imcs.MarkDispatcherFailed("DispatcherNotReady", "Dispatcher Deployment is not ready: %s : %s", cond.Reason, cond.Message) - } else { + if cond.Status == corev1.ConditionTrue { imcCondSet.Manage(imcs).MarkTrue(InMemoryChannelConditionDispatcherReady) + } else if cond.Status == corev1.ConditionFalse { + imcs.MarkDispatcherFailed("DispatcherDeploymentFalse", "The status of Dispatcher Deployment is False: %s : %s", cond.Reason, cond.Message) + } else if cond.Status == corev1.ConditionUnknown { + imcs.MarkDispatcherUnknown("DispatcherDeploymentUnknown", "The status of Dispatcher Deployment is Unknown: %s : %s", cond.Reason, cond.Message) } } } @@ -104,6 +110,10 @@ func (imcs *InMemoryChannelStatus) MarkServiceFailed(reason, messageFormat strin imcCondSet.Manage(imcs).MarkFalse(InMemoryChannelConditionServiceReady, reason, messageFormat, messageA...) } +func (imcs *InMemoryChannelStatus) MarkServiceUnknown(reason, messageFormat string, messageA ...interface{}) { + imcCondSet.Manage(imcs).MarkUnknown(InMemoryChannelConditionServiceReady, reason, messageFormat, messageA...) +} + func (imcs *InMemoryChannelStatus) MarkServiceTrue() { imcCondSet.Manage(imcs).MarkTrue(InMemoryChannelConditionServiceReady) } @@ -112,6 +122,10 @@ func (imcs *InMemoryChannelStatus) MarkChannelServiceFailed(reason, messageForma imcCondSet.Manage(imcs).MarkFalse(InMemoryChannelConditionChannelServiceReady, reason, messageFormat, messageA...) } +func (imcs *InMemoryChannelStatus) MarkChannelServiceUnknown(reason, messageFormat string, messageA ...interface{}) { + imcCondSet.Manage(imcs).MarkUnknown(InMemoryChannelConditionChannelServiceReady, reason, messageFormat, messageA...) +} + func (imcs *InMemoryChannelStatus) MarkChannelServiceTrue() { imcCondSet.Manage(imcs).MarkTrue(InMemoryChannelConditionChannelServiceReady) } @@ -120,6 +134,10 @@ func (imcs *InMemoryChannelStatus) MarkEndpointsFailed(reason, messageFormat str imcCondSet.Manage(imcs).MarkFalse(InMemoryChannelConditionEndpointsReady, reason, messageFormat, messageA...) } +func (imcs *InMemoryChannelStatus) MarkEndpointsUnknown(reason, messageFormat string, messageA ...interface{}) { + imcCondSet.Manage(imcs).MarkUnknown(InMemoryChannelConditionEndpointsReady, reason, messageFormat, messageA...) +} + func (imcs *InMemoryChannelStatus) MarkEndpointsTrue() { imcCondSet.Manage(imcs).MarkTrue(InMemoryChannelConditionEndpointsReady) } diff --git a/pkg/apis/messaging/v1alpha1/sequence_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/sequence_lifecycle_test.go index 95633522832..b0e67ef009c 100644 --- a/pkg/apis/messaging/v1alpha1/sequence_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/sequence_lifecycle_test.go @@ -63,7 +63,7 @@ func getSubscription(name string, ready bool) *Subscription { s.Status.MarkReferencesResolved() s.Status.MarkAddedToChannel() } else { - s.Status.MarkChannelNotReady("testInducedFailure", "Test Induced failure") + s.Status.MarkChannelFailed("testInducedFailure", "Test Induced failure") s.Status.MarkReferencesNotResolved("testInducedFailure", "Test Induced failure") s.Status.MarkNotAddedToChannel("testInducedfailure", "Test Induced failure") } diff --git a/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go b/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go index 3e793300069..b753a96f7eb 100644 --- a/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go @@ -49,6 +49,11 @@ func (ss *SubscriptionStatus) IsReady() bool { return subCondSet.Manage(ss).IsHappy() } +// IsUnknown returns true if the resource is unknown overall. +func (ss *SubscriptionStatus) IsUnknown() bool { + return subCondSet.Manage(ss).IsUnknown() +} + // IsAddedToChannel returns true if SubscriptionConditionAddedToChannel is true func (ss *SubscriptionStatus) IsAddedToChannel() bool { return ss.GetCondition(SubscriptionConditionAddedToChannel).IsTrue() @@ -84,11 +89,21 @@ func (ss *SubscriptionStatus) MarkReferencesNotResolved(reason, messageFormat st subCondSet.Manage(ss).MarkFalse(SubscriptionConditionReferencesResolved, reason, messageFormat, messageA...) } -// MarkChannelNotReady sets the ChannelReady condition to False state. -func (ss *SubscriptionStatus) MarkChannelNotReady(reason, messageFormat string, messageA ...interface{}) { +// MarkReferencesNotResolved sets the ReferencesResolved condition to Unknown state. +func (ss *SubscriptionStatus) MarkReferencesResolvedUnknown(reason, messageFormat string, messageA ...interface{}) { + subCondSet.Manage(ss).MarkUnknown(SubscriptionConditionReferencesResolved, reason, messageFormat, messageA...) +} + +// MarkChannelFailed sets the ChannelReady condition to False state. +func (ss *SubscriptionStatus) MarkChannelFailed(reason, messageFormat string, messageA ...interface{}) { subCondSet.Manage(ss).MarkFalse(SubscriptionConditionChannelReady, reason, messageFormat, messageA) } +// MarkChannelFailed sets the ChannelReady condition to Unknown state. +func (ss *SubscriptionStatus) MarkChannelUnknown(reason, messageFormat string, messageA ...interface{}) { + subCondSet.Manage(ss).MarkUnknown(SubscriptionConditionChannelReady, reason, messageFormat, messageA) +} + // MarkNotAddedToChannel sets the AddedToChannel condition to False state. func (ss *SubscriptionStatus) MarkNotAddedToChannel(reason, messageFormat string, messageA ...interface{}) { subCondSet.Manage(ss).MarkFalse(SubscriptionConditionAddedToChannel, reason, messageFormat, messageA) diff --git a/pkg/apis/sources/v1alpha1/apiserver_lifecycle.go b/pkg/apis/sources/v1alpha1/apiserver_lifecycle.go index e0103ada2c7..d18d72eba9e 100644 --- a/pkg/apis/sources/v1alpha1/apiserver_lifecycle.go +++ b/pkg/apis/sources/v1alpha1/apiserver_lifecycle.go @@ -64,7 +64,7 @@ func (s *ApiServerSourceStatus) MarkSink(uri string) { if len(uri) > 0 { apiserverCondSet.Manage(s).MarkTrue(ApiServerConditionSinkProvided) } else { - apiserverCondSet.Manage(s).MarkUnknown(ApiServerConditionSinkProvided, "SinkEmpty", "Sink has resolved to empty.%s", "") + apiserverCondSet.Manage(s).MarkFalse(ApiServerConditionSinkProvided, "SinkEmpty", "Sink has resolved to empty.%s", "") } } diff --git a/pkg/apis/sources/v1alpha1/apiserver_lifecycle_test.go b/pkg/apis/sources/v1alpha1/apiserver_lifecycle_test.go index 627eaf30070..5e93250da9e 100644 --- a/pkg/apis/sources/v1alpha1/apiserver_lifecycle_test.go +++ b/pkg/apis/sources/v1alpha1/apiserver_lifecycle_test.go @@ -217,6 +217,24 @@ func TestApiServerSourceStatusGetCondition(t *testing.T) { Type: ApiServerConditionReady, Status: corev1.ConditionTrue, }, + }, { + name: "mark sink empty and enough permissions and deployed and event types", + s: func() *ApiServerSourceStatus { + s := &ApiServerSourceStatus{} + s.InitializeConditions() + s.MarkSink("") + s.MarkSufficientPermissions() + s.PropagateDeploymentAvailability(availableDeployment) + s.MarkEventTypes() + return s + }(), + condQuery: ContainerConditionReady, + want: &apis.Condition{ + Type: ContainerConditionReady, + Status: corev1.ConditionFalse, + Reason: "SinkEmpty", + Message: "Sink has resolved to empty.", + }, }} for _, test := range tests { diff --git a/pkg/apis/sources/v1alpha1/containersource_lifecycle.go b/pkg/apis/sources/v1alpha1/containersource_lifecycle.go index dfe8670c9fe..efe7d7ef499 100644 --- a/pkg/apis/sources/v1alpha1/containersource_lifecycle.go +++ b/pkg/apis/sources/v1alpha1/containersource_lifecycle.go @@ -58,7 +58,7 @@ func (s *ContainerSourceStatus) MarkSink(uri string) { if len(uri) > 0 { containerCondSet.Manage(s).MarkTrue(ContainerConditionSinkProvided) } else { - containerCondSet.Manage(s).MarkUnknown(ContainerConditionSinkProvided, "SinkEmpty", "Sink has resolved to empty.%s", "") + containerCondSet.Manage(s).MarkFalse(ContainerConditionSinkProvided, "SinkEmpty", "Sink has resolved to empty.%s", "") } } diff --git a/pkg/apis/sources/v1alpha1/containersource_lifecycle_test.go b/pkg/apis/sources/v1alpha1/containersource_lifecycle_test.go index c943c759463..6c43667dd13 100644 --- a/pkg/apis/sources/v1alpha1/containersource_lifecycle_test.go +++ b/pkg/apis/sources/v1alpha1/containersource_lifecycle_test.go @@ -291,7 +291,7 @@ func TestContainerSourceStatusGetCondition(t *testing.T) { condQuery: ContainerConditionReady, want: &apis.Condition{ Type: ContainerConditionReady, - Status: corev1.ConditionUnknown, + Status: corev1.ConditionFalse, Reason: "SinkEmpty", Message: "Sink has resolved to empty.", }, diff --git a/pkg/apis/sources/v1alpha1/cron_job_lifecycle.go b/pkg/apis/sources/v1alpha1/cron_job_lifecycle.go index 67434fa0747..319c738cc48 100644 --- a/pkg/apis/sources/v1alpha1/cron_job_lifecycle.go +++ b/pkg/apis/sources/v1alpha1/cron_job_lifecycle.go @@ -83,7 +83,7 @@ func (s *CronJobSourceStatus) MarkSink(uri string) { if len(uri) > 0 { cronJobSourceCondSet.Manage(s).MarkTrue(CronJobConditionSinkProvided) } else { - cronJobSourceCondSet.Manage(s).MarkUnknown(CronJobConditionSinkProvided, "SinkEmpty", "Sink has resolved to empty.%s", "") + cronJobSourceCondSet.Manage(s).MarkFalse(CronJobConditionSinkProvided, "SinkEmpty", "Sink has resolved to empty.%s", "") } } diff --git a/pkg/apis/sources/v1alpha1/cron_job_lifecycle_test.go b/pkg/apis/sources/v1alpha1/cron_job_lifecycle_test.go index 23f120bf7ae..3b24f7a9200 100644 --- a/pkg/apis/sources/v1alpha1/cron_job_lifecycle_test.go +++ b/pkg/apis/sources/v1alpha1/cron_job_lifecycle_test.go @@ -391,7 +391,7 @@ func TestCronJobSourceStatusGetCondition(t *testing.T) { condQuery: v1alpha1.CronJobConditionReady, want: &apis.Condition{ Type: v1alpha1.CronJobConditionReady, - Status: corev1.ConditionUnknown, + Status: corev1.ConditionFalse, Reason: "SinkEmpty", Message: "Sink has resolved to empty.", }, diff --git a/pkg/reconciler/eventtype/eventtype.go b/pkg/reconciler/eventtype/eventtype.go index bbe77f0b7bb..9bef2daf4c6 100644 --- a/pkg/reconciler/eventtype/eventtype.go +++ b/pkg/reconciler/eventtype/eventtype.go @@ -115,8 +115,14 @@ func (r *Reconciler) reconcile(ctx context.Context, et *v1alpha1.EventType) erro b, err := r.getBroker(ctx, et) if err != nil { - logging.FromContext(ctx).Error("Unable to get the Broker", zap.Error(err)) - et.Status.MarkBrokerDoesNotExist() + + if apierrs.IsNotFound(err) { + logging.FromContext(ctx).Error("Broker does not exist", zap.Error(err)) + et.Status.MarkBrokerDoesNotExist() + } else { + logging.FromContext(ctx).Error("Unable to get the Broker", zap.Error(err)) + et.Status.MarkBrokerExistsUnknown("BrokerGetFailed", "Failed to get broker: %v", err) + } return err } et.Status.MarkBrokerExists() @@ -127,12 +133,7 @@ func (r *Reconciler) reconcile(ctx context.Context, et *v1alpha1.EventType) erro return err } - if !b.Status.IsReady() { - logging.FromContext(ctx).Error("Broker is not ready", zap.String("broker", b.Name)) - et.Status.MarkBrokerNotReady() - return nil - } - et.Status.MarkBrokerReady() + et.Status.PropagateBrokerStatus(&b.Status) return nil } diff --git a/pkg/reconciler/eventtype/eventtype_test.go b/pkg/reconciler/eventtype/eventtype_test.go index 32dc503c852..815e59eab92 100644 --- a/pkg/reconciler/eventtype/eventtype_test.go +++ b/pkg/reconciler/eventtype/eventtype_test.go @@ -106,7 +106,7 @@ func TestReconcile(t *testing.T) { }, }, { - Name: "Broker not ready", + Name: "The status of Broker is False", Key: testKey, Objects: []runtime.Object{ NewEventType(eventTypeName, testNS, @@ -116,6 +116,7 @@ func TestReconcile(t *testing.T) { ), NewBroker(eventTypeBroker, testNS, WithInitBrokerConditions, + WithIngressFailed("DeploymentFailure", "inducing failure for create deployments"), ), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ @@ -124,7 +125,30 @@ func TestReconcile(t *testing.T) { WithEventTypeSource(eventTypeSource), WithEventTypeBroker(eventTypeBroker), WithEventTypeBrokerExists, - WithEventTypeBrokerNotReady, + WithEventTypeBrokerFailed("BrokerFalse", "The status of Broker is False: inducing failure for create deployments"), + ), + }}, + }, + { + Name: "The status of Broker is Unknown", + Key: testKey, + Objects: []runtime.Object{ + NewEventType(eventTypeName, testNS, + WithEventTypeType(eventTypeType), + WithEventTypeSource(eventTypeSource), + WithEventTypeBroker(eventTypeBroker), + ), + NewBroker(eventTypeBroker, testNS, + WithInitBrokerConditions, + ), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewEventType(eventTypeName, testNS, + WithEventTypeType(eventTypeType), + WithEventTypeSource(eventTypeSource), + WithEventTypeBroker(eventTypeBroker), + WithEventTypeBrokerExists, + WithEventTypeBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: "), ), }}, }, diff --git a/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go b/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go index 6436175f67b..13d55b4d68b 100644 --- a/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go +++ b/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go @@ -153,7 +153,7 @@ func (r *Reconciler) reconcile(ctx context.Context, imc *v1alpha1.InMemoryChanne imc.Status.MarkDispatcherFailed("DispatcherDeploymentDoesNotExist", "Dispatcher Deployment does not exist") } else { logging.FromContext(ctx).Error("Unable to get the dispatcher Deployment", zap.Error(err)) - imc.Status.MarkDispatcherFailed("DispatcherDeploymentGetFailed", "Failed to get dispatcher Deployment") + imc.Status.MarkDispatcherUnknown("DispatcherDeploymentGetFailed", "Failed to get dispatcher Deployment") } return err } @@ -168,7 +168,7 @@ func (r *Reconciler) reconcile(ctx context.Context, imc *v1alpha1.InMemoryChanne imc.Status.MarkServiceFailed("DispatcherServiceDoesNotExist", "Dispatcher Service does not exist") } else { logging.FromContext(ctx).Error("Unable to get the dispatcher service", zap.Error(err)) - imc.Status.MarkServiceFailed("DispatcherServiceGetFailed", "Failed to get dispatcher service") + imc.Status.MarkServiceUnknown("DispatcherServiceGetFailed", "Failed to get dispatcher service") } return err } @@ -183,7 +183,7 @@ func (r *Reconciler) reconcile(ctx context.Context, imc *v1alpha1.InMemoryChanne imc.Status.MarkEndpointsFailed("DispatcherEndpointsDoesNotExist", "Dispatcher Endpoints does not exist") } else { logging.FromContext(ctx).Error("Unable to get the dispatcher endpoints", zap.Error(err)) - imc.Status.MarkEndpointsFailed("DispatcherEndpointsGetFailed", "Failed to get dispatcher endpoints") + imc.Status.MarkEndpointsUnknown("DispatcherEndpointsGetFailed", "Failed to get dispatcher endpoints") } return err } @@ -200,7 +200,6 @@ func (r *Reconciler) reconcile(ctx context.Context, imc *v1alpha1.InMemoryChanne // ExternalName svc, err := r.reconcileChannelService(ctx, imc) if err != nil { - imc.Status.MarkChannelServiceFailed("ChannelServiceFailed", fmt.Sprintf("Channel Service failed: %s", err)) return err } imc.Status.MarkChannelServiceTrue() @@ -225,22 +224,27 @@ func (r *Reconciler) reconcileChannelService(ctx context.Context, imc *v1alpha1. svc, err = resources.NewK8sService(imc, resources.ExternalService(r.dispatcherNamespace, r.dispatcherServiceName)) if err != nil { logging.FromContext(ctx).Error("failed to create the channel service object", zap.Error(err)) + imc.Status.MarkChannelServiceFailed("ChannelServiceFailed", fmt.Sprintf("Channel Service failed: %s", err)) return nil, err } svc, err = r.KubeClientSet.CoreV1().Services(imc.Namespace).Create(svc) if err != nil { logging.FromContext(ctx).Error("failed to create the channel service", zap.Error(err)) + imc.Status.MarkChannelServiceFailed("ChannelServiceFailed", fmt.Sprintf("Channel Service failed: %s", err)) return nil, err } return svc, nil } logging.FromContext(ctx).Error("Unable to get the channel service", zap.Error(err)) + imc.Status.MarkChannelServiceUnknown("ChannelServiceGetFailed", fmt.Sprintf("Unable to get the channel service: %s", err)) return nil, err } // Check to make sure that our IMC owns this service and if not, complain. if !metav1.IsControlledBy(svc, imc) { - return nil, fmt.Errorf("inmemorychannel: %s/%s does not own Service: %q", imc.Namespace, imc.Name, svc.Name) + err := fmt.Errorf("inmemorychannel: %s/%s does not own Service: %q", imc.Namespace, imc.Name, svc.Name) + imc.Status.MarkChannelServiceFailed("ChannelServiceFailed", fmt.Sprintf("Channel Service failed: %s", err)) + return nil, err } return svc, nil } diff --git a/pkg/reconciler/inmemorychannel/controller/inmemorychannel_test.go b/pkg/reconciler/inmemorychannel/controller/inmemorychannel_test.go index 41eab3646df..a5d954228d0 100644 --- a/pkg/reconciler/inmemorychannel/controller/inmemorychannel_test.go +++ b/pkg/reconciler/inmemorychannel/controller/inmemorychannel_test.go @@ -110,11 +110,61 @@ func TestAllCases(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: reconciletesting.NewInMemoryChannel(imcName, testNS, reconciletesting.WithInitInMemoryChannelConditions, - reconciletesting.WithInMemoryChannelDeploymentNotReady("DispatcherDeploymentDoesNotExist", "Dispatcher Deployment does not exist")), + reconciletesting.WithInMemoryChannelDeploymentFailed("DispatcherDeploymentDoesNotExist", "Dispatcher Deployment does not exist")), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "ReconcileFailed", "InMemoryChannel reconciliation failed: deployment.apps \"test-deployment\" not found"), }, + }, { + Name: "the status of deployment is false", + Key: imcKey, + Objects: []runtime.Object{ + makeFalseDeployment(), + makeService(), + makeReadyEndpoints(), + reconciletesting.NewInMemoryChannel(imcName, testNS), + }, + WantErr: false, + WantCreates: []runtime.Object{ + makeChannelService(reconciletesting.NewInMemoryChannel(imcName, testNS)), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: reconciletesting.NewInMemoryChannel(imcName, testNS, + reconciletesting.WithInitInMemoryChannelConditions, + reconciletesting.WithInMemoryChannelDeploymentFailed("DispatcherDeploymentFalse", "The status of Dispatcher Deployment is False: Deployment Failed : Deployment Failed"), + reconciletesting.WithInMemoryChannelServiceReady(), + reconciletesting.WithInMemoryChannelEndpointsReady(), + reconciletesting.WithInMemoryChannelChannelServiceReady(), + reconciletesting.WithInMemoryChannelAddress(channelServiceAddress)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Reconciled", "InMemoryChannel reconciled"), + }, + }, { + Name: "the status of deployment is unknown", + Key: imcKey, + Objects: []runtime.Object{ + makeUnknownDeployment(), + makeService(), + makeReadyEndpoints(), + reconciletesting.NewInMemoryChannel(imcName, testNS), + }, + WantErr: false, + WantCreates: []runtime.Object{ + makeChannelService(reconciletesting.NewInMemoryChannel(imcName, testNS)), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: reconciletesting.NewInMemoryChannel(imcName, testNS, + reconciletesting.WithInitInMemoryChannelConditions, + reconciletesting.WithInMemoryChannelDeploymentUnknown("DispatcherDeploymentUnknown", "The status of Dispatcher Deployment is Unknown: Deployment Unknown : Deployment Unknown"), + reconciletesting.WithInMemoryChannelServiceReady(), + reconciletesting.WithInMemoryChannelEndpointsReady(), + reconciletesting.WithInMemoryChannelChannelServiceReady(), + reconciletesting.WithInMemoryChannelAddress(channelServiceAddress)), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Reconciled", "InMemoryChannel reconciled"), + }, }, { Name: "Service does not exist", Key: imcKey, @@ -317,6 +367,18 @@ func makeReadyDeployment() *appsv1.Deployment { return d } +func makeFalseDeployment() *appsv1.Deployment { + d := makeDeployment() + d.Status.Conditions = []appsv1.DeploymentCondition{{Type: appsv1.DeploymentAvailable, Status: corev1.ConditionFalse, Reason: "Deployment Failed", Message: "Deployment Failed"}} + return d +} + +func makeUnknownDeployment() *appsv1.Deployment { + d := makeDeployment() + d.Status.Conditions = []appsv1.DeploymentCondition{{Type: appsv1.DeploymentAvailable, Status: corev1.ConditionUnknown, Reason: "Deployment Unknown", Message: "Deployment Unknown"}} + return d +} + func makeService() *corev1.Service { return &corev1.Service{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/reconciler/subscription/subscription.go b/pkg/reconciler/subscription/subscription.go index 5fa2587eca4..e963b7ce44e 100644 --- a/pkg/reconciler/subscription/subscription.go +++ b/pkg/reconciler/subscription/subscription.go @@ -151,7 +151,7 @@ func (r *Reconciler) reconcile(ctx context.Context, subscription *v1alpha1.Subsc zap.Error(err), zap.Any("channel", subscription.Spec.Channel)) r.Recorder.Eventf(subscription, corev1.EventTypeWarning, channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) - subscription.Status.MarkReferencesNotResolved(channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) + subscription.Status.MarkReferencesResolvedUnknown(channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) return err } err := r.syncPhysicalChannel(ctx, subscription, channel, true) @@ -171,7 +171,7 @@ func (r *Reconciler) reconcile(ctx context.Context, subscription *v1alpha1.Subsc zap.Error(err), zap.Any("channel", subscription.Spec.Channel)) r.Recorder.Eventf(subscription, corev1.EventTypeWarning, channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) - subscription.Status.MarkReferencesNotResolved(channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) + subscription.Status.MarkReferencesResolvedUnknown(channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) return err } if err := r.validateChannel(ctx, channel); err != nil { @@ -273,36 +273,40 @@ func (r *Reconciler) reconcile(ctx context.Context, subscription *v1alpha1.Subsc zap.Error(err), zap.Any("channel", subscription.Spec.Channel)) r.Recorder.Eventf(subscription, corev1.EventTypeWarning, channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) - subscription.Status.MarkChannelNotReady(channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) + subscription.Status.MarkChannelUnknown(channelReferenceFailed, "Failed to get Spec.Channel as Channelable duck type. %s", err) return err } - if err := r.subMarkedReadyByChannel(subscription, channel); err != nil { + ss, err := r.getSubStatusByChannel(subscription, channel) + if err != nil { logging.FromContext(ctx).Warn("Subscription not marked by Channel as Ready.", zap.Error(err)) r.Recorder.Eventf(subscription, corev1.EventTypeWarning, subscriptionNotMarkedReadyByChannel, err.Error()) - subscription.Status.MarkChannelNotReady(subscriptionNotMarkedReadyByChannel, "Subscription not marked by Channel as Ready: %s", err) + subscription.Status.MarkChannelFailed(subscriptionNotMarkedReadyByChannel, "Subscription not marked by Channel as Ready: %s", err) return err } - - subscription.Status.MarkChannelReady() + subStatus := ss.Ready + if subStatus == corev1.ConditionTrue { + subscription.Status.MarkChannelReady() + } else if subStatus == corev1.ConditionUnknown { + subscription.Status.MarkChannelUnknown(subscriptionNotMarkedReadyByChannel, "Subscription marked by Channel as Unknown") + } else if subStatus == corev1.ConditionFalse { + subscription.Status.MarkChannelFailed(subscriptionNotMarkedReadyByChannel, "Subscription marked by Channel as False") + } return nil } -func (r Reconciler) subMarkedReadyByChannel(subscription *v1alpha1.Subscription, channel *eventingduckv1alpha1.Channelable) error { +func (r Reconciler) getSubStatusByChannel(subscription *v1alpha1.Subscription, channel *eventingduckv1alpha1.Channelable) (eventingduckv1alpha1.SubscriberStatus, error) { subscribableStatus := channel.Status.GetSubscribableTypeStatus() if subscribableStatus == nil { - return fmt.Errorf("channel.Status.SubscribableStatus is nil") + return eventingduckv1alpha1.SubscriberStatus{}, fmt.Errorf("channel.Status.SubscribableStatus is nil") } for _, sub := range subscribableStatus.Subscribers { if sub.UID == subscription.GetUID() && sub.ObservedGeneration == subscription.GetGeneration() { - if sub.Ready == corev1.ConditionTrue { - return nil - } - return fmt.Errorf(sub.Message) + return sub, nil } } - return fmt.Errorf("subscription %q not present in channel %q subscriber's list", subscription.Name, channel.Name) + return eventingduckv1alpha1.SubscriberStatus{}, fmt.Errorf("subscription %q not present in channel %q subscriber's list", subscription.Name, channel.Name) } func (r Reconciler) subPresentInChannelSpec(subscription *v1alpha1.Subscription, channel *eventingduckv1alpha1.Channelable) bool { diff --git a/pkg/reconciler/subscription/subscription_test.go b/pkg/reconciler/subscription/subscription_test.go index 82cc980aec1..171ec703e63 100644 --- a/pkg/reconciler/subscription/subscription_test.go +++ b/pkg/reconciler/subscription/subscription_test.go @@ -159,7 +159,7 @@ func TestAllCases(t *testing.T) { WithSubscriptionSubscriberRef(subscriberGVK, subscriberName), // The first reconciliation will initialize the status conditions. WithInitSubscriptionConditions, - WithSubscriptionReferencesNotResolved(channelReferenceFailed, fmt.Sprintf("Failed to get Spec.Channel as Channelable duck type. channels.messaging.knative.dev %q not found", channelName)), + WithSubscriptionReferencesResolvedUnknown(channelReferenceFailed, fmt.Sprintf("Failed to get Spec.Channel as Channelable duck type. channels.messaging.knative.dev %q not found", channelName)), ), }}, }, { @@ -188,7 +188,7 @@ func TestAllCases(t *testing.T) { WithSubscriptionSubscriberRef(subscriberGVK, subscriberName), // The first reconciliation will initialize the status conditions. WithInitSubscriptionConditions, - WithSubscriptionReferencesNotResolved(channelReferenceFailed, fmt.Sprintf("Failed to get Spec.Channel as Channelable duck type. channels.messaging.knative.dev %q not found", channelName)), + WithSubscriptionReferencesResolvedUnknown(channelReferenceFailed, fmt.Sprintf("Failed to get Spec.Channel as Channelable duck type. channels.messaging.knative.dev %q not found", channelName)), ), }}, }, { diff --git a/pkg/reconciler/testing/eventtype.go b/pkg/reconciler/testing/eventtype.go index 2036e6c67b9..e9c51fd5e27 100644 --- a/pkg/reconciler/testing/eventtype.go +++ b/pkg/reconciler/testing/eventtype.go @@ -100,9 +100,16 @@ func WithEventTypeBrokerExists(et *v1alpha1.EventType) { et.Status.MarkBrokerExists() } -// WithEventTypeBrokerNotReady calls .Status.MarkBrokerNotReady on the EventType. -func WithEventTypeBrokerNotReady(et *v1alpha1.EventType) { - et.Status.MarkBrokerNotReady() +func WithEventTypeBrokerFailed(reason, message string) EventTypeOption { + return func(et *v1alpha1.EventType) { + et.Status.MarkBrokerFailed(reason, message) + } +} + +func WithEventTypeBrokerUnknown(reason, message string) EventTypeOption { + return func(et *v1alpha1.EventType) { + et.Status.MarkBrokerUnknown(reason, message) + } } // WithEventTypeBrokerReady calls .Status.MarkBrokerReady on the EventType. diff --git a/pkg/reconciler/testing/inmemorychannel.go b/pkg/reconciler/testing/inmemorychannel.go index 2e852610aaf..25ce4755340 100644 --- a/pkg/reconciler/testing/inmemorychannel.go +++ b/pkg/reconciler/testing/inmemorychannel.go @@ -62,12 +62,18 @@ func WithInMemoryChannelSubscribers(subscribers []duckv1alpha1.SubscriberSpec) I } } -func WithInMemoryChannelDeploymentNotReady(reason, message string) InMemoryChannelOption { +func WithInMemoryChannelDeploymentFailed(reason, message string) InMemoryChannelOption { return func(imc *v1alpha1.InMemoryChannel) { imc.Status.MarkDispatcherFailed(reason, message) } } +func WithInMemoryChannelDeploymentUnknown(reason, message string) InMemoryChannelOption { + return func(imc *v1alpha1.InMemoryChannel) { + imc.Status.MarkDispatcherUnknown(reason, message) + } +} + func WithInMemoryChannelDeploymentReady() InMemoryChannelOption { return func(imc *v1alpha1.InMemoryChannel) { imc.Status.PropagateDispatcherStatus(&appsv1.DeploymentStatus{Conditions: []appsv1.DeploymentCondition{{Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue}}}) diff --git a/pkg/reconciler/testing/subscription.go b/pkg/reconciler/testing/subscription.go index d82b0fc7343..2ef82e8df07 100644 --- a/pkg/reconciler/testing/subscription.go +++ b/pkg/reconciler/testing/subscription.go @@ -167,6 +167,12 @@ func WithSubscriptionReferencesNotResolved(reason, msg string) SubscriptionOptio } } +func WithSubscriptionReferencesResolvedUnknown(reason, msg string) SubscriptionOption { + return func(s *v1alpha1.Subscription) { + s.Status.MarkReferencesResolvedUnknown(reason, msg) + } +} + func WithSubscriptionReply(gvk metav1.GroupVersionKind, name string) SubscriptionOption { return func(s *v1alpha1.Subscription) { s.Spec.Reply = &duckv1.Destination{ diff --git a/pkg/reconciler/testing/trigger.go b/pkg/reconciler/testing/trigger.go index 6856fdaf02c..9a01ff5eb8c 100644 --- a/pkg/reconciler/testing/trigger.go +++ b/pkg/reconciler/testing/trigger.go @@ -110,12 +110,25 @@ func WithTriggerBrokerFailed(reason, message string) TriggerOption { } } +// WithTriggerBrokerUnknown marks the Broker as unknown +func WithTriggerBrokerUnknown(reason, message string) TriggerOption { + return func(t *v1alpha1.Trigger) { + t.Status.MarkBrokerUnknown(reason, message) + } +} + func WithTriggerNotSubscribed(reason, message string) TriggerOption { return func(t *v1alpha1.Trigger) { t.Status.MarkNotSubscribed(reason, message) } } +func WithTriggerSubscribedUnknown(reason, message string) TriggerOption { + return func(t *v1alpha1.Trigger) { + t.Status.MarkSubscribedUnknown(reason, message) + } +} + func WithTriggerSubscribed() TriggerOption { return func(t *v1alpha1.Trigger) { t.Status.PropagateSubscriptionStatus(v1alpha1.TestHelper.ReadySubscriptionStatus()) diff --git a/pkg/reconciler/trigger/trigger.go b/pkg/reconciler/trigger/trigger.go index da11edac1a1..1ad7fa29cdc 100644 --- a/pkg/reconciler/trigger/trigger.go +++ b/pkg/reconciler/trigger/trigger.go @@ -171,7 +171,7 @@ func (r *Reconciler) reconcile(ctx context.Context, t *v1alpha1.Trigger) error { } } } else { - t.Status.MarkBrokerFailed("BrokerGetFailed", "Failed to get broker") + t.Status.MarkBrokerUnknown("BrokerGetFailed", "Failed to get broker: %v", err) } return err } @@ -260,7 +260,7 @@ func (r *Reconciler) propagateDependencyReadiness(ctx context.Context, t *v1alph dependencyObj, err := lister.ByNamespace(t.GetNamespace()).Get(dependencyObjRef.Name) if err != nil { if apierrs.IsNotFound(err) { - t.Status.MarkDependencyUnknown("DependencyDoesNotExist", "Dependency does not exist: %v", err) + t.Status.MarkDependencyFailed("DependencyDoesNotExist", "Dependency does not exist: %v", err) } else { t.Status.MarkDependencyUnknown("DependencyGetFailed", "Failed to get dependency: %v", err) } diff --git a/pkg/reconciler/trigger/trigger_test.go b/pkg/reconciler/trigger/trigger_test.go index baec196d529..90331196eed 100644 --- a/pkg/reconciler/trigger/trigger_test.go +++ b/pkg/reconciler/trigger/trigger_test.go @@ -295,7 +295,7 @@ func TestAllCases(t *testing.T) { ), }}, }, { - Name: "Broker get failure", + Name: "Broker does not exist", Key: triggerKey, Objects: []runtime.Object{ reconciletesting.NewTrigger(triggerName, testNS, brokerName, @@ -319,7 +319,7 @@ func TestAllCases(t *testing.T) { ), }}, }, { - Name: "Broker get failure, status update fail", + Name: "Broker does not exist, status update fail", Key: triggerKey, Objects: []runtime.Object{ reconciletesting.NewTrigger(triggerName, testNS, brokerName, @@ -344,6 +344,29 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerBrokerFailed("DoesNotExist", "Broker does not exist"), ), }}, + }, { + Name: "The status of Broker is Unknown", + Key: triggerKey, + Objects: []runtime.Object{ + makeUnknownStatusBroker(), + reconciletesting.NewTrigger(triggerName, testNS, brokerName, + reconciletesting.WithTriggerUID(triggerUID), + reconciletesting.WithTriggerSubscriberURI(subscriberURI)), + }, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "TriggerChannelFailed", "Broker's Trigger channel not found"), + Eventf(corev1.EventTypeWarning, "TriggerReconcileFailed", "Trigger reconciliation failed: failed to find Broker's Trigger channel"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: reconciletesting.NewTrigger(triggerName, testNS, brokerName, + reconciletesting.WithTriggerUID(triggerUID), + reconciletesting.WithTriggerSubscriberURI(subscriberURI), + // The first reconciliation will initialize the status conditions. + reconciletesting.WithInitTriggerConditions, + reconciletesting.WithTriggerBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: nil"), + ), + }}, }, { Name: "Trigger being deleted", Key: triggerKey, @@ -567,7 +590,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionNotReady", "Subscription is not ready: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -602,7 +625,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionNotReady", "Subscription is not ready: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -635,7 +658,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionNotReady", "Subscription is not ready: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -668,7 +691,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionNotReady", "Subscription is not ready: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberResolvedTargetURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -701,7 +724,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionNotReady", "Subscription is not ready: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), reconciletesting.WithTriggerStatusSubscriberURI(k8sServiceResolvedURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -742,7 +765,7 @@ func TestAllCases(t *testing.T) { Objects: []runtime.Object{ makeReadyBroker(), makeBrokerFilterService(), - makeNotReadySubscription(), + makeFalseStatusSubscription(), reconciletesting.NewTrigger(triggerName, testNS, brokerName, reconciletesting.WithTriggerUID(triggerUID), reconciletesting.WithTriggerSubscriberURI(subscriberURI), @@ -760,7 +783,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionNotReady", "Subscription is not ready: test induced [error]"), + reconciletesting.WithTriggerNotSubscribed("SubscriptionFalse", "The status of Subscription is False: test induced [error]"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -826,17 +849,17 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), - reconciletesting.WithTriggerDependencyUnknown("DependencyDoesNotExist", "Dependency does not exist: cronjobsources.sources.eventing.knative.dev \"test-cronjob-source\" not found"), + reconciletesting.WithTriggerDependencyFailed("DependencyDoesNotExist", "Dependency does not exist: cronjobsources.sources.eventing.knative.dev \"test-cronjob-source\" not found"), ), }}, }, { - Name: "Dependency not ready", + Name: "The status of Dependency is False", Key: triggerKey, Objects: []runtime.Object{ makeReadyBroker(), makeBrokerFilterService(), makeReadySubscription(), - makeNotReadyCronJobSource(), + makeFalseStatusCronJobSource(), reconciletesting.NewTrigger(triggerName, testNS, brokerName, reconciletesting.WithTriggerUID(triggerUID), reconciletesting.WithTriggerSubscriberURI(subscriberURI), @@ -858,10 +881,43 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), - reconciletesting.WithTriggerDependencyFailed("DependencyNotReady", "Dependency is not ready: "), + reconciletesting.WithTriggerDependencyFailed("DependencyFalse", "The status of Dependency is False: "), ), }}, }, { + Name: "The status of Dependency is Unknown", + Key: triggerKey, + Objects: []runtime.Object{ + makeReadyBroker(), + makeBrokerFilterService(), + makeReadySubscription(), + makeUnknownStatusCronJobSource(), + reconciletesting.NewTrigger(triggerName, testNS, brokerName, + reconciletesting.WithTriggerUID(triggerUID), + reconciletesting.WithTriggerSubscriberURI(subscriberURI), + reconciletesting.WithInitTriggerConditions, + reconciletesting.WithDependencyAnnotation(dependencyAnnotation), + ), + }, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "TriggerReconciled", "Trigger reconciled")}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: reconciletesting.NewTrigger(triggerName, testNS, brokerName, + reconciletesting.WithTriggerUID(triggerUID), + reconciletesting.WithTriggerSubscriberURI(subscriberURI), + // The first reconciliation will initialize the status conditions. + reconciletesting.WithInitTriggerConditions, + reconciletesting.WithDependencyAnnotation(dependencyAnnotation), + reconciletesting.WithTriggerBrokerReady(), + reconciletesting.WithTriggerSubscribed(), + reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), + reconciletesting.WithTriggerSubscriberResolvedSucceeded(), + reconciletesting.WithTriggerDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: nil"), + ), + }}, + }, + { Name: "Dependency generation not equal", Key: triggerKey, Objects: []runtime.Object{ @@ -1010,6 +1066,12 @@ func makeReadyBroker() *v1alpha1.Broker { return b } +func makeUnknownStatusBroker() *v1alpha1.Broker { + b := makeBroker() + b.Status = *v1alpha1.TestHelper.UnknownBrokerStatus() + return b +} + func makeReadyDefaultBroker() *v1alpha1.Broker { b := makeReadyBroker() b.Name = "default" @@ -1091,7 +1153,7 @@ func makeIngressSubscriptionNotOwnedByTrigger() *messagingv1alpha1.Subscription func makeDifferentReadySubscription() *messagingv1alpha1.Subscription { s := makeIngressSubscription() s.Spec.Subscriber.URI = apis.HTTP("different.example.com") - s.Status = *v1alpha1.TestHelper.ReadySubscriptionStatus() + s.Status = *v1alpha1.TestHelper.FalseSubscriptionStatus() return s } @@ -1101,18 +1163,24 @@ func makeReadySubscription() *messagingv1alpha1.Subscription { return s } -func makeNotReadySubscription() *messagingv1alpha1.Subscription { +func makeFalseStatusSubscription() *messagingv1alpha1.Subscription { s := makeIngressSubscription() - s.Status = *v1alpha1.TestHelper.NotReadySubscriptionStatus() + s.Status = *v1alpha1.TestHelper.FalseSubscriptionStatus() return s } -func makeNotReadyCronJobSource() *sourcesv1alpha1.CronJobSource { +func makeFalseStatusCronJobSource() *sourcesv1alpha1.CronJobSource { return NewCronJobSource(cronJobSourceName, testNS, WithCronJobApiVersion(cronJobSourceAPIVersion), WithCronJobSourceSinkNotFound) } +func makeUnknownStatusCronJobSource() *sourcesv1alpha1.CronJobSource { + cjs := NewCronJobSource(cronJobSourceName, testNS, WithCronJobApiVersion(cronJobSourceAPIVersion)) + cjs.Status = *v1alpha1.TestHelper.UnknownCronJobSourceStatus() + return cjs +} + func makeGenerationNotEqualCronJobSource() *sourcesv1alpha1.CronJobSource { - c := makeNotReadyCronJobSource() + c := makeFalseStatusCronJobSource() c.Generation = currentGeneration c.Status.ObservedGeneration = outdatedGeneration return c diff --git a/vendor/knative.dev/pkg/apis/condition_set.go b/vendor/knative.dev/pkg/apis/condition_set.go index 7f1917e89c4..389b5977e07 100644 --- a/vendor/knative.dev/pkg/apis/condition_set.go +++ b/vendor/knative.dev/pkg/apis/condition_set.go @@ -52,6 +52,17 @@ type ConditionManager interface { // set to true. IsHappy() bool + // IsUnknown looks at the happy condition and returns true if that condition is + // set to unknown or that condition is nil. + IsUnknown() bool + + // IsFalse looks at the happy condition and returns true if that condition is + // set to false. + IsFalse() bool + + // GetCondition finds and returns the Condition that matches the ConditionType happy. + GetHappyCondition() *Condition + // GetCondition finds and returns the Condition that matches the ConditionType // previously set on Conditions. GetCondition(t ConditionType) *Condition @@ -148,6 +159,29 @@ func (r conditionsImpl) IsHappy() bool { return true } +// IsUnknown looks at the happy condition and returns true if that condition is +// set to Unknown or that condition is nil. +func (r conditionsImpl) IsUnknown() bool { + if c := r.GetCondition(r.happy); !c.IsUnknown() { + return false + } + return true +} + +// IsFalse looks at the happy condition and returns true if that condition is +// set to False. +func (r conditionsImpl) IsFalse() bool { + if c := r.GetCondition(r.happy); c == nil || !c.IsFalse() { + return false + } + return true +} + +// GetHappyCondition finds and returns the Condition that matches the ConditionType happy. +func (r conditionsImpl) GetHappyCondition() *Condition { + return r.GetCondition(r.happy) +} + // GetCondition finds and returns the Condition that matches the ConditionType // previously set on Conditions. func (r conditionsImpl) GetCondition(t ConditionType) *Condition { From d97efb1f617accd09ea22c850db1455677944d1b Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Thu, 2 Jan 2020 13:33:46 -0800 Subject: [PATCH 2/5] modified the code based on CR --- Gopkg.lock | 4 +- .../eventing/v1alpha1/broker_lifecycle.go | 10 ++-- .../eventing/v1alpha1/eventtype_lifecycle.go | 18 +++--- pkg/apis/eventing/v1alpha1/test_helper.go | 4 +- .../eventing/v1alpha1/trigger_lifecycle.go | 58 ++++++++++--------- .../v1alpha1/trigger_lifecycle_test.go | 2 +- .../messaging/v1alpha1/channel_lifecycle.go | 16 +---- .../v1alpha1/channel_lifecycle_test.go | 4 +- .../v1alpha1/subscription_lifecycle.go | 33 +++++------ pkg/reconciler/subscription/subscription.go | 4 +- pkg/reconciler/trigger/trigger_test.go | 16 ++--- vendor/knative.dev/pkg/apis/condition_set.go | 39 ++----------- 12 files changed, 87 insertions(+), 121 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 81b303f5acb..b067b411c22 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1254,7 +1254,7 @@ [[projects]] branch = "add_isunknown" - digest = "1:110a4a57c9443afbeca13831ca57bb3c5ddd98f4fb40b99a6adf21442fb90560" + digest = "1:34b5c01e5cee902f33f8bca94da7ab470e811a18939e9df9aaf006135e1401b4" name = "knative.dev/pkg" packages = [ "apis", @@ -1355,7 +1355,7 @@ "webhook/resourcesemantics/validation", ] pruneopts = "T" - revision = "f3014f066bdf0857ad691a4b2a77106be21fb054" + revision = "866e0fd95952c4bf95fa83ea68d1100dd1837cb0" source = "github.com/capri-xiyue/pkg" [[projects]] diff --git a/pkg/apis/eventing/v1alpha1/broker_lifecycle.go b/pkg/apis/eventing/v1alpha1/broker_lifecycle.go index 1bef94fc496..3737d79967a 100644 --- a/pkg/apis/eventing/v1alpha1/broker_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/broker_lifecycle.go @@ -47,16 +47,16 @@ func (bs *BrokerStatus) GetCondition(t apis.ConditionType) *apis.Condition { return brokerCondSet.Manage(bs).GetCondition(t) } +// GetTopLevelCondition returns the top level Condition. +func (bs *BrokerStatus) GetTopLevelCondition() *apis.Condition { + return brokerCondSet.Manage(bs).GetTopLevelCondition() +} + // IsReady returns true if the resource is ready overall. func (bs *BrokerStatus) IsReady() bool { return brokerCondSet.Manage(bs).IsHappy() } -// IsUnknown returns true if the resource is unknown overall. -func (bs *BrokerStatus) IsUnknown() bool { - return brokerCondSet.Manage(bs).IsUnknown() -} - // InitializeConditions sets relevant unset conditions to Unknown state. func (bs *BrokerStatus) InitializeConditions() { brokerCondSet.Manage(bs).InitializeConditions() diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go index 4e956c3d470..f625c04a5a4 100644 --- a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go @@ -68,17 +68,21 @@ func (et *EventTypeStatus) MarkBrokerUnknown(reason, messageFormat string, messa } func (et *EventTypeStatus) PropagateBrokerStatus(bs *BrokerStatus) { - if bs.IsReady() { + bc := brokerCondSet.Manage(bs).GetTopLevelCondition() + if bc == nil { + et.MarkBrokerUnknown("BrokerUnknown", "The condition of Broker is nil") + return + } + if bc.IsTrue() { eventTypeCondSet.Manage(et).MarkTrue(EventTypeConditionBrokerReady) } else { - msg := "nil" - if bc := brokerCondSet.Manage(bs).GetCondition(BrokerConditionReady); bc != nil { - msg = bc.Message - } - if bs.IsUnknown() { + msg := bc.Message + if bc.IsUnknown() { et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: %s", msg) - } else { + } else if bc.IsFalse() { et.MarkBrokerFailed("BrokerFalse", "The status of Broker is False: %s", msg) + } else { + et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) } } } diff --git a/pkg/apis/eventing/v1alpha1/test_helper.go b/pkg/apis/eventing/v1alpha1/test_helper.go index be68c9da676..f3e5a0031ea 100644 --- a/pkg/apis/eventing/v1alpha1/test_helper.go +++ b/pkg/apis/eventing/v1alpha1/test_helper.go @@ -78,6 +78,7 @@ func (t testHelper) ReadyBrokerStatus() *BrokerStatus { func (t testHelper) UnknownBrokerStatus() *BrokerStatus { bs := &BrokerStatus{} + bs.InitializeConditions() return bs } @@ -95,7 +96,7 @@ func (t testHelper) ReadyTriggerStatus() *TriggerStatus { ts.InitializeConditions() ts.SubscriberURI = &apis.URL{Scheme: "http", Host: "foo"} ts.PropagateBrokerStatus(t.ReadyBrokerStatus()) - ts.PropagateSubscriptionStatus(t.FalseSubscriptionStatus()) + ts.PropagateSubscriptionStatus(t.ReadySubscriptionStatus()) return ts } @@ -125,5 +126,6 @@ func (t testHelper) AvailableDeployment() *v1.Deployment { func (t testHelper) UnknownCronJobSourceStatus() *v1alpha1.CronJobSourceStatus { cjss := &v1alpha1.CronJobSourceStatus{} + cjss.InitializeConditions() return cjss } diff --git a/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go index 24630a8aeff..a2f413baea3 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go @@ -45,9 +45,9 @@ func (ts *TriggerStatus) GetCondition(t apis.ConditionType) *apis.Condition { return triggerCondSet.Manage(ts).GetCondition(t) } -// GetCondition returns the condition currently associated with the given ConditionType happy, or nil. -func (ts *TriggerStatus) GetHappyCondition() *apis.Condition { - return triggerCondSet.Manage(ts).GetHappyCondition() +// GetTopLevelCondition returns the top level Condition. +func (ts *TriggerStatus) GetTopLevelCondition() *apis.Condition { + return triggerCondSet.Manage(ts).GetTopLevelCondition() } // IsReady returns true if the resource is ready overall. @@ -55,28 +55,27 @@ func (ts *TriggerStatus) IsReady() bool { return triggerCondSet.Manage(ts).IsHappy() } -// IsReady returns true if the resource is unknown overall. -func (ts *TriggerStatus) IsUnknown() bool { - return triggerCondSet.Manage(ts).IsUnknown() -} - // InitializeConditions sets relevant unset conditions to Unknown state. func (ts *TriggerStatus) InitializeConditions() { triggerCondSet.Manage(ts).InitializeConditions() } func (ts *TriggerStatus) PropagateBrokerStatus(bs *BrokerStatus) { - if bs.IsReady() { + bc := brokerCondSet.Manage(bs).GetTopLevelCondition() + if bc == nil { + ts.MarkBrokerUnknown("BrokerUnknown", "The condition of Broker is nil") + return + } + if bc.IsTrue() { triggerCondSet.Manage(ts).MarkTrue(TriggerConditionBroker) } else { - msg := "nil" - if bc := brokerCondSet.Manage(bs).GetCondition(BrokerConditionReady); bc != nil { - msg = bc.Message - } - if bs.IsUnknown() { + msg := bc.Message + if bc.IsUnknown() { ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: %s", msg) - } else { + } else if bc.IsFalse() { ts.MarkBrokerFailed("BrokerFalse", "The status of Broker is False: %s", msg) + } else { + ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) } } } @@ -90,17 +89,21 @@ func (ts *TriggerStatus) MarkBrokerUnknown(reason, messageFormat string, message } func (ts *TriggerStatus) PropagateSubscriptionStatus(ss *messagingv1alpha1.SubscriptionStatus) { - if ss.IsReady() { + sc := messagingv1alpha1.SubCondSet.Manage(ss).GetTopLevelCondition() + if sc == nil { + ts.MarkSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil") + return + } + if sc.IsTrue() { triggerCondSet.Manage(ts).MarkTrue(TriggerConditionSubscribed) } else { - msg := "nil" - if sc := ss.Status.GetCondition(messagingv1alpha1.SubscriptionConditionReady); sc != nil { - msg = sc.Message - } - if ss.IsUnknown() { + msg := sc.Message + if sc.IsUnknown() { ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: %s", msg) - } else { + } else if sc.IsFalse() { ts.MarkNotSubscribed("SubscriptionFalse", "The status of Subscription is False: %s", msg) + } else { + ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Broker is invalid: %v", sc.Status) } } } @@ -143,13 +146,14 @@ func (ts *TriggerStatus) MarkDependencyUnknown(reason, messageFormat string, mes func (ts *TriggerStatus) PropagateDependencyStatus(ks *duckv1.KResource) { kc := ks.Status.GetCondition(apis.ConditionReady) - if kc != nil && kc.IsTrue() { + if kc == nil { + ts.MarkDependencyUnknown("DependencyUnknown", "The condition of Dependency is nil") + return + } + if kc.IsTrue() { ts.MarkDependencySucceeded() } else { - msg := "nil" - if kc != nil { - msg = kc.Message - } + msg := kc.Message if kc.IsUnknown() { ts.MarkDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: %s", msg) } else { diff --git a/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go index 0ea4891367a..3f056780d5a 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go +++ b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go @@ -360,7 +360,7 @@ func TestTriggerConditionStatus(t *testing.T) { ts.MarkDependencyFailed("The status of dependency is false", "The status of dependency is unknown: nil") } } - got := ts.GetHappyCondition().Status + got := ts.GetTopLevelCondition().Status if test.wantConditionStatus != got { t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) } diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index 8b6a49c3135..fa32b4888d1 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -42,9 +42,9 @@ func (cs *ChannelStatus) GetCondition(t apis.ConditionType) *apis.Condition { return chCondSet.Manage(cs).GetCondition(t) } -// GetHappyCondition returns the condition currently associated with the ConditionType happy. -func (cs *ChannelStatus) GetHappyCondition() *apis.Condition { - return chCondSet.Manage(cs).GetHappyCondition() +// GetTopLevelCondition returns the top level Condition. +func (cs *ChannelStatus) GetTopLevelCondition() *apis.Condition { + return chCondSet.Manage(cs).GetTopLevelCondition() } // IsReady returns true if the resource is ready overall. @@ -52,16 +52,6 @@ func (cs *ChannelStatus) IsReady() bool { return chCondSet.Manage(cs).IsHappy() } -// IsUnknown returns true if the resource is unknown overall. -func (cs *ChannelStatus) IsUnknown() bool { - return chCondSet.Manage(cs).IsUnknown() -} - -// IsFalse returns true if the resource is false overall. -func (cs *ChannelStatus) IsFalse() bool { - return chCondSet.Manage(cs).IsFalse() -} - // InitializeConditions sets relevant unset conditions to Unknown state. func (cs *ChannelStatus) InitializeConditions() { chCondSet.Manage(cs).InitializeConditions() diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index 48103eec211..e20f6d0cd80 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -215,7 +215,7 @@ func TestChannelConditionStatus(t *testing.T) { } else { cs.MarkBackingChannelUnknown("ChannelUnknown", "testing") } - got := cs.GetHappyCondition().Status + got := cs.GetTopLevelCondition().Status if test.wantConditionStatus != got { t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) } @@ -390,7 +390,7 @@ func TestChannelPropagateStatuses(t *testing.T) { t.Run(n, func(t *testing.T) { cs := &ChannelStatus{} cs.PropagateStatuses(tc.channelableStatus) - got := cs.GetHappyCondition().Status + got := cs.GetTopLevelCondition().Status if tc.wantConditionStatus != got { t.Errorf("unexpected readiness: want %v, got %v", tc.wantConditionStatus, got) } diff --git a/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go b/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go index b753a96f7eb..9c3783162b1 100644 --- a/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/subscription_lifecycle.go @@ -22,7 +22,7 @@ import ( // subCondSet is a condition set with Ready as the happy condition and // ReferencesResolved and ChannelReady as the dependent conditions. -var subCondSet = apis.NewLivingConditionSet(SubscriptionConditionReferencesResolved, SubscriptionConditionAddedToChannel, SubscriptionConditionChannelReady) +var SubCondSet = apis.NewLivingConditionSet(SubscriptionConditionReferencesResolved, SubscriptionConditionAddedToChannel, SubscriptionConditionChannelReady) const ( // SubscriptionConditionReady has status True when all subconditions below have been set to True. @@ -41,17 +41,12 @@ const ( // GetCondition returns the condition currently associated with the given type, or nil. func (ss *SubscriptionStatus) GetCondition(t apis.ConditionType) *apis.Condition { - return subCondSet.Manage(ss).GetCondition(t) + return SubCondSet.Manage(ss).GetCondition(t) } // IsReady returns true if the resource is ready overall. func (ss *SubscriptionStatus) IsReady() bool { - return subCondSet.Manage(ss).IsHappy() -} - -// IsUnknown returns true if the resource is unknown overall. -func (ss *SubscriptionStatus) IsUnknown() bool { - return subCondSet.Manage(ss).IsUnknown() + return SubCondSet.Manage(ss).IsHappy() } // IsAddedToChannel returns true if SubscriptionConditionAddedToChannel is true @@ -66,45 +61,45 @@ func (ss *SubscriptionStatus) AreReferencesResolved() bool { // InitializeConditions sets relevant unset conditions to Unknown state. func (ss *SubscriptionStatus) InitializeConditions() { - subCondSet.Manage(ss).InitializeConditions() + SubCondSet.Manage(ss).InitializeConditions() } // MarkReferencesResolved sets the ReferencesResolved condition to True state. func (ss *SubscriptionStatus) MarkReferencesResolved() { - subCondSet.Manage(ss).MarkTrue(SubscriptionConditionReferencesResolved) + SubCondSet.Manage(ss).MarkTrue(SubscriptionConditionReferencesResolved) } // MarkChannelReady sets the ChannelReady condition to True state. func (ss *SubscriptionStatus) MarkChannelReady() { - subCondSet.Manage(ss).MarkTrue(SubscriptionConditionChannelReady) + SubCondSet.Manage(ss).MarkTrue(SubscriptionConditionChannelReady) } // MarkAddedToChannel sets the AddedToChannel condition to True state. func (ss *SubscriptionStatus) MarkAddedToChannel() { - subCondSet.Manage(ss).MarkTrue(SubscriptionConditionAddedToChannel) + SubCondSet.Manage(ss).MarkTrue(SubscriptionConditionAddedToChannel) } // MarkReferencesNotResolved sets the ReferencesResolved condition to False state. func (ss *SubscriptionStatus) MarkReferencesNotResolved(reason, messageFormat string, messageA ...interface{}) { - subCondSet.Manage(ss).MarkFalse(SubscriptionConditionReferencesResolved, reason, messageFormat, messageA...) + SubCondSet.Manage(ss).MarkFalse(SubscriptionConditionReferencesResolved, reason, messageFormat, messageA...) } -// MarkReferencesNotResolved sets the ReferencesResolved condition to Unknown state. +// MarkReferencesResolvedUnknown sets the ReferencesResolved condition to Unknown state. func (ss *SubscriptionStatus) MarkReferencesResolvedUnknown(reason, messageFormat string, messageA ...interface{}) { - subCondSet.Manage(ss).MarkUnknown(SubscriptionConditionReferencesResolved, reason, messageFormat, messageA...) + SubCondSet.Manage(ss).MarkUnknown(SubscriptionConditionReferencesResolved, reason, messageFormat, messageA...) } // MarkChannelFailed sets the ChannelReady condition to False state. func (ss *SubscriptionStatus) MarkChannelFailed(reason, messageFormat string, messageA ...interface{}) { - subCondSet.Manage(ss).MarkFalse(SubscriptionConditionChannelReady, reason, messageFormat, messageA) + SubCondSet.Manage(ss).MarkFalse(SubscriptionConditionChannelReady, reason, messageFormat, messageA) } -// MarkChannelFailed sets the ChannelReady condition to Unknown state. +// MarkChannelUnknown sets the ChannelReady condition to Unknown state. func (ss *SubscriptionStatus) MarkChannelUnknown(reason, messageFormat string, messageA ...interface{}) { - subCondSet.Manage(ss).MarkUnknown(SubscriptionConditionChannelReady, reason, messageFormat, messageA) + SubCondSet.Manage(ss).MarkUnknown(SubscriptionConditionChannelReady, reason, messageFormat, messageA) } // MarkNotAddedToChannel sets the AddedToChannel condition to False state. func (ss *SubscriptionStatus) MarkNotAddedToChannel(reason, messageFormat string, messageA ...interface{}) { - subCondSet.Manage(ss).MarkFalse(SubscriptionConditionAddedToChannel, reason, messageFormat, messageA) + SubCondSet.Manage(ss).MarkFalse(SubscriptionConditionAddedToChannel, reason, messageFormat, messageA) } diff --git a/pkg/reconciler/subscription/subscription.go b/pkg/reconciler/subscription/subscription.go index e963b7ce44e..6803276d685 100644 --- a/pkg/reconciler/subscription/subscription.go +++ b/pkg/reconciler/subscription/subscription.go @@ -278,9 +278,9 @@ func (r *Reconciler) reconcile(ctx context.Context, subscription *v1alpha1.Subsc } ss, err := r.getSubStatusByChannel(subscription, channel) if err != nil { - logging.FromContext(ctx).Warn("Subscription not marked by Channel as Ready.", zap.Error(err)) + logging.FromContext(ctx).Warn("Failed to get subscription status.", zap.Error(err)) r.Recorder.Eventf(subscription, corev1.EventTypeWarning, subscriptionNotMarkedReadyByChannel, err.Error()) - subscription.Status.MarkChannelFailed(subscriptionNotMarkedReadyByChannel, "Subscription not marked by Channel as Ready: %s", err) + subscription.Status.MarkChannelUnknown(subscriptionNotMarkedReadyByChannel, "Failed to get subscription status: %s", err) return err } subStatus := ss.Ready diff --git a/pkg/reconciler/trigger/trigger_test.go b/pkg/reconciler/trigger/trigger_test.go index 90331196eed..e3c20eea931 100644 --- a/pkg/reconciler/trigger/trigger_test.go +++ b/pkg/reconciler/trigger/trigger_test.go @@ -364,7 +364,7 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscriberURI(subscriberURI), // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, - reconciletesting.WithTriggerBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: nil"), + reconciletesting.WithTriggerBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: "), ), }}, }, { @@ -590,7 +590,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -625,7 +625,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -658,7 +658,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -691,7 +691,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberResolvedTargetURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -724,7 +724,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: nil"), + reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), reconciletesting.WithTriggerStatusSubscriberURI(k8sServiceResolvedURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -913,7 +913,7 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), - reconciletesting.WithTriggerDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: nil"), + reconciletesting.WithTriggerDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: "), ), }}, }, @@ -1153,7 +1153,7 @@ func makeIngressSubscriptionNotOwnedByTrigger() *messagingv1alpha1.Subscription func makeDifferentReadySubscription() *messagingv1alpha1.Subscription { s := makeIngressSubscription() s.Spec.Subscriber.URI = apis.HTTP("different.example.com") - s.Status = *v1alpha1.TestHelper.FalseSubscriptionStatus() + s.Status = *v1alpha1.TestHelper.ReadySubscriptionStatus() return s } diff --git a/vendor/knative.dev/pkg/apis/condition_set.go b/vendor/knative.dev/pkg/apis/condition_set.go index 389b5977e07..39ec1d67a0f 100644 --- a/vendor/knative.dev/pkg/apis/condition_set.go +++ b/vendor/knative.dev/pkg/apis/condition_set.go @@ -52,16 +52,8 @@ type ConditionManager interface { // set to true. IsHappy() bool - // IsUnknown looks at the happy condition and returns true if that condition is - // set to unknown or that condition is nil. - IsUnknown() bool - - // IsFalse looks at the happy condition and returns true if that condition is - // set to false. - IsFalse() bool - - // GetCondition finds and returns the Condition that matches the ConditionType happy. - GetHappyCondition() *Condition + // GetTopLevelCondition finds and returns the top level Condition((happy Condition). + GetTopLevelCondition() *Condition // GetCondition finds and returns the Condition that matches the ConditionType // previously set on Conditions. @@ -153,32 +145,11 @@ func (r ConditionSet) Manage(status ConditionsAccessor) ConditionManager { // IsHappy looks at the happy condition and returns true if that condition is // set to true. func (r conditionsImpl) IsHappy() bool { - if c := r.GetCondition(r.happy); c == nil || !c.IsTrue() { - return false - } - return true -} - -// IsUnknown looks at the happy condition and returns true if that condition is -// set to Unknown or that condition is nil. -func (r conditionsImpl) IsUnknown() bool { - if c := r.GetCondition(r.happy); !c.IsUnknown() { - return false - } - return true -} - -// IsFalse looks at the happy condition and returns true if that condition is -// set to False. -func (r conditionsImpl) IsFalse() bool { - if c := r.GetCondition(r.happy); c == nil || !c.IsFalse() { - return false - } - return true + return r.GetTopLevelCondition().IsTrue() } -// GetHappyCondition finds and returns the Condition that matches the ConditionType happy. -func (r conditionsImpl) GetHappyCondition() *Condition { +// GetTopLevelCondition finds and returns the top level Condition(happy Condition). +func (r conditionsImpl) GetTopLevelCondition() *Condition { return r.GetCondition(r.happy) } From 254582622fe0baa20f8c3d894ed891c4c449fd8f Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Mon, 6 Jan 2020 10:59:59 -0800 Subject: [PATCH 3/5] update knative/pkg dependencies --- Gopkg.lock | 7 +- Gopkg.toml | 3 +- vendor/knative.dev/pkg/Gopkg.lock | 16 +++-- vendor/knative.dev/pkg/apis/condition_set.go | 6 +- .../pkg/configmap/manual_watcher.go | 2 +- .../pkg/injection/sharedmain/main.go | 12 ++-- vendor/knative.dev/pkg/kmeta/names.go | 2 +- vendor/knative.dev/pkg/test/README.md | 32 ++++++++++ .../pkg/test/gke/fake/credentials.json | 6 ++ vendor/knative.dev/pkg/test/gke/request.go | 64 ++++++++++++++----- vendor/knative.dev/pkg/test/spoof/spoof.go | 50 ++++++++++----- .../clustermanager/e2e-tests/boskos/boskos.go | 36 +++++++---- .../e2e-tests/boskos/fake/fake.go | 8 +-- .../testutils/clustermanager/e2e-tests/gke.go | 13 ++-- .../clustermanager/perf-tests/pkg/cluster.go | 4 ++ .../prow-cluster-operation/options/options.go | 1 + 16 files changed, 184 insertions(+), 78 deletions(-) create mode 100644 vendor/knative.dev/pkg/test/gke/fake/credentials.json diff --git a/Gopkg.lock b/Gopkg.lock index b067b411c22..f3187742c83 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1253,8 +1253,8 @@ revision = "8d271d903fe4c290aa361acfb242cff7bcee96f1" [[projects]] - branch = "add_isunknown" - digest = "1:34b5c01e5cee902f33f8bca94da7ab470e811a18939e9df9aaf006135e1401b4" + branch = "master" + digest = "1:bf00070eafe8577796327f992e18c3891003ee4ea28e9a2e20281fc0bf1c256f" name = "knative.dev/pkg" packages = [ "apis", @@ -1355,8 +1355,7 @@ "webhook/resourcesemantics/validation", ] pruneopts = "T" - revision = "866e0fd95952c4bf95fa83ea68d1100dd1837cb0" - source = "github.com/capri-xiyue/pkg" + revision = "fa1c639c93a0f5fcd3dbd00848c4e1a8297808b3" [[projects]] branch = "master" diff --git a/Gopkg.toml b/Gopkg.toml index f5c02ff55e2..6f655904087 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -36,8 +36,7 @@ required = [ # Our master branch tracks knative/pkg master or a release. [[override]] name = "knative.dev/pkg" - branch = "add_isunknown" - source = "github.com/capri-xiyue/pkg" + branch = "master" # TODO why is this overridden? [[override]] diff --git a/vendor/knative.dev/pkg/Gopkg.lock b/vendor/knative.dev/pkg/Gopkg.lock index fb8c2a68184..af19cf85b99 100644 --- a/vendor/knative.dev/pkg/Gopkg.lock +++ b/vendor/knative.dev/pkg/Gopkg.lock @@ -721,16 +721,15 @@ revision = "8b927904ee0dec805c89aaf9172f4459296ed6e8" [[projects]] - branch = "master" - digest = "1:48871545d029f0561819b6f761e11ed3111e79f29f7f1ba36e2e428e05ceaa9d" + digest = "1:081608ceb454c46b54d24b7561e5744088f3ff69478b23f50277ec83bd8636b0" name = "google.golang.org/api" packages = [ "container/v1beta1", - "gensupport", "googleapi", - "googleapi/internal/uritemplates", "googleapi/transport", "internal", + "internal/gensupport", + "internal/third_party/uritemplates", "iterator", "option", "storage/v1", @@ -741,7 +740,8 @@ "transport/http/internal/propagation", ] pruneopts = "NUT" - revision = "aa15faf3c8a1cffc77fc3dabe95703bb12c5f6a9" + revision = "aa5d4e47691e7ae1aebb5221ff8e4beea23fad72" + version = "v0.15.0" [[projects]] digest = "1:a955e7c44c2be14b61aa2ddda744edfdfbc6817e993703a16e303c277ba84449" @@ -1243,15 +1243,17 @@ [[projects]] branch = "master" - digest = "1:df1c1cf6d2c9ca0781705ef095cb15e06f777cddb493775aad41b72f79867d7a" + digest = "1:85d1708cf70b4fa56e184b16a5af5350945f3239978529719f4a53cbcd67ca2a" name = "k8s.io/test-infra" packages = [ "boskos/client", "boskos/common", "boskos/storage", + "prow/config/secret", + "prow/logrusutil", ] pruneopts = "NUT" - revision = "356470ae778d26af50f27ff8673434bf5cf73c07" + revision = "70b0b49fe2476f06f97272e77f47ba432cb54244" [[projects]] branch = "master" diff --git a/vendor/knative.dev/pkg/apis/condition_set.go b/vendor/knative.dev/pkg/apis/condition_set.go index 39ec1d67a0f..eba01e94b80 100644 --- a/vendor/knative.dev/pkg/apis/condition_set.go +++ b/vendor/knative.dev/pkg/apis/condition_set.go @@ -52,7 +52,7 @@ type ConditionManager interface { // set to true. IsHappy() bool - // GetTopLevelCondition finds and returns the top level Condition((happy Condition). + // GetTopLevelCondition finds and returns the top level Condition (happy Condition). GetTopLevelCondition() *Condition // GetCondition finds and returns the Condition that matches the ConditionType @@ -142,13 +142,13 @@ func (r ConditionSet) Manage(status ConditionsAccessor) ConditionManager { } } -// IsHappy looks at the happy condition and returns true if that condition is +// IsHappy looks at the top level Condition (happy Condition) and returns true if that condition is // set to true. func (r conditionsImpl) IsHappy() bool { return r.GetTopLevelCondition().IsTrue() } -// GetTopLevelCondition finds and returns the top level Condition(happy Condition). +// GetTopLevelCondition finds and returns the top level Condition (happy Condition). func (r conditionsImpl) GetTopLevelCondition() *Condition { return r.GetCondition(r.happy) } diff --git a/vendor/knative.dev/pkg/configmap/manual_watcher.go b/vendor/knative.dev/pkg/configmap/manual_watcher.go index 4774e5227ff..668714ee65b 100644 --- a/vendor/knative.dev/pkg/configmap/manual_watcher.go +++ b/vendor/knative.dev/pkg/configmap/manual_watcher.go @@ -40,7 +40,7 @@ func (w *ManualWatcher) Watch(name string, o ...Observer) { defer w.m.Unlock() if w.observers == nil { - w.observers = make(map[string][]Observer, 1) + w.observers = make(map[string][]Observer, len(o)) } w.observers[name] = append(w.observers[name], o...) } diff --git a/vendor/knative.dev/pkg/injection/sharedmain/main.go b/vendor/knative.dev/pkg/injection/sharedmain/main.go index bd379d9ab25..881f93e6431 100644 --- a/vendor/knative.dev/pkg/injection/sharedmain/main.go +++ b/vendor/knative.dev/pkg/injection/sharedmain/main.go @@ -106,7 +106,7 @@ func MainWithContext(ctx context.Context, component string, ctors ...injection.C cfg, err := GetConfig(*masterURL, *kubeconfig) if err != nil { - log.Fatal("Error building kubeconfig", err) + log.Fatalf("Error building kubeconfig: %v", err) } MainWithConfig(ctx, component, cfg, ctors...) } @@ -134,7 +134,7 @@ func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, cto // Set up our logger. loggingConfig, err := GetLoggingConfig(ctx) if err != nil { - log.Fatal("Error reading/parsing logging configuration:", err) + log.Fatalf("Error reading/parsing logging configuration: %v", err) } logger, atomicLevel := logging.NewLoggerFromConfig(loggingConfig, component) defer flush(logger) @@ -168,7 +168,7 @@ func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, cto metav1.GetOptions{}); err == nil { cmw.Watch(logging.ConfigMapName(), logging.UpdateLevelFromConfigMap(logger, atomicLevel, component)) } else if !apierrors.IsNotFound(err) { - logger.Fatalw("Error reading ConfigMap: "+logging.ConfigMapName(), zap.Error(err)) + logger.With(zap.Error(err)).Fatalf("Error reading ConfigMap %q", logging.ConfigMapName()) } // Watch the observability config map @@ -178,17 +178,17 @@ func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, cto metrics.UpdateExporterFromConfigMap(component, logger), profilingHandler.UpdateFromConfigMap) } else if !apierrors.IsNotFound(err) { - logger.Fatalw("Error reading ConfigMap: "+metrics.ConfigMapName(), zap.Error(err)) + logger.With(zap.Error(err)).Fatalf("Error reading ConfigMap %q", metrics.ConfigMapName()) } if err := cmw.Start(ctx.Done()); err != nil { - logger.Fatalw("failed to start configuration manager", zap.Error(err)) + logger.Fatalw("Failed to start configuration manager", zap.Error(err)) } // Start all of the informers and wait for them to sync. logger.Info("Starting informers.") if err := controller.StartInformers(ctx.Done(), informers...); err != nil { - logger.Fatalw("Failed to start informers", err) + logger.Fatalw("Failed to start informers", zap.Error(err)) } // Start all of the controllers. diff --git a/vendor/knative.dev/pkg/kmeta/names.go b/vendor/knative.dev/pkg/kmeta/names.go index fcbce72bd2c..c59090b52ba 100644 --- a/vendor/knative.dev/pkg/kmeta/names.go +++ b/vendor/knative.dev/pkg/kmeta/names.go @@ -54,7 +54,7 @@ func ChildName(parent, suffix string) string { if d := longest - len(ret); d > 0 { ret += suffix[:d] } - // If due to trumming above we're terminating the string with a `-`, + // If due to trimming above we're terminating the string with a `-`, // remove it. return strings.TrimRight(ret, "-") } diff --git a/vendor/knative.dev/pkg/test/README.md b/vendor/knative.dev/pkg/test/README.md index 0110c8148f2..d106ffec96c 100644 --- a/vendor/knative.dev/pkg/test/README.md +++ b/vendor/knative.dev/pkg/test/README.md @@ -121,6 +121,10 @@ Tests importing [`knative.dev/pkg/test`](#test-library) recognize these flags: - [`--cluster`](#specifying-cluster) - [`--namespace`](#specifying-namespace) - [`--logverbose`](#output-verbose-logs) +- [`--ingressendpoint`](#specifying-ingress-endpoint) +- [`--dockerrepo`](#specifying-docker-repo) +- [`--tag`](#specifying-tag) +- [`--imagetemplate`](#specifying-image-template) ### Specifying kubeconfig @@ -178,6 +182,34 @@ The `--logverbose` argument lets you see verbose test logs and k8s logs. go test ./test --logverbose ``` +### Specifying docker repo + +The `--dockerrepo` argument lets you specify a uri of the docker repo where you +have uploaded the test image to using `uploadtestimage.sh`. Defaults to +`$KO_DOCKER_REPO` + +```bash +go test ./test --dockerrepo myspecialdockerrepo +``` + +### Specifying tag + +The `--tag` argument lets you specify the version tag for the test images. + +```bash +go test ./test --tag v1.0 +``` + +### Specifying image template + +The `--imagetemplate` argument lets you specify a template to generate the +reference to an image from the test. Defaults to +`{{.Repository}}/{{.Name}}:{{.Tag}}` + +```bash +go test ./test --imagetemplate {{.Repository}}/{{.Name}}:{{.Tag}} +``` + --- Except as otherwise noted, the content of this page is licensed under the diff --git a/vendor/knative.dev/pkg/test/gke/fake/credentials.json b/vendor/knative.dev/pkg/test/gke/fake/credentials.json new file mode 100644 index 00000000000..820c27c5a0e --- /dev/null +++ b/vendor/knative.dev/pkg/test/gke/fake/credentials.json @@ -0,0 +1,6 @@ +{ + "client_id": "123456", + "client_secret": "123456", + "refresh_token": "123456", + "type": "authorized_user" +} diff --git a/vendor/knative.dev/pkg/test/gke/request.go b/vendor/knative.dev/pkg/test/gke/request.go index 9a64afe3319..0cb89fc6ff6 100644 --- a/vendor/knative.dev/pkg/test/gke/request.go +++ b/vendor/knative.dev/pkg/test/gke/request.go @@ -32,6 +32,11 @@ type Request struct { // GKEVersion: GKE version of the cluster, default to be latest if not provided GKEVersion string + // ReleaseChannel: GKE release channel. Only one of GKEVersion or ReleaseChannel can be + // specified at a time. + // https://cloud.google.com/kubernetes-engine/docs/concepts/release-channels + ReleaseChannel string + // ClusterName: name of the cluster ClusterName string @@ -52,20 +57,26 @@ type Request struct { // Addons: cluster addons to be added to cluster, such as istio Addons []string + + // EnableWorkloadIdentity: whether to enable Workload Identity - + // https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity or not + EnableWorkloadIdentity bool } // DeepCopy will make a deepcopy of the request struct. func (r *Request) DeepCopy() *Request { return &Request{ - Project: r.Project, - GKEVersion: r.GKEVersion, - ClusterName: r.ClusterName, - MinNodes: r.MinNodes, - MaxNodes: r.MaxNodes, - NodeType: r.NodeType, - Region: r.Region, - Zone: r.Zone, - Addons: r.Addons, + Project: r.Project, + GKEVersion: r.GKEVersion, + ReleaseChannel: r.ReleaseChannel, + ClusterName: r.ClusterName, + MinNodes: r.MinNodes, + MaxNodes: r.MaxNodes, + NodeType: r.NodeType, + Region: r.Region, + Zone: r.Zone, + Addons: r.Addons, + EnableWorkloadIdentity: r.EnableWorkloadIdentity, } } @@ -83,12 +94,14 @@ func NewCreateClusterRequest(request *Request) (*container.CreateClusterRequest, if request.NodeType == "" { return nil, errors.New("node type cannot be empty") } - - if request.GKEVersion == "" { - request.GKEVersion = defaultGKEVersion + if request.EnableWorkloadIdentity && request.Project == "" { + return nil, errors.New("project cannot be empty if you want Workload Identity") + } + if request.GKEVersion != "" && request.ReleaseChannel != "" { + return nil, errors.New("can only specify one of GKE version or release channel (not both)") } - return &container.CreateClusterRequest{ + ccr := &container.CreateClusterRequest{ Cluster: &container.Cluster{ NodePools: []*container.NodePool{ { @@ -105,9 +118,6 @@ func NewCreateClusterRequest(request *Request) (*container.CreateClusterRequest, }, }, Name: request.ClusterName, - // The default cluster version is not latest, has to explicitly - // set it as "latest" - InitialClusterVersion: request.GKEVersion, // Installing addons after cluster creation takes at least 5 // minutes, so install addons as part of cluster creation, which // doesn't seem to add much time on top of cluster creation @@ -118,5 +128,25 @@ func NewCreateClusterRequest(request *Request) (*container.CreateClusterRequest, // automatically generated by GKE SDK MasterAuth: &container.MasterAuth{Username: "admin"}, }, - }, nil + } + if request.EnableWorkloadIdentity { + // Equivalent to --identity-namespace=[PROJECT_ID].svc.id.goog, then + // we can configure a Kubernetes service account to act as a Google + // service account. + ccr.Cluster.WorkloadIdentityConfig = &container.WorkloadIdentityConfig{ + IdentityNamespace: request.Project + ".svc.id.goog", + } + } + + // Manage the GKE cluster version. Only one of initial cluster version or release channel can be specified. + if request.ReleaseChannel != "" { + ccr.Cluster.ReleaseChannel = &container.ReleaseChannel{Channel: request.ReleaseChannel} + } else if request.GKEVersion != "" { + ccr.Cluster.InitialClusterVersion = request.GKEVersion + } else { + // The default cluster version is not latest, has to explicitly + // set it as "latest" + ccr.Cluster.InitialClusterVersion = defaultGKEVersion + } + return ccr, nil } diff --git a/vendor/knative.dev/pkg/test/spoof/spoof.go b/vendor/knative.dev/pkg/test/spoof/spoof.go index 56798cc72b2..098a8772c24 100644 --- a/vendor/knative.dev/pkg/test/spoof/spoof.go +++ b/vendor/knative.dev/pkg/test/spoof/spoof.go @@ -64,7 +64,7 @@ func (r *Response) String() string { // Interface defines the actions that can be performed by the spoofing client. type Interface interface { Do(*http.Request) (*Response, error) - Poll(*http.Request, ResponseChecker) (*Response, error) + Poll(*http.Request, ResponseChecker, ...ErrorRetryChecker) (*Response, error) } // https://medium.com/stupid-gopher-tricks/ensuring-go-interface-satisfaction-at-compile-time-1ed158e8fa17 @@ -80,6 +80,10 @@ var ( // https://github.com/kubernetes/apimachinery/blob/cf7ae2f57dabc02a3d215f15ca61ae1446f3be8f/pkg/util/wait/wait.go#L172 type ResponseChecker func(resp *Response) (done bool, err error) +// ErrorRetryChecker is used to determine if an error should be retried or not. +// If an error should be retried, it should return true and the wrapped error to explain why to retry. +type ErrorRetryChecker func(e error) (retry bool, err error) + // SpoofingClient is a minimal HTTP client wrapper that spoofs the domain of requests // for non-resolvable domains. type SpoofingClient struct { @@ -199,7 +203,7 @@ func (sc *SpoofingClient) Do(req *http.Request) (*Response, error) { } // Poll executes an http request until it satisfies the inState condition or encounters an error. -func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker) (*Response, error) { +func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker, errorRetryCheckers ...ErrorRetryChecker) (*Response, error) { var ( resp *Response err error @@ -212,22 +216,15 @@ func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker) (*Res req.Header.Add(pollReqHeader, "True") resp, err = sc.Do(req) if err != nil { - if isTCPTimeout(err) { - sc.Logf("Retrying %s for TCP timeout: %v", req.URL, err) - return false, nil - } - // Retrying on DNS error, since we may be using xip.io or nip.io in tests. - if isDNSError(err) { - sc.Logf("Retrying %s for DNS error: %v", req.URL, err) - return false, nil - } - // Repeat the poll on `connection refused` errors, which are usually transient Istio errors. - if isConnectionRefused(err) { - sc.Logf("Retrying %s for connection refused: %v", req.URL, err) - return false, nil + if len(errorRetryCheckers) == 0 { + errorRetryCheckers = []ErrorRetryChecker{DefaultErrorRetryChecker} } - if isConnectionReset(err) { - sc.Logf("Retrying %s for connection reset: %v", req.URL, err) + for _, checker := range errorRetryCheckers { + retry, newErr := checker(err) + if retry { + sc.Logf("Retrying %s: %v", req.URL, newErr) + return false, nil + } } return true, err } @@ -245,6 +242,25 @@ func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker) (*Res return resp, nil } +// DefaultErrorRetryChecker implements the defaults for retrying on error. +func DefaultErrorRetryChecker(err error) (bool, error) { + if isTCPTimeout(err) { + return true, fmt.Errorf("Retrying for TCP timeout: %v", err) + } + // Retrying on DNS error, since we may be using xip.io or nip.io in tests. + if isDNSError(err) { + return true, fmt.Errorf("Retrying for DNS error: %v", err) + } + // Repeat the poll on `connection refused` errors, which are usually transient Istio errors. + if isConnectionRefused(err) { + return true, fmt.Errorf("Retrying for connection refused: %v", err) + } + if isConnectionReset(err) { + return true, fmt.Errorf("Retrying for connection reset: %v", err) + } + return false, err +} + // logZipkinTrace provides support to log Zipkin Trace for param: spoofResponse // We only log Zipkin trace for HTTP server errors i.e for HTTP status codes between 500 to 600 func (sc *SpoofingClient) logZipkinTrace(spoofResp *Response) { diff --git a/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/boskos.go b/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/boskos.go index 4d16601c190..28f309c14c6 100644 --- a/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/boskos.go +++ b/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/boskos.go @@ -37,30 +37,43 @@ var ( defaultWaitDuration = time.Minute * 20 ) +// Operation defines actions for handling GKE resources type Operation interface { - AcquireGKEProject(*string, string) (*boskoscommon.Resource, error) - ReleaseGKEProject(*string, string) error + AcquireGKEProject(string) (*boskoscommon.Resource, error) + ReleaseGKEProject(string) error } +// Client a wrapper around k8s boskos client that implements Operation type Client struct { *boskosclient.Client } -func newClient(host *string) *boskosclient.Client { - if host == nil { - hostName := common.GetOSEnv("JOB_NAME") - host = &hostName +// NewClient creates a boskos Client with GKE operation. The owner of any resources acquired +// by this client is the same as the host name. `user` and `pass` are used for basic +// authentication for boskos client where pass is a password file. `user` and `pass` fields +// are passed directly to k8s boskos client. Refer to +// [k8s boskos](https://github.com/kubernetes/test-infra/tree/master/boskos) for more details. +// If host is "", it looks up JOB_NAME environment variable and set it to be the host name. +func NewClient(host string, user string, pass string) (*Client, error) { + if host == "" { + host = common.GetOSEnv("JOB_NAME") } - return boskosclient.NewClient(*host, boskosURI) + + c, err := boskosclient.NewClient(host, boskosURI, user, pass) + if err != nil { + return nil, err + } + + return &Client{c}, nil } // AcquireGKEProject acquires GKE Boskos Project with "free" state, and not // owned by anyone, sets its state to "busy" and assign it an owner of *host, // which by default is env var `JOB_NAME`. -func (c *Client) AcquireGKEProject(host *string, resType string) (*boskoscommon.Resource, error) { +func (c *Client) AcquireGKEProject(resType string) (*boskoscommon.Resource, error) { ctx, cancel := context.WithTimeout(context.Background(), defaultWaitDuration) defer cancel() - p, err := newClient(host).AcquireWait(ctx, resType, boskoscommon.Free, boskoscommon.Busy) + p, err := c.AcquireWait(ctx, resType, boskoscommon.Free, boskoscommon.Busy) if err != nil { return nil, fmt.Errorf("boskos failed to acquire GKE project: %v", err) } @@ -75,9 +88,8 @@ func (c *Client) AcquireGKEProject(host *string, resType string) (*boskoscommon. // "dirty" for Janitor picking up. // This function is very powerful, it can release Boskos resource acquired by // other processes, regardless of where the other process is running. -func (c *Client) ReleaseGKEProject(host *string, name string) error { - client := newClient(host) - if err := client.Release(name, boskoscommon.Dirty); err != nil { +func (c *Client) ReleaseGKEProject(name string) error { + if err := c.Release(name, boskoscommon.Dirty); err != nil { return fmt.Errorf("boskos failed to release GKE project '%s': %v", name, err) } return nil diff --git a/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/fake/fake.go b/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/fake/fake.go index e060ed1e282..85cab7d8b6a 100644 --- a/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/fake/fake.go +++ b/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/boskos/fake/fake.go @@ -44,11 +44,11 @@ func (c *FakeBoskosClient) GetResources() []*boskoscommon.Resource { } // AcquireGKEProject fakes to be no op -func (c *FakeBoskosClient) AcquireGKEProject(host *string, resType string) (*boskoscommon.Resource, error) { +func (c *FakeBoskosClient) AcquireGKEProject(resType string) (*boskoscommon.Resource, error) { for _, res := range c.resources { if res.State == boskoscommon.Free { res.State = boskoscommon.Busy - res.Owner = c.getOwner(host) + res.Owner = c.getOwner(nil) res.Type = resType return res, nil } @@ -57,8 +57,8 @@ func (c *FakeBoskosClient) AcquireGKEProject(host *string, resType string) (*bos } // ReleaseGKEProject fakes to be no op -func (c *FakeBoskosClient) ReleaseGKEProject(host *string, name string) error { - owner := c.getOwner(host) +func (c *FakeBoskosClient) ReleaseGKEProject(name string) error { + owner := c.getOwner(nil) for _, res := range c.resources { if res.Name == name { if res.Owner == owner { diff --git a/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/gke.go b/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/gke.go index 93b73c33cbf..bede1e6d847 100644 --- a/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/gke.go +++ b/vendor/knative.dev/pkg/testutils/clustermanager/e2e-tests/gke.go @@ -135,11 +135,16 @@ func (gs *GKEClient) Setup(r GKERequest) ClusterOperations { client, err := gke.NewSDKClient() if err != nil { - log.Fatalf("failed to create GKE SDK client: '%v'", err) + log.Fatalf("Failed to create GKE SDK client: '%v'", err) } gc.operations = client - gc.boskosOps = &boskos.Client{} + gc.boskosOps, err = boskos.NewClient("", /* boskos owner */ + "", /* boskos user */ + "" /* boskos password file */) + if err != nil { + log.Fatalf("Failed to create boskos client: '%v", err) + } return gc } @@ -176,7 +181,7 @@ func (gc *GKECluster) Acquire() error { // Get project name from boskos if running in Prow, otherwise it should fail // since we don't know which project to use if common.IsProw() { - project, err := gc.boskosOps.AcquireGKEProject(nil, gc.Request.ResourceType) + project, err := gc.boskosOps.AcquireGKEProject(gc.Request.ResourceType) if err != nil { return fmt.Errorf("failed acquiring boskos project: '%v'", err) } @@ -264,7 +269,7 @@ func (gc *GKECluster) Delete() error { // clusters deleting if common.IsProw() { log.Printf("Releasing Boskos resource: '%v'", gc.Project) - return gc.boskosOps.ReleaseGKEProject(nil, gc.Project) + return gc.boskosOps.ReleaseGKEProject(gc.Project) } // NeedsCleanup is only true if running locally and cluster created by the diff --git a/vendor/knative.dev/pkg/testutils/clustermanager/perf-tests/pkg/cluster.go b/vendor/knative.dev/pkg/testutils/clustermanager/perf-tests/pkg/cluster.go index 65e0ae44f86..0fd59bda04d 100644 --- a/vendor/knative.dev/pkg/testutils/clustermanager/perf-tests/pkg/cluster.go +++ b/vendor/knative.dev/pkg/testutils/clustermanager/perf-tests/pkg/cluster.go @@ -230,11 +230,15 @@ func (gc *gkeClient) createClusterWithRetries(gcpProject, name string, config Cl addons = strings.Split(config.Addons, ",") } req := &gke.Request{ + Project: gcpProject, ClusterName: name, MinNodes: config.NodeCount, MaxNodes: config.NodeCount, NodeType: config.NodeType, Addons: addons, + // Enable Workload Identity for performance tests because we need to use a Kubernetes service account to act + // as a Google cloud service account, which is then used for authentication to the metrics data storage system. + EnableWorkloadIdentity: true, } creq, err := gke.NewCreateClusterRequest(req) if err != nil { diff --git a/vendor/knative.dev/pkg/testutils/clustermanager/prow-cluster-operation/options/options.go b/vendor/knative.dev/pkg/testutils/clustermanager/prow-cluster-operation/options/options.go index 7d0391b5cc8..739828468d3 100644 --- a/vendor/knative.dev/pkg/testutils/clustermanager/prow-cluster-operation/options/options.go +++ b/vendor/knative.dev/pkg/testutils/clustermanager/prow-cluster-operation/options/options.go @@ -55,6 +55,7 @@ func (rw *RequestWrapper) addOptions() { flag.StringVar(&rw.Request.Zone, "zone", "", "GCP zone") flag.StringVar(&rw.Request.Project, "project", "", "GCP project") flag.StringVar(&rw.Request.ClusterName, "name", "", "cluster name") + flag.StringVar(&rw.Request.ReleaseChannel, "release-channel", "", "GKE release channel") flag.StringVar(&rw.Request.ResourceType, "resource-type", "", "Boskos Resource Type") flag.StringVar(&rw.BackupRegionsStr, "backup-regions", "", "GCP regions as backup, separated by comma") flag.StringVar(&rw.AddonsStr, "addons", "", "addons to be added, separated by comma") From ab85ce817d776d8ba59b161b18b25930c19c48be Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Mon, 6 Jan 2020 11:33:48 -0800 Subject: [PATCH 4/5] added unit tests for eventtype_lifeccyle --- .../eventing/v1alpha1/eventtype_lifecycle.go | 5 ++ .../v1alpha1/eventtype_lifecycle_test.go | 64 ++++++++++--------- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go index f625c04a5a4..9d4607613fa 100644 --- a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go @@ -38,6 +38,11 @@ func (et *EventTypeStatus) IsReady() bool { return eventTypeCondSet.Manage(et).IsHappy() } +// GetTopLevelCondition returns the top level Condition. +func (et *EventTypeStatus) GetTopLevelCondition() *apis.Condition { + return eventTypeCondSet.Manage(et).GetTopLevelCondition() +} + // InitializeConditions sets relevant unset conditions to Unknown state. func (et *EventTypeStatus) InitializeConditions() { eventTypeCondSet.Manage(et).InitializeConditions() diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go index 29106828973..2b3bd1c842e 100644 --- a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go +++ b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go @@ -194,33 +194,39 @@ func TestEventTypeInitializeConditions(t *testing.T) { } } -func TestEventTypeIsReady(t *testing.T) { +func TestEventTypeConditionStatus(t *testing.T) { tests := []struct { - name string - markBrokerExists *bool - markBrokerReady *bool - wantReady bool + name string + markBrokerExists *bool + brokerStatus *BrokerStatus + wantConditionStatus corev1.ConditionStatus }{{ - name: "all happy", - markBrokerExists: &trueValue, - markBrokerReady: &trueValue, - wantReady: true, + name: "all happy", + markBrokerExists: &trueValue, + brokerStatus: TestHelper.ReadyBrokerStatus(), + wantConditionStatus: corev1.ConditionTrue, }, { - name: "broker exist sad", - markBrokerExists: &falseValue, - markBrokerReady: &trueValue, - wantReady: false, + name: "broker exist sad", + markBrokerExists: &falseValue, + brokerStatus: nil, + wantConditionStatus: corev1.ConditionFalse, }, { - name: "broker ready sad", - markBrokerExists: &trueValue, - markBrokerReady: &falseValue, - wantReady: false, + name: "broker ready sad", + markBrokerExists: &trueValue, + brokerStatus: TestHelper.FalseBrokerStatus(), + wantConditionStatus: corev1.ConditionFalse, }, { - name: "all sad", - markBrokerExists: &falseValue, - markBrokerReady: &falseValue, - wantReady: false, - }} + name: "broker ready unknown", + markBrokerExists: &trueValue, + brokerStatus: TestHelper.UnknownBrokerStatus(), + wantConditionStatus: corev1.ConditionUnknown, + }, + { + name: "all sad", + markBrokerExists: &falseValue, + brokerStatus: nil, + wantConditionStatus: corev1.ConditionFalse, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { ets := &EventTypeStatus{} @@ -231,17 +237,13 @@ func TestEventTypeIsReady(t *testing.T) { ets.MarkBrokerDoesNotExist() } } - if test.markBrokerReady != nil { - if *test.markBrokerReady { - ets.MarkBrokerReady() - } else { - ets.MarkBrokerFailed("BrokerFalse", "the status of Broker is False") - } + if test.brokerStatus != nil { + ets.PropagateBrokerStatus(test.brokerStatus) } - got := ets.IsReady() - if test.wantReady != got { - t.Errorf("unexpected readiness: want %v, got %v", test.wantReady, got) + got := ets.GetTopLevelCondition().Status + if test.wantConditionStatus != got { + t.Errorf("unexpected readiness: want %v, got %v", test.wantConditionStatus, got) } }) } From 2ec7afef0b909da38893cad8b33267224f8dfd28 Mon Sep 17 00:00:00 2001 From: XiyueYu Date: Mon, 6 Jan 2020 14:35:41 -0800 Subject: [PATCH 5/5] modified the code based on comments --- .../eventing/v1alpha1/eventtype_lifecycle.go | 26 ++++--- .../eventing/v1alpha1/trigger_lifecycle.go | 77 +++++++++++-------- .../messaging/v1alpha1/channel_lifecycle.go | 19 +++-- pkg/reconciler/eventtype/eventtype_test.go | 4 +- pkg/reconciler/testing/trigger.go | 6 ++ pkg/reconciler/trigger/trigger_test.go | 18 ++--- 6 files changed, 91 insertions(+), 59 deletions(-) diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go index 9d4607613fa..2d5d2cd8c05 100644 --- a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" ) @@ -72,22 +73,25 @@ func (et *EventTypeStatus) MarkBrokerUnknown(reason, messageFormat string, messa eventTypeCondSet.Manage(et).MarkUnknown(EventTypeConditionBrokerReady, reason, messageFormat, messageA...) } +func (et *EventTypeStatus) MarkBrokerNotConfigured() { + eventTypeCondSet.Manage(et).MarkUnknown(EventTypeConditionBrokerReady, + "BrokerNotConfigured", "Broker has not yet been reconciled.") +} + func (et *EventTypeStatus) PropagateBrokerStatus(bs *BrokerStatus) { bc := brokerCondSet.Manage(bs).GetTopLevelCondition() if bc == nil { - et.MarkBrokerUnknown("BrokerUnknown", "The condition of Broker is nil") + et.MarkBrokerNotConfigured() return } - if bc.IsTrue() { + switch { + case bc.Status == corev1.ConditionUnknown: + et.MarkBrokerUnknown(bc.Reason, bc.Message) + case bc.Status == corev1.ConditionTrue: eventTypeCondSet.Manage(et).MarkTrue(EventTypeConditionBrokerReady) - } else { - msg := bc.Message - if bc.IsUnknown() { - et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: %s", msg) - } else if bc.IsFalse() { - et.MarkBrokerFailed("BrokerFalse", "The status of Broker is False: %s", msg) - } else { - et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) - } + case bc.Status == corev1.ConditionFalse: + et.MarkBrokerFailed(bc.Reason, bc.Message) + default: + et.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) } } diff --git a/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go index a2f413baea3..68b1a11181f 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/trigger_lifecycle.go @@ -17,6 +17,7 @@ package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" messagingv1alpha1 "knative.dev/eventing/pkg/apis/messaging/v1alpha1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -63,20 +64,19 @@ func (ts *TriggerStatus) InitializeConditions() { func (ts *TriggerStatus) PropagateBrokerStatus(bs *BrokerStatus) { bc := brokerCondSet.Manage(bs).GetTopLevelCondition() if bc == nil { - ts.MarkBrokerUnknown("BrokerUnknown", "The condition of Broker is nil") + ts.MarkBrokerNotConfigured() return } - if bc.IsTrue() { + + switch { + case bc.Status == corev1.ConditionUnknown: + ts.MarkBrokerUnknown(bc.Reason, bc.Message) + case bc.Status == corev1.ConditionTrue: triggerCondSet.Manage(ts).MarkTrue(TriggerConditionBroker) - } else { - msg := bc.Message - if bc.IsUnknown() { - ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: %s", msg) - } else if bc.IsFalse() { - ts.MarkBrokerFailed("BrokerFalse", "The status of Broker is False: %s", msg) - } else { - ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) - } + case bc.Status == corev1.ConditionFalse: + ts.MarkBrokerFailed(bc.Reason, bc.Message) + default: + ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) } } @@ -88,23 +88,27 @@ func (ts *TriggerStatus) MarkBrokerUnknown(reason, messageFormat string, message triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionBroker, reason, messageFormat, messageA...) } +func (ts *TriggerStatus) MarkBrokerNotConfigured() { + triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionBroker, + "BrokerNotConfigured", "Broker has not yet been reconciled.") +} + func (ts *TriggerStatus) PropagateSubscriptionStatus(ss *messagingv1alpha1.SubscriptionStatus) { sc := messagingv1alpha1.SubCondSet.Manage(ss).GetTopLevelCondition() if sc == nil { - ts.MarkSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil") + ts.MarkSubscriptionNotConfigured() return } - if sc.IsTrue() { + + switch { + case sc.Status == corev1.ConditionUnknown: + ts.MarkSubscribedUnknown(sc.Reason, sc.Message) + case sc.Status == corev1.ConditionTrue: triggerCondSet.Manage(ts).MarkTrue(TriggerConditionSubscribed) - } else { - msg := sc.Message - if sc.IsUnknown() { - ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is Unknown: %s", msg) - } else if sc.IsFalse() { - ts.MarkNotSubscribed("SubscriptionFalse", "The status of Subscription is False: %s", msg) - } else { - ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Broker is invalid: %v", sc.Status) - } + case sc.Status == corev1.ConditionFalse: + ts.MarkNotSubscribed(sc.Reason, sc.Message) + default: + ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is invalid: %v", sc.Status) } } @@ -120,6 +124,11 @@ func (ts *TriggerStatus) MarkSubscriptionNotOwned(sub *messagingv1alpha1.Subscri triggerCondSet.Manage(ts).MarkFalse(TriggerConditionSubscribed, "SubscriptionNotOwned", "Subscription %q is not owned by this Trigger.", sub.Name) } +func (ts *TriggerStatus) MarkSubscriptionNotConfigured() { + triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionSubscribed, + "SubscriptionNotConfigured", "Subscription has not yet been reconciled.") +} + func (ts *TriggerStatus) MarkSubscriberResolvedSucceeded() { triggerCondSet.Manage(ts).MarkTrue(TriggerConditionSubscriberResolved) } @@ -144,20 +153,26 @@ func (ts *TriggerStatus) MarkDependencyUnknown(reason, messageFormat string, mes triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionDependency, reason, messageFormat, messageA...) } +func (ts *TriggerStatus) MarkDependencyNotConfigured() { + triggerCondSet.Manage(ts).MarkUnknown(EventTypeConditionBrokerReady, + "DependencyNotConfigured", "Dependency has not yet been reconciled.") +} + func (ts *TriggerStatus) PropagateDependencyStatus(ks *duckv1.KResource) { kc := ks.Status.GetCondition(apis.ConditionReady) if kc == nil { - ts.MarkDependencyUnknown("DependencyUnknown", "The condition of Dependency is nil") + ts.MarkDependencyNotConfigured() return } - if kc.IsTrue() { + + switch { + case kc.Status == corev1.ConditionUnknown: + ts.MarkDependencyUnknown(kc.Reason, kc.Message) + case kc.Status == corev1.ConditionTrue: ts.MarkDependencySucceeded() - } else { - msg := kc.Message - if kc.IsUnknown() { - ts.MarkDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: %s", msg) - } else { - ts.MarkDependencyFailed("DependencyFalse", "The status of Dependency is False: %s", msg) - } + case kc.Status == corev1.ConditionFalse: + ts.MarkDependencyFailed(kc.Reason, kc.Message) + default: + ts.MarkDependencyUnknown("DependencyUnknown", "The status of Dependency is invalid: %v", kc.Status) } } diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index fa32b4888d1..c3b4377833c 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -80,23 +80,30 @@ func (cs *ChannelStatus) MarkBackingChannelUnknown(reason, messageFormat string, chCondSet.Manage(cs).MarkUnknown(ChannelConditionBackingChannelReady, reason, messageFormat, messageA...) } +func (cs *ChannelStatus) MarkBackingChannelNotConfigured() { + chCondSet.Manage(cs).MarkUnknown(ChannelConditionBackingChannelReady, + "BackingChannelNotConfigured", "BackingChannel has not yet been reconciled.") +} + func (cs *ChannelStatus) MarkBackingChannelReady() { chCondSet.Manage(cs).MarkTrue(ChannelConditionBackingChannelReady) } func (cs *ChannelStatus) PropagateStatuses(chs *eventingduck.ChannelableStatus) { // TODO: Once you can get a Ready status from Channelable in a generic way, use it here. - readyCondition := chs.Status.GetCondition(apis.ConditionReady) if readyCondition == nil { - cs.MarkBackingChannelUnknown("readyCondition is: nil", "readyCondition is: nil") + cs.MarkBackingChannelNotConfigured() } else { - if readyCondition.Status == corev1.ConditionTrue { - cs.MarkBackingChannelReady() - } else if readyCondition.Status == corev1.ConditionUnknown { + switch { + case readyCondition.Status == corev1.ConditionUnknown: cs.MarkBackingChannelUnknown(readyCondition.Reason, readyCondition.Message) - } else { + case readyCondition.Status == corev1.ConditionTrue: + cs.MarkBackingChannelReady() + case readyCondition.Status == corev1.ConditionFalse: cs.MarkBackingChannelFailed(readyCondition.Reason, readyCondition.Message) + default: + cs.MarkBackingChannelUnknown("BackingChannelUnknown", "The status of BackingChannel is invalid: %v", readyCondition.Status) } } // Set the address and update the Addressable conditions. diff --git a/pkg/reconciler/eventtype/eventtype_test.go b/pkg/reconciler/eventtype/eventtype_test.go index 815e59eab92..43f6c5b64ff 100644 --- a/pkg/reconciler/eventtype/eventtype_test.go +++ b/pkg/reconciler/eventtype/eventtype_test.go @@ -125,7 +125,7 @@ func TestReconcile(t *testing.T) { WithEventTypeSource(eventTypeSource), WithEventTypeBroker(eventTypeBroker), WithEventTypeBrokerExists, - WithEventTypeBrokerFailed("BrokerFalse", "The status of Broker is False: inducing failure for create deployments"), + WithEventTypeBrokerFailed("DeploymentFailure", "inducing failure for create deployments"), ), }}, }, @@ -148,7 +148,7 @@ func TestReconcile(t *testing.T) { WithEventTypeSource(eventTypeSource), WithEventTypeBroker(eventTypeBroker), WithEventTypeBrokerExists, - WithEventTypeBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: "), + WithEventTypeBrokerUnknown("", ""), ), }}, }, diff --git a/pkg/reconciler/testing/trigger.go b/pkg/reconciler/testing/trigger.go index 317a878b338..08ffcede014 100644 --- a/pkg/reconciler/testing/trigger.go +++ b/pkg/reconciler/testing/trigger.go @@ -141,6 +141,12 @@ func WithTriggerSubscribedUnknown(reason, message string) TriggerOption { } } +func WithTriggerSubscriptionNotConfigured() TriggerOption { + return func(t *v1alpha1.Trigger) { + t.Status.MarkSubscriptionNotConfigured() + } +} + func WithTriggerSubscribed() TriggerOption { return func(t *v1alpha1.Trigger) { t.Status.PropagateSubscriptionStatus(v1alpha1.TestHelper.ReadySubscriptionStatus()) diff --git a/pkg/reconciler/trigger/trigger_test.go b/pkg/reconciler/trigger/trigger_test.go index 411cb4f7d8c..2b17e5cac6e 100644 --- a/pkg/reconciler/trigger/trigger_test.go +++ b/pkg/reconciler/trigger/trigger_test.go @@ -365,7 +365,7 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscriberURI(subscriberURI), // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, - reconciletesting.WithTriggerBrokerUnknown("BrokerUnknown", "The status of Broker is Unknown: "), + reconciletesting.WithTriggerBrokerUnknown("", ""), ), }}, }, { @@ -594,7 +594,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), + reconciletesting.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -629,7 +629,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), + reconciletesting.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -662,7 +662,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), + reconciletesting.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -695,7 +695,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), + reconciletesting.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberResolvedTargetURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -728,7 +728,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerSubscribedUnknown("SubscriptionUnknown", "The condition of Subscription is nil"), + reconciletesting.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(k8sServiceResolvedURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -787,7 +787,7 @@ func TestAllCases(t *testing.T) { // The first reconciliation will initialize the status conditions. reconciletesting.WithInitTriggerConditions, reconciletesting.WithTriggerBrokerReady(), - reconciletesting.WithTriggerNotSubscribed("SubscriptionFalse", "The status of Subscription is False: test induced [error]"), + reconciletesting.WithTriggerNotSubscribed("testInducedError", "test induced [error]"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -885,7 +885,7 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), - reconciletesting.WithTriggerDependencyFailed("DependencyFalse", "The status of Dependency is False: "), + reconciletesting.WithTriggerDependencyFailed("NotFound", ""), ), }}, }, { @@ -917,7 +917,7 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), - reconciletesting.WithTriggerDependencyUnknown("DependencyUnknown", "The status of Dependency is Unknown: "), + reconciletesting.WithTriggerDependencyUnknown("", ""), ), }}, },