Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix subPath on datadir mounts (fixes #531) #538

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Deaddy
Copy link
Contributor

@Deaddy Deaddy commented Feb 22, 2024

  • subPath now does not necessarily end on /data, in particular empty subPath possible for datadir on specific PVC
  • datadir persistence now does not depend on overall persistence anymore
  • bump .minor because old installs will need to add /data back to the subPath in the values file

Pull Request

Description of the change

Fix #531

The datadir had the subPath-Suffix "/data" forced upon it and it was impossible

Possible drawbacks

If someone actually used the a different volume in the past, they will now need to add /data to their subPath value.

Applicable issues

Additional information

Checklist

  • DCO has been signed off on the commit.
  • Chart version bumped in Chart.yaml according to semver.
  • (optional) Variables are documented in the README.md
  • tested installing and running fresh nextcloud instance with existing persistent volume for datadir

Deaddy and others added 2 commits February 22, 2024 12:10
- subPath now does not necessarily end on /data, in particular empty
  subPath possible for datadir on specific PVC
- datadir persistence now does not depend on overall persistence anymore
- bump .minor because old installs will need to add `/data` back to the
  subPath in the values file

Signed-off-by: Marcel Wunderlich <[email protected]>
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an alternative that doesn't require a breaking change would be to add another value that specifies the path of the data folder inside the subpath and if that is empty it is just omitted.

charts/nextcloud/templates/_helpers.tpl Show resolved Hide resolved
@jessebot jessebot added the Persistence Anything to do with external storage or persistence. This is also where we triage things like NFS. label Jul 26, 2024
- name: nextcloud-data
mountPath: {{ .Values.nextcloud.datadir }}
subPath: {{ ternary "data" (printf "%s/data" .Values.persistence.nextcloudData.subPath) (empty .Values.persistence.nextcloudData.subPath) }}
{{- else }}
{{- if not (empty .Values.persistence.nextcloudData.subPath) }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use

{{- with .Values.wasd }}
a: {{ . }}
{{- end }}

Instatt of

{{- if not (empty .Values.wasd) }}
a: {{ .Values.wasd }}
{{- end }}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Persistence Anything to do with external storage or persistence. This is also where we triage things like NFS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to mount datadir without /data suffix
4 participants