diff --git a/Gopkg.lock b/Gopkg.lock index 7ee86a20bd0..85862cf05a9 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1254,7 +1254,7 @@ [[projects]] branch = "master" - digest = "1:39b9c37fd5c6e531b1a2e740ab2ed2fbe237b3f142255fbd8ebae905a957d942" + digest = "1:bf00070eafe8577796327f992e18c3891003ee4ea28e9a2e20281fc0bf1c256f" name = "knative.dev/pkg" packages = [ "apis", @@ -1355,7 +1355,7 @@ "webhook/resourcesemantics/validation", ] pruneopts = "T" - revision = "169ef0797c1f7e548574e6aee289002c0f22dc0a" + revision = "fa1c639c93a0f5fcd3dbd00848c4e1a8297808b3" [[projects]] branch = "master" diff --git a/pkg/apis/eventing/v1alpha1/broker_lifecycle.go b/pkg/apis/eventing/v1alpha1/broker_lifecycle.go index 0b564ca1a93..3737d79967a 100644 --- a/pkg/apis/eventing/v1alpha1/broker_lifecycle.go +++ b/pkg/apis/eventing/v1alpha1/broker_lifecycle.go @@ -47,6 +47,11 @@ 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() diff --git a/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle.go index 6783fb36811..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" ) @@ -38,6 +39,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() @@ -51,10 +57,41 @@ 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) 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.MarkBrokerNotConfigured() + return + } + switch { + case bc.Status == corev1.ConditionUnknown: + et.MarkBrokerUnknown(bc.Reason, bc.Message) + case bc.Status == corev1.ConditionTrue: + eventTypeCondSet.Manage(et).MarkTrue(EventTypeConditionBrokerReady) + 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/eventtype_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/eventtype_lifecycle_test.go index 31d29983df3..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.MarkBrokerNotReady() - } + 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) } }) } diff --git a/pkg/apis/eventing/v1alpha1/test_helper.go b/pkg/apis/eventing/v1alpha1/test_helper.go index 582480e83e4..3324427c1f1 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/legacysources/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,8 +76,18 @@ func (t testHelper) ReadyBrokerStatus() *BrokerStatus { return bs } -func (t testHelper) NotReadyBrokerStatus() *BrokerStatus { +func (t testHelper) UnknownBrokerStatus() *BrokerStatus { bs := &BrokerStatus{} + bs.InitializeConditions() + 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 } @@ -112,3 +123,9 @@ func (t testHelper) AvailableDeployment() *v1.Deployment { } return d } + +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 093f5894126..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" @@ -45,6 +46,11 @@ func (ts *TriggerStatus) GetCondition(t apis.ConditionType) *apis.Condition { return triggerCondSet.Manage(ts).GetCondition(t) } +// 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. func (ts *TriggerStatus) IsReady() bool { return triggerCondSet.Manage(ts).IsHappy() @@ -56,14 +62,21 @@ func (ts *TriggerStatus) InitializeConditions() { } func (ts *TriggerStatus) PropagateBrokerStatus(bs *BrokerStatus) { - if bs.IsReady() { + bc := brokerCondSet.Manage(bs).GetTopLevelCondition() + if bc == nil { + ts.MarkBrokerNotConfigured() + return + } + + switch { + case bc.Status == corev1.ConditionUnknown: + ts.MarkBrokerUnknown(bc.Reason, bc.Message) + case bc.Status == corev1.ConditionTrue: triggerCondSet.Manage(ts).MarkTrue(TriggerConditionBroker) - } else { - msg := "nil" - if bc := brokerCondSet.Manage(bs).GetCondition(BrokerConditionReady); bc != nil { - msg = bc.Message - } - ts.MarkBrokerFailed("BrokerNotReady", "Broker is not ready: %s", msg) + case bc.Status == corev1.ConditionFalse: + ts.MarkBrokerFailed(bc.Reason, bc.Message) + default: + ts.MarkBrokerUnknown("BrokerUnknown", "The status of Broker is invalid: %v", bc.Status) } } @@ -71,15 +84,31 @@ 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) MarkBrokerNotConfigured() { + triggerCondSet.Manage(ts).MarkUnknown(TriggerConditionBroker, + "BrokerNotConfigured", "Broker has not yet been reconciled.") +} + func (ts *TriggerStatus) PropagateSubscriptionStatus(ss *messagingv1alpha1.SubscriptionStatus) { - if ss.IsReady() { + sc := messagingv1alpha1.SubCondSet.Manage(ss).GetTopLevelCondition() + if sc == nil { + ts.MarkSubscriptionNotConfigured() + return + } + + switch { + case sc.Status == corev1.ConditionUnknown: + ts.MarkSubscribedUnknown(sc.Reason, sc.Message) + case sc.Status == corev1.ConditionTrue: triggerCondSet.Manage(ts).MarkTrue(TriggerConditionSubscribed) - } else { - msg := "nil" - if sc := ss.Status.GetCondition(messagingv1alpha1.SubscriptionConditionReady); sc != nil { - msg = sc.Message - } - ts.MarkNotSubscribed("SubscriptionNotReady", "Subscription is not ready: %s", msg) + case sc.Status == corev1.ConditionFalse: + ts.MarkNotSubscribed(sc.Reason, sc.Message) + default: + ts.MarkSubscribedUnknown("SubscriptionUnknown", "The status of Subscription is invalid: %v", sc.Status) } } @@ -87,10 +116,19 @@ 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) } +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) } @@ -115,15 +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 && kc.IsTrue() { + if kc == nil { + ts.MarkDependencyNotConfigured() + return + } + + switch { + case kc.Status == corev1.ConditionUnknown: + ts.MarkDependencyUnknown(kc.Reason, kc.Message) + case kc.Status == corev1.ConditionTrue: ts.MarkDependencySucceeded() - } else { - msg := "nil" - if kc != nil { - msg = kc.Message - } - ts.MarkDependencyFailed("DependencyNotReady", "Dependency is not ready: %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/eventing/v1alpha1/trigger_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go index 6bcb567c0f8..3f056780d5a 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.GetTopLevelCondition().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/legacysources/v1alpha1/apiserver_lifecycle.go b/pkg/apis/legacysources/v1alpha1/apiserver_lifecycle.go index e0103ada2c7..d18d72eba9e 100644 --- a/pkg/apis/legacysources/v1alpha1/apiserver_lifecycle.go +++ b/pkg/apis/legacysources/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/legacysources/v1alpha1/apiserver_lifecycle_test.go b/pkg/apis/legacysources/v1alpha1/apiserver_lifecycle_test.go index 627eaf30070..5e93250da9e 100644 --- a/pkg/apis/legacysources/v1alpha1/apiserver_lifecycle_test.go +++ b/pkg/apis/legacysources/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/legacysources/v1alpha1/containersource_lifecycle.go b/pkg/apis/legacysources/v1alpha1/containersource_lifecycle.go index dfe8670c9fe..efe7d7ef499 100644 --- a/pkg/apis/legacysources/v1alpha1/containersource_lifecycle.go +++ b/pkg/apis/legacysources/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/legacysources/v1alpha1/containersource_lifecycle_test.go b/pkg/apis/legacysources/v1alpha1/containersource_lifecycle_test.go index c943c759463..6c43667dd13 100644 --- a/pkg/apis/legacysources/v1alpha1/containersource_lifecycle_test.go +++ b/pkg/apis/legacysources/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/legacysources/v1alpha1/cron_job_lifecycle.go b/pkg/apis/legacysources/v1alpha1/cron_job_lifecycle.go index 67434fa0747..319c738cc48 100644 --- a/pkg/apis/legacysources/v1alpha1/cron_job_lifecycle.go +++ b/pkg/apis/legacysources/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/legacysources/v1alpha1/cron_job_lifecycle_test.go b/pkg/apis/legacysources/v1alpha1/cron_job_lifecycle_test.go index 39ed4d17678..358d85c202e 100644 --- a/pkg/apis/legacysources/v1alpha1/cron_job_lifecycle_test.go +++ b/pkg/apis/legacysources/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/apis/messaging/v1alpha1/channel_lifecycle.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go index 1d97922ea12..c3b4377833c 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle.go @@ -42,6 +42,11 @@ func (cs *ChannelStatus) GetCondition(t apis.ConditionType) *apis.Condition { return chCondSet.Manage(cs).GetCondition(t) } +// 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. func (cs *ChannelStatus) IsReady() bool { return chCondSet.Manage(cs).IsHappy() @@ -71,6 +76,15 @@ 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) MarkBackingChannelNotConfigured() { + chCondSet.Manage(cs).MarkUnknown(ChannelConditionBackingChannelReady, + "BackingChannelNotConfigured", "BackingChannel has not yet been reconciled.") +} + func (cs *ChannelStatus) MarkBackingChannelReady() { chCondSet.Manage(cs).MarkTrue(ChannelConditionBackingChannelReady) } @@ -78,11 +92,18 @@ func (cs *ChannelStatus) MarkBackingChannelReady() { 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.MarkBackingChannelNotConfigured() + } else { + switch { + case readyCondition.Status == corev1.ConditionUnknown: + cs.MarkBackingChannelUnknown(readyCondition.Reason, readyCondition.Message) + 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/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index 6bad722a94d..e20f6d0cd80 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.GetTopLevelCondition().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.GetTopLevelCondition().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..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,12 +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() + return SubCondSet.Manage(ss).IsHappy() } // IsAddedToChannel returns true if SubscriptionConditionAddedToChannel is true @@ -61,35 +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...) } -// MarkChannelNotReady sets the ChannelReady condition to False state. -func (ss *SubscriptionStatus) MarkChannelNotReady(reason, messageFormat string, messageA ...interface{}) { - subCondSet.Manage(ss).MarkFalse(SubscriptionConditionChannelReady, reason, messageFormat, messageA) +// 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...) +} + +// 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) +} + +// 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) } // 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/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..43f6c5b64ff 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("DeploymentFailure", "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("", ""), ), }}, }, diff --git a/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go b/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go index bfd982f3586..886ca0368cc 100644 --- a/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go +++ b/pkg/reconciler/inmemorychannel/controller/inmemorychannel.go @@ -157,7 +157,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 } @@ -172,7 +172,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 } @@ -187,7 +187,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 } @@ -204,7 +204,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() @@ -229,22 +228,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 3fdb2050152..868c55fcc11 100644 --- a/pkg/reconciler/inmemorychannel/controller/inmemorychannel_test.go +++ b/pkg/reconciler/inmemorychannel/controller/inmemorychannel_test.go @@ -112,11 +112,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, @@ -322,6 +372,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 fff414d40e5..e9e6356ab14 100644 --- a/pkg/reconciler/subscription/subscription.go +++ b/pkg/reconciler/subscription/subscription.go @@ -155,7 +155,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) @@ -175,7 +175,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 { @@ -277,36 +277,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 { - logging.FromContext(ctx).Warn("Subscription not marked by Channel as Ready.", zap.Error(err)) + ss, err := r.getSubStatusByChannel(subscription, channel) + if err != nil { + logging.FromContext(ctx).Warn("Failed to get subscription status.", 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.MarkChannelUnknown(subscriptionNotMarkedReadyByChannel, "Failed to get subscription status: %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 eb3d004755e..7d441f068b1 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 5105906e8b9..7762ad5a7ab 100644 --- a/pkg/reconciler/testing/inmemorychannel.go +++ b/pkg/reconciler/testing/inmemorychannel.go @@ -74,12 +74,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 a238ac23269..d6cb030bbb3 100644 --- a/pkg/reconciler/testing/subscription.go +++ b/pkg/reconciler/testing/subscription.go @@ -173,6 +173,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 77fe6d46601..08ffcede014 100644 --- a/pkg/reconciler/testing/trigger.go +++ b/pkg/reconciler/testing/trigger.go @@ -122,12 +122,31 @@ 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 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.go b/pkg/reconciler/trigger/trigger.go index 51949762e1b..ae32c05d5e6 100644 --- a/pkg/reconciler/trigger/trigger.go +++ b/pkg/reconciler/trigger/trigger.go @@ -175,7 +175,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 } @@ -264,7 +264,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 f48d1e99097..2b17e5cac6e 100644 --- a/pkg/reconciler/trigger/trigger_test.go +++ b/pkg/reconciler/trigger/trigger_test.go @@ -296,7 +296,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, @@ -320,7 +320,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, @@ -345,6 +345,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("", ""), + ), + }}, }, { Name: "Trigger being deleted", Key: triggerKey, @@ -571,7 +594,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.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -606,7 +629,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.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -639,7 +662,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.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -672,7 +695,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.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberResolvedTargetURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -705,7 +728,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.WithTriggerSubscriptionNotConfigured(), reconciletesting.WithTriggerStatusSubscriberURI(k8sServiceResolvedURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -746,7 +769,7 @@ func TestAllCases(t *testing.T) { Objects: []runtime.Object{ makeReadyBroker(), makeBrokerFilterService(), - makeNotReadySubscription(), + makeFalseStatusSubscription(), reconciletesting.NewTrigger(triggerName, testNS, brokerName, reconciletesting.WithTriggerUID(triggerUID), reconciletesting.WithTriggerSubscriberURI(subscriberURI), @@ -764,7 +787,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("testInducedError", "test induced [error]"), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), reconciletesting.WithTriggerDependencyReady(), @@ -830,17 +853,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), @@ -862,10 +885,43 @@ func TestAllCases(t *testing.T) { reconciletesting.WithTriggerSubscribed(), reconciletesting.WithTriggerStatusSubscriberURI(subscriberURI), reconciletesting.WithTriggerSubscriberResolvedSucceeded(), - reconciletesting.WithTriggerDependencyFailed("DependencyNotReady", "Dependency is not ready: "), + reconciletesting.WithTriggerDependencyFailed("NotFound", ""), ), }}, }, { + 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("", ""), + ), + }}, + }, + { Name: "Dependency generation not equal", Key: triggerKey, Objects: []runtime.Object{ @@ -1014,6 +1070,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" @@ -1105,18 +1167,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..eba01e94b80 100644 --- a/vendor/knative.dev/pkg/apis/condition_set.go +++ b/vendor/knative.dev/pkg/apis/condition_set.go @@ -52,6 +52,9 @@ type ConditionManager interface { // set to true. IsHappy() bool + // 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. GetCondition(t ConditionType) *Condition @@ -139,13 +142,15 @@ 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 { - if c := r.GetCondition(r.happy); c == nil || !c.IsTrue() { - return false - } - return true + return r.GetTopLevelCondition().IsTrue() +} + +// GetTopLevelCondition finds and returns the top level Condition (happy Condition). +func (r conditionsImpl) GetTopLevelCondition() *Condition { + return r.GetCondition(r.happy) } // GetCondition finds and returns the Condition that matches the ConditionType 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.