Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify volume zone checker codes #84611

Merged
merged 1 commit into from Dec 19, 2019
Merged

Simplify volume zone checker codes #84611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
103 changes: 55 additions & 48 deletions pkg/scheduler/algorithm/predicates/predicates.go
View file
Open in desktop
Expand Up @@ -677,62 +677,69 @@ func (c *VolumeZoneChecker) predicate(pod *v1.Pod, meta PredicateMetadata, nodeI
manifest := &(pod.Spec)
for i := range manifest.Volumes {
volume := &manifest.Volumes[i]
if volume.PersistentVolumeClaim != nil {
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 volume.PersistentVolumeClaim == nil {
continue
}
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)
}
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 {
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)
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")
}

pv, err := c.pvLister.Get(pvName)
if err != nil {
return false, nil, err
}
class, _ := c.scLister.Get(scName)
if class == nil {
return false, nil, fmt.Errorf("StorageClass %q claimed by PersistentVolumeClaim %q not found",
scName, pvcName)

if pv == nil {
return false, nil, fmt.Errorf("PersistentVolume was not found: %q", pvName)
}
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
}

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
}
return false, nil, fmt.Errorf("PersistentVolume had no name")
}

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
}
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
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go
View file
Open in desktop
Expand Up @@ -317,16 +317,18 @@ func TestWithBinding(t *testing.T) {
Node: testNode,
},
{
name: "unbound volume empty storage class",
Pod: createPodWithVolume("pod_1", "vol_1", "PVC_EmptySC"),
Node: testNode,
wantStatus: framework.NewStatus(framework.Error, "PersistentVolumeClaim was not found: \"PVC_EmptySC\""),
name: "unbound volume empty storage class",
Pod: createPodWithVolume("pod_1", "vol_1", "PVC_EmptySC"),
Node: testNode,
wantStatus: framework.NewStatus(framework.Error,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this format seems wrong, please revert

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alculquicondor I have go fmt the codes, it is not wrong.

see line334 and 340 also have the similar format

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see, you put a new line

"PersistentVolumeClaim had no pv name and storageClass name"),
},
{
name: "unbound volume no storage class",
Pod: createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"),
Node: testNode,
wantStatus: framework.NewStatus(framework.Error, "PersistentVolumeClaim was not found: \"PVC_NoSC\""),
name: "unbound volume no storage class",
Pod: createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"),
Node: testNode,
wantStatus: framework.NewStatus(framework.Error,
"StorageClass \"Class_0\" claimed by PersistentVolumeClaim \"PVC_NoSC\" not found"),
},
{
name: "unbound volume immediate binding mode",
Expand Down