Skip to content

Commit b14fcc5

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#92846 from Huang-Wei/rm-pvcLister
Remove pvcLister from genericScheduler
2 parents f64f687 + 185ba08 commit b14fcc5

File tree

9 files changed

+89
-87
lines changed

9 files changed

+89
-87
lines changed

‎pkg/scheduler/core/BUILD‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ go_library(
1010
visibility = ["//visibility:public"],
1111
deps = [
1212
"//pkg/api/v1/pod:go_default_library",
13-
"//pkg/features:go_default_library",
1413
"//pkg/scheduler/apis/config:go_default_library",
1514
"//pkg/scheduler/framework/runtime:go_default_library",
1615
"//pkg/scheduler/framework/v1alpha1:go_default_library",
@@ -20,11 +19,8 @@ go_library(
2019
"//pkg/scheduler/profile:go_default_library",
2120
"//pkg/scheduler/util:go_default_library",
2221
"//staging/src/k8s.io/api/core/v1:go_default_library",
23-
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
2422
"//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library",
2523
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
26-
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
27-
"//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
2824
"//staging/src/k8s.io/client-go/rest:go_default_library",
2925
"//staging/src/k8s.io/kube-scheduler/extender/v1:go_default_library",
3026
"//vendor/k8s.io/klog/v2:go_default_library",
@@ -40,15 +36,16 @@ go_test(
4036
],
4137
embed = [":go_default_library"],
4238
deps = [
39+
"//pkg/controller/volume/persistentvolume/util:go_default_library",
4340
"//pkg/scheduler/apis/config:go_default_library",
4441
"//pkg/scheduler/framework/plugins/defaultbinder:go_default_library",
4542
"//pkg/scheduler/framework/plugins/noderesources:go_default_library",
4643
"//pkg/scheduler/framework/plugins/podtopologyspread:go_default_library",
4744
"//pkg/scheduler/framework/plugins/queuesort:go_default_library",
4845
"//pkg/scheduler/framework/plugins/selectorspread:go_default_library",
46+
"//pkg/scheduler/framework/plugins/volumebinding:go_default_library",
4947
"//pkg/scheduler/framework/runtime:go_default_library",
5048
"//pkg/scheduler/framework/v1alpha1:go_default_library",
51-
"//pkg/scheduler/framework/v1alpha1/fake:go_default_library",
5249
"//pkg/scheduler/internal/cache:go_default_library",
5350
"//pkg/scheduler/internal/queue:go_default_library",
5451
"//pkg/scheduler/profile:go_default_library",

‎pkg/scheduler/core/extender_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
272272
fwk, err := st.NewFramework(
273273
test.registerPlugins,
274274
runtime.WithClientSet(client),
275+
runtime.WithInformerFactory(informerFactory),
275276
runtime.WithPodNominator(internalqueue.NewPodNominator()),
276277
)
277278
if err != nil {
@@ -285,7 +286,6 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
285286
cache,
286287
emptySnapshot,
287288
extenders,
288-
informerFactory.Core().V1().PersistentVolumeClaims().Lister(),
289289
schedulerapi.DefaultPercentageOfNodesToScore)
290290
podIgnored := &v1.Pod{}
291291
result, err := scheduler.Schedule(context.Background(), prof, framework.NewCycleState(), podIgnored)

‎pkg/scheduler/core/generic_scheduler.go‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,8 @@ import (
2929
"k8s.io/klog/v2"
3030

3131
v1 "k8s.io/api/core/v1"
32-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
33-
utilfeature "k8s.io/apiserver/pkg/util/feature"
34-
corelisters "k8s.io/client-go/listers/core/v1"
3532
extenderv1 "k8s.io/kube-scheduler/extender/v1"
3633
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
37-
"k8s.io/kubernetes/pkg/features"
3834
"k8s.io/kubernetes/pkg/scheduler/framework/runtime"
3935
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
4036
internalcache "k8s.io/kubernetes/pkg/scheduler/internal/cache"
@@ -119,7 +115,6 @@ type genericScheduler struct {
119115
cache internalcache.Cache
120116
extenders []framework.Extender
121117
nodeInfoSnapshot *internalcache.Snapshot
122-
pvcLister corelisters.PersistentVolumeClaimLister
123118
percentageOfNodesToScore int32
124119
nextStartNodeIndex int
125120
}
@@ -138,11 +133,6 @@ func (g *genericScheduler) Schedule(ctx context.Context, prof *profile.Profile,
138133
trace := utiltrace.New("Scheduling", utiltrace.Field{Key: "namespace", Value: pod.Namespace}, utiltrace.Field{Key: "name", Value: pod.Name})
139134
defer trace.LogIfLong(100 * time.Millisecond)
140135

141-
if err := podPassesBasicChecks(pod, g.pvcLister); err != nil {
142-
return result, err
143-
}
144-
trace.Step("Basic checks done")
145-
146136
if err := g.snapshot(); err != nil {
147137
return result, err
148138
}
@@ -273,7 +263,6 @@ func (g *genericScheduler) findNodesThatFitPod(ctx context.Context, prof *profil
273263
filteredNodesStatuses[n.Node().Name] = s
274264
}
275265
return nil, filteredNodesStatuses, nil
276-
277266
}
278267

279268
feasibleNodes, err := g.findNodesThatPassFilters(ctx, prof, state, pod, filteredNodesStatuses)
@@ -584,57 +573,16 @@ func (g *genericScheduler) prioritizeNodes(
584573
return result, nil
585574
}
586575

587-
// podPassesBasicChecks makes sanity checks on the pod if it can be scheduled.
588-
func podPassesBasicChecks(pod *v1.Pod, pvcLister corelisters.PersistentVolumeClaimLister) error {
589-
// Check PVCs used by the pod
590-
namespace := pod.Namespace
591-
manifest := &(pod.Spec)
592-
for i := range manifest.Volumes {
593-
volume := &manifest.Volumes[i]
594-
var pvcName string
595-
ephemeral := false
596-
switch {
597-
case volume.PersistentVolumeClaim != nil:
598-
pvcName = volume.PersistentVolumeClaim.ClaimName
599-
case volume.Ephemeral != nil &&
600-
utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume):
601-
pvcName = pod.Name + "-" + volume.Name
602-
ephemeral = true
603-
default:
604-
// Volume is not using a PVC, ignore
605-
continue
606-
}
607-
pvc, err := pvcLister.PersistentVolumeClaims(namespace).Get(pvcName)
608-
if err != nil {
609-
// The error has already enough context ("persistentvolumeclaim "myclaim" not found")
610-
return err
611-
}
612-
613-
if pvc.DeletionTimestamp != nil {
614-
return fmt.Errorf("persistentvolumeclaim %q is being deleted", pvc.Name)
615-
}
616-
617-
if ephemeral &&
618-
!metav1.IsControlledBy(pvc, pod) {
619-
return fmt.Errorf("persistentvolumeclaim %q was not created for the pod", pvc.Name)
620-
}
621-
}
622-
623-
return nil
624-
}
625-
626576
// NewGenericScheduler creates a genericScheduler object.
627577
func NewGenericScheduler(
628578
cache internalcache.Cache,
629579
nodeInfoSnapshot *internalcache.Snapshot,
630580
extenders []framework.Extender,
631-
pvcLister corelisters.PersistentVolumeClaimLister,
632581
percentageOfNodesToScore int32) ScheduleAlgorithm {
633582
return &genericScheduler{
634583
cache: cache,
635584
extenders: extenders,
636585
nodeInfoSnapshot: nodeInfoSnapshot,
637-
pvcLister: pvcLister,
638586
percentageOfNodesToScore: percentageOfNodesToScore,
639587
}
640588
}

‎pkg/scheduler/core/generic_scheduler_test.go‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import (
2020
"context"
2121
"fmt"
2222
"math"
23-
"reflect"
2423
"strconv"
24+
"strings"
2525
"testing"
2626
"time"
2727

@@ -34,15 +34,16 @@ import (
3434
"k8s.io/apimachinery/pkg/util/wait"
3535
"k8s.io/client-go/informers"
3636
clientsetfake "k8s.io/client-go/kubernetes/fake"
37+
pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util"
3738
schedulerapi "k8s.io/kubernetes/pkg/scheduler/apis/config"
3839
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder"
3940
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/noderesources"
4041
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread"
4142
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort"
4243
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/selectorspread"
44+
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding"
4345
frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime"
4446
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
45-
fakeframework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1/fake"
4647
internalcache "k8s.io/kubernetes/pkg/scheduler/internal/cache"
4748
internalqueue "k8s.io/kubernetes/pkg/scheduler/internal/queue"
4849
"k8s.io/kubernetes/pkg/scheduler/profile"
@@ -418,13 +419,19 @@ func TestGenericScheduler(t *testing.T) {
418419
// Pod with existing PVC
419420
registerPlugins: []st.RegisterPluginFunc{
420421
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
422+
st.RegisterPreFilterPlugin(volumebinding.Name, volumebinding.New),
421423
st.RegisterFilterPlugin("TrueFilter", st.NewTrueFilterPlugin),
422424
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
423425
},
424426
nodes: []string{"machine1", "machine2"},
425-
pvcs: []v1.PersistentVolumeClaim{{ObjectMeta: metav1.ObjectMeta{Name: "existingPVC"}}},
427+
pvcs: []v1.PersistentVolumeClaim{
428+
{
429+
ObjectMeta: metav1.ObjectMeta{Name: "existingPVC", UID: types.UID("existingPVC"), Namespace: v1.NamespaceDefault},
430+
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "existingPV"},
431+
},
432+
},
426433
pod: &v1.Pod{
427-
ObjectMeta: metav1.ObjectMeta{Name: "ignore", UID: types.UID("ignore")},
434+
ObjectMeta: metav1.ObjectMeta{Name: "ignore", UID: types.UID("ignore"), Namespace: v1.NamespaceDefault},
428435
Spec: v1.PodSpec{
429436
Volumes: []v1.Volume{
430437
{
@@ -445,6 +452,7 @@ func TestGenericScheduler(t *testing.T) {
445452
// Pod with non existing PVC
446453
registerPlugins: []st.RegisterPluginFunc{
447454
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
455+
st.RegisterPreFilterPlugin(volumebinding.Name, volumebinding.New),
448456
st.RegisterFilterPlugin("TrueFilter", st.NewTrueFilterPlugin),
449457
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
450458
},
@@ -470,13 +478,14 @@ func TestGenericScheduler(t *testing.T) {
470478
// Pod with deleting PVC
471479
registerPlugins: []st.RegisterPluginFunc{
472480
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
481+
st.RegisterPreFilterPlugin(volumebinding.Name, volumebinding.New),
473482
st.RegisterFilterPlugin("TrueFilter", st.NewTrueFilterPlugin),
474483
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
475484
},
476485
nodes: []string{"machine1", "machine2"},
477-
pvcs: []v1.PersistentVolumeClaim{{ObjectMeta: metav1.ObjectMeta{Name: "existingPVC", DeletionTimestamp: &metav1.Time{}}}},
486+
pvcs: []v1.PersistentVolumeClaim{{ObjectMeta: metav1.ObjectMeta{Name: "existingPVC", UID: types.UID("existingPVC"), Namespace: v1.NamespaceDefault, DeletionTimestamp: &metav1.Time{}}}},
478487
pod: &v1.Pod{
479-
ObjectMeta: metav1.ObjectMeta{Name: "ignore", UID: types.UID("ignore")},
488+
ObjectMeta: metav1.ObjectMeta{Name: "ignore", UID: types.UID("ignore"), Namespace: v1.NamespaceDefault},
480489
Spec: v1.PodSpec{
481490
Volumes: []v1.Volume{
482491
{
@@ -728,10 +737,22 @@ func TestGenericScheduler(t *testing.T) {
728737
cache.AddNode(node)
729738
}
730739

740+
ctx := context.Background()
741+
cs := clientsetfake.NewSimpleClientset()
742+
informerFactory := informers.NewSharedInformerFactory(cs, 0)
743+
for _, pvc := range test.pvcs {
744+
metav1.SetMetaDataAnnotation(&pvc.ObjectMeta, pvutil.AnnBindCompleted, "true")
745+
cs.CoreV1().PersistentVolumeClaims(pvc.Namespace).Create(ctx, &pvc, metav1.CreateOptions{})
746+
if pvName := pvc.Spec.VolumeName; pvName != "" {
747+
pv := v1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Name: pvName}}
748+
cs.CoreV1().PersistentVolumes().Create(ctx, &pv, metav1.CreateOptions{})
749+
}
750+
}
731751
snapshot := internalcache.NewSnapshot(test.pods, nodes)
732752
fwk, err := st.NewFramework(
733753
test.registerPlugins,
734754
frameworkruntime.WithSnapshotSharedLister(snapshot),
755+
frameworkruntime.WithInformerFactory(informerFactory),
735756
frameworkruntime.WithPodNominator(internalqueue.NewPodNominator()),
736757
)
737758
if err != nil {
@@ -741,19 +762,18 @@ func TestGenericScheduler(t *testing.T) {
741762
Framework: fwk,
742763
}
743764

744-
var pvcs []v1.PersistentVolumeClaim
745-
pvcs = append(pvcs, test.pvcs...)
746-
pvcLister := fakeframework.PersistentVolumeClaimLister(pvcs)
747-
748765
scheduler := NewGenericScheduler(
749766
cache,
750767
snapshot,
751768
[]framework.Extender{},
752-
pvcLister,
753-
schedulerapi.DefaultPercentageOfNodesToScore)
754-
result, err := scheduler.Schedule(context.Background(), prof, framework.NewCycleState(), test.pod)
755-
if !reflect.DeepEqual(err, test.wErr) {
756-
t.Errorf("want: %v, got: %v", test.wErr, err)
769+
schedulerapi.DefaultPercentageOfNodesToScore,
770+
)
771+
informerFactory.Start(ctx.Done())
772+
informerFactory.WaitForCacheSync(ctx.Done())
773+
774+
result, err := scheduler.Schedule(ctx, prof, framework.NewCycleState(), test.pod)
775+
if err != test.wErr && !strings.Contains(err.Error(), test.wErr.Error()) {
776+
t.Errorf("Unexpected error: %v, expected: %v", err.Error(), test.wErr)
757777
}
758778
if test.expectedHosts != nil && !test.expectedHosts.Has(result.SuggestedHost) {
759779
t.Errorf("Expected: %s, got: %s", test.expectedHosts, result.SuggestedHost)
@@ -775,7 +795,7 @@ func makeScheduler(nodes []*v1.Node) *genericScheduler {
775795
s := NewGenericScheduler(
776796
cache,
777797
emptySnapshot,
778-
nil, nil,
798+
nil,
779799
schedulerapi.DefaultPercentageOfNodesToScore)
780800
cache.UpdateSnapshot(s.(*genericScheduler).nodeInfoSnapshot)
781801
return s.(*genericScheduler)
@@ -1069,7 +1089,6 @@ func TestZeroRequest(t *testing.T) {
10691089
nil,
10701090
emptySnapshot,
10711091
[]framework.Extender{},
1072-
nil,
10731092
schedulerapi.DefaultPercentageOfNodesToScore).(*genericScheduler)
10741093
scheduler.nodeInfoSnapshot = snapshot
10751094

‎pkg/scheduler/factory.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ func (c *Configurator) create() (*Scheduler, error) {
180180
c.schedulerCache,
181181
c.nodeInfoSnapshot,
182182
extenders,
183-
c.informerFactory.Core().V1().PersistentVolumeClaims().Lister(),
184183
c.percentageOfNodesToScore,
185184
)
186185

‎pkg/scheduler/framework/plugins/volumebinding/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ go_library(
1111
"//pkg/scheduler/apis/config:go_default_library",
1212
"//pkg/scheduler/framework/v1alpha1:go_default_library",
1313
"//staging/src/k8s.io/api/core/v1:go_default_library",
14+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
1415
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
1516
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
17+
"//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
1618
"//vendor/k8s.io/klog/v2:go_default_library",
1719
],
1820
)

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

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import (
2323
"time"
2424

2525
v1 "k8s.io/api/core/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
"k8s.io/apimachinery/pkg/runtime"
2728
utilfeature "k8s.io/apiserver/pkg/util/feature"
29+
corelisters "k8s.io/client-go/listers/core/v1"
2830
"k8s.io/klog/v2"
2931
"k8s.io/kubernetes/pkg/controller/volume/scheduling"
3032
"k8s.io/kubernetes/pkg/features"
@@ -62,6 +64,7 @@ func (d *stateData) Clone() framework.StateData {
6264
// Reserve and PreBind phases.
6365
type VolumeBinding struct {
6466
Binder scheduling.SchedulerVolumeBinder
67+
PVCLister corelisters.PersistentVolumeClaimLister
6568
GenericEphemeralVolumeFeatureEnabled bool
6669
}
6770

@@ -78,22 +81,50 @@ func (pl *VolumeBinding) Name() string {
7881
return Name
7982
}
8083

81-
func (pl *VolumeBinding) podHasPVCs(pod *v1.Pod) bool {
84+
// podHasPVCs returns 2 values:
85+
// - the first one to denote if the given "pod" has any PVC defined.
86+
// - the second one to return any error if the requested PVC is illegal.
87+
func (pl *VolumeBinding) podHasPVCs(pod *v1.Pod) (bool, error) {
88+
hasPVC := false
8289
for _, vol := range pod.Spec.Volumes {
83-
if vol.PersistentVolumeClaim != nil ||
84-
pl.GenericEphemeralVolumeFeatureEnabled && vol.Ephemeral != nil {
85-
return true
90+
var pvcName string
91+
ephemeral := false
92+
switch {
93+
case vol.PersistentVolumeClaim != nil:
94+
pvcName = vol.PersistentVolumeClaim.ClaimName
95+
case vol.Ephemeral != nil && pl.GenericEphemeralVolumeFeatureEnabled:
96+
pvcName = pod.Name + "-" + vol.Name
97+
ephemeral = true
98+
default:
99+
// Volume is not using a PVC, ignore
100+
continue
101+
}
102+
hasPVC = true
103+
pvc, err := pl.PVCLister.PersistentVolumeClaims(pod.Namespace).Get(pvcName)
104+
if err != nil {
105+
// The error has already enough context ("persistentvolumeclaim "myclaim" not found")
106+
return hasPVC, err
107+
}
108+
109+
if pvc.DeletionTimestamp != nil {
110+
return hasPVC, fmt.Errorf("persistentvolumeclaim %q is being deleted", pvc.Name)
111+
}
112+
113+
if ephemeral && !metav1.IsControlledBy(pvc, pod) {
114+
return hasPVC, fmt.Errorf("persistentvolumeclaim %q was not created for the pod", pvc.Name)
86115
}
87116
}
88-
return false
117+
return hasPVC, nil
89118
}
90119

91120
// PreFilter invoked at the prefilter extension point to check if pod has all
92121
// immediate PVCs bound. If not all immediate PVCs are bound, an
93122
// UnschedulableAndUnresolvable is returned.
94123
func (pl *VolumeBinding) PreFilter(ctx context.Context, state *framework.CycleState, pod *v1.Pod) *framework.Status {
95124
// If pod does not reference any PVC, we don't need to do anything.
96-
if !pl.podHasPVCs(pod) {
125+
if hasPVC, err := pl.podHasPVCs(pod); err != nil {
126+
return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error())
127+
} else if !hasPVC {
97128
state.Write(stateKey, &stateData{skip: true})
98129
return nil
99130
}
@@ -271,6 +302,7 @@ func New(plArgs runtime.Object, fh framework.FrameworkHandle) (framework.Plugin,
271302
binder := scheduling.NewVolumeBinder(fh.ClientSet(), podInformer, nodeInformer, csiNodeInformer, pvcInformer, pvInformer, storageClassInformer, capacityCheck, time.Duration(args.BindTimeoutSeconds)*time.Second)
272303
return &VolumeBinding{
273304
Binder: binder,
305+
PVCLister: pvcInformer.Lister(),
274306
GenericEphemeralVolumeFeatureEnabled: utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume),
275307
}, nil
276308
}

0 commit comments

Comments
 (0)