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

Helm 3.13 is not backward compatible with 3.12 #12460

Closed
zevisert opened this issue Oct 5, 2023 · 7 comments · Fixed by #12480
Closed

Helm 3.13 is not backward compatible with 3.12 #12460

zevisert opened this issue Oct 5, 2023 · 7 comments · Fixed by #12480
Labels
bug Categorizes issue or PR as related to a bug.

Comments

@zevisert
Copy link

zevisert commented Oct 5, 2023

Output of helm version:

version.BuildInfo{Version:"v3.13.0", GitCommit:"825e86f6a7a38cef1112bfa606e4127a706749b1", GitTreeState:"clean", GoVersion:"go1.20.8"}

Output of kubectl version:

Client Version: v1.28.2
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.3-gke.100

Cloud Provider/Platform (AKS, GKE, Minikube etc.): Not applicable


I have found in a few projects that helm 3.13 is not rendering the same output as 3.12. I've set up a simple minimal reproducible example at https://github.com/zevisert/helm-3-13-rendering-mre

Either way, here's some console output showing what I'm running into:

$ helm version
version.BuildInfo{Version:"v3.12.3", GitCommit:"3a31588ad33fe3b89af5a2a54ee1d25bfe6eaa5e", GitTreeState:"clean", GoVersion:"go1.20.7"}

$ helm create example
Creating example

$ rm -r example/charts/ example/templates/*

$ echo '{{ include "helmet.app" . }}' > example/templates/app.yaml

$ yq e '. += ({"dependencies": [{"name": "helmet", "version": "~0.9.1", "repository": "https://companyinfo.github.io/helm-charts", "import-values": ["defaults"]}]})' < example/Chart.yaml | sponge example/Chart.yaml

$ helm dependency build example
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "helmet" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading helmet from repo https://companyinfo.github.io/helm-charts
Deleting outdated charts

$ helm template example > templated-3.12.yaml

$ curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3
$ chmod 700 get_helm.sh
$ ./get_helm.sh
Helm v3.13.0 is available. Changing from version v3.12.3.
Downloading https://get.helm.sh/helm-v3.13.0-linux-amd64.tar.gz
Verifying checksum... Done.
Preparing to install helm into /opt/tooling/helm/bin
helm installed into /opt/tooling/helm/bin/helm

$ helm version
version.BuildInfo{Version:"v3.13.0", GitCommit:"825e86f6a7a38cef1112bfa606e4127a706749b1", GitTreeState:"clean", GoVersion:"go1.20.8"}

$ helm dependency build example
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "helmet" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading helmet from repo https://companyinfo.github.io/helm-charts
Deleting outdated charts

$ helm template example > templated-3.13.yaml

Here is the side-by-side diff between the two outputs (templated-3.12.yaml vs templated 3.13.yaml)

---                                                             ---
# Source: example/templates/app.yaml                            # Source: example/templates/app.yaml
apiVersion: v1                                                  apiVersion: v1
kind: Service                                                   kind: Service
metadata:                                                       metadata:
  name: release-name-example                                      name: release-name-example
  namespace: "default"                                            namespace: "default"
  labels:                                                         labels:
    app.kubernetes.io/name: example                                 app.kubernetes.io/name: example
    helm.sh/chart: example-0.1.0                                    helm.sh/chart: example-0.1.0
    app.kubernetes.io/instance: release-name                        app.kubernetes.io/instance: release-name
    app.kubernetes.io/managed-by: Helm                              app.kubernetes.io/managed-by: Helm
spec:                                                           spec:
  type: ClusterIP                                                 type: ClusterIP
  sessionAffinity: None                                           sessionAffinity: None
  ports:                                                          ports:
    - name: http                                                    - name: http
      port: 80                                                        port: 80
      protocol: TCP                                                   protocol: TCP
      targetPort: http                                                targetPort: http
  selector:                                                       selector:
    app.kubernetes.io/name: example                                 app.kubernetes.io/name: example
    app.kubernetes.io/instance: release-name                        app.kubernetes.io/instance: release-name
---                                                           <
# Source: example/templates/app.yaml                          <
apiVersion: apps/v1                                           <
kind: Deployment                                              <
metadata:                                                     <
  name: release-name-example                                  <
  namespace: "default"                                        <
  labels:                                                     <
    app.kubernetes.io/name: example                           <
    helm.sh/chart: example-0.1.0                              <
    app.kubernetes.io/instance: release-name                  <
    app.kubernetes.io/managed-by: Helm                        <
spec:                                                         <
  replicas: 1                                                 <
  revisionHistoryLimit: 10                                    <
  selector:                                                   <
    matchLabels:                                              <
      app.kubernetes.io/name: example                         <
      app.kubernetes.io/instance: release-name                <
  strategy:                                                   <
    type: RollingUpdate                                       <
  template:                                                   <
    metadata:                                                 <
      annotations:                                            <
        helm.sh/revision: "1"                                 <
      labels:                                                 <
        app.kubernetes.io/name: example                       <
        helm.sh/chart: example-0.1.0                          <
        app.kubernetes.io/instance: release-name              <
        app.kubernetes.io/managed-by: Helm                    <
    spec:                                                     <
      restartPolicy: Always                                   <
      containers:                                             <
        - name: example                                       <
          image: docker.io/hello-world:latest                 <
          imagePullPolicy: Always                             <
          ports:                                              <
            - containerPort: 8080                             <
              name: http                                      <
              protocol: TCP                                   <
          resources:                                          <
            limits: {}                                        <
            requests: {}                                      <
      affinity:                                               <
        podAntiAffinity:                                      <
          preferredDuringSchedulingIgnoredDuringExecution:    <
            - podAffinityTerm:                                <
                labelSelector:                                <
                  matchLabels:                                <
                    app.kubernetes.io/name: example           <
                    app.kubernetes.io/instance: release-name  <
                topologyKey: kubernetes.io/hostname           <
              weight: 1                                       <
      serviceAccountName: default                             <
@zevisert
Copy link
Author

zevisert commented Oct 6, 2023

Largely this seems to be due to 0a5148f / #12162 changing how imported values are overridden. This is also a case for #11162

@mattfarina mattfarina added bug Categorizes issue or PR as related to a bug. and removed question/support labels Oct 11, 2023
mattfarina added a commit to mattfarina/helm that referenced this issue Oct 11, 2023
The ordering should be:
1. User specified values (e.g CLI)
2. Parent chart values
3. Imported values
4. Sub-chart values

This enables parnet charts to import large set of values from a
child and then override select values.

This change is needed for backwards compatibility.

Fixes helm#12460

Signed-off-by: Matt Farina <matt.farina@suse.com>
@mattfarina
Copy link
Collaborator

@zevisert I think #12480 fixes your issue. I tested it against your example chart. Can you take a look.

@zevisert
Copy link
Author

@mattfarina I think this should make things work like most people would expect again. I'm a little wary about all these precedence changes going on in helm 3.13, but I suppose it's for the better.

I also think #11162 deserves some attention since you initially must have had a use-case for the values precedence when you were working on #12162

mattfarina added a commit that referenced this issue Oct 12, 2023
The ordering should be:
1. User specified values (e.g CLI)
2. Parent chart values
3. Imported values
4. Sub-chart values

This enables parnet charts to import large set of values from a
child and then override select values.

This change is needed for backwards compatibility.

Fixes #12460

Signed-off-by: Matt Farina <matt.farina@suse.com>
(cherry picked from commit 25371e2)
@hedayat
Copy link

hedayat commented Jan 2, 2024

#10998

Helm 3.13 was actually compatible with anything below Helm 3.8.1. :( Helm 3.8.2 - 3.12 were the incompatible ones!

@kingdonb
Copy link

Can we have some comment from the maintainers of Helm whether 3.13 or 3.14 was an error and might be reverted again - right now Flux v2.2.2 is running Helm 3.13 and at least @hedayat here has a use case that prefers the 3.13 (and <3.8.2) behavior. I understand that it has been changed again in 3.14.

Flux is not able to upgrade to 3.14 immediately due to some unrelated issues that may be resolved in their own time.

I refer to all these issues, I'm not sure if there is any response. But if there is any chance that it will be reverted again, then Flux devs may abandon their efforts to migrate to Helm 3.14.

Ref:

Again, I am asking for clarification only because right now there is an incompatibility that I'm not fully briefed on which prevents Flux from upgrading to Helm 3.14.x, if someone from the Helm project can weigh in, it may save us from some waste cycles, or help us better direct our efforts here.

Thanks friends

@scottrigby
Copy link
Member

Can we have some comment from the maintainers of Helm whether 3.13 or 3.14 was an error and might be reverted again - right now Flux v2.2.2 is running Helm 3.13 and at least @hedayat here has a use case that prefers the 3.13 (and <3.8.2) behavior. I understand that it has been changed again in 3.14.

Hi @kingdonb and @hedayat, appreciate you raising this and asking again. Thanks for your patience and community collaboration.

We know there are a handful of people who are trying to reconcile which version of Helm had breaking changes and which ones fix them.

Maintainers have replied to questions and comments in issues and PRs, and have discussed these in weekly Helm dev meetings. However, I acknowledge we could be better about communicating where we are at with this, given that it seems to be not fully resolved for all use cases.

About the versions. While Helm values handling has worked for the majority of users, for less common use cases there have been several previous bugs for years. We attempted to fix some of these in Helm 3.13, but that introduced breaking changes we missed. We fixed the breaking changes in 3.14, but that seems to have introduced related bugs for other use cases like @hedayat’s. There may be versions or details I missed here, but we can test/document them together.

For now, we are working on Helm values handling as a high priority:

  • several dedicated working sessions over the past few weeks
  • discussed again in yesterday's weekly Helm dev meeting
  • currently clarifying the intended behavior for each method of managing values in Helm charts and their dependencies in this Google doc

Feel free to comment on that Google doc, just bear in mind it is very much a work in progress at present.

A good way to help this effort is to document your use cases. This would help us ensure there are automated tests for each, to catch bugs that would impact your use cases before any future PRs are merged.

If you have any further questions about this or other ideas on how you can help, the weekly Helm dev meetings are a good place to engage or coordinate with maintainers working on this.

@hedayat
Copy link

hedayat commented Feb 3, 2024

@scottrigby

Thanks for your comprehensive reply. I've added a comment and a use-case to the doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants