Merge pull request #56598 from msau42/pv-binding

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Return no volume match if prebound PV node affinity doesn't match node

**What this PR does / why we need it**:
VolumeBindingChecker predicate needs to return false for prebound PVs if the NodeAffinity doesn't match the node.

Also fix log formatting in predicate.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #56596

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

pkg/controller/volume/persistentvolume/index.go

@@ -169,12 +169,13 @@ func findMatchingVolume(
 			continue
 		}
 
+		nodeAffinityValid := true
 		if node != nil {
 			// Scheduler path, check that the PV NodeAffinity
 			// is satisfied by the node
 			err := volumeutil.CheckNodeAffinity(volume, node.Labels)
 			if err != nil {
-				continue
+				nodeAffinityValid = false
 			}
 		}
 
@@ -185,6 +186,14 @@ func findMatchingVolume(
 			if volumeQty.Cmp(requestedQty) < 0 {
 				continue
 			}
+
+			// If PV node affinity is invalid, return no match.
+			// This means the prebound PV (and therefore PVC)
+			// is not suitable for this node.
+			if !nodeAffinityValid {
+				return nil, nil
+			}
+
 			return volume, nil
 		}
 
@@ -199,6 +208,7 @@ func findMatchingVolume(
 		// - volumes bound to another claim
 		// - volumes whose labels don't match the claim's selector, if specified
 		// - volumes in Class that is not requested
+		// - volumes whose NodeAffinity does not match the node
 		if volume.Spec.ClaimRef != nil {
 			continue
 		} else if selector != nil && !selector.Matches(labels.Set(volume.Labels)) {
@@ -207,6 +217,9 @@ func findMatchingVolume(
 		if v1helper.GetPersistentVolumeClass(volume) != requestedClass {
 			continue
 		}
+		if !nodeAffinityValid {
+			continue
+		}
 
 		if node != nil {
 			// Scheduler path

pkg/controller/volume/persistentvolume/index_test.go

@@ -1218,7 +1218,7 @@ func TestFindMatchVolumeWithNode(t *testing.T) {
 				pvc.Spec.StorageClassName = &classWait
 				pvc.Name = "claim02"
 			}),
-			node: node2,
+			node: node3,
 		},
 		"success-bad-and-good-node-affinity": {
 			expectedMatch: "affinity-pv3",

plugin/pkg/scheduler/algorithm/predicates/predicates.go

@@ -1475,12 +1475,12 @@ func (c *VolumeBindingChecker) predicate(pod *v1.Pod, meta algorithm.PredicateMe
 
 	failReasons := []algorithm.PredicateFailureReason{}
 	if !boundSatisfied {
-		glog.V(5).Info("Bound PVs not satisfied for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name)
+		glog.V(5).Infof("Bound PVs not satisfied for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name)
 		failReasons = append(failReasons, ErrVolumeNodeConflict)
 	}
 
 	if !unboundSatisfied {
-		glog.V(5).Info("Couldn't find matching PVs for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name)
+		glog.V(5).Infof("Couldn't find matching PVs for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name)
 		failReasons = append(failReasons, ErrVolumeBindConflict)
 	}
 
@@ -1489,6 +1489,6 @@ func (c *VolumeBindingChecker) predicate(pod *v1.Pod, meta algorithm.PredicateMe
 	}
 
 	// All volumes bound or matching PVs found for all unbound PVCs
-	glog.V(5).Info("All PVCs found matches for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name)
+	glog.V(5).Infof("All PVCs found matches for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name)
 	return true, nil, nil
 }