Merge pull request #86623 from zouyee/vz

Move VolumeZone predicate to its Filter plugin

pkg/scheduler/algorithm/predicates/BUILD

@@ -34,7 +34,6 @@ go_library(
         "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
         "//staging/src/k8s.io/client-go/listers/storage/v1:go_default_library",
         "//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
-        "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library",
         "//staging/src/k8s.io/csi-translation-lib:go_default_library",
         "//staging/src/k8s.io/csi-translation-lib/plugins:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",

pkg/scheduler/algorithm/predicates/predicates.go

@@ -34,7 +34,6 @@ import (
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	corelisters "k8s.io/client-go/listers/core/v1"
 	storagelisters "k8s.io/client-go/listers/storage/v1"
-	volumehelpers "k8s.io/cloud-provider/volume/helpers"
 	csilibplugins "k8s.io/csi-translation-lib/plugins"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 	"k8s.io/kubernetes/pkg/features"
@@ -535,137 +534,6 @@ var CinderVolumeFilter = VolumeFilter{
 	},
 }
 
-// VolumeZoneChecker contains information to check the volume zone for a predicate.
-type VolumeZoneChecker struct {
-	pvLister  corelisters.PersistentVolumeLister
-	pvcLister corelisters.PersistentVolumeClaimLister
-	scLister  storagelisters.StorageClassLister
-}
-
-// NewVolumeZonePredicate evaluates if a pod can fit due to the volumes it requests, given
-// that some volumes may have zone scheduling constraints.  The requirement is that any
-// volume zone-labels must match the equivalent zone-labels on the node.  It is OK for
-// the node to have more zone-label constraints (for example, a hypothetical replicated
-// volume might allow region-wide access)
-//
-// Currently this is only supported with PersistentVolumeClaims, and looks to the labels
-// only on the bound PersistentVolume.
-//
-// Working with volumes declared inline in the pod specification (i.e. not
-// using a PersistentVolume) is likely to be harder, as it would require
-// determining the zone of a volume during scheduling, and that is likely to
-// require calling out to the cloud provider.  It seems that we are moving away
-// from inline volume declarations anyway.
-func NewVolumeZonePredicate(pvLister corelisters.PersistentVolumeLister, pvcLister corelisters.PersistentVolumeClaimLister, scLister storagelisters.StorageClassLister) FitPredicate {
-	c := &VolumeZoneChecker{
-		pvLister:  pvLister,
-		pvcLister: pvcLister,
-		scLister:  scLister,
-	}
-	return c.predicate
-}
-
-func (c *VolumeZoneChecker) predicate(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) {
-	// If a pod doesn't have any volume attached to it, the predicate will always be true.
-	// Thus we make a fast path for it, to avoid unnecessary computations in this case.
-	if len(pod.Spec.Volumes) == 0 {
-		return true, nil, nil
-	}
-
-	node := nodeInfo.Node()
-	if node == nil {
-		return false, nil, fmt.Errorf("node not found")
-	}
-
-	nodeConstraints := make(map[string]string)
-	for k, v := range node.ObjectMeta.Labels {
-		if k != v1.LabelZoneFailureDomain && k != v1.LabelZoneRegion {
-			continue
-		}
-		nodeConstraints[k] = v
-	}
-
-	if len(nodeConstraints) == 0 {
-		// The node has no zone constraints, so we're OK to schedule.
-		// In practice, when using zones, all nodes must be labeled with zone labels.
-		// We want to fast-path this case though.
-		return true, nil, nil
-	}
-
-	namespace := pod.Namespace
-	manifest := &(pod.Spec)
-	for i := range manifest.Volumes {
-		volume := &manifest.Volumes[i]
-		if volume.PersistentVolumeClaim == nil {
-			continue
-		}
-		pvcName := volume.PersistentVolumeClaim.ClaimName
-		if pvcName == "" {
-			return false, nil, fmt.Errorf("PersistentVolumeClaim had no name")
-		}
-		pvc, err := c.pvcLister.PersistentVolumeClaims(namespace).Get(pvcName)
-		if err != nil {
-			return false, nil, err
-		}
-
-		if pvc == nil {
-			return false, nil, fmt.Errorf("PersistentVolumeClaim was not found: %q", pvcName)
-		}
-
-		pvName := pvc.Spec.VolumeName
-		if pvName == "" {
-			scName := v1helper.GetPersistentVolumeClaimClass(pvc)
-			if len(scName) == 0 {
-				return false, nil, fmt.Errorf("PersistentVolumeClaim had no pv name and storageClass name")
-			}
-
-			class, _ := c.scLister.Get(scName)
-			if class == nil {
-				return false, nil, fmt.Errorf("StorageClass %q claimed by PersistentVolumeClaim %q not found",
-					scName, pvcName)
-
-			}
-			if class.VolumeBindingMode == nil {
-				return false, nil, fmt.Errorf("VolumeBindingMode not set for StorageClass %q", scName)
-			}
-			if *class.VolumeBindingMode == storage.VolumeBindingWaitForFirstConsumer {
-				// Skip unbound volumes
-				continue
-			}
-
-			return false, nil, fmt.Errorf("PersistentVolume had no name")
-		}
-
-		pv, err := c.pvLister.Get(pvName)
-		if err != nil {
-			return false, nil, err
-		}
-
-		if pv == nil {
-			return false, nil, fmt.Errorf("PersistentVolume was not found: %q", pvName)
-		}
-
-		for k, v := range pv.ObjectMeta.Labels {
-			if k != v1.LabelZoneFailureDomain && k != v1.LabelZoneRegion {
-				continue
-			}
-			nodeV, _ := nodeConstraints[k]
-			volumeVSet, err := volumehelpers.LabelZonesToSet(v)
-			if err != nil {
-				klog.Warningf("Failed to parse label for %q: %q. Ignoring the label. err=%v. ", k, v, err)
-				continue
-			}
-
-			if !volumeVSet.Has(nodeV) {
-				klog.V(10).Infof("Won't schedule pod %q onto node %q due to volume %q (mismatch on %q)", pod.Name, node.Name, pvName, k)
-				return false, []PredicateFailureReason{ErrVolumeZoneConflict}, nil
-			}
-		}
-	}
-
-	return true, nil, nil
-}
-
 // GetResourceRequest returns a *schedulernodeinfo.Resource that covers the largest
 // width in each resource dimension. Because init-containers run sequentially, we collect
 // the max in each dimension iteratively. In contrast, we sum the resource vectors for

pkg/scheduler/algorithm/predicates/predicates_test.go

@@ -24,7 +24,6 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -33,7 +32,6 @@ import (
 	api "k8s.io/kubernetes/pkg/apis/core"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 	"k8s.io/kubernetes/pkg/features"
-	fakelisters "k8s.io/kubernetes/pkg/scheduler/listers/fake"
 	schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
 )
 
@@ -1722,364 +1720,6 @@ func TestPodToleratesTaints(t *testing.T) {
 	}
 }
 
-func createPodWithVolume(pod, pv, pvc string) *v1.Pod {
-	return &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{Name: pod, Namespace: "default"},
-		Spec: v1.PodSpec{
-			Volumes: []v1.Volume{
-				{
-					Name: pv,
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: pvc,
-						},
-					},
-				},
-			},
-		},
-	}
-}
-
-func TestVolumeZonePredicate(t *testing.T) {
-	pvLister := fakelisters.PersistentVolumeLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_1", Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a"}},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_2", Labels: map[string]string{v1.LabelZoneRegion: "us-west1-b", "uselessLabel": "none"}},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_3", Labels: map[string]string{v1.LabelZoneRegion: "us-west1-c"}},
-		},
-	}
-
-	pvcLister := fakelisters.PersistentVolumeClaimLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_2", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_2"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_3", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_3"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_4", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_not_exist"},
-		},
-	}
-
-	tests := []struct {
-		name string
-		Pod  *v1.Pod
-		Fits bool
-		Node *v1.Node
-	}{
-		{
-			name: "pod without volume",
-			Pod: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{Name: "pod_1", Namespace: "default"},
-			},
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a"},
-				},
-			},
-			Fits: true,
-		},
-		{
-			name: "node without labels",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_1"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "host1",
-				},
-			},
-			Fits: true,
-		},
-		{
-			name: "label zone failure domain matched",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_1"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a", "uselessLabel": "none"},
-				},
-			},
-			Fits: true,
-		},
-		{
-			name: "label zone region matched",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_2"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneRegion: "us-west1-b", "uselessLabel": "none"},
-				},
-			},
-			Fits: true,
-		},
-		{
-			name: "label zone region failed match",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_2"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneRegion: "no_us-west1-b", "uselessLabel": "none"},
-				},
-			},
-			Fits: false,
-		},
-		{
-			name: "label zone failure domain failed match",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_1"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneFailureDomain: "no_us-west1-a", "uselessLabel": "none"},
-				},
-			},
-			Fits: false,
-		},
-	}
-
-	expectedFailureReasons := []PredicateFailureReason{ErrVolumeZoneConflict}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			fit := NewVolumeZonePredicate(pvLister, pvcLister, nil)
-			node := &schedulernodeinfo.NodeInfo{}
-			node.SetNode(test.Node)
-
-			fits, reasons, err := fit(test.Pod, nil, node)
-			if err != nil {
-				t.Errorf("unexpected error: %v", err)
-			}
-			if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) {
-				t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons)
-			}
-			if fits != test.Fits {
-				t.Errorf("expected %v got %v", test.Fits, fits)
-			}
-		})
-	}
-}
-
-func TestVolumeZonePredicateMultiZone(t *testing.T) {
-	pvLister := fakelisters.PersistentVolumeLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_1", Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a"}},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_2", Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-b", "uselessLabel": "none"}},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_3", Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-c__us-west1-a"}},
-		},
-	}
-
-	pvcLister := fakelisters.PersistentVolumeClaimLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_2", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_2"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_3", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_3"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_4", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_not_exist"},
-		},
-	}
-
-	tests := []struct {
-		name string
-		Pod  *v1.Pod
-		Fits bool
-		Node *v1.Node
-	}{
-		{
-			name: "node without labels",
-			Pod:  createPodWithVolume("pod_1", "Vol_3", "PVC_3"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "host1",
-				},
-			},
-			Fits: true,
-		},
-		{
-			name: "label zone failure domain matched",
-			Pod:  createPodWithVolume("pod_1", "Vol_3", "PVC_3"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a", "uselessLabel": "none"},
-				},
-			},
-			Fits: true,
-		},
-		{
-			name: "label zone failure domain failed match",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_1"),
-			Node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "host1",
-					Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-b", "uselessLabel": "none"},
-				},
-			},
-			Fits: false,
-		},
-	}
-
-	expectedFailureReasons := []PredicateFailureReason{ErrVolumeZoneConflict}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			fit := NewVolumeZonePredicate(pvLister, pvcLister, nil)
-			node := &schedulernodeinfo.NodeInfo{}
-			node.SetNode(test.Node)
-
-			fits, reasons, err := fit(test.Pod, nil, node)
-			if err != nil {
-				t.Errorf("unexpected error: %v", err)
-			}
-			if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) {
-				t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons)
-			}
-			if fits != test.Fits {
-				t.Errorf("expected %v got %v", test.Fits, fits)
-			}
-		})
-	}
-}
-
-func TestVolumeZonePredicateWithVolumeBinding(t *testing.T) {
-	var (
-		modeWait = storagev1.VolumeBindingWaitForFirstConsumer
-
-		class0         = "Class_0"
-		classWait      = "Class_Wait"
-		classImmediate = "Class_Immediate"
-	)
-
-	scLister := fakelisters.StorageClassLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: classImmediate},
-		},
-		{
-			ObjectMeta:        metav1.ObjectMeta{Name: classWait},
-			VolumeBindingMode: &modeWait,
-		},
-	}
-
-	pvLister := fakelisters.PersistentVolumeLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_1", Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a"}},
-		},
-	}
-
-	pvcLister := fakelisters.PersistentVolumeClaimLister{
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_NoSC", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{StorageClassName: &class0},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_EmptySC", Namespace: "default"},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_WaitSC", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{StorageClassName: &classWait},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{Name: "PVC_ImmediateSC", Namespace: "default"},
-			Spec:       v1.PersistentVolumeClaimSpec{StorageClassName: &classImmediate},
-		},
-	}
-
-	testNode := &v1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:   "host1",
-			Labels: map[string]string{v1.LabelZoneFailureDomain: "us-west1-a", "uselessLabel": "none"},
-		},
-	}
-
-	tests := []struct {
-		name          string
-		Pod           *v1.Pod
-		Fits          bool
-		Node          *v1.Node
-		ExpectFailure bool
-	}{
-		{
-			name: "label zone failure domain matched",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_1"),
-			Node: testNode,
-			Fits: true,
-		},
-		{
-			name:          "unbound volume empty storage class",
-			Pod:           createPodWithVolume("pod_1", "vol_1", "PVC_EmptySC"),
-			Node:          testNode,
-			Fits:          false,
-			ExpectFailure: true,
-		},
-		{
-			name:          "unbound volume no storage class",
-			Pod:           createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"),
-			Node:          testNode,
-			Fits:          false,
-			ExpectFailure: true,
-		},
-		{
-			name:          "unbound volume immediate binding mode",
-			Pod:           createPodWithVolume("pod_1", "vol_1", "PVC_ImmediateSC"),
-			Node:          testNode,
-			Fits:          false,
-			ExpectFailure: true,
-		},
-		{
-			name: "unbound volume wait binding mode",
-			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_WaitSC"),
-			Node: testNode,
-			Fits: true,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			fit := NewVolumeZonePredicate(pvLister, pvcLister, scLister)
-			node := &schedulernodeinfo.NodeInfo{}
-			node.SetNode(test.Node)
-
-			fits, _, err := fit(test.Pod, nil, node)
-			if !test.ExpectFailure && err != nil {
-				t.Errorf("unexpected error: %v", err)
-			}
-			if test.ExpectFailure && err == nil {
-				t.Errorf("expected error, got success")
-			}
-			if fits != test.Fits {
-				t.Errorf("expected %v got %v", test.Fits, fits)
-			}
-		})
-	}
-
-}
-
 func TestGetMaxVols(t *testing.T) {
 	previousValue := os.Getenv(KubeMaxPDVols)
 

pkg/scheduler/framework/plugins/volumezone/BUILD

@@ -6,12 +6,17 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumezone",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/apis/core/v1/helper:go_default_library",
         "//pkg/scheduler/algorithm/predicates:go_default_library",
-        "//pkg/scheduler/framework/plugins/migration:go_default_library",
         "//pkg/scheduler/framework/v1alpha1:go_default_library",
         "//pkg/scheduler/nodeinfo: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/runtime:go_default_library",
+        "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
+        "//staging/src/k8s.io/client-go/listers/storage/v1:go_default_library",
+        "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 

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

@@ -18,18 +18,26 @@ package volumezone
 
 import (
 	"context"
+	"fmt"
 
 	v1 "k8s.io/api/core/v1"
+	storage "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	corelisters "k8s.io/client-go/listers/core/v1"
+	storagelisters "k8s.io/client-go/listers/storage/v1"
+	volumehelpers "k8s.io/cloud-provider/volume/helpers"
+	"k8s.io/klog"
+	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 	"k8s.io/kubernetes/pkg/scheduler/algorithm/predicates"
-	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/migration"
 	framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 	"k8s.io/kubernetes/pkg/scheduler/nodeinfo"
 )
 
 // VolumeZone is a plugin that checks volume zone.
 type VolumeZone struct {
-	predicate predicates.FitPredicate
+	pvLister  corelisters.PersistentVolumeLister
+	pvcLister corelisters.PersistentVolumeClaimLister
+	scLister  storagelisters.StorageClassLister
 }
 
 var _ framework.FilterPlugin = &VolumeZone{}
@@ -43,10 +51,113 @@ func (pl *VolumeZone) Name() string {
 }
 
 // Filter invoked at the filter extension point.
+//
+// It evaluates if a pod can fit due to the volumes it requests, given
+// that some volumes may have zone scheduling constraints.  The requirement is that any
+// volume zone-labels must match the equivalent zone-labels on the node.  It is OK for
+// the node to have more zone-label constraints (for example, a hypothetical replicated
+// volume might allow region-wide access)
+//
+// Currently this is only supported with PersistentVolumeClaims, and looks to the labels
+// only on the bound PersistentVolume.
+//
+// Working with volumes declared inline in the pod specification (i.e. not
+// using a PersistentVolume) is likely to be harder, as it would require
+// determining the zone of a volume during scheduling, and that is likely to
+// require calling out to the cloud provider.  It seems that we are moving away
+// from inline volume declarations anyway.
 func (pl *VolumeZone) Filter(ctx context.Context, _ *framework.CycleState, pod *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status {
-	// metadata is not needed
+	// If a pod doesn't have any volume attached to it, the predicate will always be true.
-	_, reasons, err := pl.predicate(pod, nil, nodeInfo)
+	// Thus we make a fast path for it, to avoid unnecessary computations in this case.
-	return migration.PredicateResultToFrameworkStatus(reasons, err)
+	if len(pod.Spec.Volumes) == 0 {
+		return nil
+	}
+	node := nodeInfo.Node()
+	if node == nil {
+		return framework.NewStatus(framework.Error, "node not found")
+	}
+	nodeConstraints := make(map[string]string)
+	for k, v := range node.ObjectMeta.Labels {
+		if k != v1.LabelZoneFailureDomain && k != v1.LabelZoneRegion {
+			continue
+		}
+		nodeConstraints[k] = v
+	}
+	if len(nodeConstraints) == 0 {
+		// The node has no zone constraints, so we're OK to schedule.
+		// In practice, when using zones, all nodes must be labeled with zone labels.
+		// We want to fast-path this case though.
+		return nil
+	}
+
+	for i := range pod.Spec.Volumes {
+		volume := pod.Spec.Volumes[i]
+		if volume.PersistentVolumeClaim == nil {
+			continue
+		}
+		pvcName := volume.PersistentVolumeClaim.ClaimName
+		if pvcName == "" {
+			return framework.NewStatus(framework.Error, "PersistentVolumeClaim had no name")
+		}
+		pvc, err := pl.pvcLister.PersistentVolumeClaims(pod.Namespace).Get(pvcName)
+		if err != nil {
+			return framework.NewStatus(framework.Error, err.Error())
+		}
+
+		if pvc == nil {
+			return framework.NewStatus(framework.Error, fmt.Sprintf("PersistentVolumeClaim was not found: %q", pvcName))
+		}
+
+		pvName := pvc.Spec.VolumeName
+		if pvName == "" {
+			scName := v1helper.GetPersistentVolumeClaimClass(pvc)
+			if len(scName) == 0 {
+				return framework.NewStatus(framework.Error, fmt.Sprint("PersistentVolumeClaim had no pv name and storageClass name"))
+			}
+
+			class, _ := pl.scLister.Get(scName)
+			if class == nil {
+				return framework.NewStatus(framework.Error, fmt.Sprintf("StorageClass %q claimed by PersistentVolumeClaim %q not found", scName, pvcName))
+
+			}
+			if class.VolumeBindingMode == nil {
+				return framework.NewStatus(framework.Error, fmt.Sprintf("VolumeBindingMode not set for StorageClass %q", scName))
+			}
+			if *class.VolumeBindingMode == storage.VolumeBindingWaitForFirstConsumer {
+				// Skip unbound volumes
+				continue
+			}
+
+			return framework.NewStatus(framework.Error, fmt.Sprint("PersistentVolume had no name"))
+		}
+
+		pv, err := pl.pvLister.Get(pvName)
+		if err != nil {
+			return framework.NewStatus(framework.Error, err.Error())
+		}
+
+		if pv == nil {
+			return framework.NewStatus(framework.Error, fmt.Sprintf("PersistentVolume was not found: %q", pvName))
+		}
+
+		for k, v := range pv.ObjectMeta.Labels {
+			if k != v1.LabelZoneFailureDomain && k != v1.LabelZoneRegion {
+				continue
+			}
+			nodeV, _ := nodeConstraints[k]
+			volumeVSet, err := volumehelpers.LabelZonesToSet(v)
+			if err != nil {
+				klog.Warningf("Failed to parse label for %q: %q. Ignoring the label. err=%v. ", k, v, err)
+				continue
+			}
+
+			if !volumeVSet.Has(nodeV) {
+				klog.V(10).Infof("Won't schedule pod %q onto node %q due to volume %q (mismatch on %q)", pod.Name, node.Name, pvName, k)
+				return framework.NewStatus(framework.UnschedulableAndUnresolvable, predicates.ErrVolumeZoneConflict.GetReason())
+			}
+		}
+	}
+	return nil
 }
 
 // New initializes a new plugin and returns it.
@@ -56,6 +167,8 @@ func New(_ *runtime.Unknown, handle framework.FrameworkHandle) (framework.Plugin
 	pvcLister := informerFactory.Core().V1().PersistentVolumeClaims().Lister()
 	scLister := informerFactory.Storage().V1().StorageClasses().Lister()
 	return &VolumeZone{
-		predicate: predicates.NewVolumeZonePredicate(pvLister, pvcLister, scLister),
+		pvLister,
+		pvcLister,
+		scLister,
 	}, nil
 }

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

@@ -156,7 +156,9 @@ func TestSingleZone(t *testing.T) {
 			node := &schedulernodeinfo.NodeInfo{}
 			node.SetNode(test.Node)
 			p := &VolumeZone{
-				predicate: predicates.NewVolumeZonePredicate(pvLister, pvcLister, nil),
+				pvLister,
+				pvcLister,
+				nil,
 			}
 			gotStatus := p.Filter(context.Background(), nil, test.Pod, node)
 			if !reflect.DeepEqual(gotStatus, test.wantStatus) {
@@ -241,7 +243,9 @@ func TestMultiZone(t *testing.T) {
 			node := &schedulernodeinfo.NodeInfo{}
 			node.SetNode(test.Node)
 			p := &VolumeZone{
-				predicate: predicates.NewVolumeZonePredicate(pvLister, pvcLister, nil),
+				pvLister,
+				pvcLister,
+				nil,
 			}
 			gotStatus := p.Filter(context.Background(), nil, test.Pod, node)
 			if !reflect.DeepEqual(gotStatus, test.wantStatus) {
@@ -348,7 +352,9 @@ func TestWithBinding(t *testing.T) {
 			node := &schedulernodeinfo.NodeInfo{}
 			node.SetNode(test.Node)
 			p := &VolumeZone{
-				predicate: predicates.NewVolumeZonePredicate(pvLister, pvcLister, scLister),
+				pvLister,
+				pvcLister,
+				scLister,
 			}
 			gotStatus := p.Filter(context.Background(), nil, test.Pod, node)
 			if !reflect.DeepEqual(gotStatus, test.wantStatus) {