hip introducing `export-values` for better composability #242

c0d1ngm0nk3y · 2022-02-04T14:42:12Z

Composing Helm charts using dependencies is currently limited because it is not possible to create a chart with a defined set of values and distribute them to subcharts.

This hip proposes an export-values structure to address this issue.

Co-authored-by: Johannes Dillmann <[email protected]> Co-authored-by: Ralf Pannemans <[email protected]> Co-authored-by: Pavel Busko <[email protected]> Co-authored-by: Jan von Löwenstein <[email protected]> Co-authored-by: Philipp Stehle <[email protected]> Co-authored-by: I546443 <[email protected]> Signed-off-by: Ralf Pannemans <[email protected]>

c0d1ngm0nk3y · 2022-02-15T07:56:52Z

@hickeyma , @marckhouzam HI. Is there anything missing? It would be great to get some feedback for this HIP, but we cannot add reviewers. What is the preferred approach?

c0d1ngm0nk3y · 2022-02-22T07:56:36Z

Would really be great to get any reaction at all. Is the HIP still used at all?

roulettedares · 2022-03-10T02:54:55Z

It's currently next to impossible to use the parent - subchart pattern with the helmfile values that jenkins-x passes to each release. This would unblock me in that scenario and also enable me to make a bunch of my other chart values way more DRY.

bridgetkromhout · 2022-03-17T16:42:40Z

Discussion in March 17, 2022 helm developer call: https://docs.google.com/document/d/1d-6xJEx0C78csIYSPKJzRPeWaHG_8W1Hjl72OJggwdc/edit?usp=sharing

marckhouzam · 2022-03-24T17:02:02Z

Looks like the next available number for a HIP is "0016". @c0d1ngm0nk3y you can replace the "9999" with "0016" in the file and in the name of the file.

Signed-off-by: Philipp Stehle <[email protected]>

phil9909 · 2022-03-24T17:11:50Z

Looks like the next available number for a HIP is "0016". @c0d1ngm0nk3y you can replace the "9999" with "0016" in the file and in the name of the file.

Done

Co-authored-by: Sumit Kulhadia <[email protected]> Signed-off-by: Ralf Pannemans <[email protected]>

loewenstein · 2022-04-10T11:23:34Z

@jdolitsky anything missing to get this HIP merged?

mattfarina

I have some concerns about backwards compatibility. How will older Helm clients handle this situation? Older versions are widely in use.

mattfarina · 2022-04-14T18:25:33Z

hips/hip-0016.md

+
+## Backwards Compatibility
+
+This change won't break any existing Helm chart since it only adds new mechanisms.


How will older Helm clients (e.g. v3.6.2) work with charts generated with these properties. Older versions of Helm are in wide use.

This is indeed a tricky problem.
My first idea was: We need to bump the apiVersion of the Chart.yaml and make the feature only available, if the newer version is used. But I noticed that my local helm installation happily templates my dummy chart, even if I set apiVersion: v100 in the Chart.yaml (which IMHO is a bug).

One alternative I see is recommending to chart authors who use the feature (in the docs of that feature) to include something like this in their templates (maybe in min-version.yaml by convention):

{{- if semverCompare ">3.10.0" .Capabilities.HelmVersion.Version }} {{- fail "This chart requires helm version 3.10.0 or higher" }} {{- end }}

3.10.0 is obviously only a placeholder for whatever version we will include this in.

Another way could be to make it only available, if you target a Kubernetes version to high for older helm versions (as defined in Supported Version Skew) by setting kubeVersion in the Chart.yaml. But this would be rather hard to document and I also don't think that the helm cli enforces this in any way. But at least it is documented as "not recommended" for a long time.

How about introducing a new function minimumHelmVersion?
The export-values would be only avilalbe, if you called minimumHelmVersion with 3.10.0 or higher.

That way, people using an older version of helm would get an error message function "minimumHelmVersion" not defined which should be a good hint that they might need to upgrade their helm version.

This approach could allow other changes, that couldn't otherwise be delivered in a backwards compatible way.

See #242 (review)

mattfarina · 2022-04-14T18:26:20Z

hips/hip-0016.md

+      child: "serverPort"
+  - name: server
+    version: 1.0.0
+    export-values:


How will older versions of Helm (e.g., v3.6.2) handle running into this? What will the chart do if someone tries to install it?

Matt good point. See my suggestion here #242 (review)

phil9909 · 2022-04-28T15:24:41Z

@jdolitsky anything missing to get this HIP merged?

HIPs need two approvals to get merged

This is a bit unorthodox because (logically) the gfd daemonset template should live inside the gfd chart. However, there is currently no good way to pass values from a parent chart into a subchart without jumping through alot of undesirabe hoops. This makes it hard to support the new shared config we have between gfd and the plugin. By moving the gfd daemonset into the plugin chart, we now have the ability to share values from the plugin into gfd. Note, that we don't move the contents of the values.yml from the gfd subchart into the plugin's values.yml. This is done on purpose, so that once proper suppport is in place to share values from a parent to a subchart, there won't be much work to move this file back where it belongs. A subsequent commit will show how we access these subchart values from gfd.yml, such that minimal code changes will be needed once we move gfd.yml back into the subchart. Below is a link to the Helm Improvement Proposal (HIP) that (once available) will allow us to move gfd.yml back where it belongs: helm/community#242 Signed-off-by: Kevin Klues <[email protected]>

scottrigby

I met with @phil9909 to better understand the use case, and now I see the value here beyond requiring users of parent charts to specify the same value over and over in the way that child charts require. This is essentially a mapping function that does the reverse of import-values. Phiiip, would you please write up the use case with the personas you mentioned? It would be faster for you to do this than for me right now.

I just have one suggestion for the HIP backwards compatibility section below, which addresses concerns @mattfarina mentioned. Otherwise, I think this looks good!

hips/hip-0016.md

Signed-off-by: Philipp Stehle <[email protected]> Co-authored-by: Pavel Busko <[email protected]> Co-authored-by: Scott Rigby <[email protected]>

scottrigby

I think this is looking good now for merging as a draft HIP 👍

@mattfarina How is this looking from your side? I think this has been addressed now in the HIP.

loewenstein · 2022-10-01T07:37:45Z

With two approvals (from @scottrigby & @jdolitsky) and @mattfarina's comments supposedly addressed, could we maybe proceed and try to get this HIP merged soon and the implementation into 3.11.0?

3.11.0 is the next feature release and will be released on January 18, 2023

helm-bot added the size/L label Feb 4, 2022

c0d1ngm0nk3y force-pushed the hip-export-values-reviewed branch from a2b9f12 to 39abfc9 Compare February 4, 2022 14:44

loewenstein mentioned this pull request Feb 4, 2022

feat: implement export-values helm/helm#10059

Open

3 tasks

jdolitsky approved these changes Mar 24, 2022

View reviewed changes

assign hip number 0016

23aeb73

Signed-off-by: Philipp Stehle <[email protected]>

phil9909 force-pushed the hip-export-values-reviewed branch from f5c7ea6 to 23aeb73 Compare March 24, 2022 17:10

c0d1ngm0nk3y mentioned this pull request Mar 25, 2022

feat: implement export-values helm/helm#10804

Open

3 tasks

michaelbeaumont mentioned this pull request Apr 4, 2022

feat(helm): add templates to modify expected registry and compatible version kumahq/kuma#4113

Merged

Add second way of defining export-values (see import-values)

1b66ff5

Co-authored-by: Sumit Kulhadia <[email protected]> Signed-off-by: Ralf Pannemans <[email protected]>

c0d1ngm0nk3y force-pushed the hip-export-values-reviewed branch from 6a5d241 to 1b66ff5 Compare April 7, 2022 17:16

mattfarina requested changes Apr 14, 2022

View reviewed changes

phil9909 mentioned this pull request May 25, 2022

install/upgrade/template ignore unknown apiVersions in Chart.yaml helm/helm#10990

Open

mkhpalm mentioned this pull request Jun 3, 2022

[Enhancement][opensearch] Add global support for common values opensearch-project/helm-charts#223

Open

scottrigby requested changes Jul 1, 2022

View reviewed changes

hips/hip-0016.md Outdated Show resolved Hide resolved

Add detials on backwards compatibility and motivation

4d02864

Signed-off-by: Philipp Stehle <[email protected]> Co-authored-by: Pavel Busko <[email protected]> Co-authored-by: Scott Rigby <[email protected]>

c0d1ngm0nk3y requested review from scottrigby and mattfarina July 6, 2022 06:17

bacongobbler mentioned this pull request Jul 7, 2022

Increment HIP number from most recent #258

Merged

scottrigby approved these changes Jul 8, 2022

View reviewed changes

scottrigby merged commit 0158df7 into helm:main Nov 3, 2022

pbusko deleted the hip-export-values-reviewed branch November 4, 2022 07:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hip introducing `export-values` for better composability #242

hip introducing `export-values` for better composability #242

c0d1ngm0nk3y commented Feb 4, 2022

c0d1ngm0nk3y commented Feb 15, 2022

c0d1ngm0nk3y commented Feb 22, 2022

roulettedares commented Mar 10, 2022

bridgetkromhout commented Mar 17, 2022

marckhouzam commented Mar 24, 2022

phil9909 commented Mar 24, 2022

loewenstein commented Apr 10, 2022

mattfarina left a comment

mattfarina Apr 14, 2022

phil9909 Apr 28, 2022

phil9909 Jun 30, 2022

scottrigby Jul 1, 2022

mattfarina Apr 14, 2022

scottrigby Jul 1, 2022

phil9909 commented Apr 28, 2022

scottrigby left a comment

scottrigby left a comment •

edited

Loading

loewenstein commented Oct 1, 2022


		## Backwards Compatibility

		This change won't break any existing Helm chart since it only adds new mechanisms.

hip introducing export-values for better composability #242

hip introducing export-values for better composability #242

Conversation

c0d1ngm0nk3y commented Feb 4, 2022

c0d1ngm0nk3y commented Feb 15, 2022

c0d1ngm0nk3y commented Feb 22, 2022

roulettedares commented Mar 10, 2022

bridgetkromhout commented Mar 17, 2022

marckhouzam commented Mar 24, 2022

phil9909 commented Mar 24, 2022

loewenstein commented Apr 10, 2022

mattfarina left a comment

Choose a reason for hiding this comment

mattfarina Apr 14, 2022

Choose a reason for hiding this comment

phil9909 Apr 28, 2022

Choose a reason for hiding this comment

phil9909 Jun 30, 2022

Choose a reason for hiding this comment

scottrigby Jul 1, 2022

Choose a reason for hiding this comment

mattfarina Apr 14, 2022

Choose a reason for hiding this comment

scottrigby Jul 1, 2022

Choose a reason for hiding this comment

phil9909 commented Apr 28, 2022

scottrigby left a comment

Choose a reason for hiding this comment

scottrigby left a comment • edited Loading

Choose a reason for hiding this comment

loewenstein commented Oct 1, 2022

hip introducing `export-values` for better composability #242

hip introducing `export-values` for better composability #242

scottrigby left a comment •

edited

Loading