From ad60ed9fc45e2d779aeba03f5b7d8f7c072ba342 Mon Sep 17 00:00:00 2001 From: Scott Nichols Date: Thu, 15 Apr 2021 15:00:31 -0700 Subject: [PATCH 1/3] test trigger delivery specs fully --- test/rekt/broker_test.go | 12 ++++ test/rekt/features/broker/control_plane.go | 65 +++++++++--------- test/rekt/resources/trigger/trigger_test.go | 75 +++++++++++++++++++-- 3 files changed, 115 insertions(+), 37 deletions(-) diff --git a/test/rekt/broker_test.go b/test/rekt/broker_test.go index 2f13b88dee0..9ad7c6a0962 100644 --- a/test/rekt/broker_test.go +++ b/test/rekt/broker_test.go @@ -123,3 +123,15 @@ func TestBrokerConformance(t *testing.T) { env.TestSet(ctx, t, broker.ControlPlaneConformance("default")) env.TestSet(ctx, t, broker.DataPlaneConformance("default")) } + +func TestBrokerOneOff(t *testing.T) { + ctx, env := global.Environment( + knative.WithKnativeNamespace(system.Namespace()), + knative.WithLoggingConfig, + knative.WithTracingConfig, + k8s.WithEventListener, + environment.Managed(t), + ) + + env.Test(ctx, t, broker.ControlPlaneDelivery()) +} diff --git a/test/rekt/features/broker/control_plane.go b/test/rekt/features/broker/control_plane.go index fa6cf157adf..97d39120a90 100644 --- 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) diff --git a/test/rekt/resources/trigger/trigger_test.go b/test/rekt/resources/trigger/trigger_test.go index 217ddc9324c..a2aaea42493 100644 --- 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" +} From 5c1cc92192298af11af75313921e794d3de13fe6 Mon Sep 17 00:00:00 2001 From: Scott Nichols Date: Thu, 15 Apr 2021 15:30:35 -0700 Subject: [PATCH 2/3] drop test helper --- test/rekt/broker_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/rekt/broker_test.go b/test/rekt/broker_test.go index 9ad7c6a0962..2f13b88dee0 100644 --- a/test/rekt/broker_test.go +++ b/test/rekt/broker_test.go @@ -123,15 +123,3 @@ func TestBrokerConformance(t *testing.T) { env.TestSet(ctx, t, broker.ControlPlaneConformance("default")) env.TestSet(ctx, t, broker.DataPlaneConformance("default")) } - -func TestBrokerOneOff(t *testing.T) { - ctx, env := global.Environment( - knative.WithKnativeNamespace(system.Namespace()), - knative.WithLoggingConfig, - knative.WithTracingConfig, - k8s.WithEventListener, - environment.Managed(t), - ) - - env.Test(ctx, t, broker.ControlPlaneDelivery()) -} From dbbcb1b720b1a661404ad0e6379b1f79b3efad70 Mon Sep 17 00:00:00 2001 From: Scott Nichols Date: Fri, 16 Apr 2021 08:11:33 -0700 Subject: [PATCH 3/3] we do not check timing yet --- test/rekt/features/broker/control_plane.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/rekt/features/broker/control_plane.go b/test/rekt/features/broker/control_plane.go index 97d39120a90..497c73631ed 100644 --- a/test/rekt/features/broker/control_plane.go +++ b/test/rekt/features/broker/control_plane.go @@ -717,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) + // } + //} } } }