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

Can the docker image name be changed from keda-metrics-adapter to keda-metrics-apiserver? #1105

Closed
ckuduvalli opened this issue Sep 8, 2020 · 15 comments · Fixed by #1108 or kedacore/charts#68
Labels
feature-request All issues for new features that have not been committed to needs-discussion
Milestone

Comments

@ckuduvalli
Copy link
Contributor

A clear and concise description of what you want to happen.

Currently we are using the name keda-metrics-apiserver for POD_NAME, SERVICE_NAME and so on. Only the docker image is called keda-metrics-adapter. Can we please change the docker image name also to keda-metrics-apiserver when v2 is released?

@zroubalik @tomkerkhove
Kindly suggest

@ckuduvalli ckuduvalli added feature-request All issues for new features that have not been committed to needs-discussion labels Sep 8, 2020
@tomkerkhove
Copy link
Member

I'll leave this up to @zroubalik but for me this sounds ok.

@zroubalik
Copy link
Member

Sounds good to me. @ckuduvalli are you willing to contribute this?

@zroubalik zroubalik added this to the v2.0 milestone Sep 8, 2020
@ckuduvalli
Copy link
Contributor Author

yes i will change it and send a pr

@tomkerkhove
Copy link
Member

Awesome, thanks! Would you mind updating our Helm charts as well?

@ckuduvalli
Copy link
Contributor Author

@tomkerkhove @zroubalik
For this change, I am not sure what to add and where to add documentation in keda-docs. Also, not sure about tests and changelog. Kindly help.

@tomkerkhove
could you please let me know how to update helm charts for this change?

@zroubalik
Copy link
Member

Reopening for helm charts

@zroubalik zroubalik reopened this Sep 8, 2020
@zroubalik
Copy link
Member

@ckuduvalli tests are not needed imho, and for changelog. I'll include this note to my already opened PR for changelog:#1086

@ckuduvalli
Copy link
Contributor Author

Thank you @zroubalik

@ckuduvalli
Copy link
Contributor Author

@zroubalik @tomkerkhove
Have made the helm charts change
kedacore/charts#66

@tomkerkhove
Copy link
Member

@zroubalik
Copy link
Member

@tomkerkhove IMHO it is not needed, it is covered in Changelog and this change is not something that is directly affecting users.

@flyingfang
Copy link

flyingfang commented Sep 11, 2020

Hey, there is a bug in the pr...The image key "metricsAdapter" in values.yaml has not been changed.

@tomkerkhove
Copy link
Member

@flyingfang
Copy link

This you mean? https://github.com/kedacore/charts/pull/66/files#diff-c7bc2dee6b0e6a8755bd9c0383b7a103R7

should be "metricsApiServer: docker.io/kedacore/keda-metrics-apiserver:1.5.0"

@tomkerkhove
Copy link
Member

Woops, thanks for letting us know! Fixing it now.

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
None yet
Development

Successfully merging a pull request may close this issue.

4 participants