Skip to content

Commit 4032177

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#129557 from googs1025/feature/add_QueueingHint_for_VolumeAttachment_deletion_events
feature(scheduler): add queueinghint for volumeattachment deletion
2 parents 0a08529 + 86f5042 commit 4032177

File tree

3 files changed

+251
-1
lines changed

3 files changed

+251
-1
lines changed

‎pkg/scheduler/framework/plugins/nodevolumelimits/csi.go‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func (pl *CSILimits) EventsToRegister(_ context.Context) ([]framework.ClusterEve
8989
{Event: framework.ClusterEvent{Resource: framework.CSINode, ActionType: framework.Add}},
9090
{Event: framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Delete}, QueueingHintFn: pl.isSchedulableAfterPodDeleted},
9191
{Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add}, QueueingHintFn: pl.isSchedulableAfterPVCAdded},
92-
{Event: framework.ClusterEvent{Resource: framework.VolumeAttachment, ActionType: framework.Delete}},
92+
{Event: framework.ClusterEvent{Resource: framework.VolumeAttachment, ActionType: framework.Delete}, QueueingHintFn: pl.isSchedulableAfterVolumeAttachmentDeleted},
9393
}, nil
9494
}
9595

@@ -149,6 +149,44 @@ func (pl *CSILimits) isSchedulableAfterPVCAdded(logger klog.Logger, pod *v1.Pod,
149149
return framework.QueueSkip, nil
150150
}
151151

152+
func (pl *CSILimits) isSchedulableAfterVolumeAttachmentDeleted(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
153+
deletedVolumeAttachment, _, err := util.As[*storagev1.VolumeAttachment](oldObj, newObj)
154+
if err != nil {
155+
return framework.Queue, fmt.Errorf("unexpected objects in isSchedulableAfterVolumeAttachmentDeleted: %w", err)
156+
}
157+
158+
for _, vol := range pod.Spec.Volumes {
159+
// Check if the pod volume uses a PVC
160+
// If it does, return Queue
161+
if vol.PersistentVolumeClaim != nil {
162+
logger.V(5).Info("Pod volume uses PersistentVolumeClaim, which might make this pod schedulable due to VolumeAttachment deletion", "pod", klog.KObj(pod), "volumeAttachment", klog.KObj(deletedVolumeAttachment), "volume", vol.Name)
163+
return framework.Queue, nil
164+
}
165+
166+
if !pl.translator.IsInlineMigratable(&vol) {
167+
continue
168+
}
169+
170+
translatedPV, err := pl.translator.TranslateInTreeInlineVolumeToCSI(logger, &vol, pod.Namespace)
171+
if err != nil || translatedPV == nil {
172+
return framework.Queue, fmt.Errorf("converting volume(%s) from inline to csi: %w", vol.Name, err)
173+
}
174+
175+
if translatedPV.Spec.CSI != nil && deletedVolumeAttachment.Spec.Attacher == translatedPV.Spec.CSI.Driver {
176+
// deleted VolumeAttachment Attacher matches the translated PV CSI driver
177+
logger.V(5).Info("Pod volume is an Inline Migratable volume that matches the CSI driver, which might make this pod schedulable due to VolumeAttachment deletion",
178+
"pod", klog.KObj(pod), "volumeAttachment", klog.KObj(deletedVolumeAttachment),
179+
"volume", vol.Name, "csiDriver", translatedPV.Spec.CSI.Driver,
180+
)
181+
return framework.Queue, nil
182+
}
183+
}
184+
185+
logger.V(5).Info("the VolumeAttachment deletion wouldn't make this pod schedulable because the pod has no volume related to a deleted VolumeAttachment",
186+
"pod", klog.KObj(pod), "volumeAttachment", klog.KObj(deletedVolumeAttachment))
187+
return framework.QueueSkip, nil
188+
}
189+
152190
// PreFilter invoked at the prefilter extension point
153191
//
154192
// If the pod haven't those types of volumes, we'll skip the Filter phase

‎pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,179 @@ func TestCSILimitsAddedPVCQHint(t *testing.T) {
792792
}
793793
}
794794

795+
func TestCSILimitsDeletedVolumeAttachmentQHint(t *testing.T) {
796+
tests := []struct {
797+
test string
798+
newPod *v1.Pod
799+
existingPVC *v1.PersistentVolumeClaim
800+
deletedVA *storagev1.VolumeAttachment
801+
wantQHint framework.QueueingHint
802+
}{
803+
{
804+
test: "a pod has PVC when VolumeAttachment is deleting",
805+
newPod: st.MakePod().Namespace("ns1").Volume(
806+
v1.Volume{
807+
VolumeSource: v1.VolumeSource{
808+
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
809+
ClaimName: "pvc1",
810+
},
811+
},
812+
},
813+
).Obj(),
814+
existingPVC: st.MakePersistentVolumeClaim().Name("pvc1").Namespace("ns1").
815+
VolumeName("pv1").Obj(),
816+
deletedVA: st.MakeVolumeAttachment().Name("volumeattachment1").
817+
NodeName("fake-node").
818+
Attacher("test.storage.gke.io").
819+
Source(storagev1.VolumeAttachmentSource{PersistentVolumeName: ptr.To("pv1")}).Obj(),
820+
wantQHint: framework.Queue,
821+
},
822+
{
823+
test: "a pod has an Inline Migratable volume (AWSEBSDriver) when VolumeAttachment (AWSEBSDriver) is deleting (match)",
824+
newPod: st.MakePod().Namespace("ns1").Volume(
825+
v1.Volume{
826+
VolumeSource: v1.VolumeSource{
827+
AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
828+
VolumeID: "test",
829+
},
830+
},
831+
},
832+
).Obj(),
833+
deletedVA: st.MakeVolumeAttachment().Name("volumeattachment1").
834+
NodeName("fake-node").
835+
Attacher("ebs.csi.aws.com").
836+
Source(storagev1.VolumeAttachmentSource{
837+
InlineVolumeSpec: &v1.PersistentVolumeSpec{
838+
PersistentVolumeSource: v1.PersistentVolumeSource{
839+
CSI: &v1.CSIPersistentVolumeSource{
840+
Driver: "ebs.csi.aws.com",
841+
},
842+
},
843+
},
844+
}).Obj(),
845+
wantQHint: framework.Queue,
846+
},
847+
{
848+
test: "a pod has an Inline Migratable volume (GCEPDDriver) when VolumeAttachment (AWSEBSDriver) is deleting (no match)",
849+
newPod: st.MakePod().Namespace("ns1").Volume(
850+
v1.Volume{
851+
VolumeSource: v1.VolumeSource{
852+
GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
853+
PDName: "test",
854+
},
855+
},
856+
},
857+
).Obj(),
858+
deletedVA: st.MakeVolumeAttachment().Name("volumeattachment1").
859+
NodeName("fake-node").
860+
Attacher("ebs.csi.aws.com").
861+
Source(storagev1.VolumeAttachmentSource{
862+
InlineVolumeSpec: &v1.PersistentVolumeSpec{
863+
PersistentVolumeSource: v1.PersistentVolumeSource{
864+
CSI: &v1.CSIPersistentVolumeSource{
865+
Driver: "ebs.csi.aws.com",
866+
},
867+
},
868+
},
869+
}).Obj(),
870+
wantQHint: framework.QueueSkip,
871+
},
872+
{
873+
test: "a pod has an Inline Migratable volume (AWSEBSDriver) and PVC when VolumeAttachment (AWSEBSDriver) is deleting",
874+
newPod: st.MakePod().Namespace("ns1").Volume(
875+
v1.Volume{
876+
VolumeSource: v1.VolumeSource{
877+
AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
878+
VolumeID: "test",
879+
},
880+
},
881+
},
882+
).Volume(
883+
v1.Volume{
884+
VolumeSource: v1.VolumeSource{
885+
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
886+
ClaimName: "pvc1",
887+
},
888+
},
889+
},
890+
).Obj(),
891+
existingPVC: st.MakePersistentVolumeClaim().Name("pvc1").Namespace("ns1").
892+
VolumeName("pv1").Obj(),
893+
deletedVA: st.MakeVolumeAttachment().Name("volumeattachment1").
894+
NodeName("fake-node").
895+
Attacher("ebs.csi.aws.com").
896+
Source(storagev1.VolumeAttachmentSource{
897+
InlineVolumeSpec: &v1.PersistentVolumeSpec{
898+
PersistentVolumeSource: v1.PersistentVolumeSource{
899+
CSI: &v1.CSIPersistentVolumeSource{
900+
Driver: "ebs.csi.aws.com",
901+
},
902+
},
903+
},
904+
}).Obj(),
905+
wantQHint: framework.Queue,
906+
},
907+
{
908+
test: "a pod has an Inline Migratable volume (AWSEBSDriver) and PVC when VolumeAttachment (AWSEBSDriver) is deleting",
909+
newPod: st.MakePod().Namespace("ns1").Volume(
910+
v1.Volume{
911+
VolumeSource: v1.VolumeSource{
912+
AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
913+
VolumeID: "test",
914+
},
915+
},
916+
},
917+
).Volume(
918+
v1.Volume{
919+
VolumeSource: v1.VolumeSource{
920+
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
921+
ClaimName: "pvc1",
922+
},
923+
},
924+
},
925+
).Obj(),
926+
existingPVC: st.MakePersistentVolumeClaim().Name("pvc1").Namespace("ns1").
927+
VolumeName("pv1").Obj(),
928+
deletedVA: st.MakeVolumeAttachment().Name("volumeattachment1").
929+
NodeName("fake-node").
930+
Attacher("test.storage.gke.io").
931+
Source(storagev1.VolumeAttachmentSource{PersistentVolumeName: ptr.To("pv1")}).Obj(),
932+
wantQHint: framework.Queue,
933+
},
934+
{
935+
test: "a pod has no PVC when VolumeAttachment is deleting",
936+
newPod: st.MakePod().Namespace("ns1").Obj(),
937+
deletedVA: st.MakeVolumeAttachment().Name("volumeattachment1").
938+
NodeName("fake-node").
939+
Attacher("test.storage.gke.io").
940+
Source(storagev1.VolumeAttachmentSource{PersistentVolumeName: ptr.To("pv1")}).Obj(),
941+
wantQHint: framework.QueueSkip,
942+
},
943+
}
944+
945+
for _, test := range tests {
946+
t.Run(test.test, func(t *testing.T) {
947+
var pvcList tf.PersistentVolumeClaimLister
948+
if test.existingPVC != nil {
949+
pvcList = append(pvcList, *test.existingPVC)
950+
}
951+
p := &CSILimits{
952+
pvcLister: pvcList,
953+
translator: csitrans.New(),
954+
}
955+
logger, _ := ktesting.NewTestContext(t)
956+
957+
qhint, err := p.isSchedulableAfterVolumeAttachmentDeleted(logger, test.newPod, test.deletedVA, nil)
958+
if err != nil {
959+
t.Errorf("isSchedulableAfterVolumeAttachmentDeleted failed: %v", err)
960+
}
961+
if qhint != test.wantQHint {
962+
t.Errorf("QHint does not match: %v, want: %v", qhint, test.wantQHint)
963+
}
964+
})
965+
}
966+
}
967+
795968
func getFakeVolumeAttachmentLister(count int, driverNames ...string) tf.VolumeAttachmentLister {
796969
vaLister := tf.VolumeAttachmentLister{}
797970
for _, driver := range driverNames {

‎pkg/scheduler/testing/wrappers.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,3 +1298,42 @@ func (c *CSIStorageCapacityWrapper) Capacity(capacity *resource.Quantity) *CSISt
12981298
c.CSIStorageCapacity.Capacity = capacity
12991299
return c
13001300
}
1301+
1302+
// VolumeAttachmentWrapper wraps a VolumeAttachment inside.
1303+
type VolumeAttachmentWrapper struct{ storagev1.VolumeAttachment }
1304+
1305+
// MakeVolumeAttachment creates a VolumeAttachment wrapper.
1306+
func MakeVolumeAttachment() *VolumeAttachmentWrapper {
1307+
return &VolumeAttachmentWrapper{}
1308+
}
1309+
1310+
// Obj returns the inner VolumeAttachment.
1311+
func (c *VolumeAttachmentWrapper) Obj() *storagev1.VolumeAttachment {
1312+
return &c.VolumeAttachment
1313+
}
1314+
1315+
// Name sets `n` as the name of the inner VolumeAttachment.
1316+
func (c *VolumeAttachmentWrapper) Name(n string) *VolumeAttachmentWrapper {
1317+
c.SetName(n)
1318+
return c
1319+
}
1320+
1321+
func (c *VolumeAttachmentWrapper) Attacher(attacher string) *VolumeAttachmentWrapper {
1322+
c.Spec.Attacher = attacher
1323+
return c
1324+
}
1325+
1326+
func (c *VolumeAttachmentWrapper) NodeName(nodeName string) *VolumeAttachmentWrapper {
1327+
c.Spec.NodeName = nodeName
1328+
return c
1329+
}
1330+
1331+
func (c *VolumeAttachmentWrapper) Source(source storagev1.VolumeAttachmentSource) *VolumeAttachmentWrapper {
1332+
c.Spec.Source = source
1333+
return c
1334+
}
1335+
1336+
func (c *VolumeAttachmentWrapper) Attached(attached bool) *VolumeAttachmentWrapper {
1337+
c.Status.Attached = attached
1338+
return c
1339+
}

0 commit comments

Comments
 (0)