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 inconsistency in documentation of default storageclass #48200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,11 @@ and automatically adds a default storage class to them.
This way, users that do not request any special storage class do not need to care about them at all and they
will get the default one.

This admission controller does not do anything when no default storage class is configured. When more than one storage
class is marked as default, it rejects any creation of `PersistentVolumeClaim` with an error and an administrator
must revisit their `StorageClass` objects and mark only one as default.
This admission controller does nothing when no default `StorageClass` exists. When more than one storage
class is marked as default, and you then create a `PersistentVolumeClaim` with no `storageClassName` set,
Kubernetes uses the most recently created default `StorageClass`.
When a `PersistentVolumeClaim` is created with a specified `volumeName`, it remains in a pending state
if the static volume's `storageClassName` does not match the `StorageClass` on the `PersistentVolumeClaim`.
Copy link
Contributor

Choose a reason for hiding this comment

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

What if I'm creating a PVC with 'volumeName' specified, but no 'storageClassName'?
The PVC still has to be checked for storage class matching?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the PVC still checks for matching storage class, the below show the events on a PVC with a specified volumeName that has been mutated by the DefaultStroageClass admission controller.

controlplane $ k describe pvc pvc-nginx 
Name:          pvc-nginx
Namespace:     default
StorageClass:  nfs-csi
Status:        Pending
Volume:        pv-nginx
Labels:        <none>
Annotations:   <none>
Finalizers:    [kubernetes.io/pvc-protection]
Capacity:      0
Access Modes:  
VolumeMode:    Filesystem
Used By:       nginx-nfs-example
Events:
  Type     Reason          Age               From                         Message
  ----     ------          ----              ----                         -------
  Warning  VolumeMismatch  3s (x3 over 23s)  persistentvolume-controller  Cannot bind to requested volume "pv-nginx": storageClassName does not match

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean ... remove the 'storageClassName' from the PVC, setting only 'volumeName'.
Will this binding succeed? I guess so.
The admission controller has nothing to check for this PVC, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

The admission controller may backfill the storage class for the PVC when the binding is a success. But that is a different topic. The PVC will not be pending for storage class matching.

Copy link
Contributor Author

@iheartNathan iheartNathan Oct 14, 2024

Choose a reason for hiding this comment

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

I can't remove the storageClass from the PVC, setting only volumeName, the spec is immutable after creation.
As long as here is at least one storage class setup as default, here is also no way to create a PVC without a storageClass, the admission controller is always on the look out for PVC without one specified to mutate it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remove the storageClass from the PVC, setting only volumeName, the spec is immutable after creation.

Immutable should be fine.

As long as here is at least one storage class setup as default, here is also no way to create a PVC without a storageClass, the admission controller is always on the look out for PVC without one specified to mutate it.

This is a surprise for me. Maybe we need to consult SIG Storage for this.

Copy link
Member

Choose a reason for hiding this comment

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

We discussed this at sig storage triage today. The intended behavior is that if you set storageClassName to nil and there is a default storageclass set, then storageClassName will be set with the default storageclass.

If you specifically don't want a default storageclass to be set, then you need to set storageClassName to empty string.

There is a bug with the "retroactive default storageclass" feature though where it's not following this intended behavior, but I don't think we're discussing the retroactive scenario here (ie default storageclass gets configured after the PVC is created)

Copy link
Contributor

@tengqm tengqm Oct 16, 2024

Choose a reason for hiding this comment

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

The question is this, @msau42:

If I am creating a PVC in a cluster, and the cluster has a default storage class.
I'm not specifying storageClassName in my PVC spec, and I'm only specifying volumeName, can the PVC be created?

Right. I can understand that the PVC may and may not receive a storageClassName after creation, but that is a different topic.

This admission controller ignores any `PersistentVolumeClaim` updates; it acts only on creation.

See [persistent volume](/docs/concepts/storage/persistent-volumes/) documentation about persistent volume claims and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,13 @@ for details about addon manager and how to disable individual addons.
kubectl patch storageclass gold -p '{"metadata": {"annotations":{"storageclass.kubernetes.io/is-default-class":"true"}}}'
```

Please note that at most one StorageClass can be marked as default. If two
or more of them are marked as default, a `PersistentVolumeClaim` without
`storageClassName` explicitly specified cannot be created.
Please note you can have multiple `StorageClass` marked as default. If more
than one `StorageClass` is marked as default, a `PersistentVolumeClaim` without
an explicitly defined `storageClassName` will be created using the most recently
created default `StorageClass`.
When a `PersistentVolumeClaim` is created with a specified `volumeName`, it remains
in a pending state if the static volume's `storageClassName` does not match the
`StorageClass` on the `PersistentVolumeClaim`.

1. Verify that your chosen StorageClass is default:

Expand Down