Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Broker class spec based default #6621

Closed
wants to merge 9 commits into from
Closed

Add Broker class spec based default #6621

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
520c7cb
Add Broker class spec based default
liuchangyan Nov 21, 2022
3ef5c30
code cleanup and bug fix
liuchangyan Nov 21, 2022
ca0f0e7
unit test bug fix
liuchangyan Nov 22, 2022
2ad62d2
Update codegen
liuchangyan Nov 28, 2022
e942ada
broker setDefault unit test add
liuchangyan Dec 12, 2022
1fa7225
remove Deprecated StringPtr/Int32Ptr to String/Int32
liuchangyan Dec 12, 2022
d247b0f
fix codegen error
liuchangyan Dec 12, 2022
375ef8e
broker setDefault unit test add
liuchangyan Dec 16, 2022
3ba0387
unit tests cleanup
liuchangyan Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 122 additions & 45 deletions pkg/apis/eventing/v1/broker_defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ var (
},
},
"mynamespace4": {
BrokerClass: "KafkaBroker",
BrokerConfig: &config.BrokerConfig{
KReference: &duckv1.KReference{
APIVersion: "v1",
Expand Down Expand Up @@ -169,47 +168,8 @@ var (
)

func TestBrokerSetDefaults(t *testing.T) {
brokerClasses := make(map[string]*config.BrokerConfigSpec)
brokerSpec1 := &config.BrokerSpec{
Config: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "kafka-test",
},
Delivery: &eventingduckv1.DeliverySpec{
DeadLetterSink: &duckv1.Destination{
Ref: &duckv1.KReference{
Kind: "Service",
Namespace: "default",
Name: "kafka-error",
APIVersion: "serving.knative.dev/v1",
},
},
Retry: pointer.Int32(5),
BackoffPolicy: (*eventingduckv1.BackoffPolicyType)(pointer.String("exponential")),
BackoffDelay: pointer.String("5s"),
},
}
brokerConfigSpec1 := &config.BrokerConfigSpec{
Spec: brokerSpec1,
}
brokerSpec2 := &config.BrokerSpec{
Config: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "rabbitmq-test",
},
Delivery: nil,
}
brokerConfigSpec2 := &config.BrokerConfigSpec{
Spec: brokerSpec2,
}
brokerClasses["KafkaBroker"] = brokerConfigSpec1
brokerClasses["RabbitmqBroker"] = brokerConfigSpec2
defaultConfig.Defaults.ClusterDefault.BrokerClasses = brokerClasses
testCases := map[string]struct {
// testCases1 is the original testcases
testCases1 := map[string]struct {
initial Broker
expected Broker
}{
Expand Down Expand Up @@ -531,15 +491,97 @@ func TestBrokerSetDefaults(t *testing.T) {
},
},
},
"no config, uses namespace4's broker class and config": {
}
for n, tc := range testCases1 {
t.Run(n, func(t *testing.T) {
tc.initial.SetDefaults(config.ToContext(context.Background(), defaultConfig))
if diff := cmp.Diff(tc.expected, tc.initial); diff != "" {
t.Fatal("Unexpected defaults (-want, +got):", diff)
}
})
}

// Add cluster-level multi-class-based configs to defaultConfig
brokerClasses := make(map[string]*config.BrokerConfigSpec)
brokerSpec1 := &config.BrokerSpec{
Copy link
Member

@pierDipi pierDipi Dec 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call all brokerSpec<X> variables with the associated class for clarity like brokerSpecMTChannelBasedBroker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

Config: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "mt-test",
},
Delivery: &eventingduckv1.DeliverySpec{
DeadLetterSink: &duckv1.Destination{
Ref: &duckv1.KReference{
Kind: "Service",
Namespace: "default",
Name: "mt-error",
APIVersion: "serving.knative.dev/v1",
},
},
Retry: pointer.Int32(5),
BackoffPolicy: (*eventingduckv1.BackoffPolicyType)(pointer.String("exponential")),
BackoffDelay: pointer.String("5s"),
},
}
brokerConfigSpec1 := &config.BrokerConfigSpec{
Spec: brokerSpec1,
}
brokerSpec2 := &config.BrokerSpec{
Config: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "kafka-test",
},
Delivery: &eventingduckv1.DeliverySpec{
DeadLetterSink: &duckv1.Destination{
Ref: &duckv1.KReference{
Kind: "Service",
Namespace: "default",
Name: "kafka-error",
APIVersion: "serving.knative.dev/v1",
},
},
Retry: pointer.Int32(5),
BackoffPolicy: (*eventingduckv1.BackoffPolicyType)(pointer.String("exponential")),
BackoffDelay: pointer.String("5s"),
},
}
brokerConfigSpec2 := &config.BrokerConfigSpec{
Spec: brokerSpec2,
}
brokerSpec3 := &config.BrokerSpec{
Config: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "rabbitmq-test",
},
Delivery: nil,
}
brokerConfigSpec3 := &config.BrokerConfigSpec{
Spec: brokerSpec3,
}
brokerClasses["MTChannelBasedBroker"] = brokerConfigSpec1
brokerClasses["KafkaBroker"] = brokerConfigSpec2
brokerClasses["RabbitmqBroker"] = brokerConfigSpec3
defaultConfig.Defaults.ClusterDefault.BrokerClasses = brokerClasses

// After adding cluster-level multi-class-based configs to defaultConfig(BrokerClasses)
testCases2 := map[string]struct {
initial Broker
expected Broker
}{
"no config, uses clusterDefault broker class and namespace4's broker config": {
initial: Broker{
ObjectMeta: metav1.ObjectMeta{Namespace: "mynamespace4"},
},
expected: Broker{
ObjectMeta: metav1.ObjectMeta{
Namespace: "mynamespace4",
Annotations: map[string]string{
eventing.BrokerClassKey: "KafkaBroker",
eventing.BrokerClassKey: eventing.MTChannelBrokerClassValue,
},
},
Spec: BrokerSpec{
Expand Down Expand Up @@ -621,13 +663,48 @@ func TestBrokerSetDefaults(t *testing.T) {
},
},
},
"no config, missing namespace, uses clusterDefault broker class and corresponding config ": {
initial: Broker{
ObjectMeta: metav1.ObjectMeta{Namespace: "mynamespace-random"},
},
expected: Broker{
ObjectMeta: metav1.ObjectMeta{
Namespace: "mynamespace-random",
Annotations: map[string]string{
eventing.BrokerClassKey: eventing.MTChannelBrokerClassValue,
},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Kind: "ConfigMap",
APIVersion: "v1",
Namespace: "knative-eventing",
Name: "mt-test",
},
Delivery: &eventingduckv1.DeliverySpec{
DeadLetterSink: &duckv1.Destination{
Ref: &duckv1.KReference{
Kind: "Service",
Namespace: "default",
Name: "mt-error",
APIVersion: "serving.knative.dev/v1",
},
},
Retry: pointer.Int32(5),
BackoffPolicy: (*eventingduckv1.BackoffPolicyType)(pointer.String("exponential")),
BackoffDelay: pointer.String("5s"),
},
},
},
},
}
for n, tc := range testCases {
for n, tc := range testCases2 {
t.Run(n, func(t *testing.T) {
tc.initial.SetDefaults(config.ToContext(context.Background(), defaultConfig))
if diff := cmp.Diff(tc.expected, tc.initial); diff != "" {
t.Fatal("Unexpected defaults (-want, +got):", diff)
}
})
}

}