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 the option to disable api server registration for metrics-server #609

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

raphapr
Copy link

@raphapr raphapr commented Feb 15, 2024

Add the metricsServer.registerAPIService option to disable external metrics API registration.

This is useful when you already have an API registration (e.g. Datadog Cluster Agent) and want to deploy KEDA without impacting the current external metrics API.

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable) learn more
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Signed-off-by: Raphael Ribeiro <raphaelpr01@gmail.com>
@raphapr raphapr requested a review from a team as a code owner February 15, 2024 19:14
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.

KEDA Metrics Server is mandatory to run KEDA, you cannot use the one that's shipped with Datadog for example. The implementation is different

@raphapr
Copy link
Author

raphapr commented Feb 15, 2024

@zroubalik I understand that is mandatory. My rationale is there are potential scenarios where deploying KEDA without registering its metrics server becomes useful for migration purposes.

For example, in my case, I manage a cluster in which 1beta1.external.metrics.k8s.io is pointed to datadog; so deploying KEDA without deregistering the actual metrics provider is desired to facilitate the creation of the KEDA objects - I want to migrate the datadogmetric to scaledobject first. And subsequently, I can switch the API by using metricsServer.registerAPIService.

Does that make sense?

@JorTurFer
Copy link
Member

JorTurFer commented Feb 15, 2024

Does that make sense?

Yes, it can make sense IMO.

I guess that we could accept that feature now because I'll give another try to the proxy for supporting multiple metric servers (there have been some poeple reluctant to the subdomain idea in control plane side), so probably we will have to integrate it sooner or later and it also allows using KEDA only for ScaledJobs

Of course, this is something that requires a good explanation in docs, but I'd say that we could continue

WDYT?

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.

I understand your use case, it is a rare one though. I am not extremely happy with this addition, but I won't block it 😄 I let others to decide.

My only requirement is that this is properly documented, with consequencies for users (if they enable this setting).

@JorTurFer
Copy link
Member

I've been thinking about this and I'm not totally sure. Even though I understand the use case, I'm not sure if we are in the moment for supporting this feature as the proxy is still an on-going idea.
I won't block it either but it has to be really well documented with the side effects of this. @tomkerkhove ?

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.

3 participants