allenporter / flux-local

flux-local is a set of tools and libraries for managing a local flux gitops repository focused on validation steps to help improve quality of commits, PRs, and general local testing.
https://allenporter.github.io/flux-local/
Apache License 2.0
156 stars 22 forks source link

HelmRelease Error: template due to substituteFrom not being supported #617

Closed tropnikovvl closed 7 months ago

tropnikovvl commented 7 months ago

Hello!

I use flux-local as a github action to view the diff between versions of helm releases.

I encountered a problem that it is impossible to template the parameters that I pass as flux variables.

flux_local.exceptions.HelmException: Command 'helm template external-dns flux-system-bitnami/external-dns --namespace external-dns --skip-crds --skip-tests --api-versions policy/v1/PodDisruptionBudget --version 7.1.0 --values /tmp/tmp1fe8o8lo/external-dns-external-dns-values.yaml --registry-config /dev/null --repository-cache /tmp/tmp40mhc7f7 --repository-config /tmp/tmp1fe8o8lo/repository-config.yaml' failed with return code 1
Error: template: external-dns/templates/dep-ds.yaml:133:29: executing "external-dns/templates/dep-ds.yaml" at <.Values.domainFilters>: range can't iterate over ${EXTERNAL_DNS_DONAINS}
apiVersion: helm.toolkit.fluxcd.io/v2beta2
kind: HelmRelease
metadata:
  name: external-dns
  namespace: external-dns
spec:
  chart:
    spec:
      chart: external-dns
      version: 7.1.0
      sourceRef:
        kind: HelmRepository
        name: bitnami
        namespace: flux-system
  releaseName: external-dns
  values:
    domainFilters: ${EXTERNAL_DNS_DONAINS}

Is it possible to add to your product the ability to pass parameters to it during startup in order to override flux variables?

For example:

flux-local test --path clusters/dev --enable-helm -v --parametes EXTERNAL_DNS_DONAINS=["foo.com", "bar.com"]
allenporter commented 7 months ago

How do you configure these variables in production flux? (Link to docs could help me understand).

Seems reasonable to try and support something like this.

allenporter commented 7 months ago

If possible I'd like 'flux build' or 'kustomize' to do the expansion so looking for a built in mechanism to hook into here.

tropnikovvl commented 7 months ago

It is https://fluxcd.io/flux/components/kustomize/kustomizations/#post-build-variable-substitution

How it works for me.

I create a configmap when creating a cluster with a set of parameters

apiVersion: v1
kind: ConfigMap
metadata:
  name: flux-cluster-settings
  namespace: flux-system
data:
  CLUSTER_NAME: test
  AWS_REGION: eu-central-1
  EXTERNAL_DNS_DONAINS: ["foo.com"]

then these configs are used below

---
apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
  name: base-stack
  namespace: flux-system
spec:
  interval: 3h
  retryInterval: 5m
  timeout: 5m
  sourceRef:
    kind: GitRepository
    name: flux-system
  path: ./infrastructure/base-stack
  prune: true
  wait: true
  postBuild:
    substituteFrom:
      - kind: ConfigMap
        name: flux-cluster-settings
tropnikovvl commented 7 months ago

Hello @allenporter !

I also found perhaps a better example for you. These are the variables from your code:

internal_ingress_ip from configmap network-config https://github.com/allenporter/k8s-gitops/blob/73c9eb34d1efa26fd5665f9313111d02d19eb54b/kubernetes/clusters/prod/network-config.yaml#L17

substituteFrom from https://github.com/allenporter/k8s-gitops/blob/73c9eb34d1efa26fd5665f9313111d02d19eb54b/kubernetes/clusters/prod/ml.yaml#L17

and as final part helmrelease with internal_ingress_ip variable https://github.com/allenporter/k8s-gitops/blob/73c9eb34d1efa26fd5665f9313111d02d19eb54b/kubernetes/network/prod/ingress/nginx-values.yaml#L16

allenporter commented 7 months ago

Yeah, thanks, definitely see the need here. I think in my case I get lucky and the values work fine in the template as unreplaced variables.

allenporter commented 7 months ago

I added a test of postbuild.substitute in #624 and it already works out of the box from flux build. That is promosing, so maybe flux local doesn't have to do its own bash variable manipulation.

allenporter commented 7 months ago

I think a plan to support this could be similar to valuesFrom in #338

allenporter commented 7 months ago

There will be a performance hit from supporting this.

Right now we can build kustomizations in parallel since we don't need to wait for the output of any of them. To support this, we need to build kustomizations in order and wait for the results to reference the outputs of previous kustomizations to find the config maps and values to substitute from.

I am wondering if it would be prefered to not support this, and set default placeholder values for the substitutions.

tropnikovvl commented 7 months ago

Hello!

I think this would be a good solution. If it is possible to pass values that will be used as placeholders. Some parameter or something like that.

allenporter commented 7 months ago

This has a 20-30% performance loss in my personal cluster, but the absolute numbers aren't huge.

# time flux-local get hr -A | tail -3
...
real    0m1.709s
user    0m4.229s
sys     0m0.621s
# time flux-local get hr -A | tail -3
...
real    0m2.164s
user    0m5.316s
sys     0m0.590s
allenporter commented 7 months ago

I will proceed with adding support for this.