Merge pull request #41 from containscafeine/error-out-on-unsupported-resource-names

Validate resource names to pass to thresholds

pkg/descheduler/strategies/lownodeutilization.go

@@ -77,22 +77,24 @@ func LowNodeUtilization(ds *options.DeschedulerServer, strategy api.DeschedulerS
 }
 
 func validateThresholds(thresholds api.ResourceThresholds) bool {
-	if thresholds == nil {
+	if thresholds == nil || len(thresholds) == 0 {
 		glog.V(1).Infof("no resource threshold is configured")
 		return false
 	}
-	found := false
 	for name := range thresholds {
-		if name == v1.ResourceCPU || name == v1.ResourceMemory || name == v1.ResourcePods {
-			found = true
-			break
+		switch name {
+		case v1.ResourceCPU:
+			continue
+		case v1.ResourceMemory:
+			continue
+		case v1.ResourcePods:
+			continue
+		default:
+			glog.Errorf("only cpu, memory, or pods thresholds can be specified")
+			return false
 		}
 	}
-	if !found {
-		glog.V(1).Infof("one of cpu, memory, or pods resource threshold must be configured")
-		return false
-	}
-	return found
+	return true
 }
 
 //This function could be merged into above once we are clear.

pkg/descheduler/strategies/lownodeutilization_test.go

@@ -18,6 +18,9 @@ package strategies
 
 import (
 	"fmt"
+	"strings"
+	"testing"
+
 	"github.com/kubernetes-incubator/descheduler/pkg/api"
 	"github.com/kubernetes-incubator/descheduler/test"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -25,8 +28,6 @@ import (
 	core "k8s.io/client-go/testing"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
-	"strings"
-	"testing"
 )
 
 // TODO: Make this table driven.
@@ -109,3 +110,55 @@ func TestLowNodeUtilization(t *testing.T) {
 	}
 
 }
+
+func TestValidateThresholds(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   api.ResourceThresholds
+		succeed bool
+	}{
+		{
+			name:    "passing nil map for threshold",
+			input:   nil,
+			succeed: false,
+		},
+		{
+			name:    "passing no threshold",
+			input:   api.ResourceThresholds{},
+			succeed: false,
+		},
+		{
+			name: "passing unsupported resource name",
+			input: api.ResourceThresholds{
+				v1.ResourceCPU:     40,
+				v1.ResourceStorage: 25.5,
+			},
+			succeed: false,
+		},
+		{
+			name: "passing invalid resource name",
+			input: api.ResourceThresholds{
+				v1.ResourceCPU: 40,
+				"coolResource": 42.0,
+			},
+			succeed: false,
+		},
+		{
+			name: "passing a valid threshold with cpu, memory and pods",
+			input: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 30,
+				v1.ResourcePods:   40,
+			},
+			succeed: true,
+		},
+	}
+
+	for _, test := range tests {
+		isValid := validateThresholds(test.input)
+
+		if isValid != test.succeed {
+			t.Errorf("expected validity of threshold: %#v\nto be %v but got %v instead", test.input, test.succeed, isValid)
+		}
+	}
+}