From f2206682496054cce17fb7da76df05a659d43d3f Mon Sep 17 00:00:00 2001
From: Ville Aikas <11279988+vaikas@users.noreply.github.com>
Date: Mon, 1 Mar 2021 16:46:49 -0800
Subject: [PATCH] add more coverage (#4999)

---
 .../mtbroker/trigger/controller_test.go       | 36 +++++++++
 pkg/reconciler/mtbroker/trigger/trigger.go    |  2 +-
 .../mtbroker/trigger/trigger_test.go          | 75 +++++++++++++++++++
 3 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/pkg/reconciler/mtbroker/trigger/controller_test.go b/pkg/reconciler/mtbroker/trigger/controller_test.go
index 14a8055299c..f5ad07bc11d 100644
--- 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
index 7d196a30a09..609b2a04c68 100644
--- 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
index edc49e3bc1c..6b5e16982ae 100644
--- 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,