Added broker and trigger tests to keda reconciler tests

[NeerajNagure]

Fixes #3814

Proposed Changes

• Added broker and trigger tests to keda reconciler tests so as to test them in keda environment.

• 🎁 Added broker and trigger tests

[knative-prow]

Hi @NeerajNagure. Thanks for your PR.

I'm waiting for a knative-extensions member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.72%. Comparing base (551691c) to head (504e7e9).
Report is 36 commits behind head on main.

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #3819       +/-   ##
=============================================
- Coverage     73.90%   49.72%   -24.18%     
=============================================
  Files           100      246      +146     
  Lines          3407    14819    +11412     
  Branches        288        0      -288     
=============================================
+ Hits           2518     7369     +4851     
- Misses          716     6695     +5979     
- Partials        173      755      +582     

Flag	Coverage Δ
java-unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pierDipi]

Thanks @NeerajNagure, left a few cleanup comments

[NeerajNagure]

@pierDipi I think so the tests that are left now in my changes are broker tests.How can I add the trigger tests then?

[pierDipi]

@NeerajNagure broker tests are also trigger tests, so the PR looks good to me

/lgtm

/assign @Cali0707

[Cali0707]

/ok-to-test

[knative-prow]

New changes are detected. LGTM label has been removed.

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: NeerajNagure
Once this PR has been reviewed and has the lgtm label, please ask for approval from cali0707. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Cali0707]

@NeerajNagure don't worry about the failing tests - those are unrelated to your changes. It looks like our CI is having issues this morning

[NeerajNagure]

@Cali0707 I have made the changes and removed -run=kafkasource flag test

[NeerajNagure]

/retest-required

[NeerajNagure]

Adding that command didn't work.I dont know how can we fix this.Can I get some help please @Cali0707 @pierDipi ?

[Cali0707]

/retest-required

[NeerajNagure]

/retest-required

[NeerajNagure]

/retest-required

[NeerajNagure]

/retest-required

[Cali0707]

/retest-required

[NeerajNagure]

@Cali0707 there is this error in keda tests broker_auth.go:193: no consumergroup template in config-kafka-features due to which its failing.I am not able to figure out how to fix it,can I please get some help?

[pierDipi]

@Cali0707 can you help @NeerajNagure ?

[Cali0707]

/retest

[pierDipi]

Same error

broker_auth.go:193: no consumergroup template in config-kafka-features

[Cali0707]

Same error

I'll look into it today 👍

[Cali0707]

Hey @NeerajNagure I think I found the problem that you are seeing in your tests - the enable-keda-autoscaling.yaml file only includes the feature flag for keda, so we are deleting all the other feature flags when we apply it. To fix this, I would recommend copying the values from https://github.com/knative-extensions/eventing-kafka-broker/blob/main/control-plane/config/eventing-kafka-broker/200-controller/100-config-kafka-features.yaml into https://github.com/knative-extensions/eventing-kafka-broker/blob/main/test/keda/enable-keda-autoscaling.yaml

You'll notice that this will leave you with two keys that seemingly do the same thing:

1. controller.autoscaler
2. controller-autoscaler-keda

We should remove the controller.autoscaler key from the enable-keda-autoscaling.yaml file, and switch the value of controller-autoscaler-keda from disabled to enabled

Hopefully this makes sense, if you have any questions please feel free to ask!

[NeerajNagure]

@Cali0707 thanks for the help I have made the changes

[NeerajNagure]

/retest-required

[NeerajNagure]

/retest-required

[Cali0707]

Hey @NeerajNagure sorry for missing this earlier, I think we also need something like

eventing-kafka-broker/test/reconciler-tests.sh

Line 24 in abaec81

kubectl apply -f ./test/e2e_new/config/features.yaml

before we actually run the tests

[Cali0707]

/retest-required

[Cali0707]

@NeerajNagure looks like these tests caught a bug! Would you be interested in investigating it? If you're not sure, I'm happy to try and investigate and resolve the bug myself as well

[NeerajNagure]

@Cali0707 I couldnt understand the bug so you please go ahead in investigating the bug

[Cali0707]

Hey @NeerajNagure I think my PR linked above should fix the problems here - we just need to merge that into eventing, and then update the dependencies here. If you want to, it would be great if you review the PR in eventing!

[Cali0707]

/retest-required

[Cali0707]

/test reconciler-tests-keda

[knative-prow]

@NeerajNagure: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
reconciler-tests-keda_eventing-kafka-broker_main	504e7e9	link	true	/test reconciler-tests-keda

Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[NeerajNagure]

@Cali0707 I think so we are getting this error tracker.go:139: Waiting for eventing-e2e0 to be deleted. I could not figure this out. Is this something related to deleting something in knative/eventing?