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

Add support to customize HPA name #3057

Closed
aviadlevy opened this issue May 22, 2022 · 8 comments · Fixed by #3068
Closed

Add support to customize HPA name #3057

aviadlevy opened this issue May 22, 2022 · 8 comments · Fixed by #3068
Assignees
Labels
feature-request All issues for new features that have not been committed to needs-discussion

Comments

@aviadlevy
Copy link
Contributor

Proposal

Add new field to allow custom hpa name (and not just keda-hpa-<name>)

Use-Case

For example:

apiVersion: keda.sh/v1alpha1
kind: ScaledObject
metadata:
  annotations:
    ...
  labels:
    ...
  name: ...
spec:
  advanced:
    horizontalPodAutoscalerConfig:
      hpaName: my-prsonal-hpa-name

Then keda will generate hpa named my-prsonal-hpa-name

Anything else?

If you're thinking it's a good proposal, I would like to try implement it myself and open PR 🙏

@aviadlevy aviadlevy added feature-request All issues for new features that have not been committed to needs-discussion labels May 22, 2022
@tomkerkhove tomkerkhove moved this to Proposed in Roadmap - KEDA Core May 22, 2022
@tomkerkhove
Copy link
Member

Fair ask, I like it.

I would, however, suggest to do the following as we try to avoid using abbreviations and it's already in horizontalPodAutoscalerConfig

apiVersion: keda.sh/v1alpha1
kind: ScaledObject
metadata:
  annotations:
    ...
  labels:
    ...
  name: ...
spec:
  advanced:
    horizontalPodAutoscalerConfig:
-      hpaName: my-prsonal-hpa-name
+      name: my-prsonal-hpa-name

@JorTurFer
Copy link
Member

JorTurFer commented May 23, 2022

I like the idea, I'm afraid about the implementation. I mean, right now it's easy to manage the resources (in k8s) created by KEDA because are deterministic in the names and they don't change,
if we proceed with this feature, we should ensure that we manage the resources properly in all edge cases like if we modify the custom name in the SO with the operator down. I mean, the old HPA has to be removed and then create the new one, otherwise we will have 2 HPA. It's not a problem IMO, but we must should it

@aviadlevy
Copy link
Contributor Author

aviadlevy commented May 23, 2022

I like the idea, I'm afraid about the implementation. I mean, right now it's easy to manage the resources (in k8s) created by KEDA because are deterministic in the names and they don't change, if we proceed with this feature, we should ensure that we manage the resources properly in all edge cases like if we modify the custom name in the SO with the operator down. I mean, the old HPA has to be removed and then create the new one, otherwise we will have 2 HPA. It's not a problem IMO, but we must should it

Yep, saw that while I start browsing the code. Do you have any suggestion how to know that we changed the hpa name and we need to clean the "old" one?
One idea I had is to use an annotation with fixed name (like "app.kubernetes.io/part-of": scaledObject.Name,) and search for it. what do you say?

@JorTurFer
Copy link
Member

Maybe we could add the "currently created" hpa's name in the SO status 🤔
WDYT @zroubalik ?

@zroubalik
Copy link
Member

zroubalik commented May 24, 2022

Yeah, SO.Status is the only field where we can imho save it.

I think that if we are going to implement this change, a solid test coverage needs to be done.

@JorTurFer
Copy link
Member

JorTurFer commented May 24, 2022

I think that if we are going to implement this change, a solid test coverage needs to be done.

Agree

@aviadlevy
Copy link
Contributor Author

Sounds good.
Can I still get a chance to try do it myself (though golang is not my primary language)?
If I'll see it's too much for me, I'll let you know

@zroubalik
Copy link
Member

@aviadlevy definitely! :)

@aviadlevy aviadlevy changed the title Customised hpa name Add support to custom hpa name May 25, 2022
@aviadlevy aviadlevy changed the title Add support to custom hpa name Add support to customize HPA name May 25, 2022
@tomkerkhove tomkerkhove moved this from Proposed to In Review in Roadmap - KEDA Core May 31, 2022
Repository owner moved this from In Review to Ready To Ship in Roadmap - KEDA Core Jun 20, 2022
@tomkerkhove tomkerkhove moved this from Ready To Ship to Done in Roadmap - KEDA Core Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request All issues for new features that have not been committed to needs-discussion
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants