search

traefik / traefik-helm-chart

Traefik Proxy Helm Chart
https://traefik.io
Apache License 2.0
1.05k stars 745 forks source link

persistent volume annotations bug #471

Closed corbosman closed 3 years ago

corbosman commented 3 years ago

Welcome!

What version of the Traefik's Helm Chart are you using?

10.1.1

What version of Traefik are you using?

2.4.9

What did you do?

I tried passing annotations to the persistent volume for acme certs.

persistence:
  enabled: true
  name: data
  accessMode: ReadWriteMany
  size: 128Mi
  storageClass: "nfs-client"
  path: /data
  annotations:
    nfs.io/storage-path: "acme"

I expected a PVC containing those annotations. Instead I got a yaml parsing error.

What did you see instead?

The helm chart creates this yaml which seems to be indented wrong.

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: traefik
  annotations:
      nfs.io/storage-path: acme
    helm.sh/resource-policy: keep
  labels:
    app.kubernetes.io/name: traefik
    helm.sh/chart: traefik-10.1.1
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: traefik

What is your environment & configuration?

k8s 1.21.4, helm 3.6.3, homelab system

Additional Information

No response

faust64 commented 3 years ago

Indeed, template uses some indent, while it should be nindent.

I did submit a PR, that would generate something like:

...
# Source: traefik/templates/pvc.yaml
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: traefik
  annotations:

    foo: bar
    helm.sh/resource-policy: keep
...
faust64 commented 3 years ago

FYI, PR was merged yesterday. Can you confirm it solved your issue, @corbosman ?

corbosman commented 3 years ago

I actually now see a new problem, there is now an empty line after the annotations.

This is what my values.yaml holds:

persistence:
  enabled: true
  name: data
  # existingClaim: "traefik"
  accessMode: ReadWriteMany
  size: 128Mi
  storageClass: "nfs"
  path: /data
  annotations:
    foo: bar
  # subPath: "acme" # only mount a subpath of the Volume into the pod

And resulting manifest:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: traefik
  annotations:

    foo: bar
    helm.sh/resource-policy: keep
  labels:
    app.kubernetes.io/name: traefik
    helm.sh/chart: traefik-10.2.0
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: traefik
spec:
  accessModes:
    - "ReadWriteMany"
  resources:
    requests:
      storage: "128Mi"
  storageClassName: "nfs"
faust64 commented 3 years ago

Sure, this is to be expected (as shown in https://github.com/traefik/traefik-helm-chart/issues/471#issuecomment-897589450). It's working though, isn't it?

If we want to remove those empty lines, we could revert back to indent instead of nindent, and move the block to the left / remove all indentation before its inclusion.

Looking at the rest of the code in this Chart, I'm not sure what's the norm, but there doesn't seem to be anything like this. We would however find other places, introducing carriage returns, canceling variable indentation:

And many other, every time that Chart indents some variable, it was done with a nindent. The only exception being https://github.com/traefik/traefik-helm-chart/blob/master/traefik/templates/hpa.yaml#L19 -- which has no indentation before calling the variable.

Now, for sure, if we want the generated yaml to be exempt of any un-needed character, we could consider refactoring all of those. I'm not sure that's such a big deal: once submitted to the API, they would go away, it doesn't prevent objects from being applied. If you're up for it, feel free to open a PR.

corbosman commented 3 years ago

Hi, it seems to work just fine, so as far as I'm concerned the issue is closed. Thanks!