diff --git a/pkg/controller/volume/persistentvolume/util/util.go b/pkg/controller/volume/persistentvolume/util/util.go index 3f526ae09d9..5c132f7e2a0 100644 --- a/pkg/controller/volume/persistentvolume/util/util.go +++ b/pkg/controller/volume/persistentvolume/util/util.go @@ -211,9 +211,14 @@ func FindMatchingVolume( // Skip volumes in the excluded list continue } + if volume.Spec.ClaimRef != nil && !IsVolumeBoundToClaim(volume, claim) { + continue + } volumeQty := volume.Spec.Capacity[v1.ResourceStorage] - + if volumeQty.Cmp(requestedQty) < 0 { + continue + } // filter out mismatching volumeModes if CheckVolumeModeMismatches(&claim.Spec, &volume.Spec) { continue @@ -230,6 +235,8 @@ func FindMatchingVolume( if node != nil { // Scheduler path, check that the PV NodeAffinity // is satisfied by the node + // volumeutil.CheckNodeAffinity is the most expensive call in this loop. + // We should check cheaper conditions first or consider optimizing this function. err := volumeutil.CheckNodeAffinity(volume, node.Labels) if err != nil { nodeAffinityValid = false @@ -237,13 +244,6 @@ func FindMatchingVolume( } 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 - 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. @@ -263,7 +263,6 @@ func FindMatchingVolume( // filter out: // - volumes in non-available phase - // - 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 @@ -273,8 +272,6 @@ func FindMatchingVolume( // them now has high chance of encountering unnecessary failures // due to API conflicts. continue - } else if volume.Spec.ClaimRef != nil { - continue } else if selector != nil && !selector.Matches(labels.Set(volume.Labels)) { continue } @@ -293,11 +290,9 @@ func FindMatchingVolume( } } - if volumeQty.Cmp(requestedQty) >= 0 { - if smallestVolume == nil || smallestVolumeQty.Cmp(volumeQty) > 0 { - smallestVolume = volume - smallestVolumeQty = volumeQty - } + if smallestVolume == nil || smallestVolumeQty.Cmp(volumeQty) > 0 { + smallestVolume = volume + smallestVolumeQty = volumeQty } } diff --git a/test/utils/runners.go b/test/utils/runners.go index 6c4218f5f50..3d0498cfd21 100644 --- a/test/utils/runners.go +++ b/test/utils/runners.go @@ -1385,12 +1385,11 @@ func CreatePodWithPersistentVolume(client clientset.Interface, namespace string, createError = fmt.Errorf("error creating PV: %s", err) return } - // We need to update status separately, as creating persistentvolumes resets status to the default one - // (so with Status.Phase will be equal to PersistentVolumePhase). + // We need to update statuses separately, as creating pv/pvc resets status to the default one. if _, err := client.CoreV1().PersistentVolumes().UpdateStatus(context.TODO(), pv, metav1.UpdateOptions{}); err != nil { lock.Lock() defer lock.Unlock() - createError = fmt.Errorf("error creating PV: %s", err) + createError = fmt.Errorf("error updating PV status: %s", err) return } @@ -1400,6 +1399,12 @@ func CreatePodWithPersistentVolume(client clientset.Interface, namespace string, createError = fmt.Errorf("error creating PVC: %s", err) return } + if _, err := client.CoreV1().PersistentVolumeClaims(namespace).UpdateStatus(context.TODO(), pvc, metav1.UpdateOptions{}); err != nil { + lock.Lock() + defer lock.Unlock() + createError = fmt.Errorf("error updating PVC status: %s", err) + return + } // pod pod := podTemplate.DeepCopy()