keda
keda copied to clipboard
kedacore
Add dapr checkpointing strategy for Azure Event Hubs scaler
This PR adds the dapr checkpointing strategy to the Azure Event Hubs scalar. The new checkpoint strategy is based on the existing goSdk checkpointing strategy but implements the dapr specific naming scheme for the offset file on the Azure Blob Storage. To use this strategy the field checkpointStrategy in the trigger specification needs to be set to the value dapr.
Checklist
- [x] Commits are signed with Developer Certificate of Origin (DCO - learn more)
- [x] Tests have been added
- [x] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
- [x] A PR is opened to update the documentation on (repo) (if applicable)
- [x] Changelog has been updated and is aligned with our changelog requirements
Fixes #
Relates to https://github.com/kedacore/keda/issues/3141 Relates to https://github.com/kedacore/keda-docs/pull/775
I think it could be e2e testable. But, this would definitely take some more time to implement, at least for me as I am still a novice with Go. I can try to implement it but I probably would suggest to create a new issue for it. I'm pretty busy next few weeks so I don't know if I'll have the time in the very near future.
@brainslush Any thought if we can close this PR soon? We are planning to release in 2 weeks.
@tomkerkhove I have no concerns so far. I tested it for some time now in our development infrastructure w/o any issues. Do I still need to do something?
Concerning the e2e test, I currently don't have the time as I'll move in a month to another city but after the move I'll take a shot at it.
No problem at all, but we'll hold off merging in that case until we have some e2e tests though.
Good luck with your move!
No problem at all, but we'll hold off merging in that case until we have some e2e tests though.
Good luck with your move!
I have seen that you are currently moving e2e to go. Was that already done for Azure Event Hubs? When exactly do you plan to do the release? I have to see if I possibly can squeeze it in somewhen.
We are shooting for the first week of August. I'll ask @JorTurFer about Event Hubs though, he should know
Was that already done for Azure Event Hubs?
not yet, you can do it if you want :)
@brainslush any update on this please? We are going to ship a new release next week.
@zroubalik I'm currently looking into how to do integration tests in golang. I'll do it. As I said I am currently moving to a new city, so free time is very sparse.
hey @brainslush
Today we have merged a PR with 2 new e2e test for Event Hub, one for blobMetadata strategy and another for goSdk strategy.
You can create a new test based on them but you should add also Dapr installation/removal as part of the e2e tests.
You have an example about how to install and uninstall with AWS Identity Components, but in this case the installation is not conditional, you should install and uninstall Dapr on every execution
@JorTurFer @brainslush what is the status of this PR, please?
I don't have more info about this, waiting e2e tests xD
If @brainslush needs support, I could jump in for that. (I created another duplicate PR because I missed this one). So if wanted, i could create a little e2e test.
I would be great if we could merge this for the next release because my dapr change is merged within dapr 1.9 (https://github.com/dapr/components-contrib/pull/1940) I have consolidated the naming schema between pubsub and bindings.
Additionally, related to this pr i would add to the docs pr a version-info for the new dapr checkpointer.
so the new dapr checkpointer works for: pubsub components: >= Dapr 1.6 (older versions need the GoSdk checkpointer) binding components: >= Dapr 1.9 (older versions need the GoSdk checkpointer)
So we need for newer dapr versions, this new checkpointer. I can add this version Infos to the docs pr
If @brainslush needs support, I could jump in for that. (I created another duplicate PR because I missed this one). So if wanted, i could create a little e2e test.
I would be great if we could merge this for the next release because my dapr change is merged within dapr 1.9 (dapr/components-contrib#1940) I have consolidated the naming schema between pubsub and bindings.
Additionally, related to this pr i would add to the docs pr a version-info for the new dapr checkpointer.
so the new dapr checkpointer works for: pubsub components: >= Dapr 1.6 (older versions need the GoSdk checkpointer) binding components: >= Dapr 1.9 (older versions need the GoSdk checkpointer)
So we need for newer dapr versions, this new checkpointer. I can add this version Infos to the docs pr
@christle sounds like a good plan, do you think you can follow up on the work by @brainslush? btw the next release is in ~2 weeks
Yes, I could create an e2e test this week and prepare a docs pr for 2.9
thanks a lot!
@christle That would be great. I still haven't gotten around to implement the e2e-test.
We are going to release KEDA v2.9 on Thursday. Do you think you can complete the open work by Wednesday @christle?
yes, I think so. I'm nearly ready with the e2e test. I found an easy way to integrate Dapr into the test setup. I will also open a PR for the test-tools repo