SchwarzIT / node-red-chart

Node-red Helm Chart
Apache License 2.0
36 stars 23 forks source link

Persisence-related chart values not being respected #321

Open rossigee opened 4 months ago

rossigee commented 4 months ago

What happened?

I expected to be able to define various aspects of the PVC manifest.

It turns out that the docs and values.yaml are out-of-date/incorrect, as only the persitence.enabled and an undocumented subPath value are being respected.

How can we reproduce this?

https://github.com/search?q=repo%3ASchwarzIT%2Fnode-red-chart%20persistence&type=code

Helm Chart version

$ helm -n nodered list
NAME    NAMESPACE   REVISION    UPDATED                                 STATUS      CHART           APP VERSION
nodered nodered     4           2024-03-01 09:03:22.916041598 +0000 UTC deployed    node-red-0.28.1 3.0.2      


### Search

- [X] I did search for other open and closed issues before opening this.

### Code of Conduct

- [X] I agree to follow this project's Code of Conduct

### Additional context

As a workaround I've simply disabled persistence in the chart and created the PVC separately.
dirien commented 1 month ago

Hey @rossigee,

Can you elaborate more on the issue and why you had to use the workaround you went?

rossigee commented 1 month ago

Never mind. I don't think this is a bug, just a documentation issue that sent me off on a tangent.

The values.yaml (without comments) suggests we configure persistence using values in the chart like this:

persistence:
  enabled: false
  # storageClass: "-"
  # existingClaim: your-claim
  accessMode: ReadWriteOnce
  size: 5Gi
  keepPVC: false

However, the template file for the deployment contains an undocumented subPath attribute which led me to think the docs were out-of-sync. Also I notice it mounts the /data volume regardless of the value of enabled or not, which prevents us using the extraVolumes if we needed more customisation than just subPath.

Anyway, it took me a while to figure it out how it worked but I got there in the end. Perhaps adding a brief 'Storage' section to the README would be useful for others?

In the end what I was trying to do was easily achieved using this:

    persistence:
      enabled: true
      existingClaim: nodered-data