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

Values precedence in helm 3.13.1 #12511

Open
intetinte opened this issue Oct 19, 2023 · 15 comments
Open

Values precedence in helm 3.13.1 #12511

intetinte opened this issue Oct 19, 2023 · 15 comments

Comments

@intetinte
Copy link

With this commit (#12480) the precedence was changed to:
1. User specified values (e.g CLI)
2. Sub-chart values
3. Imported values
4. Parent chart values

This makes it only possible to add new values to sub-charts from parent or imported values. Modify existing values is no longer possible. In helm versions 3.8.2-3.12.3 it was possible to fill in empty or null values in the sub-charts from imports or parent chart. This is no longer possible in helm 3.13.1
In helm 3.13.0, it was possible to change any value in sub-charts from the imported or parent charts.

Is the precedence in helm 3.13.1 the final word on this?

@z4ce
Copy link
Contributor

z4ce commented Oct 20, 2023

Can you give an example of what you're trying to achieve with expected input and output?

@intetinte
Copy link
Author

intetinte commented Oct 23, 2023

I need a way to set values in the sub-charts from the parent chart.
Prior to 3.8.2 and in 3.13.0 it is possible to import values in the parent and apply those to sub-charts.
In 3.8.2-3.12 it is possible to set values in the sub-charts from an import in the parent, if those values had null or empty definitions in the sub-charts.
In 3.13.1, it is no longer possible to either update values in sub-charts from the parent or from imports in the parent. Default value, null or empty remains. The only option is when the sub-chart does not have the parameter names defined.

@z4ce
Copy link
Contributor

z4ce commented Oct 23, 2023

If you give an example of what you're trying and what's not working as you expect, that would help. The functionality here should still work: https://helm.sh/docs/chart_template_guide/subcharts_and_globals/#overriding-values-from-a-parent-chart

@intetinte
Copy link
Author

Here an import is done with three helm versions. With three different outcome:
Charts:

$ cat mychart/values.yaml
favorite:
drink: coffee
food: pizza
pizzaToppings:

  • mushrooms
  • cheese
  • peppers
  • onions

profile:
myimport: true

mysubchart:
dessert: ice cream


$ cat mychart/charts/mysubchart/values.yaml
dessert: cake
topping:

$ cat mychart/Chart.yaml
apiVersion: v2
appVersion: "1.0"
name: test
version: 1.0.0
description: test
dependencies:

  • condition: profile.myimport
    import-values:
    • myimport
      name: myimport
      repository: file://../charts/myimport
      version: 1.2.3
  • name: mysubchart
    repository: file://../charts/mysubchart
    version: 0.1.0

$ cat mychart/charts/mysubchart/templates/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ .Release.Name }}-cfgmap2
data:
dessert: {{ .Values.dessert }}
topping: {{ .Values.topping }}
servedon: {{ .Values.servedon }}

$ cat mychart/charts/myimport/values.yaml
exports:
myimport:
mysubchart:
dessert: mousse
topping: sugar
servedon: plate

$ ./helm-3.13.1 install --generate-name --dry-run --debug mychart
install.go:214: [debug] Original chart version: ""
install.go:231: [debug] CHART PATH: /home/epktojo/helmtest/mychart

NAME: mychart-1698132901
LAST DEPLOYED: Tue Oct 24 09:35:02 2023
NAMESPACE: default
STATUS: pending-install
REVISION: 1
TEST SUITE: None
USER-SUPPLIED VALUES:
{}

COMPUTED VALUES:
favorite:
drink: coffee
food: pizza
myimport:
exports:
myimport:
mysubchart:
dessert: mousse
servedon: plate
topping: sugar
global: {}
mysubchart:
dessert: ice cream
global: {}
servedon: plate
pizzaToppings:

  • mushrooms
  • cheese
  • peppers
  • onions
    profile:
    myimport: true

HOOKS:
MANIFEST:

Source: test/charts/mysubchart/templates/configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
name: mychart-1698132901-cfgmap2
data:
dessert: ice cream
topping:
servedon: plate

Tests:

$ ./helm-3.13.0 install --generate-name --dry-run --debug mychart
install.go:214: [debug] Original chart version: ""
install.go:231: [debug] CHART PATH: /home/epktojo/helmtest/mychart

NAME: mychart-1698132934
LAST DEPLOYED: Tue Oct 24 09:35:35 2023
NAMESPACE: default
STATUS: pending-install
REVISION: 1
TEST SUITE: None
USER-SUPPLIED VALUES:
{}

COMPUTED VALUES:
favorite:
drink: coffee
food: pizza
myimport:
exports:
myimport:
mysubchart:
dessert: mousse
servedon: plate
topping: sugar
global: {}
mysubchart:
dessert: mousse
global: {}
servedon: plate
topping: sugar
pizzaToppings:

  • mushrooms
  • cheese
  • peppers
  • onions
    profile:
    myimport: true

HOOKS:
MANIFEST:

Source: test/charts/mysubchart/templates/configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
name: mychart-1698132934-cfgmap2
data:
dessert: mousse
topping: sugar
servedon: plate

$ ./helm-3.8.2 install --generate-name --dry-run --debug mychart
install.go:178: [debug] Original chart version: ""
install.go:195: [debug] CHART PATH: /home/epktojo/helmtest/mychart

NAME: mychart-1698132978
LAST DEPLOYED: Tue Oct 24 09:36:18 2023
NAMESPACE: default
STATUS: pending-install
REVISION: 1
TEST SUITE: None
USER-SUPPLIED VALUES:
{}

COMPUTED VALUES:
favorite:
drink: coffee
food: pizza
myimport:
exports:
myimport:
mysubchart:
dessert: mousse
servedon: plate
topping: sugar
global: {}
mysubchart:
dessert: ice cream
global: {}
servedon: plate
topping: sugar
pizzaToppings:

  • mushrooms
  • cheese
  • peppers
  • onions
    profile:
    myimport: true

HOOKS:
MANIFEST:

Source: test/charts/mysubchart/templates/configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
name: mychart-1698132978-cfgmap2
data:
dessert: ice cream
topping: sugar
servedon: plate

@z4ce
Copy link
Contributor

z4ce commented Oct 24, 2023

Can you upload this test harness as a gist?
I think you have a couple of options to get the behavior you want.

I believe library charts maintains the behavior you want. Try and see what you what happens marking type: library in myimport/Chart.yaml.

The other thing I think you could do to maintain similar behavior is use the global values.

@intetinte
Copy link
Author

intetinte commented Oct 25, 2023

Not sure how to upload the complete package. Anyway there is no difference if I add type: library in the myimport Chart.yaml

Basically the target is to create a parent chart consisting of a number of subcharts. The parent chart gives a fixed number of flavors with content defined by imports. The values from the imports will be realized in the subcharts.
This has been possible prior to helm 3.13.1.

@intetinte
Copy link
Author

@intetinte
Copy link
Author

I was looking into this test:
func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) {
c := loadChart(t, "testdata/three-level-dependent-chart/umbrella")

e := make(map[string]string)

// The order of precedence should be:
// 1. User specified values (e.g CLI)
// 2. Parent chart values
// 3. Imported values
// 4. Sub-chart values
// The 4 app charts here deal with things differently:
// - app1 has a port value set in the umbrella chart. It does not import any
//   values so the value from the umbrella chart should be used.
// - app2 has a value in the app chart and imports from the library. The
//   app chart value should take precedence.
// - app3 has no value in the app chart and imports the value from the library
//   chart. The library chart value should be used.
// - app4 has a value in the app chart and does not import the value from the
//   library chart. The app charts value should be used.

For app2 it states that "has a value in the app chart and imports from the library. The app chart value should take precedence." That makes the order of precedence for imports to be lower than the sub.charts values. Contradicts the value precedence in the initial text.
For app3 it states that "app3 has no value in the app chart and imports the value from the library chart. The library chart value should be used.". What is considered as no value here? In the test chart the key "service.port" is missing. It could however have an empty definition or have null/nil value. Is that also considered as a value? Hence, that "value" overrides the library?

@intetinte
Copy link
Author

I actually fixed the problem and did a test verifying it.
helm_changes.patch
helm_testdata.patch

Please check and see if this is something that could be used.

@xkiranj
Copy link

xkiranj commented Nov 22, 2023

We are facing the exact same problem with Helm ver 3.13.1, that is being reported by @intetinte .
Seems like the patch could fix the issues.

@gjenkins8
Copy link
Contributor

@xkiranj

We are facing the exact same problem with Helm ver 3.13.1, that is being reported by @intetinte .
Seems like the patch could fix the issues.

Can you please retry with Helm 3.13.2 or higher. #12480 was the fix applied to Helm 3.13.2. And report if you still encounter unexpected behavior.

@gjenkins8
Copy link
Contributor

gjenkins8 commented Dec 21, 2023

@intetinte -- I apologize, Ive been trying to find time to get to the bottom of this issue. Helm 3.13 series did try to fix bugs in the import-values process (#12162, then fix/patch: #12480).

It would help me greatly if the reproducing your issue was easily accessible. In particular, the gist doesn't accurately represent a chart file structure. It would be very helpful if I could do something like:

$ git clone https://gist.github.com/intetinte/93b57e71e1173c0bcc262363b7020f57
$ helm template 93b57e71e1173c0bcc262363b7020f57/ # command to reproduce issue, and in particular if I can see the any behavioral differences between Helm 3.12 and 3.13 series

@intetinte
Copy link
Author

intetinte commented Dec 21, 2023

@gjenkins8
With the gist package I get these results from the different helm versions:

helm-3.12.3 template 93b57e71e1173c0bcc262363b7020f57

Source: test/charts/mysubchart/templates/configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
name: release-name-cfgmap2
data:
dessert: ice cream
topping: sugar
servedon: plate

helm-3.13.0 template 93b57e71e1173c0bcc262363b7020f57

Source: test/charts/mysubchart/templates/configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
name: release-name-cfgmap2
data:
dessert: mousse
topping: sugar
servedon: plate

helm-3.13.1 template 93b57e71e1173c0bcc262363b7020f57

Source: test/charts/mysubchart/templates/configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
name: release-name-cfgmap2
data:
dessert: ice cream
topping:
servedon: plate

I did a pull request that fix the issue without interfering with the other recent fixes with imports:
#12590

I added helm charts and tests for the fix in the pull request that maybe illustrates better then the gist.

@intetinte
Copy link
Author

Initial data that the import-function starts with in helm:
helm

The flow in the current helm (from 3.13.1):
helmflow
The data referred in the flow above:
helmImport
The suggested flow:
helmfix
Data in suggested flow:
helmflowfix

The suggested above makes imports on top-level take precedence of the values in the subcharts. This is what the documentation for helm suggests. The change is backwards compatible (i.e don't interfere with other updates done in the import area).
This can be verified in the pull request #12590

@nrledo
Copy link

nrledo commented Apr 22, 2024

We are facing the same issues that are reported here by @intetinte and checked that, with his version the charts are rendered as expected (import values taking precedence of the sub-chart values), as it's documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants