-
Notifications
You must be signed in to change notification settings - Fork 32
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
✨ add VersionTemplate
support for HelmChartProxy
#292
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Furkan <furkan.turkal@trendyol.com> Co-authored-by: Batuhan <batuhan.apaydin@trendyol.com> Signed-off-by: Furkan <furkan.turkal@trendyol.com>
@Dentrax: GitHub didn't allow me to request PR reviews from the following users: developer-guy. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Dentrax The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @Dentrax! |
Hi @Dentrax. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/ok-to-test |
@Dentrax: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Thanks for the contribution! This is definitely an interesting idea. I'm wondering, however, if we can simplify this. If all we need is to map the compatibility matrix of K8s versions to Helm chart versions, it would be pretty complicated to have a ton of statements like
Do you have any use case where we would need to look up any field other than the K8s version to determine which Helm chart version to install? |
@mboersma @jackfrancis Any thoughts? |
Thanks @Jont828 for the
|
My thinking was that if our goal is to only support the compatibility matrix like the one linked for Descheduler, we might want to a design a scoped solution around that use-case. For ranges, my thinking was that we could try to leverage the semver package, which supports Compare(). We could then take the K8s version and compare it to the values in a map like this by iterating through the chart versions and seeing if the K8s version either is a direct match or falls in the inclusive range.
This is, however, just some brainstorming and the idea would need to be fleshed out if we wanted to implement something similar. |
I think if we need information other than just the K8s value, however, we should use a more generic approach like yours. Is it possible to use semver the |
I'm not sure if I'm following you here. Could you provide some example as above but for Since |
Sure thing. I'm thinking that because of how Go templates work, it can get pretty long to try to encode the version selection logic. Also, since there aren't that many functions available, we would be limited on what we can express. For example, in your use case here, we only have the function
Also, it's not very easy to handle version ranges. It would be nice to be able to do something like this, but that would require us defining new functions since the basic
I think that the version templating worked well for |
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This PR introduces a new field called
VersionTemplate
in theHelmChartProxy
CRD.Background
Most apps that provide Helm Charts include a version compatibility matrix as part of their versioning policy. For example, see the Descheduler and KEDA documentation. Given this, it's important to offer a way to override or set the Helm Chart version by comparing values against cluster metadata (such as version, region, data center, etc.). This ensures that the desired Helm Chart version is installed based on specific requirements.
Motivation
As early adopters of this Helm provider, we've found that adjusting the Helm Chart version based on metadata can be difficult and often requires workarounds. This negatively impacts the overall developer experience when encountering such cases. Currently, there is no straightforward method for managing chart versions effectively.
Current Workarounds
#@
tagsIn your
HelmChartProxy.yaml
file, put the following values on top of the file:Then pass
version: #@ chart_version
Example:
And then, you will need to set
clusterSelector.matchExpressions
to add:The
<OP>
should be set toIn
formetrics-server-k8s-v1.16
andNotIn
formetrics-server-k8s-not-v1.16
.Current Limitation
If you manage a flat mono-repo structure for kustomization manifests and need to upgrade a Helm Chart, but some clusters must rely on an older version of the chart, you'll have to completely restructure your folders and kustomization setup to accommodate this. This would create additional complexity and effort in maintaining the repository and HelmChartProxies.
Proposal
This PR introduces a new field to
HelmChartProxy
CRD, and its calledVersionTemplate
:It's quite similar to
ValuesTemplate
. By leveraging Go templates, we can gain greater control over the currentversion
field.#@
tagsUse Cases
Cluster
CR, allowing full automation of which Helm Chart version should be installed on each Kubernetes version. This enables dynamically retrieving the desired version based on the given labels by the API.version
andvalues
are core Helm flags that are typically set dynamically during the initial installation of an app, so they shouldn't be static fields. To get align withValuesTemplate
./kind feature
What this PR does / why we need it:
See above.
Which issue(s) this PR fixes
-
/cc @developer-guy @melikeiremguler