Option to override default volumeclaimtemplate for statefulsets

[ThelonKarrde]

What this PR does

Adding an option to helm chart to be able to override the default volumeClaimTemplates which is very limited in terms of customization and you can't apply for example local-volume storage or so.

This is not a breaking change and it's fully compatible with current installations.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[CLAassistant]

All committers have signed the CLA.

[krajorama]

Hi,

thank you for the contribution, what is it that you're trying to achieve ? Is it adding an extra PVC or replacing the current "storage" claim with something different ?

thanks, krajo

[ThelonKarrde]

Hi,

thank you for the contribution, what is it that you're trying to achieve ? Is it adding an extra PVC or replacing the current "storage" claim with something different ?

thanks, krajo

Yes, the idea is to have the ability to replace the original claims and/or be able to add more claims directly into the sts template. I do understand that there is a possibility to make an override for directories where Mimir writes + to make additional attachments + self-made claims, but I believe if it's all nested into original sts it's far easier to manage since everything is in one place.

[dimitarvdimitrov]

An alternative I see to this is to extend the configuration set under *.persistentVolume to support your use case. I think it's worth considering this because then when using the chart you only need to override what you care about wrt PVCs, and not have to redefine the whole PVC block. These are the fields of a PersistentVolumeClaim (taken from the API spec):

* accessModes ([]string)                     # configurable

* selector (LabelSelector)                   # not configurable

* resources (ResourceRequirements)

  * resources.limits (map[string]Quantity)   # not configurable, but does not seem related to your use case

  * resources.requests (map[string]Quantity) # configurable

* volumeName (string)                        # not configurable

* storageClassName (string)                  # configurable

* volumeMode (string)                        # not configurable, but I doubt someone would want to run with a Block volume mode instead of Filesystem. Mimir requires a filesystem

@ThelonKarrde in your PR description you mention that you cannot use local volume storage. Is it the volumeName or the selectors that you need to be able to use local volumes? Or is it something I missed?

[krajorama]

To be more consistent (with extraEnv, extraEnvFrom, etc, etc) the new value (if we decide to do add it) should be called extraVolumeClaimTemplates and it should be added after the existing conditional section.
If could be added from line 42 (original) as something like:

    {{- with .Values.alertmanager.persistentVolume.extraVolumeClaimTemplates }}
    {{- toYaml . | nindent 4 }}
    {{- end }}

also for the other components.

[ThelonKarrde]

@dimitarvdimitrov actually that's my bad. I mixed PVs and PVCs so with an initial goal this PR is pointless since I'm able to do what I need to. Essentially generate local-volume PVs for PVCs as I can in a separate template. But maybe I should include it here as an option to generate one with correct naming aka $pv-name-$pod-name-$count?

Other than that, I can refactor PR to what @krajorama suggested and just keep it as extraPersistentVolumeTemplates option if needed.

[dimitarvdimitrov]

Essentially generate local-volume PVs for PVCs as I can in a separate template. But maybe I should include it here as an option to generate one with correct naming aka $pv-name-$pod-name-$count?

Aren't PVs resources that k8s operators manage outside of PVCs? My impression was that the separation between PVs and PVCs exists so that they can be decoupled. Having said that, I don't think PVs are in scope of the mimir helm chart. But my k8s understanding is not super up-to-date, I'm happy to be corrected.

[ThelonKarrde]

Aren't PVs resources that k8s operators manage outside of PVCs? My impression was that the separation between PVs and PVCs exists so that they can be decoupled. Having said that, I don't think PVs are in scope of the mimir helm chart. But my k8s understanding is not super up-to-date, I'm happy to be corrected.

Yes, generally PVs are managed outside of the installation. I'm only considering a specific case when it's a local mount with directories from the host.
Something like

{{- range until (int .Values.mimir-distributed.ingester.replicas) }}
apiVersion: v1
kind: PersistentVolume
metadata:
  name: storage-store-ingester-{{ . }}
spec:
  capacity:
    storage: {{ .Values.mimir-distributed.ingester.persistentVolume.size }} 
  volumeMode: Filesystem
  accessModes:
  - ReadWriteOnce
  persistentVolumeReclaimPolicy: Retain
  storageClassName: local-storage
  local:
    path: /mimir/ingester
  nodeAffinity:
    required:
      nodeSelectorTerms:
      - matchExpressions:
        - key: role
          operator: In
          values:
          - mimir 
---
{{- end }}

The only reason why it's nice to have in the main chart it's the ability to reuse naming strings {{ include "mimir.resourceName" (dict "ctx" . "component" "ingester") }} so it's easier to predict names for PVs.

But that is a very specific use case I think so overall this PR can be closed if not to just use as add option extraPersistentVolumeClaims.

[ThelonKarrde]

@dimitarvdimitrov @krajorama just a follow-up here if this should be closed or remain open with changes to discuss?

[dimitarvdimitrov]

I think we are ok to close this since it affects your use case only marginally. If it turns out that there's a bigger need for it, we can consider reopening and merging. Thanks for the proposal and patience :)

[matagyula]

Please excuse the necro-bump, this was the only remotely related topic I could find for an issue that I am facing.

I am struggling to configure our Mimir deployment on EKS with Fargate profiles - EBS volumes can't be used, and so I have set-up EFS. I have also created the PVs in advance for the Mimir components that need it (alert manager, compactor, ingester, and store gateway).

The problem is, that I am unable to provide the volumeName and storageClassName parameters to the aforementioned components via the values.yaml file. The helm install command processes the parameters (they are visible when I do a dry-run), but the resulting Kubernetes resources (PVCs) disregard them completely. Looking at the discussion thread I believe it's because this part of the configuration (stateful sets) isn't "templated".

Thank you for any and all inputs in advance.

Snippet of the values.yaml:

alertmanager:
  persistentVolume:
    enabled: true
    storageClassName: efs-sc
    volumeName: mimir-alertmanager