Merge pull request #84611 from cwdsuzhou/Nov/simplyfy_zone_checker

Simplify volume zone checker codes

pkg/scheduler/algorithm/predicates/predicates.go

@@ -667,62 +667,69 @@ func (c *VolumeZoneChecker) predicate(pod *v1.Pod, meta Metadata, nodeInfo *sche
 	manifest := &(pod.Spec)
 	for i := range manifest.Volumes {
 		volume := &manifest.Volumes[i]
-		if volume.PersistentVolumeClaim != nil {
+		if volume.PersistentVolumeClaim == nil {
-			pvcName := volume.PersistentVolumeClaim.ClaimName
+			continue
-			if pvcName == "" {
+		}
-				return false, nil, fmt.Errorf("PersistentVolumeClaim had no name")
+		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")
 			}
-			pvc, err := c.pvcLister.PersistentVolumeClaims(namespace).Get(pvcName)
+
+			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 {
-				return false, nil, err
+				klog.Warningf("Failed to parse label for %q: %q. Ignoring the label. err=%v. ", k, v, err)
+				continue
 			}
 
-			if pvc == nil {
+			if !volumeVSet.Has(nodeV) {
-				return false, nil, fmt.Errorf("PersistentVolumeClaim was not found: %q", pvcName)
+				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
-
-			pvName := pvc.Spec.VolumeName
-			if pvName == "" {
-				scName := v1helper.GetPersistentVolumeClaimClass(pvc)
-				if len(scName) > 0 {
-					class, _ := c.scLister.Get(scName)
-					if class != nil {
-						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("PersistentVolumeClaim was not found: %q", pvcName)
-			}
-
-			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
-				}
 			}
 		}
 	}

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

@@ -317,16 +317,18 @@ func TestWithBinding(t *testing.T) {
 			Node: testNode,
 		},
 		{
-			name:       "unbound volume empty storage class",
+			name: "unbound volume empty storage class",
-			Pod:        createPodWithVolume("pod_1", "vol_1", "PVC_EmptySC"),
+			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_EmptySC"),
-			Node:       testNode,
+			Node: testNode,
-			wantStatus: framework.NewStatus(framework.Error, "PersistentVolumeClaim was not found: \"PVC_EmptySC\""),
+			wantStatus: framework.NewStatus(framework.Error,
+				"PersistentVolumeClaim had no pv name and storageClass name"),
 		},
 		{
-			name:       "unbound volume no storage class",
+			name: "unbound volume no storage class",
-			Pod:        createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"),
+			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"),
-			Node:       testNode,
+			Node: testNode,
-			wantStatus: framework.NewStatus(framework.Error, "PersistentVolumeClaim was not found: \"PVC_NoSC\""),
+			wantStatus: framework.NewStatus(framework.Error,
+				"StorageClass \"Class_0\" claimed by PersistentVolumeClaim \"PVC_NoSC\" not found"),
 		},
 		{
 			name:       "unbound volume immediate binding mode",