kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
11.09k stars 2.26k forks source link

How to properly override helmChart values using overlays? #4658

Closed paul-bormans closed 2 years ago

paul-bormans commented 2 years ago

Consider following "base":

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

helmGlobals:
  chartHome: ../../../charts
helmCharts:
- name: foo
  repo: foo
  version: 0.1.0
  # ValuesFile: <Defaults to {ChartHome}/{Name}/values.yaml>
  valuesInline:
    image:
      repository: my-repo
      tag: abc
    nodeSelector:
      role: test

Then on a "develop" overlay i need to pass in additional overrides compared to the baseline (provided by the values.yml o/t chart + some defaults in the kustomized/base layer.

I know this isn't working :-)....:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

bases:
- ../../base

namePrefix: dev-

helmCharts:
- name: foo
  valuesInline:
    resources:
      limits:
        cpu: 200m
        memory: 128Mi
      requests:
        cpu: 100m
        memory: 64Mi

I'm puzzled here how to bring in this seem to be trivial need:

The only way seems to be a full copy of the base helmChart entry; but that defeats the purpose of having overlays (what it comes to the charts).

Any advise?

Paul

k8s-ci-robot commented 2 years ago

@paul-bormans: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
HariSekhon commented 2 years ago

I override Helm charts at the later patches stage using standard Kustomize patching.

You can see examples of this throughout my main public Kubernetes repo, starting at the Kustomization.yaml:

https://github.com/HariSekhon/Kubernetes-configs

KnVerey commented 2 years ago

To do this, you would currently need to use the "transformed transformers" technique: https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/#transformed-transformers

This is not particularly elegant, and we have a new Kustomize kind called "Composition" that is intended to improve workflows like these. You can read more about it in this KEP: https://github.com/kubernetes/enhancements/issues/2299 The KEP also contains an example of how to build the two structures (transformed transformers vs Composition): https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2299-kustomize-plugin-composition/example

This PR details the work left to do to get Composition merged: #4323

/kind support /close

k8s-ci-robot commented 2 years ago

@KnVerey: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/kustomize/issues/4658#issuecomment-1150141920): >To do this, you would currently need to use the "transformed transformers" technique: https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/#transformed-transformers > >This is not particularly elegant, and we have a new Kustomize kind called "Composition" that is intended to improve workflows like these. You can read more about it in this KEP: https://github.com/kubernetes/enhancements/issues/2299 >The KEP also contains an example of how to build the two structures (transformed transformers vs Composition): https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2299-kustomize-plugin-composition/example > >This PR details the work left to do to get Composition merged: #4323 > >/kind support >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
cedvan commented 2 years ago

Why close ?

Proposed solution is not enough, can we have a sample please ? It seems to me extremely complicated for such a simple need...

KnVerey commented 2 years ago

I closed it because there is an entire KEP tracking the problem and solution for the "transformed transformers" technique being too complicated. I don't think there is value in having separate issues open about it for individual transformers. The docs I linked to do already provide examples of the current and future techniques.

cedvan commented 2 years ago

If I understand the proposed composition solution correctly, you are redefining the generated manifests after the Helm build. While it would be much simpler and also safer to be able to patch the values.yaml before the Helm build.

So when the Helm chart comes to evolve, nothing is to be done on the kustomize side except modify the version and possibly update the values.yaml if necessary. In the case of composition, the patched manifest may have been modified, even deleted, etc... Maintenance therefore becomes a real problem with this solution.

NickLarsenNZ commented 2 years ago

I would like this too. I have a base which includes a helm chart with sane values. But I would like the environment overlays to be able to adjust (like enable debugging) rather than making small changes in kustomize patches (which is actually tedious since in my current case, I have to rewrite the whole command/args in the pod specs just to enable debug logging - not to mention making some values ineffective since the args are overwritten and related values have no effect anymore).

I would much prefer to override some chart values, and then have kustomize render the chart manifests.

HariSekhon commented 2 years ago

@NickLarsenNZ you can actually add command args without having to rewrite the whole command args array.

Simple example:

https://github.com/HariSekhon/Kubernetes-configs/blob/master/argocd/base/argocd-server.http.patch.yaml

More complicated example with positional-based replacement of an existing arg:

https://github.com/HariSekhon/Kubernetes-configs/blob/master/cluster-autoscaler/overlay/cluster-autoscaler-deployment.eks.patch.yaml

I do a lot of Kustomize installations and patching, so you may be interested in seeing the master kustomization.yaml with patching strategies and examples as well as various app deployments at the top level of that repo:

https://github.com/HariSekhon/Kubernetes-configs

I agree it would be nice to support some additional Helm overrides in kustomization.yaml without need for patching, but until that is done, these are the best workarounds I've come up with so far using Kustomize's native capabilities.

AndrewFarley commented 2 years ago

@NickLarsenNZ

I would much prefer to override some chart values, and then have kustomize render the chart manifests.

This is already possible without Kustomize (if I understand what you're trying to do). Helm supports multiple values files, and although it seems to be black magic that almost no one uses or even knows about, when you specify multiple values files it automatically does shallow merges of those values files. This means you don't need Kustomize at all for this use case. I've been consulting and teaching this method at various companies and conferences over the last few years, this is a path to simplicity and success. Here's an example.

First part of the trick is to use sub-charts...

# requirements.yaml
dependencies:
- name: volume-autoscaler
  version: 1.0.5
  repository: "https://devops-nirvana.s3.amazonaws.com/helm-charts/"

Then specify your "global" settings applied to all your envs...

# values.yaml
volume-autoscaler:
  slack_webhook_url: "https://hooks.slack.com/services/1234/123/1234"
  slack_channel: "INTENTIONALLY-INVALID-WILL-OVERRIDE-IN-ENV-VALUES-FILES"
  resources:
    limits:
      cpu: 20m
      memory: 75Mi
    requests:
      cpu: 20m
      memory: 75Mi

Then you do overrides in an values-env file...

# values-stage.yaml
volume-autoscaler:
  slack_channel: "devops-stage"

Then you use helm with two values files. And ideally, you bake this logic into your CI/CD framework, and a README in your Helm Chart's folder so people know to use a specific command that includes both values files. The command examples are...

# First download dependency (subchart)
helm dependencies update volume-autoscaler
# Then helm diff with multiple values to check what will change (YOU should ALWAYS diff before applying)
helm diff upgrade --namespace infrastructure --allow-unreleased volume-autoscaler ./volume-autoscaler -f volume-autoscaler/values.yaml -f volume-autoscaler/values-stage.yaml
# And finally helm upgrade
helm upgrade --install --namespace infrastructure volume-autoscaler ./volume-autoscaler -f volume-autoscaler/values.yaml -f volume-autoscaler/values-stage.yaml

The way "shallow merge" works, is that if you have an map (dict) it'll merge the values from both the values.yaml and any subsequent values. However, if you have a list, it will override that list, there is no way to merge a list. Also, this is not limited to two values files, it's as many as you want. I have customers that setup multiple "sub-environments" on one kubernetes cluster, eg: one per-namespace. In this type of setup, we'd have three values files. A values.yaml, a values-envname.yaml and an values-envname-subenvname.yaml Eg: values-prod-disney.yaml. Keep in mind the order in this is CRITICAL, and you ALWAYS want to include the most global values file first. Meaning...

# this one everything is values-stage will override anything in values, which is the desired outcome
-f volume-autoscaler/values.yaml -f volume-autoscaler/values-stage.yaml
# is not the same as this, which everything in values will override everything in staging config, which
# is generally the exact opposite of what you want
-f volume-autoscaler/values-stage.yaml -f volume-autoscaler/values.yaml
AndrewFarley commented 2 years ago

And @paul-bormans as well might be relevant (see above message). As long as your upstream chart has the features in it, you don't need Kustomize. Just use helm only, keep things simpler. I only (personally, opinionated) recommend using Kustomize when your upstream chart doesn't support customizing something.

NickLarsenNZ commented 2 years ago

Thanks @AndrewFarley

And @paul-bormans as well might be relevant (see above message). As long as your upstream chart has the features in it, you don't need Kustomize. Just use helm only, keep things simpler. I only (personally, opinionated) recommend using Kustomize when your upstream chart doesn't support customizing something.

That's exactly why I'm using Kustomize with the chart. Unfortunately, Helm is closed for extension, while Kustomize is open for extension. So I'm having to add a few additional resources along with it.

NickLarsenNZ commented 2 years ago

@paul-bormans, if I understand what you meant correctly, I just add a second values file in the base that is generically for any environment overrides, but then I have to place that file (say with a CI/CD step).

If I understood correctly, I have thought of doing that, but I really want the environment overlays to represent the modifications as they are - so we have a clearer picture of environmental disparity (one of Kustomizes strengths). Having to do some background file placement feels a bit too hacky (though it works) and interrupts my Kustomize development workflow (where I render various environments locally and pipe the output through some utilities that help me see the bits I'm interested in).

Perhaps I misunderstood, so apologies if that's the case.

cedvan commented 2 years ago

Personally I changed my approach, where before I asked kustomize to build my chart helm. Now I ask Helm to build with the helm template command. Then I apply a kustomize on the resulting manifests if necessary.

Each tool retains its scope of operation and I do not distort the kustomize system by the helm chart build.

To automate this need I only created a helm binary wrapper (used by my argocd) :

#!/usr/bin/env sh

KUSTOMIZE_PATH="/usr/local/bin/kustomize"
KUSTOMIZE_BUILD_FILE="argocd.build.kustomization.yaml"
HELM_BUILD_FILE="argocd.build.helm.yaml"
HELM_PATH="/usr/local/bin/_helm"

# On kustomization file exist, ovveride to apply kustomize after helm template
if [ -f kustomization.yaml ]; then

    # Add Helm result to kustomization resources
    if [ -z "$(yq eval '.resources[]' kustomization.yaml)" ]; then
        file="$HELM_BUILD_FILE" yq -i '.resources = [strenv(file)]' kustomization.yaml
    else
        if [ "$(yq eval '.resources[-1]' kustomization.yaml)" != $HELM_BUILD_FILE ]; then
            file="$HELM_BUILD_FILE" yq -i '.resources += strenv(file)' kustomization.yaml
        fi
    fi

    $HELM_PATH "$@" > $HELM_BUILD_FILE && $KUSTOMIZE_PATH build
else
    $HELM_PATH "$@"
fi

Next, simply use @AndrewFarley solution to override helm chart values :

aeb-dev commented 1 year ago

@KnVerey Can you elaborate how "transformed transformers" would solve this issue? As far as I understand everything you suggested works after Helm chart is build because transformers, transformed transformers and composition works after generators. This is very error prone and subject to unexpected behavior.

Can't we have something like configMapGenerator so that helmChartInflationGenerator can be merged/patched?

xujihui1985 commented 1 year ago

can we just overwrite the helmCharts by name if it is defined in overlays?

NickLarsenNZ commented 1 year ago

can we just overwrite the helmCharts by name if it is defined in overlays?

How do you mean?

AFAIK, the overwriting (patching) occurs on the resulting manifests, which neither the helm values, nor the helmChart configuration are any part of.

xujihui1985 commented 1 year ago

@NickLarsenNZ sorry for the confuse. I mean if I define a helmCharts with the save name as the charts from base in the overlays, can we merge the properties of charts? for example

I have such config in base

helmCharts:
- name: minecraft
  includeCRDs: false
  valuesInline:
    minecraftServer:
      eula: true
      difficulty: hard
      rcon:
        enabled: true

then in overlay/dev, I have

namePrefix:  dev-
resources:
- ../../base

helmCharts:
- name: minecraft
  valuesInline:
    minecraftServer:
      eula: false
      difficulty: easy

then we can merge the helmCharts part before helm expand the template

NickLarsenNZ commented 1 year ago

then we can merge the helmCharts part before helm expand the template

@xujihui1985, I wish it could overlay helm values (or properties as you mention), but it can only operate on the resulting manifests that helm spits out.

So Kustomize only passes the values to helm to use. You don't see the values in overlays or other generators/transforms.

That means you have to apply patches as you would if it was just plain manifests from a base kustomization, which is sometimes awkward when it would be more ideal to patch the helm values yaml.

I should mention, the helm template is rendered where it is defined, so overlays without a helmCharts config never see anything about Helm.

I hope one day it works in a way that you and I would find useful.

xujihui1985 commented 1 year ago

@NickLarsenNZ Understood. Thanks for clarify.

tal19987 commented 1 year ago

Why is it closed? There still isn't an option to do it

tmsquill commented 1 year ago

This issue should remain open as it's highly relevant.

zeppelinen commented 1 year ago

Can we reopen it? The above mentioned composition API is closed, we need some good practice with Helm!

Smana commented 1 year ago

We still need this. Especially now that we have multiple sources for ArgoCD. That would be awesome to be able to merge valuesObjects

oldium commented 1 year ago

Composition API is not abandoned - see https://github.com/kubernetes-sigs/kustomize/issues/5198

oldium commented 1 year ago

But I wonder how the Composition API would help with Helm charts? For example, if I want to override just few fields, like the version (to have different version on staging and production environments) and only few values (like some base/redirection URIs)?

mtrin commented 1 year ago

I found a way to do values overlay by using valuesInline field but you need to disable load restrictor if you want to load the base values file from outside of your current overlay. That is, the values.yaml file you'll have on base won't be really part of the kustomize but called directly by the Helm inflator.

Here's an example that I used for grafana-agent Let's say you have your base values.yaml sitting on base folder with all of your desired defaults. base/values.yaml

agent:
  resources: 
    requests:
      cpu: "150m"
      memory: "256Mi"
    limits: 
      cpu: "200m"
      memory: "1Gi"

overlays/dev/kustomization.yaml

helmCharts:
  - name: grafana-agent
    version: 0.27.2
    repo: https://grafana.github.io/helm-charts
    releaseName: grafana-agent
    namespace: grafana
    includeCRDs: false
    valuesFile: ../../base/values.yaml
    valuesMerge: override # how to treat valuesInline with respect to Values. Legal values: ‘merge’, ‘override’, ‘replace’. Defaults to ‘override’.
    valuesInline:
      agent:
        resources: 
          requests:
            # cpu: "150m" # Will inherit from base
            memory: "2Gi" # Will override
          limits: 
            # cpu: "200m"  # Will inherit from base
            memory: "3Gi" # Will override

now you can build but make sure to disable load restrictor

kustomize build --enable-helm --load-restrictor=LoadRestrictionsNone > manifests.yaml

mtrin commented 1 year ago

The other venue I'm thinking is just use configmap generator which can merge yaml properties I think. Then use replacements feature and somehow make helm call that, maybe use the helm generator as a file and try to transform the transformers as mentioned here.

acidpizza commented 10 months ago

kustomize v5 added the field additionalValuesFiles which can overlay helm values. Example usage here.

xujihui1985 commented 10 months ago

kustomize v5 added the field additionalValuesFiles which can overlay helm values. Example usage here.

this is awesome

adnanQB commented 9 months ago

Hello guys,

any news on this one? Still not possible to do the override like suggested by [xujihui1985]?

Thanks.

Br, aDi

oldium commented 9 months ago

any news on this one? Still not possible to do the override like suggested by [xujihui1985]?

As far as I understand it this is not how it works now. You can override values by specifying the base values first with valuesFile: ../../base/values.yaml within your overlay kustomization.yaml. You also specify valuesMerge: override, and then valuesInline as shown in this comment and/or by additionalValuesFiles: values.yaml, which takes the values from the overlay directory (the path is relative). This means that the helmChart is executed from the overlay context, but the values are combined from both the base and overlay.

trexx commented 7 months ago

Just be sure to disable the load restrictor --load-restrictor=LoadRestrictionsNone if your overlayed values.yaml file is in another directory than where you are generating the helm chart.

uluzox commented 7 months ago

I am managing multiple clusters (e.g. development and production) in one repository and overwrite helmCharts like this

cluster-base/
└── devops-tools/
    └── kube-prometheus/
        ├── values.yml
        ├── namespace.yml
        └── kustomization.yml
cluster-overlay/
└── development/
    └── components/
        └── kube-prometheus/
            ├── values-overwrite.yml
            └── kustomization.yml
└── production/
    └── components/
        └── kube-prometheus/
            ├── values-overwrite.yml
            └── kustomization.yml

The cluster-base/devops-tools/kube-prometheus/kustomization.yml looks like this

---
apiVersion: kustomize.config.k8s.io/v1alpha1
kind: Component

namespace: monitoring
resources:
  - namespace.yml

The cluster-overlay/development/components/kube-prometheus/kustomization.yml looks like this

---
components:
  - ../../../../cluster-base/devops-tools/kube-prometheus/

helmCharts:
  - name: kube-prometheus-stack
    repo: https://prometheus-community.github.io/helm-charts
    namespace: monitoring
    releaseName: monitoring
    version: 57.2.1
    includeCRDs: true
    valuesFile: ../../../../cluster-base/devops-tools/kube-prometheus/values.yml
    additionalValuesFiles:
      - values-overwrite.yml

And in cluster-base/devops-tools/kube-prometheus/values.yml, I define the default values for the chart. While I overwrite values with values-overwrite.yml like below:

---
prometheus:
  ingress:
    hosts:
      - prometheus.caas.my-url.com
    tls:
      - secretName: prometheus-tls
        hosts:
          - prometheus.caas.my-url.com

grafana:
  env:
    GF_SERVER_ROOT_URL: "https://monitoring.caas.my-url.com/"
  ingress:
    hosts:
      - monitoring.caas.my-url.com
    tls:
      - secretName: monitoring-tls
        hosts:
          - monitoring.caas.my-url.com
seanturner026 commented 5 months ago

@uluzox this definitely works? Does it require --load-restrictor=LoadRestrictionsNone? I was getting the is not in or below error, I believe I followed your example exactly.

That said, using --load-restrictor=LoadRestrictionsNone allows one to do the following:

├── apps
│   ├── data
│   │   └── simple-chart
│   │       ├── overlays
│   │       │   ├── production
│   │       │   │   ├── kustomization.yaml
│   │       │   │   └── values.yaml <--- env specific
│   │       │   └── staging
│   │       │       ├── kustomization.yaml
│   │       │       └── values.yaml <--- env specific
│   │       └── values.yaml <--- base
uluzox commented 5 months ago

Hi @seanturner026, Yeah, my approach requires --load-restrictor=LoadRestrictionsNone. I forgot to mention this.

Kaslie commented 4 months ago

@uluzox Is it possible to merge instead of overwrite the base values ? I.E

base.yaml

deployments:
- command: ["./main"]
  resources:
    requests:
      cpu: 500m
overlays/production.yaml
- command: ["./main", "example"]

Expected Result:

kind: Deployment
metadata:
  spec:
    template:
      spec:
        containers:
          - command: ["./main", "example"]
            resources:
                requests:
                cpu: 500m

Current Result:

kind: Deployment
metadata:
  spec:
    template:
      spec:
        containers:
          - command: ["./main", "example"]
seanturner026 commented 4 months ago

Perhaps try swapping what you pass in for values and additionalValues? Maybe one has precedence over the other?

uluzox commented 4 months ago

@uluzox Is it possible to merge instead of overwrite the base values ? I.E

base.yaml

deployments:
- command: ["./main"]
  resources:
    requests:
      cpu: 500m
overlays/production.yaml
- command: ["./main", "example"]

Expected Result:

kind: Deployment
metadata:
  spec:
    template:
      spec:
        containers:
          - command: ["./main", "example"]
            resources:
                requests:
                cpu: 500m

Current Result:

kind: Deployment
metadata:
  spec:
    template:
      spec:
        containers:
          - command: ["./main", "example"]

Hi, I don't think this is possible since merging array members is not possible the way you would like to have it.

xujihui1985 commented 4 months ago

@uluzox Is it possible to merge instead of overwrite the base values ? I.E

base.yaml

deployments:
- command: ["./main"]
  resources:
    requests:
      cpu: 500m
overlays/production.yaml
- command: ["./main", "example"]

Expected Result:

kind: Deployment
metadata:
  spec:
    template:
      spec:
        containers:
          - command: ["./main", "example"]
            resources:
                requests:
                cpu: 500m

Current Result:

kind: Deployment
metadata:
  spec:
    template:
      spec:
        containers:
          - command: ["./main", "example"]

Hi, I don't think this is possible since merging array members is not possible the way you would like to have it.

you can't merge command array, but it's total valid to merge other fields under container