helm icon indicating copy to clipboard operation
helm copied to clipboard

Unable to mount datadir without `/data` suffix

Open Deaddy opened this issue 1 year ago • 6 comments

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?

Deaddy avatar Feb 06 '24 09:02 Deaddy

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.

provokateurin avatar Feb 06 '24 09:02 provokateurin

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.

Deaddy avatar Feb 06 '24 09:02 Deaddy

Ah ok, I see :+1:

provokateurin avatar Feb 06 '24 09:02 provokateurin

Excellent!

Which approach do you favor? :-)

Deaddy avatar Feb 06 '24 10:02 Deaddy

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

provokateurin avatar Feb 06 '24 10:02 provokateurin

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

  • I think the subPath variable is appropriate in the scenario where we use one persistent volume for everything
  • at the moment I am unable to install a fresh nextcloud with the change and the official image ~~because some of the script magic there still expects the data subdirectory and due to permission issues (listening on port 80/running as root) I am unable to manually try it out with occ, so I have to take a look at that~~ Edit: I just overlooked the _helpers.tpl

Deaddy avatar Feb 19 '24 14:02 Deaddy