From 07bc06ba5026a31eb61713fc21593d039f36b026 Mon Sep 17 00:00:00 2001 From: Abhishek Gupta Date: Fri, 10 Jun 2016 17:15:50 -0700 Subject: [PATCH 1/2] Counting pod volume towards PV limit even if PV/PVC is missing --- .../algorithm/predicates/predicates.go | 20 ++++++- .../algorithm/predicates/predicates_test.go | 60 ++++++++++++++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index a5c63f4a7c..60a412c7c0 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -18,6 +18,9 @@ package predicates import ( "fmt" + "math/rand" + "strconv" + "time" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -156,7 +159,13 @@ func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []api.Volume, namespace } pvc, err := c.pvcInfo.GetPersistentVolumeClaimInfo(namespace, pvcName) if err != nil { - return err + // if the PVC is not found, log the error and count the PV towards the PV limit + // generate a random volume ID since its required for de-dup + glog.Error(err) + source := rand.NewSource(time.Now().UnixNano()) + generatedID := "missingPVC" + strconv.Itoa(rand.New(source).Intn(1000000)) + filteredVolumes[generatedID] = true + return nil } pvName := pvc.Spec.VolumeName @@ -166,7 +175,14 @@ func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []api.Volume, namespace pv, err := c.pvInfo.GetPersistentVolumeInfo(pvName) if err != nil { - return err + // if the PV is not found, log the error + // and count the PV towards the PV limit + // generate a random volume ID since its required for de-dup + glog.Error(err) + source := rand.NewSource(time.Now().UnixNano()) + generatedID := "missingPV" + strconv.Itoa(rand.New(source).Intn(1000000)) + filteredVolumes[generatedID] = true + return nil } if id, ok := c.filter.FilterPersistentVolume(pv); ok { diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index 1221b663a8..cd1a4679bd 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -1392,6 +1392,32 @@ func TestEBSVolumeCountConflicts(t *testing.T) { }, }, } + deletedPVCPod := &api.Pod{ + Spec: api.PodSpec{ + Volumes: []api.Volume{ + { + VolumeSource: api.VolumeSource{ + PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{ + ClaimName: "deletedPVC", + }, + }, + }, + }, + }, + } + deletedPVPod := &api.Pod{ + Spec: api.PodSpec{ + Volumes: []api.Volume{ + { + VolumeSource: api.VolumeSource{ + PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{ + ClaimName: "deletedPV", + }, + }, + }, + }, + }, + } emptyPod := &api.Pod{ Spec: api.PodSpec{}, } @@ -1466,6 +1492,34 @@ func TestEBSVolumeCountConflicts(t *testing.T) { fits: true, test: "the same EBS volumes are not counted multiple times", }, + { + newPod: ebsPVCPod, + existingPods: []*api.Pod{oneVolPod, deletedPVCPod}, + maxVols: 2, + fits: false, + test: "pod with missing PVC is counted towards the PV limit", + }, + { + newPod: ebsPVCPod, + existingPods: []*api.Pod{oneVolPod, deletedPVCPod}, + maxVols: 3, + fits: true, + test: "pod with missing PVC is counted towards the PV limit", + }, + { + newPod: ebsPVCPod, + existingPods: []*api.Pod{oneVolPod, deletedPVPod}, + maxVols: 2, + fits: false, + test: "pod with missing PV is counted towards the PV limit", + }, + { + newPod: ebsPVCPod, + existingPods: []*api.Pod{oneVolPod, deletedPVPod}, + maxVols: 3, + fits: true, + test: "pod with missing PV is counted towards the PV limit", + }, } pvInfo := FakePersistentVolumeInfo{ @@ -1473,7 +1527,7 @@ func TestEBSVolumeCountConflicts(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "someEBSVol"}, Spec: api.PersistentVolumeSpec{ PersistentVolumeSource: api.PersistentVolumeSource{ - AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{}, + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{VolumeID: "ebsVol"}, }, }, }, @@ -1494,6 +1548,10 @@ func TestEBSVolumeCountConflicts(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "someNonEBSVol"}, Spec: api.PersistentVolumeClaimSpec{VolumeName: "someNonEBSVol"}, }, + { + ObjectMeta: api.ObjectMeta{Name: "deletedPV"}, + Spec: api.PersistentVolumeClaimSpec{VolumeName: "deletedPV"}, + }, } filter := VolumeFilter{ From 20ce8b71ab41ef223a787068e80242838a719faa Mon Sep 17 00:00:00 2001 From: Abhishek Gupta Date: Mon, 13 Jun 2016 13:44:12 -0700 Subject: [PATCH 2/2] Improving error messages and naming to be clear --- plugin/pkg/scheduler/algorithm/predicates/predicates.go | 5 +++-- .../pkg/scheduler/algorithm/predicates/predicates_test.go | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index 60a412c7c0..e91b1ecef7 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -28,6 +28,7 @@ import ( "k8s.io/kubernetes/pkg/client/cache" qosutil "k8s.io/kubernetes/pkg/kubelet/qos/util" "k8s.io/kubernetes/pkg/labels" + utilruntime "k8s.io/kubernetes/pkg/util/runtime" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" priorityutil "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities/util" "k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache" @@ -161,7 +162,7 @@ func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []api.Volume, namespace if err != nil { // if the PVC is not found, log the error and count the PV towards the PV limit // generate a random volume ID since its required for de-dup - glog.Error(err) + utilruntime.HandleError(fmt.Errorf("Unable to look up PVC info for %s/%s, assuming PVC matches predicate when counting limits: %v", namespace, pvcName, err)) source := rand.NewSource(time.Now().UnixNano()) generatedID := "missingPVC" + strconv.Itoa(rand.New(source).Intn(1000000)) filteredVolumes[generatedID] = true @@ -178,7 +179,7 @@ func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []api.Volume, namespace // if the PV is not found, log the error // and count the PV towards the PV limit // generate a random volume ID since its required for de-dup - glog.Error(err) + utilruntime.HandleError(fmt.Errorf("Unable to look up PV info for %s/%s/%s, assuming PV matches predicate when counting limits: %v", namespace, pvcName, pvName, err)) source := rand.NewSource(time.Now().UnixNano()) generatedID := "missingPV" + strconv.Itoa(rand.New(source).Intn(1000000)) filteredVolumes[generatedID] = true diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index cd1a4679bd..bbf5a54c32 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -1411,7 +1411,7 @@ func TestEBSVolumeCountConflicts(t *testing.T) { { VolumeSource: api.VolumeSource{ PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{ - ClaimName: "deletedPV", + ClaimName: "pvcWithDeletedPV", }, }, }, @@ -1549,8 +1549,8 @@ func TestEBSVolumeCountConflicts(t *testing.T) { Spec: api.PersistentVolumeClaimSpec{VolumeName: "someNonEBSVol"}, }, { - ObjectMeta: api.ObjectMeta{Name: "deletedPV"}, - Spec: api.PersistentVolumeClaimSpec{VolumeName: "deletedPV"}, + ObjectMeta: api.ObjectMeta{Name: "pvcWithDeletedPV"}, + Spec: api.PersistentVolumeClaimSpec{VolumeName: "pvcWithDeletedPV"}, }, }