Skip to content

Commit 5b49afa

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#125000 from Gekko0114/vz_pvc
volumezone: scheduler queueing hints: pvc
2 parents 37f733a + 4ccae88 commit 5b49afa

File tree

2 files changed

+149
-17
lines changed

2 files changed

+149
-17
lines changed

‎pkg/scheduler/framework/plugins/volumezone/volume_zone.go‎

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"k8s.io/klog/v2"
3434
"k8s.io/kubernetes/pkg/scheduler/framework"
3535
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
36+
"k8s.io/kubernetes/pkg/scheduler/util"
3637
)
3738

3839
// VolumeZone is a plugin that checks volume zone.
@@ -105,7 +106,8 @@ func (pl *VolumeZone) Name() string {
105106
// Currently, this is only supported with PersistentVolumeClaims,
106107
// and only looks for the bound PersistentVolume.
107108
func (pl *VolumeZone) PreFilter(ctx context.Context, cs *framework.CycleState, pod *v1.Pod) (*framework.PreFilterResult, *framework.Status) {
108-
podPVTopologies, status := pl.getPVbyPod(ctx, pod)
109+
logger := klog.FromContext(ctx)
110+
podPVTopologies, status := pl.getPVbyPod(logger, pod)
109111
if !status.IsSuccess() {
110112
return nil, status
111113
}
@@ -116,16 +118,12 @@ func (pl *VolumeZone) PreFilter(ctx context.Context, cs *framework.CycleState, p
116118
return nil, nil
117119
}
118120

119-
func (pl *VolumeZone) getPVbyPod(ctx context.Context, pod *v1.Pod) ([]pvTopology, *framework.Status) {
120-
logger := klog.FromContext(ctx)
121+
// getPVbyPod gets PVTopology from pod
122+
func (pl *VolumeZone) getPVbyPod(logger klog.Logger, pod *v1.Pod) ([]pvTopology, *framework.Status) {
121123
podPVTopologies := make([]pvTopology, 0)
122124

123-
for i := range pod.Spec.Volumes {
124-
volume := pod.Spec.Volumes[i]
125-
if volume.PersistentVolumeClaim == nil {
126-
continue
127-
}
128-
pvcName := volume.PersistentVolumeClaim.ClaimName
125+
pvcNames := pl.getPersistentVolumeClaimNameFromPod(pod)
126+
for _, pvcName := range pvcNames {
129127
if pvcName == "" {
130128
return nil, framework.NewStatus(framework.UnschedulableAndUnresolvable, "PersistentVolumeClaim had no name")
131129
}
@@ -212,7 +210,7 @@ func (pl *VolumeZone) Filter(ctx context.Context, cs *framework.CycleState, pod
212210
if err != nil {
213211
// Fallback to calculate pv list here
214212
var status *framework.Status
215-
podPVTopologies, status = pl.getPVbyPod(ctx, pod)
213+
podPVTopologies, status = pl.getPVbyPod(logger, pod)
216214
if !status.IsSuccess() {
217215
return status
218216
}
@@ -291,13 +289,59 @@ func (pl *VolumeZone) EventsToRegister() []framework.ClusterEventWithHint {
291289
// See: https://github.com/kubernetes/kubernetes/issues/110175
292290
{Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.UpdateNodeLabel | framework.UpdateNodeTaint}},
293291
// A new pvc may make a pod schedulable.
294-
// Due to fields are immutable except `spec.resources`, pvc update events are ignored.
295-
{Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add}},
292+
// Also, if pvc's VolumeName is filled, that also could make a pod schedulable.
293+
{Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterPersistentVolumeClaimChange},
296294
// A new pv or updating a pv's volume zone labels may make a pod schedulable.
297295
{Event: framework.ClusterEvent{Resource: framework.PersistentVolume, ActionType: framework.Add | framework.Update}},
298296
}
299297
}
300298

299+
// getPersistentVolumeClaimNameFromPod gets pvc names bound to a pod.
300+
func (pl *VolumeZone) getPersistentVolumeClaimNameFromPod(pod *v1.Pod) []string {
301+
var pvcNames []string
302+
for i := range pod.Spec.Volumes {
303+
volume := pod.Spec.Volumes[i]
304+
if volume.PersistentVolumeClaim == nil {
305+
continue
306+
}
307+
pvcName := volume.PersistentVolumeClaim.ClaimName
308+
pvcNames = append(pvcNames, pvcName)
309+
}
310+
return pvcNames
311+
}
312+
313+
// isSchedulableAfterPersistentVolumeClaimChange is invoked whenever a PersistentVolumeClaim added or updated.
314+
// It checks whether the change of PVC has made a previously unschedulable pod schedulable.
315+
func (pl *VolumeZone) isSchedulableAfterPersistentVolumeClaimChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
316+
_, modifiedPVC, err := util.As[*v1.PersistentVolumeClaim](oldObj, newObj)
317+
if err != nil {
318+
return framework.Queue, fmt.Errorf("unexpected objects in isSchedulableAfterPersistentVolumeClaimChange: %w", err)
319+
}
320+
if pl.isPVCRequestedFromPod(logger, modifiedPVC, pod) {
321+
logger.V(5).Info("PVC that is referred from the pod was created or updated, which might make this pod schedulable", "pod", klog.KObj(pod), "PVC", klog.KObj(modifiedPVC))
322+
return framework.Queue, nil
323+
}
324+
325+
logger.V(5).Info("PVC irrelevant to the Pod was created or updated, which doesn't make this pod schedulable", "pod", klog.KObj(pod), "PVC", klog.KObj(modifiedPVC))
326+
return framework.QueueSkip, nil
327+
}
328+
329+
// isPVCRequestedFromPod verifies if the PVC is requested from a given Pod.
330+
func (pl *VolumeZone) isPVCRequestedFromPod(logger klog.Logger, pvc *v1.PersistentVolumeClaim, pod *v1.Pod) bool {
331+
if (pvc == nil) || (pod.Namespace != pvc.Namespace) {
332+
return false
333+
}
334+
pvcNames := pl.getPersistentVolumeClaimNameFromPod(pod)
335+
for _, pvcName := range pvcNames {
336+
if pvc.Name == pvcName {
337+
logger.V(5).Info("PVC is referred from the pod", "pod", klog.KObj(pod), "PVC", klog.KObj(pvc))
338+
return true
339+
}
340+
}
341+
logger.V(5).Info("PVC is not referred from the pod", "pod", klog.KObj(pod), "PVC", klog.KObj(pvc))
342+
return false
343+
}
344+
301345
// New initializes a new plugin and returns it.
302346
func New(_ context.Context, _ runtime.Object, handle framework.Handle) (framework.Plugin, error) {
303347
informerFactory := handle.SharedInformerFactory()

‎pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go‎

Lines changed: 93 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"testing"
2323

2424
"github.com/google/go-cmp/cmp"
25+
2526
v1 "k8s.io/api/core/v1"
2627
storagev1 "k8s.io/api/storage/v1"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -539,6 +540,93 @@ func TestWithBinding(t *testing.T) {
539540
}
540541
}
541542

543+
func TestIsSchedulableAfterPersistentVolumeClaimAdded(t *testing.T) {
544+
testcases := map[string]struct {
545+
pod *v1.Pod
546+
oldObj, newObj interface{}
547+
expectedHint framework.QueueingHint
548+
expectedErr bool
549+
}{
550+
"error-wrong-new-object": {
551+
pod: createPodWithVolume("pod_1", "PVC_1"),
552+
newObj: "not-a-pvc",
553+
expectedHint: framework.Queue,
554+
expectedErr: true,
555+
},
556+
"pvc-was-added-but-pod-refers-no-pvc": {
557+
pod: st.MakePod().Name("pod_1").Namespace("default").Obj(),
558+
newObj: &v1.PersistentVolumeClaim{
559+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
560+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
561+
},
562+
expectedHint: framework.QueueSkip,
563+
},
564+
"pvc-was-added-and-pod-was-bound-to-different-pvc": {
565+
pod: createPodWithVolume("pod_1", "PVC_2"),
566+
newObj: &v1.PersistentVolumeClaim{
567+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
568+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
569+
},
570+
expectedHint: framework.QueueSkip,
571+
},
572+
"pvc-was-added-and-pod-was-bound-to-pvc-but-different-ns": {
573+
pod: createPodWithVolume("pod_1", "PVC_1"),
574+
newObj: &v1.PersistentVolumeClaim{
575+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "ns1"},
576+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
577+
},
578+
expectedHint: framework.QueueSkip,
579+
},
580+
"pvc-was-added-and-pod-was-bound-to-the-pvc": {
581+
pod: createPodWithVolume("pod_1", "PVC_1"),
582+
newObj: &v1.PersistentVolumeClaim{
583+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
584+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
585+
},
586+
expectedHint: framework.Queue,
587+
},
588+
"pvc-was-updated-and-pod-was-bound-to-the-pvc": {
589+
pod: createPodWithVolume("pod_1", "PVC_1"),
590+
oldObj: &v1.PersistentVolumeClaim{
591+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
592+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: ""},
593+
},
594+
newObj: &v1.PersistentVolumeClaim{
595+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
596+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
597+
},
598+
expectedHint: framework.Queue,
599+
},
600+
"pvc-was-updated-but-pod-refers-no-pvc": {
601+
pod: st.MakePod().Name("pod_1").Namespace(metav1.NamespaceDefault).Obj(),
602+
oldObj: &v1.PersistentVolumeClaim{
603+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
604+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: ""},
605+
},
606+
newObj: &v1.PersistentVolumeClaim{
607+
ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
608+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
609+
},
610+
expectedHint: framework.QueueSkip,
611+
},
612+
}
613+
614+
for name, tc := range testcases {
615+
t.Run(name, func(t *testing.T) {
616+
logger, _ := ktesting.NewTestContext(t)
617+
p := &VolumeZone{}
618+
619+
got, err := p.isSchedulableAfterPersistentVolumeClaimChange(logger, tc.pod, tc.oldObj, tc.newObj)
620+
if err != nil && !tc.expectedErr {
621+
t.Errorf("unexpected error: %v", err)
622+
}
623+
if got != tc.expectedHint {
624+
t.Errorf("isSchedulableAfterPersistentVolumeClaimChange() = %v, want %v", got, tc.expectedHint)
625+
}
626+
})
627+
}
628+
}
629+
542630
func BenchmarkVolumeZone(b *testing.B) {
543631
tests := []struct {
544632
Name string
@@ -572,7 +660,7 @@ func BenchmarkVolumeZone(b *testing.B) {
572660
defer cancel()
573661
nodes := makeNodesWithTopologyZone(tt.NumNodes)
574662
pl := newPluginWithListers(ctx, b, []*v1.Pod{tt.Pod}, nodes, makePVCsWithPV(tt.NumPVC), makePVsWithZoneLabel(tt.NumPV))
575-
nodeInfos := make([]*framework.NodeInfo, len(nodes), len(nodes))
663+
nodeInfos := make([]*framework.NodeInfo, len(nodes))
576664
for i := 0; i < len(nodes); i++ {
577665
nodeInfo := &framework.NodeInfo{}
578666
nodeInfo.SetNode(nodes[i])
@@ -609,7 +697,7 @@ func newPluginWithListers(ctx context.Context, tb testing.TB, pods []*v1.Pod, no
609697
}
610698

611699
func makePVsWithZoneLabel(num int) []*v1.PersistentVolume {
612-
pvList := make([]*v1.PersistentVolume, num, num)
700+
pvList := make([]*v1.PersistentVolume, num)
613701
for i := 0; i < len(pvList); i++ {
614702
pvName := fmt.Sprintf("Vol_Stable_%d", i)
615703
zone := fmt.Sprintf("us-west-%d", i)
@@ -621,7 +709,7 @@ func makePVsWithZoneLabel(num int) []*v1.PersistentVolume {
621709
}
622710

623711
func makePVCsWithPV(num int) []*v1.PersistentVolumeClaim {
624-
pvcList := make([]*v1.PersistentVolumeClaim, num, num)
712+
pvcList := make([]*v1.PersistentVolumeClaim, num)
625713
for i := 0; i < len(pvcList); i++ {
626714
pvcName := fmt.Sprintf("PVC_Stable_%d", i)
627715
pvName := fmt.Sprintf("Vol_Stable_%d", i)
@@ -634,10 +722,10 @@ func makePVCsWithPV(num int) []*v1.PersistentVolumeClaim {
634722
}
635723

636724
func makeNodesWithTopologyZone(num int) []*v1.Node {
637-
nodeList := make([]*v1.Node, num, num)
725+
nodeList := make([]*v1.Node, num)
638726
for i := 0; i < len(nodeList); i++ {
639727
nodeName := fmt.Sprintf("host_%d", i)
640-
zone := fmt.Sprintf("us-west-0")
728+
zone := "us-west-0"
641729
nodeList[i] = &v1.Node{
642730
ObjectMeta: metav1.ObjectMeta{
643731
Name: nodeName,

0 commit comments

Comments
 (0)