Feedback

README.md

@@ -182,7 +182,7 @@ strategies:
      enabled: true
      params:
        podsHavingTooManyRestarts:
-         podeRestartThresholds: 100
+         podRestartThresholds: 100
          includingInitContainers: true
 ```
 

examples/policy.yaml

@@ -21,5 +21,5 @@ strategies:
      enabled: true
      params:
        podsHavingTooManyRestarts:
-         podeRestartThresholds: 100
+         podRestartThresholds: 100
          includingInitContainers: true

go.mod

@@ -4,7 +4,6 @@ go 1.13
 
 require (
 	github.com/gogo/protobuf v1.3.1 // indirect
-	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/spf13/cobra v0.0.5
 	github.com/spf13/pflag v1.0.5
 	k8s.io/api v0.17.0

pkg/api/types.go

@@ -61,6 +61,6 @@ type NodeResourceUtilizationThresholds struct {
 }
 
 type PodsHavingTooManyRestarts struct {
-	PodeRestartThresholds   int32
+	PodRestartThresholds    int32
 	IncludingInitContainers bool
 }

pkg/api/v1alpha1/types.go

@@ -61,6 +61,6 @@ type NodeResourceUtilizationThresholds struct {
 }
 
 type PodsHavingTooManyRestarts struct {
-	PodeRestartThresholds   int32 `json:"podeRestartThresholds,omitempty"`
+	PodRestartThresholds    int32 `json:"podRestartThresholds,omitempty"`
 	IncludingInitContainers bool  `json:"includingInitContainers,omitempty"`
 }

pkg/descheduler/strategies/toomanyrestarts.go

@@ -17,9 +17,8 @@ limitations under the License.
 package strategies
 
 import (
-	"github.com/golang/glog"
-
 	"k8s.io/api/core/v1"
+	"k8s.io/klog"
 
 	"sigs.k8s.io/descheduler/cmd/descheduler/app/options"
 	"sigs.k8s.io/descheduler/pkg/api"
@@ -32,24 +31,21 @@ import (
 // There are too many cases leading this issue: Volume mount failed, app error due to nodes' different settings.
 // As of now, this strategy won't evict daemonsets, mirror pods, critical pods and pods with local storages.
 func RemovePodsHavingTooManyRestarts(ds *options.DeschedulerServer, strategy api.DeschedulerStrategy, policyGroupVersion string, nodes []*v1.Node, nodePodCount utils.NodePodEvictedCount) {
+	if !strategy.Enabled || strategy.Params.PodsHavingTooManyRestarts.PodRestartThresholds < 1 {
+		return
+	}
 	evictionCount := removePodsHavingTooManyRestarts(ds, strategy, policyGroupVersion, nodes, nodePodCount, ds.EvictLocalStoragePods)
-	glog.V(1).Infof("RemovePodsHavingTooManyRestarts evicted %v pods", evictionCount)
+	klog.V(1).Infof("evicted %v pods", evictionCount)
 }
 
 func removePodsHavingTooManyRestarts(ds *options.DeschedulerServer, strategy api.DeschedulerStrategy, policyGroupVersion string, nodes []*v1.Node, nodePodCount utils.NodePodEvictedCount, evictLocalStoragePods bool) int {
 	podsEvicted := 0
-
-	if !strategy.Enabled || strategy.Params.PodsHavingTooManyRestarts.PodeRestartThresholds < 1 {
-		glog.Info("RemovePodsHavingTooManyRestarts policy is disabled.")
-		return podsEvicted
-	}
-
 	for _, node := range nodes {
-		glog.V(1).Infof("RemovePodsHavingTooManyRestarts Processing node: %#v", node.Name)
+		klog.V(1).Infof("Processing node: %s", node.Name)
 		pods, err := podutil.ListEvictablePodsOnNode(ds.Client, node, evictLocalStoragePods)
 
 		if err != nil {
-			glog.Errorf("RemovePodsHavingTooManyRestarts Error when list pods at node %s", node.Name)
+			klog.Errorf("Error when list pods at node %s", node.Name)
 			continue
 		}
 		for _, pod := range pods {
@@ -59,20 +55,20 @@ func removePodsHavingTooManyRestarts(ds *options.DeschedulerServer, strategy api
 
 			restarts, initRestarts := calcContainerRestarts(pod)
 			if strategy.Params.PodsHavingTooManyRestarts.IncludingInitContainers {
-				if restarts+initRestarts <= strategy.Params.PodsHavingTooManyRestarts.PodeRestartThresholds {
+				if restarts+initRestarts < strategy.Params.PodsHavingTooManyRestarts.PodRestartThresholds {
 					continue
 				}
-			} else if restarts <= strategy.Params.PodsHavingTooManyRestarts.PodeRestartThresholds {
+			} else if restarts < strategy.Params.PodsHavingTooManyRestarts.PodRestartThresholds {
 				continue
 			}
 
-			glog.V(1).Infof("RemovePodsHavingTooManyRestarts will evicted pod: %#v, container restarts: %d, initContainer restarts: %d", pod.Name, restarts, initRestarts)
+			klog.V(1).Infof("will evict pod: %#v, container restarts: %d, initContainer restarts: %d", pod.Name, restarts, initRestarts)
 			success, err := evictions.EvictPod(ds.Client, pod, policyGroupVersion, ds.DryRun)
 			if !success {
-				glog.Infof("RemovePodsHavingTooManyRestarts Error when evicting pod: %#v (%#v)", pod.Name, err)
+				klog.Infof("Error when evicting pod: %#v (%#v)", pod.Name, err)
 			} else {
 				nodePodCount[node]++
-				glog.V(1).Infof("RemovePodsHavingTooManyRestarts Evicted pod: %#v (%#v)", pod.Name, err)
+				klog.V(1).Infof("Evicted pod: %#v (%#v)", pod.Name, err)
 			}
 		}
 		podsEvicted += nodePodCount[node]
@@ -82,10 +78,7 @@ func removePodsHavingTooManyRestarts(ds *options.DeschedulerServer, strategy api
 
 // calcContainerRestarts get container restarts and init container restarts.
 func calcContainerRestarts(pod *v1.Pod) (int32, int32) {
-	var (
-		restarts     int32 = 0
-		initRestarts int32 = 0
-	)
+	var restarts, initRestarts int32
 
 	for _, cs := range pod.Status.ContainerStatuses {
 		restarts += cs.RestartCount

pkg/descheduler/strategies/toomanyrestarts_test.go

@@ -33,15 +33,13 @@ import (
 )
 
 func initPods(node *v1.Node) []v1.Pod {
-
 	pods := make([]v1.Pod, 0)
 
-	var i int32 = 0
-	for i <= 9 {
+	for i := int32(0); i <= 9; i++ {
 		pod := test.BuildTestPod(fmt.Sprintf("pod-%d", i), 100, 0, node.Name)
 		pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
 
-		// pod i will has 25 * i restarts.
+		// pod at index i will have 25 * i restarts.
 		pod.Status = v1.PodStatus{
 			InitContainerStatuses: []v1.ContainerStatus{
 				{
@@ -58,10 +56,9 @@ func initPods(node *v1.Node) []v1.Pod {
 			},
 		}
 		pods = append(pods, *pod)
-		i++
 	}
 
-	// The following 4 pods won't get evicted.
+	// The following 3 pods won't get evicted.
 	// A daemonset.
 	pods[6].ObjectMeta.OwnerReferences = test.GetDaemonSetOwnerRefList()
 	// A pod with local storage.
@@ -83,13 +80,12 @@ func initPods(node *v1.Node) []v1.Pod {
 }
 
 func TestRemovePodsHavingTooManyRestarts(t *testing.T) {
-
 	createStrategy := func(enabled, includingInitContainers bool, restartThresholds int32) api.DeschedulerStrategy {
 		return api.DeschedulerStrategy{
 			Enabled: enabled,
 			Params: api.StrategyParameters{
 				PodsHavingTooManyRestarts: api.PodsHavingTooManyRestarts{
-					PodeRestartThresholds:   restartThresholds,
+					PodRestartThresholds:    restartThresholds,
 					IncludingInitContainers: includingInitContainers,
 				},
 			},
@@ -112,41 +108,22 @@ func TestRemovePodsHavingTooManyRestarts(t *testing.T) {
 		expectedEvictedPodCount int
 		maxPodsToEvict          int
 	}{
-		{
-			description: "Not a PodsHavingTooManyRestarts strategy, no pod evictions",
-			strategy: api.DeschedulerStrategy{
-				Enabled: true,
-				Params: api.StrategyParameters{
-					NodeAffinityType: []string{
-						"requiredDuringSchedulingRequiredDuringExecution",
-					},
-				},
-			},
-			expectedEvictedPodCount: 0,
-			maxPodsToEvict:          0,
-		},
-		{
-			description:             "Strategy is disabled, no pod evictions",
-			strategy:                createStrategy(false, true, 10),
-			expectedEvictedPodCount: 0,
-			maxPodsToEvict:          0,
-		},
 		{
 			description:             "All pods have total restarts under threshold, no pod evictions",
 			strategy:                createStrategy(true, true, 10000),
 			expectedEvictedPodCount: 0,
 			maxPodsToEvict:          0,
 		},
 		{
-			description:             "One pod have total restarts bigger than threshold, 6 pod evictions",
+			description:             "Some pods have total restarts bigger than threshold",
 			strategy:                createStrategy(true, true, 1),
 			expectedEvictedPodCount: 6,
 			maxPodsToEvict:          0,
 		},
 		{
-			description:             "Nine pods have total restarts equals threshold(includingInitContainers=true), 5 pods evictions",
+			description:             "Nine pods have total restarts equals threshold(includingInitContainers=true), 6 pods evictions",
 			strategy:                createStrategy(true, true, 1*25),
-			expectedEvictedPodCount: 5,
+			expectedEvictedPodCount: 6,
 			maxPodsToEvict:          0,
 		},
 		{
@@ -162,9 +139,9 @@ func TestRemovePodsHavingTooManyRestarts(t *testing.T) {
 			maxPodsToEvict:          0,
 		},
 		{
-			description:             "Nine pods have total restarts equals threshold(includingInitContainers=false), 5 pods evictions",
+			description:             "Nine pods have total restarts equals threshold(includingInitContainers=false), 6 pods evictions",
 			strategy:                createStrategy(true, false, 1*20),
-			expectedEvictedPodCount: 5,
+			expectedEvictedPodCount: 6,
 			maxPodsToEvict:          0,
 		},
 		{