Fix code to handle delayed binding volumes

pkg/scheduler/algorithm/predicates/csi_volume_predicate.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/rand"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/features"
@@ -29,16 +30,20 @@ import (
 
 // CSIMaxVolumeLimitChecker defines predicate needed for counting CSI volumes
 type CSIMaxVolumeLimitChecker struct {
-	pvInfo  PersistentVolumeInfo
+	pvInfo               PersistentVolumeInfo
-	pvcInfo PersistentVolumeClaimInfo
+	pvcInfo              PersistentVolumeClaimInfo
+	scInfo               StorageClassInfo
+	randomVolumeIDPrefix string
 }
 
 // NewCSIMaxVolumeLimitPredicate returns a predicate for counting CSI volumes
 func NewCSIMaxVolumeLimitPredicate(
-	pvInfo PersistentVolumeInfo, pvcInfo PersistentVolumeClaimInfo) FitPredicate {
+	pvInfo PersistentVolumeInfo, pvcInfo PersistentVolumeClaimInfo, scInfo StorageClassInfo) FitPredicate {
 	c := &CSIMaxVolumeLimitChecker{
-		pvInfo:  pvInfo,
+		pvInfo:               pvInfo,
-		pvcInfo: pvcInfo,
+		pvcInfo:              pvcInfo,
+		scInfo:               scInfo,
+		randomVolumeIDPrefix: rand.String(32),
 	}
 	return c.attachableLimitPredicate
 }
@@ -129,28 +134,62 @@ func (c *CSIMaxVolumeLimitChecker) filterAttachableVolumes(
 			continue
 		}
 
-		pvName := pvc.Spec.VolumeName
+		driverName, volumeHandle := c.getCSIDriver(pvc)
-		// TODO - the actual handling of unbound PVCs will be fixed by late binding design.
+		// if we can't find driver name or volume handle - we don't count this volume.
-		if pvName == "" {
+		if driverName == "" || volumeHandle == "" {
-			klog.V(4).Infof("Persistent volume had no name for claim %s/%s", namespace, pvcName)
 			continue
 		}
-		pv, err := c.pvInfo.GetPersistentVolumeInfo(pvName)
-
-		if err != nil {
-			klog.V(4).Infof("Unable to look up PV info for PVC %s/%s and PV %s", namespace, pvcName, pvName)
-			continue
-		}
-
-		csiSource := pv.Spec.PersistentVolumeSource.CSI
-		if csiSource == nil {
-			klog.V(4).Infof("Not considering non-CSI volume %s/%s", namespace, pvcName)
-			continue
-		}
-		driverName := csiSource.Driver
 		volumeLimitKey := volumeutil.GetCSIAttachLimitKey(driverName)
-		result[csiSource.VolumeHandle] = volumeLimitKey
+		result[volumeHandle] = volumeLimitKey
 
 	}
 	return nil
 }
+
+func (c *CSIMaxVolumeLimitChecker) getCSIDriver(pvc *v1.PersistentVolumeClaim) (string, string) {
+	pvName := pvc.Spec.VolumeName
+	namespace := pvc.Namespace
+	pvcName := pvc.Name
+
+	placeHolderCSIDriver := ""
+	placeHolderHandle := ""
+	if pvName == "" {
+		klog.V(4).Infof("Persistent volume had no name for claim %s/%s", namespace, pvcName)
+		return c.getDriverNameFromSC(pvc)
+	}
+	pv, err := c.pvInfo.GetPersistentVolumeInfo(pvName)
+
+	if err != nil {
+		klog.V(4).Infof("Unable to look up PV info for PVC %s/%s and PV %s", namespace, pvcName, pvName)
+		return placeHolderCSIDriver, placeHolderHandle
+	}
+
+	csiSource := pv.Spec.PersistentVolumeSource.CSI
+	if csiSource == nil {
+		klog.V(4).Infof("Not considering non-CSI volume %s/%s", namespace, pvcName)
+		return placeHolderCSIDriver, placeHolderHandle
+	}
+	return csiSource.Driver, csiSource.VolumeHandle
+}
+
+func (c *CSIMaxVolumeLimitChecker) getDriverNameFromSC(pvc *v1.PersistentVolumeClaim) (string, string) {
+	namespace := pvc.Namespace
+	pvcName := pvc.Name
+	scName := *pvc.Spec.StorageClassName
+
+	placeHolderCSIDriver := ""
+	placeHolderHandle := ""
+	if scName == "" {
+		klog.V(4).Infof("pvc %s/%s has no storageClass", namespace, pvcName)
+		return placeHolderCSIDriver, placeHolderHandle
+	}
+
+	storageClass, err := c.scInfo.GetStorageClassInfo(scName)
+	if err != nil {
+		klog.V(4).Infof("no storage %s found for pvc %s/%s", scName, namespace, pvcName)
+		return placeHolderCSIDriver, placeHolderHandle
+	}
+
+	volumeHandle := fmt.Sprintf("%s-%s/%s", c.randomVolumeIDPrefix, namespace, pvcName)
+	return storageClass.Provisioner, volumeHandle
+}

pkg/scheduler/algorithm/predicates/csi_volume_predicate_test.go

@@ -77,6 +77,20 @@ func TestCSIVolumeCountPredicate(t *testing.T) {
 		},
 	}
 
+	pendingVolumePod := &v1.Pod{
+		Spec: v1.PodSpec{
+			Volumes: []v1.Volume{
+				{
+					VolumeSource: v1.VolumeSource{
+						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
+							ClaimName: "csi-ebs-4",
+						},
+					},
+				},
+			},
+		},
+	}
+
 	tests := []struct {
 		newPod       *v1.Pod
 		existingPods []*v1.Pod
@@ -101,6 +115,15 @@ func TestCSIVolumeCountPredicate(t *testing.T) {
 			fits:         false,
 			test:         "doesn't when node capacity <= pods CSI volume",
 		},
+		// should count pending PVCs
+		{
+			newPod:       oneVolPod,
+			existingPods: []*v1.Pod{pendingVolumePod, twoVolPod},
+			filterName:   "csi-ebs",
+			maxVols:      2,
+			fits:         false,
+			test:         "doesn't when node capacity <= pods CSI volume",
+		},
 	}
 
 	defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, true)()
@@ -108,7 +131,7 @@ func TestCSIVolumeCountPredicate(t *testing.T) {
 	// running attachable predicate tests with feature gate and limit present on nodes
 	for _, test := range tests {
 		node := getNodeWithPodAndVolumeLimits(test.existingPods, int64(test.maxVols), test.filterName)
-		pred := NewCSIMaxVolumeLimitPredicate(getFakeCSIPVInfo("csi-ebs", "csi-ebs"), getFakeCSIPVCInfo("csi-ebs"))
+		pred := NewCSIMaxVolumeLimitPredicate(getFakeCSIPVInfo("csi-ebs", "csi-ebs"), getFakeCSIPVCInfo("csi-ebs", "csi-ebs-gp2"), getFakeCSIStorageClassInfo("csi-ebs-gp2", "csi-ebs"))
 		fits, reasons, err := pred(test.newPod, GetPredicateMetadata(test.newPod, nil), node)
 		if err != nil {
 			t.Errorf("Using allocatable [%s]%s: unexpected error: %v", test.filterName, test.test, err)
@@ -160,7 +183,7 @@ func getFakeCSIPVInfo(volumeName, driverName string) FakePersistentVolumeInfo {
 	}
 }
 
-func getFakeCSIPVCInfo(volumeName string) FakePersistentVolumeClaimInfo {
+func getFakeCSIPVCInfo(volumeName, scName string) FakePersistentVolumeClaimInfo {
 	return FakePersistentVolumeClaimInfo{
 		{
 			ObjectMeta: metav1.ObjectMeta{Name: volumeName},
@@ -174,5 +197,18 @@ func getFakeCSIPVCInfo(volumeName string) FakePersistentVolumeClaimInfo {
 			ObjectMeta: metav1.ObjectMeta{Name: volumeName + "-3"},
 			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: volumeName + "-3"},
 		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: volumeName + "-4"},
+			Spec:       v1.PersistentVolumeClaimSpec{StorageClassName: &scName},
+		},
+	}
+}
+
+func getFakeCSIStorageClassInfo(scName, provisionerName string) FakeStorageClassInfo {
+	return FakeStorageClassInfo{
+		{
+			ObjectMeta:  metav1.ObjectMeta{Name: scName},
+			Provisioner: provisionerName,
+		},
 	}
 }

pkg/scheduler/algorithmprovider/defaults/register_predicates.go

@@ -81,7 +81,7 @@ func init() {
 	factory.RegisterFitPredicateFactory(
 		predicates.MaxCSIVolumeCountPred,
 		func(args factory.PluginFactoryArgs) predicates.FitPredicate {
-			return predicates.NewCSIMaxVolumeLimitPredicate(args.PVInfo, args.PVCInfo)
+			return predicates.NewCSIMaxVolumeLimitPredicate(args.PVInfo, args.PVCInfo, args.StorageClassInfo)
 		},
 	)
 	factory.RegisterFitPredicateFactory(