add more coverage (#4999)

knative · Mar 2, 2021 · f220668 · f220668
1 parent fba3dbf
commit f220668
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 1 deletion.
diff --git a/pkg/reconciler/mtbroker/trigger/controller_test.go b/pkg/reconciler/mtbroker/trigger/controller_test.go
@@ -17,8 +17,13 @@ limitations under the License.
 package mttrigger
 
 import (
+	"fmt"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/labels"
+	v1 "knative.dev/eventing/pkg/apis/eventing/v1"
+	v1lister "knative.dev/eventing/pkg/client/listers/eventing/v1"
+
 	"k8s.io/apimachinery/pkg/runtime"
 
 	testingv1 "knative.dev/eventing/pkg/reconciler/testing/v1"
@@ -83,3 +88,34 @@ func TestGetTriggersForBroker(t *testing.T) {
 		})
 	}
 }
+
+type TriggerListerFailer struct{}
+
+func (failer *TriggerListerFailer) List(selector labels.Selector) (ret []*v1.Trigger, err error) {
+	return nil, nil
+}
+
+func (failer *TriggerListerFailer) Triggers(namespace string) v1lister.TriggerNamespaceLister {
+	return &TriggerNamespaceListerFailer{}
+}
+
+type TriggerNamespaceListerFailer struct{}
+
+// List lists all Triggers in the indexer.
+// Objects returned here must be treated as read-only.
+func (failer *TriggerNamespaceListerFailer) List(selector labels.Selector) (ret []*v1.Trigger, err error) {
+	return nil, fmt.Errorf("Inducing test failure for List")
+}
+
+// Triggers returns an object that can list and get Triggers.
+func (failer *TriggerNamespaceListerFailer) Get(name string) (*v1.Trigger, error) {
+	return nil, nil
+}
+
+func TestListFailure(t *testing.T) {
+	logger := logtesting.TestLogger(t)
+	triggerListerFailer := &TriggerListerFailer{}
+	if len(getTriggersForBroker(logger, triggerListerFailer, ReadyBroker())) != 0 {
+		t.Fatalf("Got back triggers when not expecting any")
+	}
+}
diff --git a/pkg/reconciler/mtbroker/trigger/trigger.go b/pkg/reconciler/mtbroker/trigger/trigger.go
@@ -168,7 +168,7 @@ func (r *Reconciler) subscribeToBrokerChannel(ctx context.Context, b *eventingv1
 	sub, err := r.subscriptionLister.Subscriptions(t.Namespace).Get(expected.Name)
 	// If the resource doesn't exist, we'll create it.
 	if apierrs.IsNotFound(err) {
-		logging.FromContext(ctx).Info("Creating subscription")
+		logging.FromContext(ctx).Infow("Creating subscription", zap.Error(err))
 		sub, err = r.eventingClientSet.MessagingV1().Subscriptions(t.Namespace).Create(ctx, expected, metav1.CreateOptions{})
 		if err != nil {
 			return nil, err

diff --git a/pkg/reconciler/mtbroker/trigger/trigger_test.go b/pkg/reconciler/mtbroker/trigger/trigger_test.go
@@ -363,6 +363,81 @@ func TestReconcile(t *testing.T) {
 			WantCreates: []runtime.Object{
 				makeFilterSubscription(),
 			},
+		}, {
+			Name: "Trigger subscription update (delete) fails",
+			Key:  testKey,
+			Objects: allBrokerObjectsReadyPlus([]runtime.Object{
+				NewTrigger(triggerName, testNS, brokerName,
+					WithTriggerUID(triggerUID),
+					WithTriggerSubscriberURI(subscriberURI)),
+				makeDifferentReadySubscription()}...),
+			WithReactors: []clientgotesting.ReactionFunc{
+				InduceFailure("delete", "subscriptions"),
+			},
+			WantErr: true,
+			WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+				Object: NewTrigger(triggerName, testNS, brokerName,
+					WithTriggerUID(triggerUID),
+					WithTriggerSubscriberURI(subscriberURI),
+					WithInitTriggerConditions,
+					WithTriggerBrokerReady(),
+					// The first reconciliation will initialize the status conditions.
+					WithInitTriggerConditions,
+					WithTriggerBrokerReady(),
+					WithTriggerSubscriberResolvedSucceeded(),
+					WithTriggerStatusSubscriberURI(subscriberURI),
+					WithTriggerNotSubscribed("NotSubscribed", "inducing failure for delete subscriptions")),
+			}},
+			WantDeletes: []clientgotesting.DeleteActionImpl{{
+				ActionImpl: clientgotesting.ActionImpl{
+					Namespace: testNS,
+					Resource:  v1.SchemeGroupVersion.WithResource("subscriptions"),
+				},
+				Name: subscriptionName,
+			}},
+			WantEvents: []string{
+				Eventf(corev1.EventTypeWarning, "SubscriptionDeleteFailed", `Delete Trigger's subscription failed: inducing failure for delete subscriptions`),
+				Eventf(corev1.EventTypeWarning, "InternalError", "inducing failure for delete subscriptions"),
+			},
+		}, {
+			Name: "Trigger subscription create after delete fails",
+			Key:  testKey,
+			Objects: allBrokerObjectsReadyPlus([]runtime.Object{
+				NewTrigger(triggerName, testNS, brokerName,
+					WithTriggerUID(triggerUID),
+					WithTriggerSubscriberURI(subscriberURI)),
+				makeDifferentReadySubscription()}...),
+			WithReactors: []clientgotesting.ReactionFunc{
+				InduceFailure("create", "subscriptions"),
+			},
+			WantErr: true,
+			WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+				Object: NewTrigger(triggerName, testNS, brokerName,
+					WithTriggerUID(triggerUID),
+					WithTriggerSubscriberURI(subscriberURI),
+					WithInitTriggerConditions,
+					WithTriggerBrokerReady(),
+					// The first reconciliation will initialize the status conditions.
+					WithInitTriggerConditions,
+					WithTriggerBrokerReady(),
+					WithTriggerSubscriberResolvedSucceeded(),
+					WithTriggerStatusSubscriberURI(subscriberURI),
+					WithTriggerNotSubscribed("NotSubscribed", "inducing failure for create subscriptions")),
+			}},
+			WantEvents: []string{
+				Eventf(corev1.EventTypeWarning, "SubscriptionCreateFailed", `Create Trigger's subscription failed: inducing failure for create subscriptions`),
+				Eventf(corev1.EventTypeWarning, "InternalError", "inducing failure for create subscriptions"),
+			},
+			WantDeletes: []clientgotesting.DeleteActionImpl{{
+				ActionImpl: clientgotesting.ActionImpl{
+					Namespace: testNS,
+					Resource:  v1.SchemeGroupVersion.WithResource("subscriptions"),
+				},
+				Name: subscriptionName,
+			}},
+			WantCreates: []runtime.Object{
+				makeFilterSubscription(),
+			},
 		}, {
 			Name: "Trigger has subscriber ref exists",
 			Key:  testKey,