diff --git a/pkg/scheduler/framework/plugins/volumebinding/test_utils.go b/pkg/scheduler/framework/plugins/volumebinding/test_utils.go index a1c968d76fc..23675f969d6 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/test_utils.go +++ b/pkg/scheduler/framework/plugins/volumebinding/test_utils.go @@ -204,3 +204,14 @@ func (pb podBuilder) withGenericEphemeralVolume(name string) podBuilder { }) return pb } + +func (pb podBuilder) withCSI(driver string) podBuilder { + pb.Pod.Spec.Volumes = append(pb.Pod.Spec.Volumes, v1.Volume{ + VolumeSource: v1.VolumeSource{ + CSI: &v1.CSIVolumeSource{ + Driver: driver, + }, + }, + }) + return pb +} diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index 8bafaa52123..3eff2df8b0e 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -126,7 +126,7 @@ func (pl *VolumeBinding) EventsToRegister(_ context.Context) ([]framework.Cluste // When CSIStorageCapacity is enabled, pods may become schedulable // on CSI driver & storage capacity changes. - {Event: framework.ClusterEvent{Resource: framework.CSIDriver, ActionType: framework.Add | framework.Update}}, + {Event: framework.ClusterEvent{Resource: framework.CSIDriver, ActionType: framework.Update}, QueueingHintFn: pl.isSchedulableAfterCSIDriverChange}, {Event: framework.ClusterEvent{Resource: framework.CSIStorageCapacity, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterCSIStorageCapacityChange}, } return events, nil @@ -270,6 +270,33 @@ func (pl *VolumeBinding) isSchedulableAfterCSIStorageCapacityChange(logger klog. return framework.QueueSkip, nil } +func (pl *VolumeBinding) isSchedulableAfterCSIDriverChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { + originalCSIDriver, modifiedCSIDriver, err := util.As[*storagev1.CSIDriver](oldObj, newObj) + if err != nil { + return framework.Queue, err + } + + logger = klog.LoggerWithValues( + logger, + "Pod", klog.KObj(pod), + "CSIDriver", klog.KObj(modifiedCSIDriver), + ) + + for _, vol := range pod.Spec.Volumes { + if vol.CSI == nil || vol.CSI.Driver != modifiedCSIDriver.Name { + continue + } + if (originalCSIDriver.Spec.StorageCapacity != nil && *originalCSIDriver.Spec.StorageCapacity) && + (modifiedCSIDriver.Spec.StorageCapacity == nil || !*modifiedCSIDriver.Spec.StorageCapacity) { + logger.V(5).Info("CSIDriver was updated and storage capacity got disabled, which may make the pod schedulable") + return framework.Queue, nil + } + } + + logger.V(5).Info("CSIDriver was created or 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 127c2c606cc..1f115b52a82 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -35,6 +35,7 @@ import ( "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" "k8s.io/kubernetes/pkg/scheduler/framework/runtime" tf "k8s.io/kubernetes/pkg/scheduler/testing/framework" + "k8s.io/utils/ptr" ) var ( @@ -1372,3 +1373,188 @@ func TestIsSchedulableAfterCSIStorageCapacityChange(t *testing.T) { }) } } + +func TestIsSchedulableAfterCSIDriverChange(t *testing.T) { + table := []struct { + name string + pod *v1.Pod + newObj interface{} + oldObj interface{} + err bool + expect framework.QueueingHint + }{ + { + name: "pod has no CSIDriver", + pod: makePod("pod-a").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(true), + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + }, + err: false, + expect: framework.QueueSkip, + }, + { + name: "unexpected objects are passed", + pod: makePod("pod-a").Pod, + newObj: new(struct{}), + oldObj: new(struct{}), + err: true, + expect: framework.Queue, + }, + { + name: "driver name in pod and csidriver name are different", + pod: makePod("pod-a").withCSI("test1").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test2", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(false), + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test2", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(true), + }, + }, + err: false, + expect: framework.QueueSkip, + }, + { + name: "original StorageCapacity is nil", + pod: makePod("pod-a").withCSI("test1").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: nil, + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: nil, + }, + }, + err: false, + expect: framework.QueueSkip, + }, + { + name: "original StorageCapacity is false", + pod: makePod("pod-a").withCSI("test1").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(false), + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(false), + }, + }, + err: false, + expect: framework.QueueSkip, + }, + { + name: "modified StorageCapacity is nil", + pod: makePod("pod-a").withCSI("test1").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: nil, + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(true), + }, + }, + err: false, + expect: framework.Queue, + }, + { + name: "modified StorageCapacity is true", + pod: makePod("pod-a").withCSI("test1").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(true), + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(true), + }, + }, + err: false, + expect: framework.QueueSkip, + }, + + { + name: "modified StorageCapacity is false", + pod: makePod("pod-a").withCSI("test1").Pod, + newObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(false), + }, + }, + oldObj: &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + }, + Spec: storagev1.CSIDriverSpec{ + StorageCapacity: ptr.To(true), + }, + }, + err: false, + expect: framework.Queue, + }, + } + for _, item := range table { + t.Run(item.name, func(t *testing.T) { + pl := &VolumeBinding{} + logger, _ := ktesting.NewTestContext(t) + qhint, err := pl.isSchedulableAfterCSIDriverChange(logger, item.pod, item.oldObj, item.newObj) + if (err != nil) != item.err { + t.Errorf("isSchedulableAfterCSINodeChange failed - got: %q", err) + } + if qhint != item.expect { + t.Errorf("QHint does not match: %v, want: %v", qhint, item.expect) + } + }) + } +} diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index b146afbbd7c..e8e2c833892 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -874,7 +874,7 @@ func Test_UnionedGVKs(t *testing.T) { framework.Pod: framework.Add | framework.UpdatePodLabel | framework.Delete, framework.Node: framework.Add | framework.UpdateNodeAllocatable | framework.UpdateNodeLabel | framework.UpdateNodeTaint | framework.Delete, framework.CSINode: framework.All - framework.Delete, - framework.CSIDriver: framework.All - framework.Delete, + framework.CSIDriver: framework.Update, framework.CSIStorageCapacity: framework.All - framework.Delete, framework.PersistentVolume: framework.All - framework.Delete, framework.PersistentVolumeClaim: framework.All - framework.Delete, @@ -888,7 +888,7 @@ func Test_UnionedGVKs(t *testing.T) { framework.Pod: framework.Add | framework.UpdatePodLabel | framework.UpdatePodScaleDown | framework.Delete, framework.Node: framework.Add | framework.UpdateNodeAllocatable | framework.UpdateNodeLabel | framework.UpdateNodeTaint | framework.Delete, framework.CSINode: framework.All - framework.Delete, - framework.CSIDriver: framework.All - framework.Delete, + framework.CSIDriver: framework.Update, framework.CSIStorageCapacity: framework.All - framework.Delete, framework.PersistentVolume: framework.All - framework.Delete, framework.PersistentVolumeClaim: framework.All - framework.Delete, @@ -903,7 +903,7 @@ func Test_UnionedGVKs(t *testing.T) { framework.Pod: framework.Add | framework.UpdatePodLabel | framework.UpdatePodScaleDown | framework.UpdatePodTolerations | framework.UpdatePodSchedulingGatesEliminated | framework.Delete, framework.Node: framework.Add | framework.UpdateNodeAllocatable | framework.UpdateNodeLabel | framework.UpdateNodeTaint | framework.Delete, framework.CSINode: framework.All - framework.Delete, - framework.CSIDriver: framework.All - framework.Delete, + framework.CSIDriver: framework.Update, framework.CSIStorageCapacity: framework.All - framework.Delete, framework.PersistentVolume: framework.All - framework.Delete, framework.PersistentVolumeClaim: framework.All - framework.Delete,