Skip to content

Update Helm chart versions #1944

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.

Sign up for GitHub

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

Merged
flyrain merged 2 commits into from
Jun 25, 2025
Merged

Update Helm chart versions #1944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
685cd5e
Update Helm chart versions
Jun 25, 2025
a6bf6f5
Update the tag to make sure this Helm chart version works best with a…
Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions helm/polaris/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ apiVersion: v2
name: polaris
description: A Helm chart for Apache Polaris (incubating)
type: application
version: 0.1.0
appVersion: 1.0.0-incubating-SNAPSHOT
version: 1.0.0
Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the Helm chart versioning to always follow the versioning of the underlying app?

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to, but we need to update it every time we update the helm-chart itself, see details here, #1944 (comment). I'm OK with other version number, any suggestions?

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with 1.0.0, and I am OK releasing the Helm chart every time we release Polaris, updating version and appVersion.

I am just mentioning this because we never finished the discussion started here:

https://lists.apache.org/thread/rrsjygxdltnbtldyo3zl9x0v0h1t614h

And so I don't know if there is a consensus here.

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sharing the thread, that's helpful. I didn't see any objection there. I'm also supportive of release helm charts separately from the main source / binary bundle.

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And so I don't know if there is a consensus here.

two +1s and no objection for a long time, which is usually considered as a consensus. It'd be nice to conclude it though.

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviving the discussion 🙏

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, would you mind chiming in a bit. I hope it can conclude soon. I'm OK if we didn't figure out helm chart release for 1.0.0, but it'd be nice to have it.

Copy link
Contributor

@dimas-b dimas-b Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this discussion to the dev ML?

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yufei revived that old thread, and I just added my +1 to his suggestions, FYI.

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we did, here are new discussions: https://lists.apache.org/thread/d1vf7xpn6nkzp8gbh417m8qb58tkpcqz

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm uses version to track the chart package itself (for repo indexes, upgrades, roll-backs, dependency resolution). It has nothing to do with the container image. It must be valid SemVer. It was recommend to update every time we update the helm chart itself.

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know that. But why jump from 0.1.0 to 1.0.0 then?

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason, can you suggest a version?

appVersion: 1.0.0-incubating
Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the docker image tag. The 1.0.0-incubating docker image will be published after the vote passed per release guide suggsted.

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appVersion has nothing to do with the docker image tag. The docker image tag is set in values.yaml at image.tag.

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must keep in mind that a Helm chart published under appVersion X should stay compatible with a wide range of Docker images, roughly going from X-N to X, where N is yet to be defined. At some point, it could be useful to keep a formal compatibility matrix somewhere.

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the correction. In that case, should we update the tag from latest to 1.0.0-incubating in file values.yaml?

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must keep in mind that a Helm chart published under appVersion X should stay compatible with a wide range of Docker images, roughly going from X-N to X, where N is yet to be defined. At some point, it could be useful to keep a formal compatibility matrix somewhere.

How do we archive the the range compatibility? I guess X is 1.0.0-incubating for the Polaris release 1.0.0-incubating. Is there any default value for N?

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are pros and cons:

  • latest is very convenient, but at the same time, not very precise.
  • Specifying a fixed version in image.tag, on the contrary, has the merit of making it clear that the Helm chart works best for that specific tag.

I am personally in favor of using image.tag = latest, because if people also install the latest version of the helm chart, and provided that we publish images and charts at every release, then they should automatically get a latest chart version that works well with the latest docker image version.

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. The only concern is that if users want to revert to a historical version of helm chart, do they expect to still get the latest docker image?

Copy link
Contributor Author

@flyrain flyrain Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an example, which assumes Helm chart version matched with Polaris src/bin/docker version:

  1. When an user revert to the Helm Chart v1.0.0, it still points the latest docker image, which could be 1.1.0 already. This introduces the incompatible risks. On the other hand, if we specified a version(e.g., 1.0.0) instead of latest, the Helm reversion will always be same.
  2. if people install the latest version of the helm chart, it still points to the latest docker image as we release them together. So that use case is covered as well.

With that, I think specifying a version is preferrable than latest. WDYT?

Copy link
Contributor

@adutra adutra Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid point! latest is not very friendly to historical chart versions. I am OK with using the release version instead.

home: https://polaris.apache.org/
icon: https://raw.githubusercontent.com/apache/polaris/main/site/static/img/logos/polaris-brandmark.png
sources:
Expand Down
2 changes: 1 addition & 1 deletion helm/polaris/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ image:
# -- The image pull policy.
pullPolicy: IfNotPresent
# -- The image tag.
tag: "latest"
tag: "1.0.0-incubating"
# -- The path to the directory where the application.properties file, and other configuration
# files, if any, should be mounted.
# Note: if you are using EclipseLink, then this value must be at least two folders down to the
Expand Down