From 4c3c4128afb92025f960a23d80b1fc5be0fc56c8 Mon Sep 17 00:00:00 2001 From: bells17 Date: Mon, 20 May 2024 13:32:37 +0900 Subject: [PATCH] volumebinding: scheduler queueing hints - StorageClass --- .../plugins/volumebinding/volume_binding.go | 35 +++++++- .../volumebinding/volume_binding_test.go | 88 +++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index 2f60d25f571..0feb002bf75 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -25,6 +25,7 @@ import ( v1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" corelisters "k8s.io/client-go/listers/core/v1" @@ -98,7 +99,7 @@ func (pl *VolumeBinding) EventsToRegister() []framework.ClusterEventWithHint { // Pods may fail because of missing or mis-configured storage class // (e.g., allowedTopologies, volumeBindingMode), and hence may become // schedulable upon StorageClass Add or Update events. - {Event: framework.ClusterEvent{Resource: framework.StorageClass, ActionType: framework.Add | framework.Update}}, + {Event: framework.ClusterEvent{Resource: framework.StorageClass, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterStorageClassChange}, // We bind PVCs with PVs, so any changes may make the pods schedulable. {Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterPersistentVolumeClaimChange}, @@ -195,6 +196,38 @@ func (pl *VolumeBinding) isSchedulableAfterPersistentVolumeClaimChange(logger kl return framework.QueueSkip, nil } +// isSchedulableAfterStorageClassChange checks whether an StorageClass event might make a Pod schedulable or not. +// Any StorageClass addition and a StorageClass update to allowedTopologies +// might make a Pod schedulable. +// Note that an update to volume binding mode is not allowed and we don't have to consider while examining the update event. +func (pl *VolumeBinding) isSchedulableAfterStorageClassChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { + oldSC, newSC, err := util.As[*storagev1.StorageClass](oldObj, newObj) + if err != nil { + return framework.Queue, err + } + + logger = klog.LoggerWithValues( + logger, + "Pod", klog.KObj(pod), + "StorageClass", klog.KObj(newSC), + ) + + if oldSC == nil { + // No further filtering can be made for a creation event, + // and we just always return Queue. + logger.V(5).Info("A new StorageClass was created, which could make a Pod schedulable") + return framework.Queue, nil + } + + if !apiequality.Semantic.DeepEqual(newSC.AllowedTopologies, oldSC.AllowedTopologies) { + logger.V(5).Info("StorageClass got an update in AllowedTopologies", "AllowedTopologies", newSC.AllowedTopologies) + return framework.Queue, nil + } + + logger.V(5).Info("StorageClass was updated, but it doesn't make this pod schedulable") + return framework.QueueSkip, nil +} + // podHasPVCs returns 2 values: // - the first one to denote if the given "pod" has any PVC defined. // - the second one to return any error if the requested PVC is illegal. diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go index 232db2d56aa..2904d0cdab3 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -1093,3 +1093,91 @@ func TestIsSchedulableAfterPersistentVolumeClaimChange(t *testing.T) { }) } } + +func TestIsSchedulableAfterStorageClassChange(t *testing.T) { + table := []struct { + name string + pod *v1.Pod + oldSC interface{} + newSC interface{} + pvcLister tf.PersistentVolumeClaimLister + err bool + expect framework.QueueingHint + }{ + { + name: "When a new StorageClass is created, it returns Queue", + pod: makePod("pod-a").Pod, + oldSC: nil, + newSC: &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-a", + }, + }, + err: false, + expect: framework.Queue, + }, + { + name: "When the AllowedTopologies are changed, it returns Queue", + pod: makePod("pod-a").Pod, + oldSC: &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-a", + }, + }, + newSC: &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-a", + }, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: "kubernetes.io/hostname", + Values: []string{"node-a"}, + }, + }, + }, + }, + }, + err: false, + expect: framework.Queue, + }, + { + name: "When there are no changes to the StorageClass, it returns QueueSkip", + pod: makePod("pod-a").Pod, + oldSC: &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-a", + }, + }, + newSC: &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sc-a", + }, + }, + err: false, + expect: framework.QueueSkip, + }, + { + name: "type conversion error", + oldSC: new(struct{}), + newSC: new(struct{}), + err: true, + expect: framework.Queue, + }, + } + + for _, item := range table { + t.Run(item.name, func(t *testing.T) { + pl := &VolumeBinding{PVCLister: item.pvcLister} + logger, _ := ktesting.NewTestContext(t) + qhint, err := pl.isSchedulableAfterStorageClassChange(logger, item.pod, item.oldSC, item.newSC) + if (err != nil) != item.err { + t.Errorf("isSchedulableAfterStorageClassChange failed - got: %q", err) + } + if qhint != item.expect { + t.Errorf("QHint does not match: %v, want: %v", qhint, item.expect) + } + }) + } +}