test trigger delivery specs fully (#5268)

* test trigger delivery specs fully * drop test helper * we do not check timing yet
knative · Apr 19, 2021 · cd7c750 · cd7c750
1 parent 0452204
commit cd7c750
Show file tree

Hide file tree

Showing 2 changed files with 109 additions and 43 deletions.
diff --git a/test/rekt/features/broker/control_plane.go b/test/rekt/features/broker/control_plane.go
@@ -35,6 +35,8 @@ import (
 	brokerresources "knative.dev/eventing/test/rekt/resources/broker"
 	"knative.dev/eventing/test/rekt/resources/delivery"
 	triggerresources "knative.dev/eventing/test/rekt/resources/trigger"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+	"knative.dev/pkg/ptr"
 	"knative.dev/reconciler-test/pkg/environment"
 	"knative.dev/reconciler-test/pkg/eventshub"
 	"knative.dev/reconciler-test/pkg/feature"
@@ -53,7 +55,7 @@ func ControlPlaneConformance(brokerName string) *feature.FeatureSet {
 			*ControlPlaneTrigger_WithBrokerLifecycle(),
 			*ControlPlaneTrigger_WithValidFilters(brokerName),
 			*ControlPlaneTrigger_WithInvalidFilters(brokerName),
-			*ControlPlaneDelivery(brokerName),
+			*ControlPlaneDelivery(),
 		},
 	}
 	// TODO: This is not a control plane test, or at best it is a blend with data plane.
@@ -282,11 +284,9 @@ func ControlPlaneTrigger_WithInvalidFilters(brokerName string) *feature.Feature
 	return f
 }
 
-func ControlPlaneDelivery(brokerName string) *feature.Feature {
+func ControlPlaneDelivery() *feature.Feature {
 	f := feature.NewFeatureNamed("Delivery Spec")
 
-	f.Setup("Set Broker Name", setBrokerName(brokerName))
-
 	for i, tt := range []struct {
 		name     string
 		brokerDS *v1.DeliverySpec
@@ -300,35 +300,34 @@ func ControlPlaneDelivery(brokerName string) *feature.Feature {
 		t2FailCount uint
 	}{{
 		name: "When `BrokerSpec.Delivery` and `TriggerSpec.Delivery` are both not configured, no delivery spec SHOULD be used.",
-		// TODO: save these for a followup, just trigger spec seems to be having issues. Might be a bug in eventing?
-		//}, {
-		//	name: "When `BrokerSpec.Delivery` is configured, but not the specific `TriggerSpec.Delivery`, then the `BrokerSpec.Delivery` SHOULD be used. (Retry)",
-		//	brokerDS: &v1.DeliverySpec{
-		//		DeadLetterSink: new(duckv1.Destination),
-		//		Retry:          ptr.Int32(3),
-		//	},
-		//	t1FailCount: 3, // Should get event.
-		//	t2FailCount: 4, // Should end up in DLQ.
-		//}, {
-		//	name: "When `TriggerSpec.Delivery` is configured, then `TriggerSpec.Delivery` SHOULD be used. (Retry)",
-		//	t1DS: &v1.DeliverySpec{
-		//		DeadLetterSink: new(duckv1.Destination),
-		//		Retry:          ptr.Int32(3),
-		//	},
-		//	t1FailCount: 3, // Should get event.
-		//	t2FailCount: 1, // Should be dropped.
-		//}, {
-		//	name: "When both `BrokerSpec.Delivery` and `TriggerSpec.Delivery` is configured, then `TriggerSpec.Delivery` SHOULD be used. (Retry)",
-		//	brokerDS: &v1.DeliverySpec{
-		//		DeadLetterSink: new(duckv1.Destination),
-		//		Retry:          ptr.Int32(1),
-		//	},
-		//	t1DS: &v1.DeliverySpec{
-		//		DeadLetterSink: new(duckv1.Destination),
-		//		Retry:          ptr.Int32(3),
-		//	},
-		//	t1FailCount: 3, // Should get event.
-		//	t2FailCount: 2, // Should end up in DLQ.
+	}, {
+		name: "When `BrokerSpec.Delivery` is configured, but not the specific `TriggerSpec.Delivery`, then the `BrokerSpec.Delivery` SHOULD be used. (Retry)",
+		brokerDS: &v1.DeliverySpec{
+			DeadLetterSink: new(duckv1.Destination),
+			Retry:          ptr.Int32(3),
+		},
+		t1FailCount: 3, // Should get event.
+		t2FailCount: 4, // Should end up in DLQ.
+	}, {
+		name: "When `TriggerSpec.Delivery` is configured, then `TriggerSpec.Delivery` SHOULD be used. (Retry)",
+		t1DS: &v1.DeliverySpec{
+			DeadLetterSink: new(duckv1.Destination),
+			Retry:          ptr.Int32(3),
+		},
+		t1FailCount: 3, // Should get event.
+		t2FailCount: 1, // Should be dropped.
+	}, {
+		name: "When both `BrokerSpec.Delivery` and `TriggerSpec.Delivery` is configured, then `TriggerSpec.Delivery` SHOULD be used. (Retry)",
+		brokerDS: &v1.DeliverySpec{
+			DeadLetterSink: new(duckv1.Destination),
+			Retry:          ptr.Int32(1),
+		},
+		t1DS: &v1.DeliverySpec{
+			DeadLetterSink: new(duckv1.Destination),
+			Retry:          ptr.Int32(3),
+		},
+		t1FailCount: 3, // Should get event.
+		t2FailCount: 2, // Should end up in DLQ.
 	}} {
 		brokerName := fmt.Sprintf("dlq-test-%d", i)
 		prober := createBrokerTriggerDeliveryTopology(f, brokerName, tt.brokerDS, tt.t1DS, tt.t2DS, tt.t1FailCount, tt.t2FailCount)
@@ -718,12 +717,12 @@ func assertExpectedEvents(prober *eventshub.EventProber, expected map[string]exp
 					t.Error("unexpected event acceptance behaviour (-want, +got) =", diff)
 				}
 			}
-			// Check timing.
-			if len(want.eventInterval) != 0 && len(got.eventInterval) != 0 {
-				if diff := cmp.Diff(want.eventInterval, got.eventInterval); diff != "" {
-					t.Error("unexpected event interval behaviour (-want, +got) =", diff)
-				}
-			}
+			// TODO: Check timing.
+			//if len(want.eventInterval) != 0 && len(got.eventInterval) != 0 {
+			//	if diff := cmp.Diff(want.eventInterval, got.eventInterval); diff != "" {
+			//		t.Error("unexpected event interval behaviour (-want, +got) =", diff)
+			//	}
+			//}
 		}
 	}
 }

diff --git a/test/rekt/resources/trigger/trigger_test.go b/test/rekt/resources/trigger/trigger_test.go
@@ -19,11 +19,13 @@ package trigger_test
 import (
 	"os"
 
-	"knative.dev/reconciler-test/pkg/manifest"
-
-	v1 "knative.dev/pkg/apis/duck/v1"
-
+	duckv1 "knative.dev/eventing/pkg/apis/duck/v1"
+	"knative.dev/eventing/test/rekt/resources/delivery"
 	"knative.dev/eventing/test/rekt/resources/trigger"
+	v1 "knative.dev/pkg/apis/duck/v1"
+	"knative.dev/pkg/ptr"
+	"knative.dev/reconciler-test/pkg/manifest"
+	"knative.dev/reconciler-test/resources/svc"
 )
 
 // The following examples validate the processing of the With* helper methods
@@ -206,3 +208,68 @@ func ExampleWithFilter() {
 	//       type: "z"
 	//       x: "y"
 }
+
+func ExampleWithDeadLetterSink() {
+	images := map[string]string{}
+	cfg := map[string]interface{}{
+		"name":       "foo",
+		"namespace":  "bar",
+		"brokerName": "baz",
+	}
+
+	delivery.WithDeadLetterSink(svc.AsKReference("targetdlq"), "/uri/here")(cfg)
+
+	files, err := manifest.ExecuteLocalYAML(images, cfg)
+	if err != nil {
+		panic(err)
+	}
+
+	manifest.OutputYAML(os.Stdout, files)
+	// Output:
+	// apiVersion: eventing.knative.dev/v1
+	// kind: Trigger
+	// metadata:
+	//   name: foo
+	//   namespace: bar
+	// spec:
+	//   broker: baz
+	//   delivery:
+	//     deadLetterSink:
+	//       ref:
+	//         kind: Service
+	//         namespace: bar
+	//         name: targetdlq
+	//         apiVersion: v1
+	//       uri: /uri/here
+}
+
+func ExampleWithRetry() {
+	images := map[string]string{}
+	cfg := map[string]interface{}{
+		"name":       "foo",
+		"namespace":  "bar",
+		"brokerName": "baz",
+	}
+
+	exp := duckv1.BackoffPolicyExponential
+	delivery.WithRetry(3, &exp, ptr.String("T0"))(cfg)
+
+	files, err := manifest.ExecuteLocalYAML(images, cfg)
+	if err != nil {
+		panic(err)
+	}
+
+	manifest.OutputYAML(os.Stdout, files)
+	// Output:
+	// apiVersion: eventing.knative.dev/v1
+	// kind: Trigger
+	// metadata:
+	//   name: foo
+	//   namespace: bar
+	// spec:
+	//   broker: baz
+	//   delivery:
+	//     retry: 3
+	//     backoffPolicy: exponential
+	//     backoffDelay: "T0"
+}