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.

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

KEP-3333 Retroactive default StorageClass assignment #3337

Merged
merged 10 commits into from
Jun 15, 2022
Merged

KEP-3333 Retroactive default StorageClass assignment #3337

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c6afdd4
Add KEP for Retroactive default StorageClass assignment
RomanBednar Jun 3, 2022
03d09b5
Address review comments
RomanBednar Jun 6, 2022
4756dd6
Add PRR
RomanBednar Jun 6, 2022
7a33586
Add PRR approval file
RomanBednar Jun 6, 2022
c53beb7
Address review comments 2
RomanBednar Jun 8, 2022
cd609a8
Address review comments 3
RomanBednar Jun 9, 2022
e1b4d81
Address review comments 4
RomanBednar Jun 10, 2022
9073730
Rewrite KEP to alternative considered
RomanBednar Jun 13, 2022
6d49829
Change status to implementable
RomanBednar Jun 14, 2022
c6d69a7
Address review comments 5
RomanBednar Jun 15, 2022
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
12 changes: 8 additions & 4 deletions keps/sig-storage/3333-reconcile-default-storage-class/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,23 +96,27 @@ cases that can be problematic:
1. It’s hard to mark a different SC as the default one. Cluster admin can
RomanBednar marked this conversation as resolved.
Show resolved Hide resolved
choose between two bad solutions:

1. Cluster has two default SCs for a short time, i.e. admin marks the new
1. Option 1: Cluster has two default SCs for a short time, i.e. admin marks the new
Copy link
Member

Choose a reason for hiding this comment

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

kubernetes/kubernetes#110514

default SC as default and then marks the old default SC as non-default. When
there are two default SCs in a cluster, the PVC admission plugin refuses to
accept new PVCs with `pvc.spec.storageClassName = nil`. Hence, cluster users
may get errors when creating PVCs at the wrong time. They must know it’s a
transient error and manually retry later.

2. Cluster has no default SC for a short time, i.e. admin marks the old
2. Option 2: Cluster has no default SC for a short time, i.e. admin marks the old
default SC as non-default and then marks the new default SC as default.
Since there is no default SC for some time, PVCs with
`pvc.spec.storageClassName = nil` created during this time will not get any
SC and are Pending forever. Users must be smart enough to delete the PVC and
re-create it.

2. When users want to change the default SC parameters, they must delete the SC
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we advise them to make a new SC and flip the default as I described above?

Copy link
Member

Choose a reason for hiding this comment

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

No, there may be quota on the original SC. Admins may want to re-create a default SC to keep the quota numbers. Re-create means there is no default SC for a short time.

@RomanBednar perhaps you could add a note about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Member

Choose a reason for hiding this comment

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

If we are encouraging people to delete a class and recreate it with different params, we might as well allow them to mutate it. Do we ever refer back to the class after the volume is created?

Copy link
Member

Choose a reason for hiding this comment

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

If we are encouraging people to delete a class and recreate it with different params, we might as well allow them to mutate it

+1. Do we need a KEP for this?

and re-create it, Kubernetes API does not allow change in the SC. So there is no
default SC for some time and second case above applies here too.
and re-create it, Kubernetes API does not allow change in the SC. So there is
no default SC for some time and second case above applies here too.
Re-creating the storage class to change parameters can be useful in cases
there is a quota set for the SC, and since the quota is coupled with SC name
users can not use Option 1 because second SC would have a different name
and so the existing quota would not apply to it.

3. Defined ordering during cluster installation. Kubernetes cluster installation
tools must be currently smart enough to create a default SC before starting
Expand Down