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

Update appVersion for Vertical Pod Autoscaler Helm Chart to Use Docker Image Version 1.2.1 #1522

Open
wernerwws opened this issue Aug 26, 2024 · 4 comments · May be fixed by #1538
Open

Update appVersion for Vertical Pod Autoscaler Helm Chart to Use Docker Image Version 1.2.1 #1522

wernerwws opened this issue Aug 26, 2024 · 4 comments · May be fixed by #1538
Labels
enhancement Adding additional functionality or improvements good first issue Good for newcomers help wanted Extra attention is needed

Comments

@wernerwws
Copy link

Description

This issue requests an update to the appVersion of the Vertical Pod Autoscaler (VPA) Helm chart to use the newer Docker image version 1.2.1 instead of the current 1.0.0. Updating the appVersion will help in aligning the Helm chart with the latest available Docker image, ensuring users benefit from the latest features and fixes.

Current Behavior

The current appVersion in the Helm chart is set to 1.0.0.

Desired Behavior

Update the appVersion in the Helm chart to 1.2.1.

Additional Context

While I am aware that the image tag can be set via the values file (values.yaml), my current automation setup requires using the latest chart's appVersion for deployment processes. This change would simplify deployment strategies and ensure consistency across different environments using the VPA.

Impact

This update will benefit users who rely on automated tools and scripts that fetch the latest appVersion from the Helm chart metadata for deployment, reducing manual interventions and potential for errors.

Thank you for considering this update.

@wernerwws wernerwws added enhancement Adding additional functionality or improvements triage This bug needs triage labels Aug 26, 2024
@sudermanjr
Copy link
Member

I imagine this change would be fine, but I would love for someone to review the changes first to make sure this won't break anything. Additionally, this would be a chartVersion minor change (which is totally fine).

@sudermanjr sudermanjr added help wanted Extra attention is needed good first issue Good for newcomers and removed triage This bug needs triage labels Sep 20, 2024
@oguzhan-yilmaz
Copy link

I don't think this would be a good idea. In the helm chart all of the blow images gets their tags from .Chart.appVersion:

  • vpa-admission-controller
  • vpa-updater
  • vpa-recommender

But in the Autoscaler Releases page they don't follow the same versioning for the images.

I think these *.image.tag values should be set separately in the values.yaml or should be overridden in user defined values.yaml file.

@wernerwws
Copy link
Author

wernerwws commented Oct 23, 2024

Hi @oguzhan-yilmaz ,

I think you have to filter for the "vertical-pod-autoscaler" releases on the release page.

It is perfectly fine that the images get their tags via .Chart.appVersion by default, which could be overwritten image by image via some *.image.tag value.

You can see how it is typically handled in popular community chart e.g. in the helm chart for prometheus on artifact hub.

Best Werner

@oguzhan-yilmaz
Copy link

Hey @wernerwws, thanks for letting me know. It was an oversight on my part.

After checking out https://explore.ggcr.dev/?repo=registry.k8s.io%2Fautoscaling I saw that every image VPA uses follows the same versioning.

It's okay to change the .Chart.appVersion to 1.2.1. And I think it should be done too. I had to update my values.yaml to accommodate for this older versioning.

All the best!

@oguzhan-yilmaz oguzhan-yilmaz linked a pull request Oct 27, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding additional functionality or improvements good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@sudermanjr @wernerwws @oguzhan-yilmaz and others