From a7242fcff768658019f878cb691583dcbcfefb2d Mon Sep 17 00:00:00 2001 From: Toru Komatsu Date: Wed, 14 Aug 2024 13:02:42 +0900 Subject: [PATCH] Implement PVC/Add QueueingHint in CSILimit plugin (#124703) Signed-off-by: utam0k --- .../framework/plugins/nodevolumelimits/csi.go | 34 +++++++- .../plugins/nodevolumelimits/csi_test.go | 77 +++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go index 9c2e037fd7f..a0a1ae0cd09 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go @@ -82,7 +82,7 @@ func (pl *CSILimits) EventsToRegister(_ context.Context) ([]framework.ClusterEve // because any new CSINode could make pods that were rejected by CSI volumes schedulable. {Event: framework.ClusterEvent{Resource: framework.CSINode, ActionType: framework.Add}}, {Event: framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Delete}, QueueingHintFn: pl.isSchedulableAfterPodDeleted}, - {Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add}}, + {Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add}, QueueingHintFn: pl.isSchedulableAfterPVCAdded}, }, nil } @@ -110,6 +110,38 @@ func (pl *CSILimits) isSchedulableAfterPodDeleted(logger klog.Logger, pod *v1.Po return framework.QueueSkip, nil } +func (pl *CSILimits) isSchedulableAfterPVCAdded(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { + _, addedPvc, err := util.As[*v1.PersistentVolumeClaim](oldObj, newObj) + if err != nil { + return framework.Queue, fmt.Errorf("unexpected objects in isSchedulableAfterPVCAdded: %w", err) + } + + if addedPvc.Namespace != pod.Namespace { + return framework.QueueSkip, nil + } + + for _, volumes := range pod.Spec.Volumes { + var pvcName string + switch { + case volumes.PersistentVolumeClaim != nil: + pvcName = volumes.PersistentVolumeClaim.ClaimName + case volumes.Ephemeral != nil: + pvcName = ephemeral.VolumeClaimName(pod, &volumes) + default: + // Volume is not using a PVC, ignore + continue + } + + if pvcName == addedPvc.Name { + logger.V(5).Info("PVC that is referred from the pod was created, which might make this pod schedulable", "pod", klog.KObj(pod), "PVC", klog.KObj(addedPvc)) + return framework.Queue, nil + } + } + + logger.V(5).Info("PVC irrelevant to the Pod was created, which doesn't make this pod schedulable", "pod", klog.KObj(pod), "PVC", klog.KObj(addedPvc)) + return framework.QueueSkip, nil +} + // PreFilter invoked at the prefilter extension point // // If the pod haven't those types of volumes, we'll skip the Filter phase diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go index d74d52b2597..a10cbbd9998 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go @@ -706,6 +706,83 @@ func TestCSILimitsQHint(t *testing.T) { } } +func TestCSILimitsAddedPVCQHint(t *testing.T) { + tests := []struct { + test string + newPod *v1.Pod + addedPvc *v1.PersistentVolumeClaim + wantQHint framework.QueueingHint + }{ + { + test: "a pod isn't in the same namespace as an added PVC", + newPod: st.MakePod().Namespace("ns1").Obj(), + addedPvc: st.MakePersistentVolumeClaim().Namespace("ns2").Obj(), + wantQHint: framework.QueueSkip, + }, + { + test: "a pod is in the same namespace as an added PVC", + newPod: st.MakePod().Namespace("ns1").Volume( + v1.Volume{ + VolumeSource: v1.VolumeSource{ + PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc1", + }, + }, + }).Obj(), + addedPvc: st.MakePersistentVolumeClaim().Name("pvc1").Namespace("ns1").Obj(), + wantQHint: framework.Queue, + }, + { + test: "a pod has an ephemeral volume related to an added PVC", + newPod: st.MakePod().Name("pod1").Namespace("ns1").Volume( + v1.Volume{ + Name: "ephemeral", + VolumeSource: v1.VolumeSource{ + Ephemeral: &v1.EphemeralVolumeSource{}, + }, + }, + ).Obj(), + addedPvc: st.MakePersistentVolumeClaim().Name("pod1-ephemeral").Namespace("ns1").Obj(), + wantQHint: framework.Queue, + }, + { + test: "a pod doesn't have the same PVC as an added PVC", + newPod: st.MakePod().Namespace("ns1").Volume( + v1.Volume{ + VolumeSource: v1.VolumeSource{ + PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc1", + }, + }, + }, + ).Obj(), + addedPvc: st.MakePersistentVolumeClaim().Name("pvc2").Namespace("ns1").Obj(), + wantQHint: framework.QueueSkip, + }, + { + test: "a pod doesn't have any PVC attached", + newPod: st.MakePod().Namespace("ns1").Obj(), + addedPvc: st.MakePersistentVolumeClaim().Name("pvc2").Namespace("ns1").Obj(), + wantQHint: framework.QueueSkip, + }, + } + + for _, test := range tests { + t.Run(test.test, func(t *testing.T) { + p := &CSILimits{} + logger, _ := ktesting.NewTestContext(t) + + qhint, err := p.isSchedulableAfterPVCAdded(logger, test.newPod, nil, test.addedPvc) + if err != nil { + t.Errorf("isSchedulableAfterPVCAdded failed: %v", err) + } + if qhint != test.wantQHint { + t.Errorf("QHint does not match: %v, want: %v", qhint, test.wantQHint) + } + }) + } +} + func getFakeCSIPVLister(volumeName string, driverNames ...string) tf.PersistentVolumeLister { pvLister := tf.PersistentVolumeLister{} for _, driver := range driverNames {