Skip to content

Add support for multiple storage classes in Helm chart #974

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Add support for multiple storage classes in Helm chart #974

wants to merge 5 commits into from

Conversation

Copy link

Copilot AI commented Oct 7, 2025 •
edited
Loading

Overview

This PR adds support for creating multiple storage classes with different configurations in a single Helm chart installation, addressing the feature request to support scenarios like having separate storage classes for Delete and Retain reclaim policies.

Changes

New Feature: storageClasses Array

Added a new storageClasses array parameter in values.yaml that allows users to define multiple storage classes with different configurations:

storageClasses:
  - name: nfs-delete
    annotations:
      storageclass.kubernetes.io/is-default-class: "true"
    parameters:
      server: nfs-server.default.svc.cluster.local
      share: /
    reclaimPolicy: Delete
    volumeBindingMode: Immediate
    mountOptions:
      - nfsvers=4.1
  - name: nfs-retain
    parameters:
      server: nfs-server.default.svc.cluster.local
      share: /data
    reclaimPolicy: Retain
    volumeBindingMode: Immediate
    mountOptions:
      - nfsvers=4.1

Backward Compatibility

The existing single storageClass configuration continues to work exactly as before:

storageClass:
  create: true
  name: nfs-csi
  parameters:
    server: nfs-server.default.svc.cluster.local
    share: /
  reclaimPolicy: Delete

Both configurations can even coexist if needed.

Smart Defaults

The implementation provides sensible defaults for optional fields:

  • reclaimPolicy: Delete
  • volumeBindingMode: Immediate
  • allowVolumeExpansion: true

Technical Improvements

  • Fixed boolean handling for allowVolumeExpansion to correctly process explicit false values using hasKey check
  • Updated template to iterate over storage class array using Helm's range function
  • Proper context passing ($) for accessing global values within the loop

Files Modified

  • charts/latest/csi-driver-nfs/templates/storageclass.yaml
  • charts/latest/csi-driver-nfs/values.yaml
  • charts/v4.11.0/csi-driver-nfs/templates/storageclass.yaml
  • charts/v4.11.0/csi-driver-nfs/values.yaml
  • charts/v4.10.0/csi-driver-nfs/templates/storageclass.yaml
  • charts/v4.10.0/csi-driver-nfs/values.yaml
  • charts/README.md

Testing

All changes have been thoroughly tested:

  • ✅ helm lint passes for all chart versions
  • ✅ Backward compatibility verified with existing single storageClass configuration
  • ✅ Multiple storage classes creation tested with 2-3 classes
  • ✅ All configuration options validated (annotations, parameters, mountOptions, reclaimPolicy, volumeBindingMode, allowVolumeExpansion)
  • ✅ Repository verification scripts pass successfully

Documentation

Updated charts/README.md with:

  • New storageClasses parameter documentation
  • Example usage showing multiple storage classes
  • Installation instructions with custom values file

Fixes #[issue_number]

Original prompt

This section details on the original issue you should resolve

<issue_title>Multiple storageclasses</issue_title>
<issue_description>Hi!

It would be nice to create multiple storageclass with the hem chart.

Actually is pissoble to definte a storageclass with the helmchart, but i think it wouldbe a nice feature to create more. For example, one for delete, one for retain.

An exmaple, how i magine it.

storageclasses:
  - name: nfs-delete
    annotations:
      storageclass.kubernetes.io/is-default-class: "true"
    parameters:
    server: nfs-server.default.svc.cluster.local
    share: /
    subDir:
    mountPermissions: "0"
    csi.storage.k8s.io/provisioner-secret is only needed for providing mountOptions in DeleteVolume
    csi.storage.k8s.io/provisioner-secret-name: "mount-options"
    csi.storage.k8s.io/provisioner-secret-namespace: "default"
  reclaimPolicy: Delete
  volumeBindingMode: Immediate
  mountOptions:
    - nfsvers=4.1
  - name: nfs-retain
    annotations:
      storageclass.kubernetes.io/is-default-class: "true"
    parameters:
      server: nfs-server.default.svc.cluster.local
      share: /
      subDir:
      mountPermissions: "0"
      csi.storage.k8s.io/provisioner-secret is only needed for providing mountOptions in DeleteVolume
      csi.storage.k8s.io/provisioner-secret-name: "mount-options"
      csi.storage.k8s.io/provisioner-secret-namespace: "default"
  reclaimPolicy: Retain
  volumeBindingMode: Immediate
  mountOptions:
    - nfsvers=4.1

</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #945

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@k8s-ci-robot
Copy link
Contributor

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 7, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @Copilot. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 7, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Copilot
Once this PR has been reviewed and has the lgtm label, please ask for approval from andyzhangx. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Copilot
Once this PR has been reviewed and has the lgtm label, please ask for approval from andyzhangx. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2025
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 7, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 18305664509

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 78.693%
Totals Coverage Status
Change from base Build 18299884864: 0.0%
Covered Lines: 1108
Relevant Lines: 1408
💛 - Coveralls

Copilot AI changed the title [WIP] Add support for multiple storage classes in helm chart Add support for multiple storage classes in Helm chart Oct 7, 2025
Copilot finished work on behalf of andyzhangx October 7, 2025 08:04
Copilot AI requested a review from andyzhangx October 7, 2025 08:04
@andyzhangx andyzhangx closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andyzhangx andyzhangx Awaiting requested review from andyzhangx

Assignees

@andyzhangx andyzhangx

Copilot code review Copilot

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Multiple storageclasses

4 participants

@k8s-ci-robot @coveralls @andyzhangx