nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.
GNU Affero General Public License v3.0
323 stars 264 forks source link

volumeMount and NFS mount clash when trying to use NFS for primary data storage #552

Open miguemely opened 5 months ago

miguemely commented 5 months ago

Could be related to #264 and #531 Originally posted by @miguemely in https://github.com/nextcloud/helm/issues/264#issuecomment-2035531364

So I'm trying to deploy this helm chart to replace my old install that uses NFS.

On the old install, it mounts a NFS share to /ncdata (Nextcloud Data Directory). This is what is defined in the Nextcloud config as its "Data Directory".

Within the helm values, I defined the following: nextcloud.datadir: /ncdata nextcloud.extraVolumes:

  extraVolumes:
   - name: nfs
     nfs:
       server: "nfs-server"
       path: "/mnt/nfspath"
       readOnly: false
  extraVolumeMounts:
   - name: nfs
     mountPath: "/ncdata"

The error I get is Deployment.apps "nextcloud" is invalid: [spec.template.spec.containers[0].volumeMounts[7].mountPath: Invalid value: "/ncdata": must be unique, spec.template.spec.containers[1].volumeMounts[8].mountPath: Invalid value: "/ncdata": must be unique, spec.template.spec.containers[2].volumeMounts[7].mountPath: Invalid value: "/ncdata": must be unique, spec.template.spec.containers[3].volumeMounts[0].name: Not found: "nextcloud-data"

I'm noticing that when I do a dry run, it creates a volumeMount for nextcloud-main.

            - name: nextcloud-main
              mountPath: /ncdata
              subPath: data

Is there a way to tell it not to use/create that mountPath, and instead use the one I specified for NFS?

bcbrookman commented 3 months ago

I've also been trying to get this to work. As you alluded to, it's because the value of nextcloud.datadir is always used as a mountPath here:

https://github.com/nextcloud/helm/blob/36118d09cf6f95b7f83a8641f23ac8362e079913/charts/nextcloud/templates/deployment.yaml#L222-L230

I expect changes would be needed to allow for this common(?) use-case as it does not appear to be configurable in any way.

jessebot commented 2 months ago

@bcbrookman or @miguemely I'd like to understand this a bit better. Would setting the persistence.nextcloudData.existingClaim help here? Here's where it is in values.yaml:

https://github.com/nextcloud/helm/blob/cf19396bf25c4366bd6bcaca0e2b2531526c41cc/charts/nextcloud/values.yaml#L431-L438

And here's the pesistence docs we have right now. Please let me know if you think this won't work, and if not, what you'd like to see done instead. As this is a community maintained chart, we're always open to pull requests to improve the chart 💙

Heavybullets8 commented 2 months ago

I was able to get this working by using an existing claim:

pvc.yaml

---
apiVersion: v1
kind: PersistentVolume
metadata:
  name: nextcloud-pv
spec:
  storageClassName: nextcloud-nfs
  capacity:
    storage: 300Gi
  accessModes:
    - ReadWriteMany
  persistentVolumeReclaimPolicy: Retain
  nfs:
    path: /mnt/tank/cloud
    server: ${SECRET_TRUENAS_IP}
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: nextcloud-pvc
spec:
  storageClassName: nextcloud-nfs
  accessModes:
    - ReadWriteMany
  resources:
    requests:
      storage: 300Gi

helmrelease.yaml

    nextcloud:
       ...
      datadir: /var/www/data
       ...
    persistence:
      enabled: true
      existingClaim: *app

      nextcloudData:
        enabled: true
        existingClaim: nextcloud-pvc

Notes: I did have to chmod the data dir that was created to 0770 (on my NFS machine) with:

chmod /mnt/tank/cloud/data 0770

Afterwards I refreshed the page and everything came up fine.

miguemely commented 2 months ago

@Heavybullets8 Not understanding you here, Did you apply those files before the helm chart or?

The other problem there is the subpath of data, if you have existing data, your basically SOL.

miguemely commented 2 months ago

-snip- https://github.com/nextcloud/helm/blob/cf19396bf25c4366bd6bcaca0e2b2531526c41cc/charts/nextcloud/values.yaml#L431-L438

How exactly would this work? Would we provide the existing pvc in existingClaim??

miguemely commented 2 months ago

Actually...ignore me. I just put two and two together (once I relooked at https://github.com/nextcloud/helm/issues/552#issuecomment-2200764668)

Let me give that a go...

bcbrookman commented 2 months ago

Thanks for the responses! I did see persistence.nextcloudData.existingClaim initially but hadn't considered it with an NFS volume. As @Heavybullets8 mentioned, using it with an NFS PV should work

jessebot commented 2 months ago

Happy to help :) Please let me know if everyone is all set, and if not, how we can help further 🙏