From 731068288e112c8b5af70f676296cc44661e84f4 Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Mon, 3 Apr 2023 11:27:59 +0200 Subject: [PATCH 1/2] correct storage class selection message The function does not necessarily choose class based on the creation timestamp but can also pick alphabetically first if the timestamps are equal. The info message should not say it's choosing the newest because it is misleading. --- pkg/volume/util/storageclass.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/volume/util/storageclass.go b/pkg/volume/util/storageclass.go index d2098c25800..223eb9dc21c 100644 --- a/pkg/volume/util/storageclass.go +++ b/pkg/volume/util/storageclass.go @@ -64,7 +64,7 @@ func GetDefaultClass(lister storagev1listers.StorageClassLister) (*storagev1.Sto return defaultClasses[i].CreationTimestamp.UnixNano() > defaultClasses[j].CreationTimestamp.UnixNano() }) if len(defaultClasses) > 1 { - klog.V(4).Infof("%d default StorageClasses were found, choosing the newest: %s", len(defaultClasses), defaultClasses[0].Name) + klog.V(4).Infof("%d default StorageClasses were found, choosing: %s", len(defaultClasses), defaultClasses[0].Name) } return defaultClasses[0], nil From 1bd3f072fa7cff22cbab2e581ff1bbc6f2d7dcf4 Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Mon, 3 Apr 2023 11:32:07 +0200 Subject: [PATCH 2/2] stop ignoring storage class selection errors The GetDefaultClass() was fixed in scope of this issue: https://github.com/kubernetes/kubernetes/issues/110514 Before this change assignDefaultStorageClass() was ignoring errors from this function since it could mean there are multiple defaults - assign could safely continue and do nothing. This is no longer true because we always choose one from multiple defaults - any errors returned from GetDefaultClass() are real errors and should not be ignored. --- pkg/controller/volume/persistentvolume/pv_controller.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index a58be28164e..ba25bdb7c44 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -955,10 +955,7 @@ func (ctrl *PersistentVolumeController) assignDefaultStorageClass(ctx context.Co class, err := util.GetDefaultClass(ctrl.classLister) if err != nil { - // It is safe to ignore errors here because it means we either could not list SCs or there is more than one default. - // TODO: do not ignore errors after this PR is merged: https://github.com/kubernetes/kubernetes/pull/110559 - logger.V(4).Info("Failed to get default storage class", "err", err) - return false, nil + return false, err } else if class == nil { logger.V(4).Info("Can not assign storage class to PersistentVolumeClaim: default storage class not found", "PVC", klog.KObj(claim)) return false, nil