nextcloud / helm

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

Unable to mount datadir without `/data` suffix #531

Open Deaddy opened 4 months ago

Deaddy commented 4 months ago

Describe your Issue

At the moment it is impossible to mount a data directory which is not $somepath/data.

The offending code is in lines 218-226 of deployment.yaml:

            {{- if and .Values.persistence.nextcloudData.enabled .Values.persistence.enabled }}
            - name: nextcloud-data
              mountPath: {{ .Values.nextcloud.datadir }}
              subPath: {{ ternary "data" (printf "%s/data" .Values.persistence.nextcloudData.subPath) (empty .Values.persistence.nextcloudData.subPath) }}
            {{- else }}
            - name: nextcloud-main
              mountPath: {{ .Values.nextcloud.datadir }}
              subPath: {{ ternary "data" (printf "%s/data" .Values.persistence.subPath) (empty .Values.persistence.subPath) }}
            {{- end }}

We see that /data is being appended irrespective of the content or existence of subPath.

Another issue is that the data dir is only used if also .Values.persistence.enabled is enabled and it would be convenient to remove this unnecessary and-condition in the same PR, but I am not yet familiar with how atomic we like our PRs here. :-) (context would be #532)

Approaches

I think there are basically two ways to deal with this.

  1. The cleaner version would be to set /data as the subPath-default Value, so it would only break for people who have set a subPath and they would need to update their values accordingly.

  2. The not so nice but non-breaking change would be to add another boolean variable with a glorious name like postFixData which is by default set to true and would then use the old logic, and if it is not set to true it would drop the subPath if empty and not add /data if it is set.

So, which approach do we prefer or did I possibly even miss a trick to make this work without code change?

provokateurin commented 4 months ago

I'm not sure I understand the problem. Why do you need a to have the data dir at a different location? I'm not even sure if Nextcloud itself allows that.

Deaddy commented 4 months ago

It does.

Assume you have a non-k8s deployment of nextcloud which stores its data on some shared Storage like /nfs/nextcloud/ and want to move it over to k8s.

Now you do not want to move the data over to /nfs/nextcloud/data but keep it in place, lest you need to update all the 2 TB of oc_filecache and all the shares etc.

provokateurin commented 4 months ago

Ah ok, I see :+1:

Deaddy commented 4 months ago

Excellent!

Which approach do you favor? :-)

provokateurin commented 4 months ago

The first is definitely cleaner, even though it is a breaking change.

Deaddy commented 4 months ago

To give an update to the PR (or lack thereof):