Don't assign a storage class to claim when the user asked for a specific volume.

[carlory]

What type of PR is this?

/kind feature

What this PR does / why we need it:

There are 2 approaches to assigning a default storage class to a persistent volume claim (PVC) in Kubernetes when the storage class is not specified in the PVC object. The first approach is to use DefaultStorageClass admission plugin which assigns a default storage class to a PVC if the storage class field of the PVC is unset when it is created. The limitation of this approach is that it only works for PVCs created after the admission plugin is enabled and the default storage class existed in the cluster before the PVC was created. The second approach unlocks the limitation of the first approach by leveraging the PV controller to retroactively assign a default storage class to PVCs when it reconciles the unbound PVC objects if the PVC object does not have a storage class and no user-specified volume name is set.

There is a gap between the two approaches on how to handle the PVC when the PVC has a user-specified volume name.

• https://github.com/carlory/kubernetes/blob/master/plugin/pkg/admission/storage/storageclass/setdefault/admission.go#L108
• https://github.com/carlory/kubernetes/blob/master/pkg/controller/volume/persistentvolume/pv_controller.go#L335

It may make users confused. See kubernetes/website#42288 and kubernetes/website#48200 (comment)

StorageClass describes the parameters for a class of storage for which PersistentVolumes can be dynamically provisioned. When a user creates a PVC with a given storage class, if the PVC does not contain a user-specified volume name, the PV controller or external provisioner will dynamically provision a PV for the PVC.

If a user asks for a given volume, the user must create the PV object manually because the volume won't be dynamically provisioned.

• https://github.com/carlory/kubernetes/blob/master/pkg/controller/volume/persistentvolume/pv_controller.go#L414
• https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/blob/master/controller/controller.go#L1213

There is a side effect for the user-specified volume case, see kubernetes/website#48200 (comment).

Even if we have clarified the breaking change on the storage class field of claim in the KEP, user should assign an empty storage class to the PVC object explicitly when the PVC has an user-specified volume name (static provision).

• https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/3333-retroactive-default-storage-class#behavior-change
• https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/3333-retroactive-default-storage-class#risks-and-mitigations

And the website is updated to reflect the change. But some users may still get confused. Why need to assign an empty storage class to the PVC object explicitly when the PVC has an user-specified volume name?

I have a proposal to improve the user experience. It will clear the confusion and make the behavior more consistent and predictable.

1. DefaultStorageClass admission plugin does not again assign a default storage class to the PVC object when the PVC has a user-specified volume name.
2. kube-apiserver will reject the PVC object updation if a user/controller tries to update the storage class field of the PVC object from nil to non-nil when the PVC has a user-specified volume name. (I'm not sure whether it should be applied)

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

DefaultStorageClass admission plugins would not assign a default storage class to a claim when the user asked for a specific volume.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[k8s-ci-robot]

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: carlory
Once this PR has been reviewed and has the lgtm label, please assign jpbetz for approval. For more information see the Kubernetes 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:

• plugin/pkg/admission/OWNERS

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

[carlory]

/cc @msau42 @jsafrane

/cc @tengqm @sftim @iheartNathan

[k8s-ci-robot]

@carlory: GitHub didn't allow me to request PR reviews from the following users: iheartNathan.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @msau42 @jsafrane

/cc @tengqm @sftim @iheartNathan

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.

[carlory]

/sig storage

[carlory]

/close

due to kubernetes/website#48200 (comment)

[k8s-ci-robot]

@carlory: Closed this PR.

In response to this:

/close

due to kubernetes/website#48200 (comment)

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.