From f7b2836d7d384fac262b676c2e98eacf46cd36e1 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 19 Oct 2026 09:24:00 +0900 Subject: [PATCH] fix(helm): skip data PVCs when storageclass.dataStorageSize is empty (#8681) An empty storageclass.dataStorageSize used to be the way to run on user-supplied drives (extraVolumes/extraVolumeMounts, e.g. JBOD hostPath), but the chart now always renders data PVC templates, with an invalid empty storage request. When the value is empty, render no data volumeClaimTemplates and no data volumeMounts, drop the data directory preparation from the init container (and the init container itself when there is nothing left to prepare), and omit volumeClaimTemplates when no PVC template remains. Standalone mode skips the data PVC and data volume unless existingClaim.dataClaim is set. Pools may override the value independently. Default renders are byte-identical. Fixes #8665 --- helm/README.md | 36 ++++++++++ helm/rustfs/templates/deployment.yaml | 13 ++++ helm/rustfs/templates/pvc.yaml | 2 +- helm/rustfs/templates/statefulset.yaml | 18 +++++ helm/rustfs/values.yaml | 20 +++++- scripts/test_helm_templates.sh | 96 ++++++++++++++++++++++++++ 6 files changed, 183 insertions(+), 2 deletions(-) diff --git a/helm/README.md b/helm/README.md index 42d5b3476..4d24099ed 100644 --- a/helm/README.md +++ b/helm/README.md @@ -32,6 +32,42 @@ You can override the inference by setting `drivesPerNode` explicitly, e.g. (with `--cascade=orphan` to keep pods and PVCs) and recreate it, or perform a full reinstall. +## Bring-your-own drives (JBOD) + +By default every drive is a PersistentVolumeClaim sized by +`storageclass.dataStorageSize`. To use disks that you provide yourself, such as +JBOD `hostPath` or local volumes, set `storageclass.dataStorageSize` to an empty +string. The chart then renders no data PVCs (no data `volumeClaimTemplates` in +distributed mode, no data PVC in standalone mode) and no data `volumeMounts`, +and you supply the drives through `extraVolumes` and `extraVolumeMounts`: + +```yaml +replicaCount: 3 +drivesPerNode: 4 +storageclass: + dataStorageSize: "" +extraVolumes: + - name: disk0 + hostPath: {path: /mnt/disk0, type: Directory} + # ... disk1 to disk3 +extraVolumeMounts: + - name: disk0 + mountPath: /data/rustfs0 + # ... /data/rustfs1 to /data/rustfs3 +``` + +Mount the drives at `/data/rustfs0` … `/data/rustfs` when +`drivesPerNode` is greater than 1, otherwise at `/data`, so they match the +generated `RUSTFS_VOLUMES`. The logs PVC is unaffected; set +`config.rustfs.obs_log_directory` to an empty string to drop it as well. Each +entry of `pools.list` may override `storageclass.dataStorageSize` +independently. In standalone mode, `mode.standalone.existingClaim.dataClaim` +still mounts the named claim as `data`. + +Switching `storageclass.dataStorageSize` between empty and non-empty changes +`volumeClaimTemplates`, so the same StatefulSet recreation rules as for +`drivesPerNode` apply. + --- ## Upgrade notes diff --git a/helm/rustfs/templates/deployment.yaml b/helm/rustfs/templates/deployment.yaml index 97b663626..22284b5a7 100644 --- a/helm/rustfs/templates/deployment.yaml +++ b/helm/rustfs/templates/deployment.yaml @@ -2,6 +2,9 @@ {{- $tlsEnabled := $tls.enabled | default false -}} {{- $logDir := .Values.config.rustfs.obs_log_directory }} {{- $logDirEnabled := ne $logDir "" }} +{{- /* With an empty storageclass.dataStorageSize and no existingClaim.dataClaim, + the data volume is supplied through extraVolumes/extraVolumeMounts. */}} +{{- $dataVolumeEnabled := or .Values.mode.standalone.existingClaim.dataClaim (not (empty .Values.storageclass.dataStorageSize)) }} {{- if .Values.mode.standalone.enabled }} apiVersion: apps/v1 @@ -74,6 +77,7 @@ spec: priorityClassName: {{ .Values.priorityClassName }} securityContext: {{- toYaml .Values.podSecurityContext | nindent 8 }} + {{- if or $dataVolumeEnabled $logDirEnabled }} initContainers: - name: init-step image: "{{ .Values.image.initImage.repository }}:{{ .Values.image.initImage.tag }}" @@ -84,18 +88,23 @@ spec: - sh - -c - | + {{- if $dataVolumeEnabled }} mkdir -p /data + {{- end }} {{- if $logDirEnabled }} mkdir -p /mnt/rustfs/logs chmod 755 /mnt/rustfs/logs {{- end }} volumeMounts: + {{- if $dataVolumeEnabled }} - name: data mountPath: /data + {{- end }} {{- if $logDirEnabled }} - name: logs mountPath: /mnt/rustfs {{- end }} + {{- end }} containers: - name: {{ .Chart.Name }} image: "{{ .Values.image.rustfs.repository }}:{{ .Values.image.rustfs.tag | default .Chart.AppVersion }}" @@ -138,8 +147,10 @@ spec: mountPath: {{ $logDir }} subPath: logs {{- end }} + {{- if $dataVolumeEnabled }} - name: data mountPath: /data + {{- end }} volumes: {{- with .Values.extraVolumes }} {{- toYaml . | nindent 8 }} @@ -188,6 +199,7 @@ spec: claimName: {{ include "rustfs.fullname" $ }}-logs {{- end }} {{- end }} + {{- if $dataVolumeEnabled }} - name: data persistentVolumeClaim: {{- if .Values.mode.standalone.existingClaim.dataClaim }} @@ -195,4 +207,5 @@ spec: {{- else }} claimName: {{ include "rustfs.fullname" $ }}-data {{- end }} + {{- end }} {{- end }} diff --git a/helm/rustfs/templates/pvc.yaml b/helm/rustfs/templates/pvc.yaml index d2ddbd95a..ca7f49466 100644 --- a/helm/rustfs/templates/pvc.yaml +++ b/helm/rustfs/templates/pvc.yaml @@ -1,4 +1,4 @@ -{{- if and .Values.mode.standalone.enabled (not .Values.mode.standalone.existingClaim.dataClaim) }} +{{- if and .Values.mode.standalone.enabled (not .Values.mode.standalone.existingClaim.dataClaim) (not (empty .Values.storageclass.dataStorageSize)) }} {{- with .Values.storageclass }} apiVersion: v1 kind: PersistentVolumeClaim diff --git a/helm/rustfs/templates/statefulset.yaml b/helm/rustfs/templates/statefulset.yaml index 54de97694..ac735009b 100644 --- a/helm/rustfs/templates/statefulset.yaml +++ b/helm/rustfs/templates/statefulset.yaml @@ -39,6 +39,10 @@ {{- $pools := include "rustfs.pools" . | fromJsonArray }} {{- range $pool := $pools }} {{- $drivesPerNode := int $pool.drives }} +{{- /* An empty storageclass.dataStorageSize means the data volumes are supplied + through extraVolumes/extraVolumeMounts (for example JBOD hostPath disks), + so the chart renders no data PVC templates and no data mounts. */}} +{{- $dataPvcEnabled := not (empty $pool.storageclass.dataStorageSize) }} --- apiVersion: apps/v1 kind: StatefulSet @@ -142,19 +146,23 @@ spec: priorityClassName: {{ $.Values.priorityClassName }} securityContext: {{- toYaml $.Values.podSecurityContext | nindent 8 }} + {{- if or $dataPvcEnabled $logDirEnabled }} initContainers: - name: init-step image: "{{ $.Values.image.initImage.repository }}:{{ $.Values.image.initImage.tag }}" imagePullPolicy: {{ $.Values.image.initImage.pullPolicy }} securityContext: {{- toYaml $.Values.containerSecurityContext | nindent 12 }} + {{- if $dataPvcEnabled }} env: - name: DRIVES_PER_NODE value: {{ $drivesPerNode | quote }} + {{- end }} command: - sh - -c - | + {{- if $dataPvcEnabled }} if [ "$DRIVES_PER_NODE" -gt 1 ]; then for i in $(seq 0 $(($DRIVES_PER_NODE - 1))); do mkdir -p /data/rustfs$i @@ -162,11 +170,13 @@ spec: else mkdir -p /data fi + {{- end }} {{- if $logDirEnabled }} mkdir -p /mnt/rustfs/logs chmod 755 /mnt/rustfs/logs {{- end }} volumeMounts: + {{- if $dataPvcEnabled }} {{- if gt $drivesPerNode 1 }} {{- range $i := until $drivesPerNode }} - name: data-rustfs-{{ $i }} @@ -176,10 +186,12 @@ spec: - name: data mountPath: /data {{- end }} + {{- end }} {{- if $logDirEnabled }} - name: logs mountPath: /mnt/rustfs {{- end }} + {{- end }} containers: - name: {{ $.Chart.Name }} image: "{{ $.Values.image.rustfs.repository }}:{{ $.Values.image.rustfs.tag | default $.Chart.AppVersion }}" @@ -232,6 +244,7 @@ spec: mountPath: {{ $logDir }} subPath: logs {{- end }} + {{- if $dataPvcEnabled }} {{- if gt $drivesPerNode 1 }} {{- range $i := until $drivesPerNode }} - name: data-rustfs-{{ $i }} @@ -241,6 +254,7 @@ spec: - name: data mountPath: /data {{- end }} + {{- end }} {{- if or $.Values.mtls.enabled $tlsEnabled $.Values.extraVolumes }} volumes: {{- if or $.Values.mtls.enabled $tlsEnabled }} @@ -282,6 +296,7 @@ spec: {{- toYaml . | nindent 8 }} {{- end }} {{- end }} + {{- if or $logDirEnabled $dataPvcEnabled }} volumeClaimTemplates: {{- if $logDirEnabled }} - apiVersion: v1 @@ -302,6 +317,7 @@ spec: requests: storage: {{ $pool.storageclass.logStorageSize }} {{- end }} + {{- if $dataPvcEnabled }} {{- if gt $drivesPerNode 1 }} {{- range $i := until $drivesPerNode }} - apiVersion: v1 @@ -341,5 +357,7 @@ spec: requests: storage: {{ $pool.storageclass.dataStorageSize }} {{- end }} + {{- end }} + {{- end }} {{- end }} {{- end }} diff --git a/helm/rustfs/values.yaml b/helm/rustfs/values.yaml index b7b622d7e..680d3ef5b 100644 --- a/helm/rustfs/values.yaml +++ b/helm/rustfs/values.yaml @@ -8,7 +8,9 @@ replicaCount: 4 # Number of data drives (PVCs) per pod. Combined with replicaCount this -# determines the total drive count seen by rustfs. +# determines the total drive count seen by rustfs. When +# storageclass.dataStorageSize is empty no PVCs are created and the drives come +# from extraVolumes/extraVolumeMounts instead. # - drivesPerNode > 1: each pod gets N volumes mounted at /data/rustfs0.../data/rustfsN-1 # - drivesPerNode = 1: each pod gets a single volume mounted at /data # @@ -467,6 +469,22 @@ topologySpreadConstraints: storageclass: name: local-path + # Size of each data PVC. Set it to an empty string ("") to stop the chart from + # creating data PersistentVolumeClaims: the StatefulSet then renders no data + # volumeClaimTemplates (standalone mode renders no data PVC) and no data + # volumeMounts, so the drives must come from extraVolumes/extraVolumeMounts, + # for example JBOD hostPath or local disks. Mount them at the paths RustFS + # expects: /data/rustfs0 ... /data/rustfs when drivesPerNode + # is greater than 1, otherwise /data. Keep drivesPerNode and replicaCount + # accurate because they still drive the generated RUSTFS_VOLUMES. A pool + # under pools.list can override this value, including with "". Standalone + # mode keeps the data volume when mode.standalone.existingClaim.dataClaim is + # set. logStorageSize is independent: the logs PVC is still created unless + # config.rustfs.obs_log_directory is empty. + # WARNING: switching between an empty and a non-empty value changes the + # StatefulSet volumeClaimTemplates, which Kubernetes forbids updating in + # place. Delete the StatefulSet with --cascade=orphan and recreate it, or + # reinstall. dataStorageSize: 256Mi logStorageSize: 256Mi pvcAnnotations: diff --git a/scripts/test_helm_templates.sh b/scripts/test_helm_templates.sh index e6e7907d0..b8d1eeb44 100755 --- a/scripts/test_helm_templates.sh +++ b/scripts/test_helm_templates.sh @@ -503,6 +503,102 @@ if grep -q "name: data$" <<<"$generic_eight_by_two"; then exit 1 fi +# Issue #8665: an empty storageclass.dataStorageSize means the drives come from +# extraVolumes/extraVolumeMounts, so no data PVC templates, no data mounts and +# no empty `storage:` request may be rendered. The logs PVC is unaffected. +jbod_extra_volumes='[{"name":"disk0","hostPath":{"path":"/mnt/disk0"}},{"name":"disk1","hostPath":{"path":"/mnt/disk1"}}]' +jbod_extra_mounts='[{"name":"disk0","mountPath":"/data/rustfs0"},{"name":"disk1","mountPath":"/data/rustfs1"}]' +jbod_statefulset=$(render_distributed_statefulset \ + --set replicaCount=3 \ + --set drivesPerNode=2 \ + --set storageclass.dataStorageSize= \ + --set-json "extraVolumes=${jbod_extra_volumes}" \ + --set-json "extraVolumeMounts=${jbod_extra_mounts}") +if grep -Eq 'name: data(-rustfs-[0-9]+)?$' <<<"$jbod_statefulset"; then + echo "Empty dataStorageSize must not render data PVC templates or data volume mounts" >&2 + exit 1 +fi +if grep -Eq 'storage:[[:space:]]*$' <<<"$jbod_statefulset"; then + echo "Empty dataStorageSize must not render an empty storage request" >&2 + exit 1 +fi +if grep -Eq 'DRIVES_PER_NODE|mkdir -p /data' <<<"$jbod_statefulset"; then + echo "Empty dataStorageSize must not initialize data directories in the init container" >&2 + exit 1 +fi +if ! grep -Fq 'mountPath: /data/rustfs1' <<<"$jbod_statefulset" || ! grep -Fq 'path: /mnt/disk1' <<<"$jbod_statefulset"; then + echo "Empty dataStorageSize must keep user-supplied extraVolumes and extraVolumeMounts" >&2 + exit 1 +fi +if ! grep -Eq '^[[:space:]]+name: logs$' <<<"$jbod_statefulset"; then + echo "Empty dataStorageSize must not affect the logs PVC" >&2 + exit 1 +fi + +# With neither data nor logs PVCs there is nothing to claim or initialize. +jbod_no_pvc_statefulset=$(render_distributed_statefulset \ + --set replicaCount=3 \ + --set drivesPerNode=2 \ + --set storageclass.dataStorageSize= \ + --set config.rustfs.obs_log_directory=) +for absent_key in volumeClaimTemplates initContainers; do + if grep -Fq "${absent_key}:" <<<"$jbod_no_pvc_statefulset"; then + echo "StatefulSet without data or logs PVCs must not render ${absent_key}" >&2 + exit 1 + fi +done + +# A pool may disable its data PVCs independently of the other pools. +pools_mixed=$(helm template rustfs "$CHART_DIR" \ + --namespace rustfs \ + --set secret.rustfs.access_key=test-access-key \ + --set secret.rustfs.secret_key=test-secret-key \ + --set pools.enabled=true \ + --set-json 'pools.list=[{},{"storageclass":{"dataStorageSize":""}}]') +if grep -Eq 'storage:[[:space:]]*$' <<<"$pools_mixed"; then + echo "A pool with empty dataStorageSize must not render an empty storage request" >&2 + exit 1 +fi +statefulset_named() { + awk -v metadata_name="$1" ' + function flush() { + if (doc ~ /kind: StatefulSet/ && index(doc, "\n name: " metadata_name "\n") > 0) printf "%s", doc + doc = "" + } + /^---$/ { flush(); next } + { doc = doc "\n" $0 } + END { flush() } + ' +} +pool0_statefulset=$(statefulset_named rustfs <<<"$pools_mixed") +pool1_statefulset=$(statefulset_named rustfs-pool1 <<<"$pools_mixed") +if ! grep -Eq 'name: data-rustfs-0$' <<<"$pool0_statefulset"; then + echo "A pool that keeps dataStorageSize must still render its data PVC templates" >&2 + exit 1 +fi +if [[ -z "$pool1_statefulset" ]] || grep -Eq 'name: data(-rustfs-[0-9]+)?$' <<<"$pool1_statefulset"; then + echo "A pool with empty dataStorageSize must not render data PVC templates or mounts" >&2 + exit 1 +fi + +# Standalone mode: no data PVC and no data volume unless an existing claim is named. +standalone_jbod=$(render_chart --set storageclass.dataStorageSize=) +if grep -Eq 'rustfs-data$|storage:[[:space:]]*$' <<<"$standalone_jbod"; then + echo "Standalone mode with empty dataStorageSize must not render a data PVC" >&2 + exit 1 +fi +if grep -Eq '^[[:space:]]+- name: data$|mountPath: /data$' <<<"$standalone_jbod"; then + echo "Standalone mode with empty dataStorageSize must not mount a data volume" >&2 + exit 1 +fi +standalone_jbod_claim=$(render_chart \ + --set storageclass.dataStorageSize= \ + --set mode.standalone.existingClaim.dataClaim=existing-data) +if ! grep -Fq 'claimName: existing-data' <<<"$standalone_jbod_claim"; then + echo "Standalone existingClaim.dataClaim must still mount when dataStorageSize is empty" >&2 + exit 1 +fi + # Issue #5416's exact three-node, one-drive topology must render all three # ordinal hosts while each StatefulSet pod receives its own explicit anchor. three_by_one_configmap=$(render_distributed_configmap --set replicaCount=3 --set drivesPerNode=1)