diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 7d11120d023..a40921a51ba 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -773,23 +773,6 @@ func TestRetroactiveStorageClassAssignment(t *testing.T) { }, }, }, - { - storageClasses: []*storagev1.StorageClass{ - makeDefaultStorageClass(classGold, &modeImmediate), - makeDefaultStorageClass(classSilver, &modeImmediate)}, - tests: []controllerTest{ - { - name: "15-2 - pvc storage class is not assigned retroactively if there are multiple default storage classes", - initialVolumes: novolumes, - expectedVolumes: novolumes, - initialClaims: newClaimArray("claim15-2", "uid15-2", "1Gi", "", v1.ClaimPending, nil), - expectedClaims: newClaimArray("claim15-2", "uid15-2", "1Gi", "", v1.ClaimPending, nil), - expectedEvents: noevents, - errors: noerrors, - test: testSyncClaim, - }, - }, - }, { storageClasses: []*storagev1.StorageClass{ makeDefaultStorageClass(classGold, &modeImmediate), @@ -844,6 +827,23 @@ func TestRetroactiveStorageClassAssignment(t *testing.T) { }, }, }, + { + storageClasses: []*storagev1.StorageClass{ + makeDefaultStorageClass(classGold, &modeImmediate), + makeDefaultStorageClass(classSilver, &modeImmediate)}, + tests: []controllerTest{ + { + name: "15-2 - pvc storage class is assigned retroactively if there are multiple default storage classes", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-2", "uid15-2", "1Gi", "", v1.ClaimPending, nil), + expectedClaims: newClaimArray("claim15-2", "uid15-2", "1Gi", "", v1.ClaimPending, &classGold), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, { storageClasses: []*storagev1.StorageClass{ makeDefaultStorageClass(classGold, &modeImmediate), diff --git a/pkg/volume/util/storageclass.go b/pkg/volume/util/storageclass.go index 5350bc8e963..d2098c25800 100644 --- a/pkg/volume/util/storageclass.go +++ b/pkg/volume/util/storageclass.go @@ -17,9 +17,9 @@ limitations under the License. package util import ( - "fmt" + "sort" + storagev1 "k8s.io/api/storage/v1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" storagev1listers "k8s.io/client-go/listers/storage/v1" @@ -54,10 +54,19 @@ func GetDefaultClass(lister storagev1listers.StorageClassLister) (*storagev1.Sto if len(defaultClasses) == 0 { return nil, nil } + + // Primary sort by creation timestamp, newest first + // Secondary sort by class name, ascending order + sort.Slice(defaultClasses, func(i, j int) bool { + if defaultClasses[i].CreationTimestamp.UnixNano() == defaultClasses[j].CreationTimestamp.UnixNano() { + return defaultClasses[i].Name < defaultClasses[j].Name + } + return defaultClasses[i].CreationTimestamp.UnixNano() > defaultClasses[j].CreationTimestamp.UnixNano() + }) if len(defaultClasses) > 1 { - klog.V(4).Infof("GetDefaultClass %d defaults found", len(defaultClasses)) - return nil, errors.NewInternalError(fmt.Errorf("%d default StorageClasses were found", len(defaultClasses))) + klog.V(4).Infof("%d default StorageClasses were found, choosing the newest: %s", len(defaultClasses), defaultClasses[0].Name) } + return defaultClasses[0], nil } diff --git a/plugin/pkg/admission/storage/storageclass/setdefault/admission_test.go b/plugin/pkg/admission/storage/storageclass/setdefault/admission_test.go index da48d69e7d0..9883fd97e0b 100644 --- a/plugin/pkg/admission/storage/storageclass/setdefault/admission_test.go +++ b/plugin/pkg/admission/storage/storageclass/setdefault/admission_test.go @@ -19,6 +19,7 @@ package setdefault import ( "context" "testing" + "time" "k8s.io/klog/v2" @@ -96,6 +97,30 @@ func TestAdmission(t *testing.T) { }, Provisioner: "nondefault1", } + classWithCreateTime1 := &storagev1.StorageClass{ + TypeMeta: metav1.TypeMeta{ + Kind: "StorageClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "default1", + CreationTimestamp: metav1.NewTime(time.Date(2022, time.Month(1), 1, 0, 0, 0, 1, time.UTC)), + Annotations: map[string]string{ + storageutil.IsDefaultStorageClassAnnotation: "true", + }, + }, + } + classWithCreateTime2 := &storagev1.StorageClass{ + TypeMeta: metav1.TypeMeta{ + Kind: "StorageClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "default2", + CreationTimestamp: metav1.NewTime(time.Date(2022, time.Month(1), 1, 0, 0, 0, 0, time.UTC)), + Annotations: map[string]string{ + storageutil.IsDefaultStorageClassAnnotation: "true", + }, + }, + } claimWithClass := &api.PersistentVolumeClaim{ TypeMeta: metav1.TypeMeta{ @@ -166,13 +191,6 @@ func TestAdmission(t *testing.T) { false, "foo", }, - { - "two defaults, error with PVC with class=nil", - []*storagev1.StorageClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, - claimWithNoClass, - true, - "", - }, { "two defaults, no modification of PVC with class=''", []*storagev1.StorageClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, @@ -187,6 +205,20 @@ func TestAdmission(t *testing.T) { false, "foo", }, + { + "two defaults with same creation time, choose the one with smaller name", + []*storagev1.StorageClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + claimWithNoClass, + false, + defaultClass1.Name, + }, + { + "two defaults, choose the one with newer creation time", + []*storagev1.StorageClass{classWithCreateTime1, classWithCreateTime2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + claimWithNoClass, + false, + classWithCreateTime1.Name, + }, } for _, test := range tests {