Doesn't seem possible to use default storage class for Tenant pools

[lotyrin]

Creating a tenant with the helm chart, the chart assumes you want to specify a storageClass. If you do not, it sets storageClass to null on the resulting Tenant, which is not a valid value.

Modifying the chart or working around the chart by creating a Tenant manually, it seems that if storage class is not specified on the Tenant, it will specify empty string ('') on the resulting PVC, thereby specifying no storage class, rather than a non-specified storage class (and allowing to use the default class).

Expected Behavior

There should be a way to avoid specifying a storage class and allow the default to be selected.

Current Behavior

Either null (invalid) empty string (no class, create PV manually) or an explicit class will be specified.

Possible Solution

Do not set or propagate values (either in the helm chart or in the operator) which are not set. Distinguish between unspecified, null, and an empty string.

Steps to Reproduce (for bugs)

Create a Tenant without a storage class specified.

Context

I am creating a chart that deploys MinIO to a set of clusters that have different default storage classes. I would like to avoid specifying a class and use the default for each of the clusters.

Regression

Possibly, not sure if this worked in the past or not.

Your Environment

Various. Reproducible in a local environment, e.g. Docker Desktop or minikube.

[ramondeklein]

I tried the following on a 4-node kind cluster:

helm install --namespace minio-operator --create-namespace operator minio-operator/operator
helm install --namespace test-tenant --create-namespace test-tenant minio-operator/tenant

I used the v5.0.15 chart versions and it created a total of 16 (4 servers with 4 disks) persistent volumes using the default storage class (which is standard in my case). Do you have a standard storage class? You can check using:

 $ kubectl get storageclass
NAME                 PROVISIONER             RECLAIMPOLICY   VOLUMEBINDINGMODE      ALLOWVOLUMEEXPANSION   AGE
standard (default)   rancher.io/local-path   Delete          WaitForFirstConsumer   false                  3d17h

When I run kubectl -n test-tenant get tenant test-tenant -o yaml the volumeClaimTemplate looks like this:

      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
          - ReadWriteOnce
          resources:
            requests:
              storage: 10Gi

It also doesn't contain the storageClass, so it will request the default storage class. A dry run looks like this:

$ helm install --dry-run --namespace test-tenant --create-namespace test-tenant minio-operator/tenant
NAME: test-tenant
LAST DEPLOYED: Tue May 14 12:29:55 2024
NAMESPACE: test-tenant
STATUS: pending-install
REVISION: 1
TEST SUITE: None
HOOKS:
MANIFEST:
---
# Source: tenant/templates/tenant-configuration.yaml
# WARNING: '.secrets' is deprecated since v5.0.15 and will be removed in next minor release (i.e. v5.1.0). Please use '.tenant.configSecret' instead.
apiVersion: v1
kind: Secret
metadata:
  name: myminio-env-configuration
type: Opaque
stringData:
  config.env: |-
    export MINIO_ROOT_USER="minio"
    export MINIO_ROOT_PASSWORD="minio123"
---
# Source: tenant/templates/tenant.yaml
apiVersion: minio.min.io/v2
kind: Tenant
metadata:
  name: myminio
  ## Optionally pass labels to be applied to the statefulset pods
  labels:
    app: minio
spec:
  image: "quay.io/minio/minio:RELEASE.2024-05-01T01-11-10Z"
  imagePullPolicy: IfNotPresent
  ## Secret with default environment variable configurations
  configuration:
    name: myminio-env-configuration
  pools:
    - servers: 4
      name: pool-0
      volumesPerServer: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          storageClassName: 
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: 10Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: OnRootMismatch
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      containerSecurityContext:
        allowPrivilegeEscalation: false
        capabilities:
          drop:
          - ALL
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
        seccompProfile:
          type: RuntimeDefault
  mountPath: /export
  subPath: /data
  requestAutoCert: true
  features:
    bucketDNS: false
    enableSFTP: false
  podManagementPolicy: Parallel
  prometheusOperator: false

My version of Helm (v3.8.0) / Kubernetes (v1.29) doesn't have any problems with storageClass being empty. It just assumes it's not there and uses the default storage class.

[lotyrin]

Okay. I or my GitOps tool must have made a mistake here somewhere. Thanks for verifying.

[ramondeklein]

@lotyrin I do think that we shouldn't generate the field if it's not set, so I have submitted a PR.