Refactor PV selection into a common call for scheduler and PV controller

pkg/controller/volume/persistentvolume/index.go

@@ -90,6 +90,25 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *v1.PersistentVol
 	// example above).
 	allPossibleModes := pvIndex.allPossibleMatchingAccessModes(claim.Spec.AccessModes)
 
+	for _, modes := range allPossibleModes {
+		volumes, err := pvIndex.listByAccessModes(modes)
+		if err != nil {
+			return nil, err
+		}
+
+		bestVol, err := findMatchingVolume(claim, volumes)
+		if err != nil {
+			return nil, err
+		}
+
+		if bestVol != nil {
+			return bestVol, nil
+		}
+	}
+	return nil, nil
+}
+
+func findMatchingVolume(claim *v1.PersistentVolumeClaim, volumes []*v1.PersistentVolume) (*v1.PersistentVolume, error) {
 	var smallestVolume *v1.PersistentVolume
 	var smallestVolumeQty resource.Quantity
 	requestedQty := claim.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
@@ -105,67 +124,61 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *v1.PersistentVol
 		selector = internalSelector
 	}
 
-	for _, modes := range allPossibleModes {
+	// Go through all available volumes with two goals:
-		volumes, err := pvIndex.listByAccessModes(modes)
+	// - find a volume that is either pre-bound by user or dynamically
+	//   provisioned for this claim. Because of this we need to loop through
+	//   all volumes.
+	// - find the smallest matching one if there is no volume pre-bound to
+	//   the claim.
+	for _, volume := range volumes {
+		volumeQty := volume.Spec.Capacity[v1.ResourceStorage]
+
+		// check if volumeModes do not match (Alpha and feature gate protected)
+		isMisMatch, err := checkVolumeModeMisMatches(&claim.Spec, &volume.Spec)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("error checking if volumeMode was a mismatch: %v", err)
+		}
+		// filter out mismatching volumeModes
+		if isMisMatch {
+			continue
 		}
 
-		// Go through all available volumes with two goals:
+		if isVolumeBoundToClaim(volume, claim) {
-		// - find a volume that is either pre-bound by user or dynamically
+			// this claim and volume are pre-bound; return
-		//   provisioned for this claim. Because of this we need to loop through
+			// the volume if the size request is satisfied,
-		//   all volumes.
+			// otherwise continue searching for a match
-		// - find the smallest matching one if there is no volume pre-bound to
+			if volumeQty.Cmp(requestedQty) < 0 {
-		//   the claim.
-		for _, volume := range volumes {
-			// check if volumeModes do not match (Alpha and feature gate protected)
-			isMisMatch, err := checkVolumeModeMisMatches(&claim.Spec, &volume.Spec)
-			if err != nil {
-				return nil, fmt.Errorf("error checking if volumeMode was a mismatch: %v", err)
-			}
-			// filter out mismatching volumeModes
-			if isMisMatch {
 				continue
 			}
-
+			return volume, nil
-			if isVolumeBoundToClaim(volume, claim) {
-				// this claim and volume are pre-bound; return
-				// the volume if the size request is satisfied,
-				// otherwise continue searching for a match
-				volumeQty := volume.Spec.Capacity[v1.ResourceStorage]
-				if volumeQty.Cmp(requestedQty) < 0 {
-					continue
-				}
-				return volume, nil
-			}
-
-			// filter out:
-			// - volumes bound to another claim
-			// - volumes whose labels don't match the claim's selector, if specified
-			// - volumes in Class that is not requested
-			if volume.Spec.ClaimRef != nil {
-				continue
-			} else if selector != nil && !selector.Matches(labels.Set(volume.Labels)) {
-				continue
-			}
-			if v1helper.GetPersistentVolumeClass(volume) != requestedClass {
-				continue
-			}
-
-			volumeQty := volume.Spec.Capacity[v1.ResourceStorage]
-			if volumeQty.Cmp(requestedQty) >= 0 {
-				if smallestVolume == nil || smallestVolumeQty.Cmp(volumeQty) > 0 {
-					smallestVolume = volume
-					smallestVolumeQty = volumeQty
-				}
-			}
 		}
 
-		if smallestVolume != nil {
+		// filter out:
-			// Found a matching volume
+		// - volumes bound to another claim
-			return smallestVolume, nil
+		// - volumes whose labels don't match the claim's selector, if specified
+		// - volumes in Class that is not requested
+		if volume.Spec.ClaimRef != nil {
+			continue
+		} else if selector != nil && !selector.Matches(labels.Set(volume.Labels)) {
+			continue
+		}
+		if v1helper.GetPersistentVolumeClass(volume) != requestedClass {
+			continue
+		}
+
+		if volumeQty.Cmp(requestedQty) >= 0 {
+			if smallestVolume == nil || smallestVolumeQty.Cmp(volumeQty) > 0 {
+				smallestVolume = volume
+				smallestVolumeQty = volumeQty
+			}
 		}
 	}
+
+	if smallestVolume != nil {
+		// Found a matching volume
+		return smallestVolume, nil
+	}
+
 	return nil, nil
 }