From f12325add3fbac8a2ce31cb12b4649c01d74f248 Mon Sep 17 00:00:00 2001 From: danishprakash Date: Tue, 14 Jun 2022 10:42:03 +0530 Subject: [PATCH] pkg/admission/storageclass: pick random storageclass if >1 present Signed-off-by: danishprakash --- pkg/volume/util/storageclass.go | 15 ++++-- .../storageclass/setdefault/admission.go | 1 + .../storageclass/setdefault/admission_test.go | 46 ++++++++++++++++--- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/pkg/volume/util/storageclass.go b/pkg/volume/util/storageclass.go index 5350bc8e963..fd1e9c4fddf 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,17 @@ func GetDefaultClass(lister storagev1listers.StorageClassLister) (*storagev1.Sto if len(defaultClasses) == 0 { return nil, nil } + + 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.go b/plugin/pkg/admission/storage/storageclass/setdefault/admission.go index aa4c1c14ba7..4772996a1a7 100644 --- a/plugin/pkg/admission/storage/storageclass/setdefault/admission.go +++ b/plugin/pkg/admission/storage/storageclass/setdefault/admission.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "k8s.io/kubernetes/pkg/volume/util" "k8s.io/klog/v2" 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 {