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.

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

feat: Add kafka-go based kafka scaler #4801

Merged
merged 17 commits into from Sep 25, 2023
Merged

Conversation

sansmoraxz
Copy link
Contributor

@sansmoraxz sansmoraxz commented Jul 15, 2023

Add experimental kafka scaler based on kafka-go apache-kafka.
Changes to e2e test cases for concurrent testing of multiple kafka scalers.
Upgrade kafka version used for e2e testing.

Checklist

Fixes #4692 #3431

Relates to kedacore/keda-docs#1187

- kafka tests should run in their own namespace
- each kafka test case can have their own kafka cluster

Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
@github-actions
Copy link

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

🏖️ Over the summer, the response time will be longer than usual due to maintainers taking time off so please bear with us.

While you are waiting, make sure to:

Learn more about:

Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
@sansmoraxz
Copy link
Contributor Author

I was waiting for test results on how it goes performance wise in a production grade use case. Seems pretty chill so far.

Putting it up for review.

@sansmoraxz sansmoraxz marked this pull request as ready for review July 17, 2023 11:49
@sansmoraxz sansmoraxz requested a review from a team as a code owner July 17, 2023 11:49
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this awesome contribution!!!
I have done a first review over majority of the code

CHANGELOG.md Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler.go Outdated Show resolved Hide resolved
pkg/scalers/kafka_x_scaler_test.go Outdated Show resolved Hide resolved
pkg/scaling/scalers_builder.go Outdated Show resolved Hide resolved
pkg/util/helpers.go Show resolved Hide resolved
sansmoraxz and others added 4 commits July 27, 2023 12:02
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Souyama <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
- rename kafka-x to apache-kafka

Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
@sansmoraxz
Copy link
Contributor Author

As @JorTurFer suggested, I made the required changes.

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM!
Could you take a look @zroubalik ?

@JorTurFer
Copy link
Member

JorTurFer commented Aug 15, 2023

/run-e2e kafka
Update: You can check the progress here

@amcgreevy-r7
Copy link

Is there an update on when we think this change will be merged? Eagerly waiting on the ability to use IAM for auth.

@JorTurFer
Copy link
Member

We are waiting @zroubalik 's review as our expert in kafka. We are in summer break but we will restore the usual speed in September, so I guess that this will be merged soon

@zroubalik
Copy link
Member

zroubalik commented Sep 11, 2023

/run-e2e kafka
Update: You can check the progress here

Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
- With summary of diference between kafka scaler implementation

Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
@zroubalik
Copy link
Member

zroubalik commented Sep 14, 2023

/run-e2e kafka
Update: You can check the progress here

@sansmoraxz
Copy link
Contributor Author

Just a heads up, although there were minimal changes needed to make this compatible with v2, haven't tested it with a cluster yet.

Should not break anything, but will get around to testing it by mid of next week.

@sansmoraxz sansmoraxz force-pushed the kafka-go branch 2 times, most recently from 5648ca4 to 4f87694 Compare September 14, 2023 20:46
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
@zroubalik
Copy link
Member

zroubalik commented Sep 15, 2023

/run-e2e kafka
Update: You can check the progress here

@zroubalik
Copy link
Member

Just a heads up, although there were minimal changes needed to make this compatible with v2, haven't tested it with a cluster yet.

Should not break anything, but will get around to testing it by mid of next week.

Awesome, thanks for the great work! Keep us posted.

@sansmoraxz
Copy link
Contributor Author

Everything's working as expected.

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM, great work! Thanks

@zroubalik zroubalik merged commit b9f8d15 into kedacore:main Sep 25, 2023
18 of 19 checks passed
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
Signed-off-by: Souyama Debnath <souyama.debnath@atos.net>
Signed-off-by: Souyama <souyama.debnath@atos.net>
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: anton.lysina <alysina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change kafka library to kafka-go
7 participants