FairwindsOps / charts

Fairwinds helm chart repository
https://fairwinds.com
Apache License 2.0
141 stars 153 forks source link

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

Open wernerwws opened 2 months ago

wernerwws commented 2 months ago

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.

sudermanjr commented 1 month ago

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).

oguzhan-yilmaz commented 3 weeks ago

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:

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 commented 3 weeks ago

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 commented 3 weeks ago

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!