Skip to content

Commit f409ded

Browse files
committed
Implement QHint for CSINode
1 parent a310305 commit f409ded

File tree

2 files changed

+135
-1
lines changed

2 files changed

+135
-1
lines changed

pkg/scheduler/framework/plugins/volumebinding/volume_binding.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"time"
2525

2626
v1 "k8s.io/api/core/v1"
27+
storagev1 "k8s.io/api/storage/v1"
2728
apierrors "k8s.io/apimachinery/pkg/api/errors"
2829
"k8s.io/apimachinery/pkg/runtime"
2930
corelisters "k8s.io/client-go/listers/core/v1"
@@ -35,6 +36,7 @@ import (
3536
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature"
3637
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper"
3738
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
39+
"k8s.io/kubernetes/pkg/scheduler/util"
3840
)
3941

4042
const (
@@ -114,7 +116,8 @@ func (pl *VolumeBinding) EventsToRegister() []framework.ClusterEventWithHint {
114116
// See: https://github.com/kubernetes/kubernetes/issues/110175
115117
{Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.UpdateNodeLabel | framework.UpdateNodeTaint}},
116118
// We rely on CSI node to translate in-tree PV to CSI.
117-
{Event: framework.ClusterEvent{Resource: framework.CSINode, ActionType: framework.Add | framework.Update}},
119+
// TODO: kube-schduler will unregister the CSINode events once all the volume plugins has completed their CSI migration.
120+
{Event: framework.ClusterEvent{Resource: framework.CSINode, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterCSINodeChange},
118121
// When CSIStorageCapacity is enabled, pods may become schedulable
119122
// on CSI driver & storage capacity changes.
120123
{Event: framework.ClusterEvent{Resource: framework.CSIDriver, ActionType: framework.Add | framework.Update}},
@@ -123,6 +126,31 @@ func (pl *VolumeBinding) EventsToRegister() []framework.ClusterEventWithHint {
123126
return events
124127
}
125128

129+
func (pl *VolumeBinding) isSchedulableAfterCSINodeChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
130+
if oldObj == nil {
131+
logger.V(5).Info("CSINode creation could make the pod schedulable")
132+
return framework.Queue, nil
133+
}
134+
oldCSINode, modifiedCSINode, err := util.As[*storagev1.CSINode](oldObj, newObj)
135+
if err != nil {
136+
return framework.Queue, err
137+
}
138+
139+
logger = klog.LoggerWithValues(
140+
logger,
141+
"Pod", klog.KObj(pod),
142+
"CSINode", klog.KObj(modifiedCSINode),
143+
)
144+
145+
if oldCSINode.ObjectMeta.Annotations[v1.MigratedPluginsAnnotationKey] != modifiedCSINode.ObjectMeta.Annotations[v1.MigratedPluginsAnnotationKey] {
146+
logger.V(5).Info("CSINode's migrated plugins annotation is updated and that may make the pod schedulable")
147+
return framework.Queue, nil
148+
}
149+
150+
logger.V(5).Info("CISNode was created or updated but it doesn't make this pod schedulable")
151+
return framework.QueueSkip, nil
152+
}
153+
126154
// podHasPVCs returns 2 values:
127155
// - the first one to denote if the given "pod" has any PVC defined.
128156
// - the second one to return any error if the requested PVC is illegal.

pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,3 +890,109 @@ func TestVolumeBinding(t *testing.T) {
890890
})
891891
}
892892
}
893+
894+
func TestIsSchedulableAfterCSINodeChange(t *testing.T) {
895+
table := []struct {
896+
name string
897+
oldObj interface{}
898+
newObj interface{}
899+
err bool
900+
expect framework.QueueingHint
901+
}{
902+
{
903+
name: "unexpected objects are passed",
904+
oldObj: new(struct{}),
905+
newObj: new(struct{}),
906+
err: true,
907+
expect: framework.Queue,
908+
},
909+
{
910+
name: "CSINode is newly created",
911+
newObj: &storagev1.CSINode{
912+
ObjectMeta: metav1.ObjectMeta{
913+
Name: "csinode-a",
914+
},
915+
},
916+
oldObj: nil,
917+
err: false,
918+
expect: framework.Queue,
919+
},
920+
{
921+
name: "CSINode's migrated-plugins annotations is added",
922+
oldObj: &storagev1.CSINode{
923+
ObjectMeta: metav1.ObjectMeta{
924+
Name: "csinode-a",
925+
Annotations: map[string]string{
926+
v1.MigratedPluginsAnnotationKey: "test1",
927+
},
928+
},
929+
},
930+
newObj: &storagev1.CSINode{
931+
ObjectMeta: metav1.ObjectMeta{
932+
Name: "csinode-a",
933+
Annotations: map[string]string{
934+
v1.MigratedPluginsAnnotationKey: "test1, test2",
935+
},
936+
},
937+
},
938+
err: false,
939+
expect: framework.Queue,
940+
},
941+
{
942+
name: "CSINode's migrated-plugins annotation is updated",
943+
oldObj: &storagev1.CSINode{
944+
ObjectMeta: metav1.ObjectMeta{
945+
Name: "csinode-a",
946+
Annotations: map[string]string{
947+
v1.MigratedPluginsAnnotationKey: "test1",
948+
},
949+
},
950+
},
951+
newObj: &storagev1.CSINode{
952+
ObjectMeta: metav1.ObjectMeta{
953+
Name: "csinode-a",
954+
Annotations: map[string]string{
955+
v1.MigratedPluginsAnnotationKey: "test2",
956+
},
957+
},
958+
},
959+
err: false,
960+
expect: framework.Queue,
961+
},
962+
{
963+
name: "CSINode is updated but migrated-plugins annotation gets unchanged",
964+
oldObj: &storagev1.CSINode{
965+
ObjectMeta: metav1.ObjectMeta{
966+
Name: "csinode-a",
967+
Annotations: map[string]string{
968+
v1.MigratedPluginsAnnotationKey: "test1",
969+
},
970+
},
971+
},
972+
newObj: &storagev1.CSINode{
973+
ObjectMeta: metav1.ObjectMeta{
974+
Name: "csinode-a",
975+
Annotations: map[string]string{
976+
v1.MigratedPluginsAnnotationKey: "test1",
977+
},
978+
},
979+
},
980+
err: false,
981+
expect: framework.QueueSkip,
982+
},
983+
}
984+
for _, item := range table {
985+
t.Run(item.name, func(t *testing.T) {
986+
pl := &VolumeBinding{}
987+
pod := makePod("pod-a").Pod
988+
logger, _ := ktesting.NewTestContext(t)
989+
qhint, err := pl.isSchedulableAfterCSINodeChange(logger, pod, item.oldObj, item.newObj)
990+
if (err != nil) != item.err {
991+
t.Errorf("isSchedulableAfterCSINodeChange failed - got: %q", err)
992+
}
993+
if qhint != item.expect {
994+
t.Errorf("QHint does not match: %v, want: %v", qhint, item.expect)
995+
}
996+
})
997+
}
998+
}

0 commit comments

Comments
 (0)