bitnami / charts

Bitnami Helm Charts
https://bitnami.com
Other
9.01k stars 9.22k forks source link

Helm upgrade command does not work after mutating admission webhook changed the deployment #13012

Open tkohn opened 2 years ago

tkohn commented 2 years ago

Name and Version

bitnami/nginx-ingress-controller 9.3.3

What steps will reproduce the bug?

  1. It can be reproduced on rancher-desktop, minikube and EKS
  2. Install the chart via helm upgrade --install --repo https://charts.bitnami.com/bitnami nginx-ingress-controller nginx-ingress-controller
  3. Edit the deployment to simulate the mutating admission webhook
    • kubectl edit deployment nginx-ingress-controller
    • Add the following lines
          volumeMounts:
          - name: workdir
            mountPath: /usr/share/nginx/html
        initContainers:
        - name: install
          image: busybox:1.28
          command:
          - wget
          - "-O"
          - "/work-dir/index.html"
          - http://info.cern.ch
          volumeMounts:
          - name: workdir
            mountPath: "/work-dir"
        volumes:
        - name: workdir
          emptyDir: {}

      after

          terminationMessagePath: /dev/termination-log
          terminationMessagePolicy: File

      save the deployment and verify that the deployment contains the new volumeMounts

  4. Run helm upgrade --install --repo https://charts.bitnami.com/bitnami --set commonLabels.test=test nginx-ingress-controller nginx-ingress-controller to trigger a patch.
  5. You will see the following error: Error: UPGRADE FAILED: cannot patch "nginx-ingress-controller" with kind Deployment: Deployment.apps "nginx-ingress-controller" is invalid: spec.template.spec.initContainers[0].volumeMounts[0].name: Not found: "workdir"

What is the expected behavior?

The helm upgrade command should not fail for the volumeMounts if mutating the admission webhook adds a volumeMount to the deployment.

What do you see instead?

The helm upgrade fails with the following error:

Error: UPGRADE FAILED: cannot patch "nginx-ingress-controller" with kind Deployment: Deployment.apps "nginx-ingress-controller" is invalid: spec.template.spec.initContainers[0].volumeMounts[0].name: Not found: "workdir"

Additional information

At first, I thought this is a bug with helm. I created a small example project to re-create the issue: https://github.com/tkohn/example-issue-helm But my example chart does not fail like the nginx-ingress-controller.

My guess is, that a sub-chart or some logic in the chart is messing up the volumes.

I tested it with helm 3.9.4 and 3.10.1 on Kubernetes 1.21, 1.22 and 1.23

github-actions[bot] commented 2 years ago

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

tkohn commented 2 years ago

Hey folks, have you had time to investigate the problem?

CeliaGMqrz commented 2 years ago

Hi @tkohn ,

Sorry for the delay.

I recommend using the values specified in the values.yaml file for these implementations instead of editing the deployment. You can use these settings in values.yaml

## @param extraVolumes Optionally specify extra list of additional volumes for Controller pods
##
extraVolumes:
  - name: workdir
    emptyDir: {}
## @param extraVolumeMounts Optionally specify extra list of additional volumeMounts for Controller container(s)
##
extraVolumeMounts:
    - name: workdir
      mountPath: /usr/share/nginx/html
## @param initContainers Add init containers to the controller pods
## Example:
## initContainers:
##   - name: your-image-name
##     image: your-image
##     imagePullPolicy: Always
##     ports:
##       - name: portname
##         containerPort: 1234
##
initContainers:
  - name: install
    image: busybox:1.28
    command:
    - wget
    - "-O"
    - "/work-dir/index.html"
    - http://info.cern.ch
    volumeMounts:
     - name: workdir
       mountPath: "/work-dir"

For this run, you must indicate the values.yaml file:

helm upgrade --install --repo https://charts.bitnami.com/bitnami --set commonLabels.test=test nginx-ingress-controller nginx-ingress-controller -f values.yaml

Please try it and if you have any problems feel free to comment here. Thanks for your comments.

tkohn commented 2 years ago

Hello @CeliaGMqrz

thanks for your answer.

The current problem is, that a Admission Webhook changes the deployment in the cluster. Your solution means, users have to create a diff of the deployment before and after the Admission Webhook and add the diff to the values file.

Do you have any other ideas to debug this Chart? Because I want to understand what could trigger this issue. My helm chart example https://github.com/tkohn/example-issue-helm does not have this issue. But it could be possible that the issue is in the helm implementation and not in the bitnami/nginx-ingress-controller chart.

github-actions[bot] commented 1 year ago

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

CeliaGMqrz commented 1 year ago

Hi @tkohn

Sorry for the delay. I have been able to reproduce the issue. I will have to investigate further as this is a particular case. When I have an update I will let you know.

Thanks for your report.

github-actions[bot] commented 1 year ago

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

pacholoamit commented 10 months ago

Any updates?