From 7d079813e581a72c90941d0e3c6463e5e38ce9f0 Mon Sep 17 00:00:00 2001
From: Avesh Agarwal <avagarwa@redhat.com>
Date: Fri, 10 Nov 2017 13:07:07 -0500
Subject: [PATCH] Fix to not process empty node list and also fix error
 reporting in other places. Also fix unit test panic and if nodeLister is nil
 and refactor some code.

---
 cmd/descheduler/app/server.go     |  4 ++--
 pkg/descheduler/descheduler.go    | 11 +++++++++--
 pkg/descheduler/node/node.go      | 23 ++++++++++++++++-------
 pkg/descheduler/node/node_test.go |  4 +---
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/cmd/descheduler/app/server.go b/cmd/descheduler/app/server.go
index 34a894745..01d86343f 100644
--- a/cmd/descheduler/app/server.go
+++ b/cmd/descheduler/app/server.go
@@ -19,12 +19,12 @@ package app
 
 import (
 	"flag"
-	"fmt"
 	"io"
 
 	"github.com/kubernetes-incubator/descheduler/cmd/descheduler/app/options"
 	"github.com/kubernetes-incubator/descheduler/pkg/descheduler"
 
+	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 
 	aflag "k8s.io/apiserver/pkg/util/flag"
@@ -43,7 +43,7 @@ func NewDeschedulerCommand(out io.Writer) *cobra.Command {
 			defer logs.FlushLogs()
 			err := Run(s)
 			if err != nil {
-				fmt.Println(err)
+				glog.Errorf("%v", err)
 			}
 		},
 	}
diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go
index 2ebfa5ff8..56aa65781 100644
--- a/pkg/descheduler/descheduler.go
+++ b/pkg/descheduler/descheduler.go
@@ -19,6 +19,8 @@ package descheduler
 import (
 	"fmt"
 
+	"github.com/golang/glog"
+
 	"github.com/kubernetes-incubator/descheduler/cmd/descheduler/app/options"
 	"github.com/kubernetes-incubator/descheduler/pkg/descheduler/client"
 	eutils "github.com/kubernetes-incubator/descheduler/pkg/descheduler/evictions/utils"
@@ -39,9 +41,9 @@ func Run(rs *options.DeschedulerServer) error {
 		return err
 	}
 	if deschedulerPolicy == nil {
-		return fmt.Errorf("\ndeschedulerPolicy is nil\n")
-
+		return fmt.Errorf("deschedulerPolicy is nil")
 	}
+
 	evictionPolicyGroupVersion, err := eutils.SupportEviction(rs.Client)
 	if err != nil || len(evictionPolicyGroupVersion) == 0 {
 		return err
@@ -53,6 +55,11 @@ func Run(rs *options.DeschedulerServer) error {
 		return err
 	}
 
+	if len(nodes) == 0 {
+		glog.V(1).Infof("node list is empty")
+		return nil
+	}
+
 	strategies.RemoveDuplicatePods(rs, deschedulerPolicy.Strategies["RemoveDuplicates"], evictionPolicyGroupVersion, nodes)
 	strategies.LowNodeUtilization(rs, deschedulerPolicy.Strategies["LowNodeUtilization"], evictionPolicyGroupVersion, nodes)
 	strategies.RemovePodsViolatingInterPodAntiAffinity(rs, deschedulerPolicy.Strategies["RemovePodsViolatingInterPodAntiAffinity"], evictionPolicyGroupVersion, nodes)
diff --git a/pkg/descheduler/node/node.go b/pkg/descheduler/node/node.go
index a34c6eee0..bb4d915f8 100644
--- a/pkg/descheduler/node/node.go
+++ b/pkg/descheduler/node/node.go
@@ -32,26 +32,32 @@ import (
 // ReadyNodes returns ready nodes irrespective of whether they are
 // schedulable or not.
 func ReadyNodes(client clientset.Interface, nodeSelector string, stopChannel <-chan struct{}) ([]*v1.Node, error) {
-	nl := GetNodeLister(client, stopChannel)
-
 	ns, err := labels.Parse(nodeSelector)
 	if err != nil {
 		return []*v1.Node{}, err
 	}
 
-	nodes, err := nl.List(ns)
-
-	if err != nil {
-		return []*v1.Node{}, err
+	var nodes []*v1.Node
+	nl := GetNodeLister(client, stopChannel)
+	if nl != nil {
+		// err is defined above
+		if nodes, err = nl.List(ns); err != nil {
+			return []*v1.Node{}, err
+		}
 	}
 
 	if len(nodes) == 0 {
-		var err error
+		glog.V(2).Infof("node lister returned empty list, now fetch directly")
+
 		nItems, err := client.Core().Nodes().List(metav1.ListOptions{LabelSelector: nodeSelector})
 		if err != nil {
 			return []*v1.Node{}, err
 		}
 
+		if nItems == nil || len(nItems.Items) == 0 {
+			return []*v1.Node{}, nil
+		}
+
 		for i := range nItems.Items {
 			node := nItems.Items[i]
 			nodes = append(nodes, &node)
@@ -68,6 +74,9 @@ func ReadyNodes(client clientset.Interface, nodeSelector string, stopChannel <-c
 }
 
 func GetNodeLister(client clientset.Interface, stopChannel <-chan struct{}) corelisters.NodeLister {
+	if stopChannel == nil {
+		return nil
+	}
 	listWatcher := cache.NewListWatchFromClient(client.Core().RESTClient(), "nodes", v1.NamespaceAll, fields.Everything())
 	store := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
 	nodeLister := corelisters.NewNodeLister(store)
diff --git a/pkg/descheduler/node/node_test.go b/pkg/descheduler/node/node_test.go
index dd26fdaa7..08211d04b 100644
--- a/pkg/descheduler/node/node_test.go
+++ b/pkg/descheduler/node/node_test.go
@@ -88,10 +88,8 @@ func TestReadyNodesWithNodeSelector(t *testing.T) {
 	node2.Labels = map[string]string{"type": "infra"}
 
 	fakeClient := fake.NewSimpleClientset(node1, node2)
-
 	nodeSelector := "type=compute"
-	stopChannel := make(chan struct{})
-	nodes, _ := ReadyNodes(fakeClient, nodeSelector, stopChannel)
+	nodes, _ := ReadyNodes(fakeClient, nodeSelector, nil)
 
 	if nodes[0].Name != "node1" {
 		t.Errorf("Expected node1, got %s", nodes[0].Name)