diff --git a/pkg/controller/volume/scheduling/scheduler_binder.go b/pkg/controller/volume/scheduling/scheduler_binder.go index 4397a0ab104..0c9c9516148 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder.go +++ b/pkg/controller/volume/scheduling/scheduler_binder.go @@ -256,8 +256,6 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []* // Check PV node affinity on bound volumes if len(boundClaims) > 0 { - // TODO if node affinity does not match, we should - // UnschedulableAndUnresolvable error back to scheduler framework boundVolumesSatisfied, err = b.checkBoundClaims(boundClaims, node, podName) if err != nil { return nil, err diff --git a/pkg/scheduler/framework/plugins/volumebinding/BUILD b/pkg/scheduler/framework/plugins/volumebinding/BUILD index fc37262aca4..7119da3810a 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/BUILD +++ b/pkg/scheduler/framework/plugins/volumebinding/BUILD @@ -36,8 +36,15 @@ go_test( srcs = ["volume_binding_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/controller/volume/persistentvolume/util:go_default_library", "//pkg/controller/volume/scheduling:go_default_library", + "//pkg/scheduler/apis/config:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/api/storage/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/client-go/informers:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go index a583c15f47d..f2df794c023 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -18,99 +18,287 @@ package volumebinding import ( "context" - "fmt" "reflect" "testing" v1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" "k8s.io/kubernetes/pkg/controller/volume/scheduling" + "k8s.io/kubernetes/pkg/scheduler/apis/config" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" + "k8s.io/utils/pointer" ) -func TestVolumeBinding(t *testing.T) { - findErr := fmt.Errorf("find err") - volState := v1.PodSpec{ - Volumes: []v1.Volume{ - { - VolumeSource: v1.VolumeSource{ - PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{}, - }, - }, +var ( + immediate = storagev1.VolumeBindingImmediate + waitForFirstConsumer = storagev1.VolumeBindingWaitForFirstConsumer + immediateSC = &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "immediate-sc", + }, + VolumeBindingMode: &immediate, + } + waitSC = &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "wait-sc", + }, + VolumeBindingMode: &waitForFirstConsumer, + } +) + +func makePV(name string) *v1.PersistentVolume { + return &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, }, } +} + +func addPVNodeAffinity(pv *v1.PersistentVolume, volumeNodeAffinity *v1.VolumeNodeAffinity) *v1.PersistentVolume { + pv.Spec.NodeAffinity = volumeNodeAffinity + return pv +} + +func makePVC(name string, boundPVName string, storageClassName string) *v1.PersistentVolumeClaim { + pvc := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: v1.NamespaceDefault, + }, + Spec: v1.PersistentVolumeClaimSpec{ + StorageClassName: pointer.StringPtr(storageClassName), + }, + } + if boundPVName != "" { + pvc.Spec.VolumeName = boundPVName + metav1.SetMetaDataAnnotation(&pvc.ObjectMeta, pvutil.AnnBindCompleted, "true") + } + return pvc +} + +func makePod(name string, pvcNames []string) *v1.Pod { + p := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: v1.NamespaceDefault, + }, + } + p.Spec.Volumes = make([]v1.Volume, 0) + for _, pvcName := range pvcNames { + p.Spec.Volumes = append(p.Spec.Volumes, v1.Volume{ + VolumeSource: v1.VolumeSource{ + PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ + ClaimName: pvcName, + }, + }, + }) + } + return p +} + +func TestVolumeBinding(t *testing.T) { table := []struct { - name string - pod *v1.Pod - node *v1.Node - volumeBinderConfig *scheduling.FakeVolumeBinderConfig - wantStatus *framework.Status + name string + pod *v1.Pod + node *v1.Node + pvcs []*v1.PersistentVolumeClaim + pvs []*v1.PersistentVolume + wantPreFilterStatus *framework.Status + wantStateAfterPreFilter *stateData + wantFilterStatus *framework.Status }{ { - name: "nothing", - pod: &v1.Pod{}, - node: &v1.Node{}, - wantStatus: nil, + name: "pod has not pvcs", + pod: makePod("pod-a", nil), + node: &v1.Node{}, + wantStateAfterPreFilter: &stateData{ + skip: true, + }, }, { name: "all bound", - pod: &v1.Pod{Spec: volState}, + pod: makePod("pod-a", []string{"pvc-a"}), node: &v1.Node{}, - volumeBinderConfig: &scheduling.FakeVolumeBinderConfig{ - AllBound: true, + pvcs: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "pv-a", waitSC.Name), + }, + pvs: []*v1.PersistentVolume{ + makePV("pv-a"), + }, + wantStateAfterPreFilter: &stateData{ + boundClaims: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "pv-a", waitSC.Name), + }, + claimsToBind: []*v1.PersistentVolumeClaim{}, }, - wantStatus: nil, }, { - name: "unbound/no matches", - pod: &v1.Pod{Spec: volState}, + name: "immediate claims not bound", + pod: makePod("pod-a", []string{"pvc-a"}), node: &v1.Node{}, - volumeBinderConfig: &scheduling.FakeVolumeBinderConfig{ - FindReasons: []scheduling.ConflictReason{scheduling.ErrReasonBindConflict}, + pvcs: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "", immediateSC.Name), }, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(scheduling.ErrReasonBindConflict)), + wantPreFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, "pod has unbound immediate PersistentVolumeClaims"), + }, + { + name: "unbound claims no matches", + pod: makePod("pod-a", []string{"pvc-a"}), + node: &v1.Node{}, + pvcs: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "", waitSC.Name), + }, + wantStateAfterPreFilter: &stateData{ + boundClaims: []*v1.PersistentVolumeClaim{}, + claimsToBind: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "", waitSC.Name), + }, + }, + wantFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(scheduling.ErrReasonBindConflict)), }, { name: "bound and unbound unsatisfied", - pod: &v1.Pod{Spec: volState}, - node: &v1.Node{}, - volumeBinderConfig: &scheduling.FakeVolumeBinderConfig{ - FindReasons: []scheduling.ConflictReason{scheduling.ErrReasonBindConflict, scheduling.ErrReasonNodeConflict}, + pod: makePod("pod-a", []string{"pvc-a", "pvc-b"}), + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "foo": "barbar", + }, + }, }, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(scheduling.ErrReasonBindConflict), string(scheduling.ErrReasonNodeConflict)), + pvcs: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "pv-a", waitSC.Name), + makePVC("pvc-b", "", waitSC.Name), + }, + pvs: []*v1.PersistentVolume{ + addPVNodeAffinity(makePV("pv-a"), &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "foo", + Operator: v1.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + }, + }, + }, + }), + }, + wantStateAfterPreFilter: &stateData{ + boundClaims: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "pv-a", waitSC.Name), + }, + claimsToBind: []*v1.PersistentVolumeClaim{ + makePVC("pvc-b", "", waitSC.Name), + }, + }, + wantFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(scheduling.ErrReasonNodeConflict), string(scheduling.ErrReasonBindConflict)), }, { - name: "unbound/found matches/bind succeeds", - pod: &v1.Pod{Spec: volState}, - node: &v1.Node{}, - volumeBinderConfig: &scheduling.FakeVolumeBinderConfig{}, - wantStatus: nil, + name: "pvc not found", + pod: makePod("pod-a", []string{"pvc-a"}), + node: &v1.Node{}, + wantPreFilterStatus: framework.NewStatus(framework.Error, `error getting PVC "default/pvc-a": could not find v1.PersistentVolumeClaim "default/pvc-a"`), + wantFilterStatus: nil, }, { - name: "predicate error", - pod: &v1.Pod{Spec: volState}, + name: "pv not found", + pod: makePod("pod-a", []string{"pvc-a"}), node: &v1.Node{}, - volumeBinderConfig: &scheduling.FakeVolumeBinderConfig{ - FindErr: findErr, + pvcs: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "pv-a", waitSC.Name), }, - wantStatus: framework.NewStatus(framework.Error, findErr.Error()), + wantPreFilterStatus: nil, + wantStateAfterPreFilter: &stateData{ + boundClaims: []*v1.PersistentVolumeClaim{ + makePVC("pvc-a", "pv-a", waitSC.Name), + }, + claimsToBind: []*v1.PersistentVolumeClaim{}, + }, + wantFilterStatus: framework.NewStatus(framework.Error, `could not find v1.PersistentVolume "pv-a"`), }, } for _, item := range table { t.Run(item.name, func(t *testing.T) { - nodeInfo := framework.NewNodeInfo() - nodeInfo.SetNode(item.node) - fakeVolumeBinder := scheduling.NewFakeVolumeBinder(item.volumeBinderConfig) - p := &VolumeBinding{ - Binder: fakeVolumeBinder, + ctx := context.Background() + client := fake.NewSimpleClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) + opts := []framework.Option{ + framework.WithClientSet(client), + framework.WithInformerFactory(informerFactory), } - state := framework.NewCycleState() - p.PreFilter(context.Background(), state, item.pod) - gotStatus := p.Filter(context.Background(), state, item.pod, nodeInfo) - if !reflect.DeepEqual(gotStatus, item.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, item.wantStatus) + fh, err := framework.NewFramework(nil, nil, nil, opts...) + if err != nil { + t.Fatal(err) + } + pl, err := New(&config.VolumeBindingArgs{ + BindTimeoutSeconds: 300, + }, fh) + if err != nil { + t.Fatal(err) } + // Start informer factory after initialization + informerFactory.Start(ctx.Done()) + + // Feed testing data and wait for them to be synced + client.StorageV1().StorageClasses().Create(ctx, immediateSC, metav1.CreateOptions{}) + client.StorageV1().StorageClasses().Create(ctx, waitSC, metav1.CreateOptions{}) + if item.node != nil { + client.CoreV1().Nodes().Create(ctx, item.node, metav1.CreateOptions{}) + } + if len(item.pvcs) > 0 { + for _, pvc := range item.pvcs { + client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Create(ctx, pvc, metav1.CreateOptions{}) + } + } + if len(item.pvs) > 0 { + for _, pv := range item.pvs { + client.CoreV1().PersistentVolumes().Create(ctx, pv, metav1.CreateOptions{}) + } + } + caches := informerFactory.WaitForCacheSync(ctx.Done()) + for _, synced := range caches { + if !synced { + t.Errorf("error waiting for informer cache sync") + } + } + + // Verify + p := pl.(*VolumeBinding) + nodeInfo := framework.NewNodeInfo() + nodeInfo.SetNode(item.node) + state := framework.NewCycleState() + t.Logf("call PreFilter and check status") + gotPreFilterStatus := p.PreFilter(ctx, state, item.pod) + if !reflect.DeepEqual(gotPreFilterStatus, item.wantPreFilterStatus) { + t.Errorf("filter prefilter status does not match: %v, want: %v", gotPreFilterStatus, item.wantPreFilterStatus) + } + if !gotPreFilterStatus.IsSuccess() { + // scheduler framework will skip Filter if PreFilter fails + return + } + t.Logf("check state after prefilter phase") + stateData, err := getStateData(state) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(stateData, item.wantStateAfterPreFilter) { + t.Errorf("state got after prefilter does not match: %v, want: %v", stateData, item.wantStateAfterPreFilter) + } + t.Logf("call Filter and check status") + gotStatus := p.Filter(ctx, state, item.pod, nodeInfo) + if !reflect.DeepEqual(gotStatus, item.wantFilterStatus) { + t.Errorf("filter status does not match: %v, want: %v", gotStatus, item.wantFilterStatus) + } }) } }