From 4bee622865a3cec651bcf964feacda91245ef2f6 Mon Sep 17 00:00:00 2001
From: Claire Li <claire921@gmail.com>
Date: Thu, 6 Nov 2014 21:38:41 -0800
Subject: [PATCH] Add check for empty priority list and refactor tests.

---
 pkg/scheduler/generic_scheduler.go      |  9 ++++---
 pkg/scheduler/generic_scheduler_test.go | 34 ++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/pkg/scheduler/generic_scheduler.go b/pkg/scheduler/generic_scheduler.go
index 77c95ae9742..a65b726e9f0 100644
--- a/pkg/scheduler/generic_scheduler.go
+++ b/pkg/scheduler/generic_scheduler.go
@@ -49,10 +49,13 @@ func (g *genericScheduler) Schedule(pod api.Pod, minionLister MinionLister) (str
 	if len(priorityList) == 0 {
 		return "", fmt.Errorf("failed to find a fit for pod: %v", pod)
 	}
-	return g.selectHost(priorityList), nil
+	return g.selectHost(priorityList)
 }
 
-func (g *genericScheduler) selectHost(priorityList HostPriorityList) string {
+func (g *genericScheduler) selectHost(priorityList HostPriorityList) (string, error) {
+	if len(priorityList) == 0 {
+		return "", fmt.Errorf("empty priorityList")
+	}
 	sort.Sort(priorityList)
 
 	hosts := getMinHosts(priorityList)
@@ -60,7 +63,7 @@ func (g *genericScheduler) selectHost(priorityList HostPriorityList) string {
 	defer g.randomLock.Unlock()
 
 	ix := g.random.Int() % len(hosts)
-	return hosts[ix]
+	return hosts[ix], nil
 }
 
 func findNodesThatFit(pod api.Pod, podLister PodLister, predicates []FitPredicate, nodes api.MinionList) (api.MinionList, error) {
diff --git a/pkg/scheduler/generic_scheduler_test.go b/pkg/scheduler/generic_scheduler_test.go
index 00890708b18..c5cbbcdca33 100644
--- a/pkg/scheduler/generic_scheduler_test.go
+++ b/pkg/scheduler/generic_scheduler_test.go
@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 func falsePredicate(pod api.Pod, existingPods []api.Pod, node string) (bool, error) {
@@ -72,14 +73,16 @@ func TestSelectHost(t *testing.T) {
 	scheduler := genericScheduler{random: rand.New(rand.NewSource(0))}
 	tests := []struct {
 		list          HostPriorityList
-		possibleHosts map[string]bool
+		possibleHosts util.StringSet
+		expectsErr    bool
 	}{
 		{
 			list: []HostPriority{
 				{host: "machine1.1", score: 1},
 				{host: "machine2.1", score: 2},
 			},
-			possibleHosts: map[string]bool{"machine1.1": true},
+			possibleHosts: util.NewStringSet("machine1.1"),
+			expectsErr:    false,
 		},
 		// equal scores
 		{
@@ -89,7 +92,8 @@ func TestSelectHost(t *testing.T) {
 				{host: "machine1.3", score: 1},
 				{host: "machine2.1", score: 2},
 			},
-			possibleHosts: map[string]bool{"machine1.1": true, "machine1.2": true, "machine1.3": true},
+			possibleHosts: util.NewStringSet("machine1.1", "machine1.2", "machine1.3"),
+			expectsErr:    false,
 		},
 		// out of order scores
 		{
@@ -100,16 +104,32 @@ func TestSelectHost(t *testing.T) {
 				{host: "machine3.1", score: 3},
 				{host: "machine1.3", score: 1},
 			},
-			possibleHosts: map[string]bool{"machine1.1": true, "machine1.2": true, "machine1.3": true},
+			possibleHosts: util.NewStringSet("machine1.1", "machine1.2", "machine1.3"),
+			expectsErr:    false,
+		},
+		// empty priorityList
+		{
+			list:          []HostPriority{},
+			possibleHosts: util.NewStringSet(),
+			expectsErr:    true,
 		},
 	}
 
 	for _, test := range tests {
 		// increase the randomness
 		for i := 0; i < 10; i++ {
-			got := scheduler.selectHost(test.list)
-			if !test.possibleHosts[got] {
-				t.Errorf("got %s is not in the possible map %v", got, test.possibleHosts)
+			got, err := scheduler.selectHost(test.list)
+			if test.expectsErr {
+				if err == nil {
+					t.Error("Unexpected non-error")
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Unexpected error: %v", err)
+				}
+				if !test.possibleHosts.Has(got) {
+					t.Errorf("got %s is not in the possible map %v", got, test.possibleHosts)
+				}
 			}
 		}
 	}