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: allow skip scaled object creation with config #795

Conversation

frankliu20
Copy link

@frankliu20 frankliu20 commented Sep 22, 2023

When integrate Keda-Http with other KEDA scalers, we want to manage ScaledObject by ourselves, and add http-scaler as one of the triggers. In this PR, we allow users to do such ScaledObject management. When flag SKIP_SCALED_OBJECT_CREATION is set to true, keda-http-operator will skip the ScaledObject creation.

Checklist

@frankliu20 frankliu20 requested a review from a team as a code owner September 22, 2023 04:47
@JorTurFer
Copy link
Member

I think that this is a nice feature and simplifies the compatibility between KEDA and the HTTP add-on because avoid the requirement of implementing the whole ScaledObject spec in the add-on. WDYT @tomkerkhove @t0rr3sp3dr0 ?

@frankliu20 , Could you add an e2e test for testing this feature? IMO this requires an update in docs to explain that this can be done for other users

@frankliu20 frankliu20 force-pushed the frank/add-config-to-skip-so-creation branch from 78923b0 to 2809adb Compare September 25, 2023 06:21
@t0rr3sp3dr0
Copy link
Contributor

I like the idea but would rather have this as a field in the HTTPSO definition instead of an Operator argument. That way, you could have some services using the managed SO while others use a custom one.

@JorTurFer
Copy link
Member

@frankliu20 , any update?

@muecahit94
Copy link

Hi, any updates on that feature?

I would like to use zero scaling feature, but for that I need a combination of a keda built-in trigger and the http trigger, at the moment its not possible.

@JorTurFer
Copy link
Member

It looks that the PR has been abandoned by the OP :(
Are you willing to continue it @muecahit94 ?

@muecahit94
Copy link

@JorTurFer tbh no, just wanted to ask if there are any updates

@devopsdynamo
Copy link
Contributor

@JorTurFer Hi, I'm happy to pick this up and raise a new PR for this as I've already made the required changes for this feature on our forked repo.

@devopsdynamo
Copy link
Contributor

I've opened up new PR to allow annotation to be set on the HTTPSO instead as this allows more flexibility.
#929

@JorTurFer
Copy link
Member

This feature is already done by #929

@JorTurFer JorTurFer closed this Apr 4, 2024
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.

5 participants