Skip to content

Commit

Permalink
Merge pull request kubernetes-sigs#265 from ingvagabund/tolerations-t…
Browse files Browse the repository at this point in the history 
…ains-code-cleanup

Tolerations taints code cleanup
  • Loading branch information
(Brien Dieterle) committed Apr 21, 2020
2 parents c91fc07 + b9d2146 commit 5911b2f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 99 deletions.
79 changes: 6 additions & 73 deletions pkg/descheduler/strategies/node_taint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ import (
"k8s.io/klog"
)

const (
TolerationOpExists v1.TolerationOperator = "Exists"
TolerationOpEqual v1.TolerationOperator = "Equal"
)

// RemovePodsViolatingNodeTaints with elimination strategy
func RemovePodsViolatingNodeTaints(ds *options.DeschedulerServer, strategy api.DeschedulerStrategy, policyGroupVersion string, nodes []*v1.Node, nodePodCount utils.NodePodEvictedCount) {
if !strategy.Enabled {
Expand All @@ -56,7 +51,12 @@ func deletePodsViolatingNodeTaints(client clientset.Interface, policyGroupVersio
if maxPodsToEvict > 0 && nodePodCount[node]+1 > maxPodsToEvict {
break
}
if !checkPodsSatisfyTolerations(pods[i], node) {
if !utils.TolerationsTolerateTaintsWithFilter(
pods[i].Spec.Tolerations,
node.Spec.Taints,
func(taint *v1.Taint) bool { return taint.Effect == v1.TaintEffectNoSchedule },
) {
klog.V(2).Infof("Not all taints with NoSchedule effect are tolerated after update for pod %v on node %v", pods[i].Name, node.Name)
success, err := evictions.EvictPod(client, pods[i], policyGroupVersion, dryRun)
if !success {
klog.Errorf("Error when evicting pod: %#v (%#v)\n", pods[i].Name, err)
Expand All @@ -70,70 +70,3 @@ func deletePodsViolatingNodeTaints(client clientset.Interface, policyGroupVersio
}
return podsEvicted
}

// checkPodsSatisfyTolerations checks if the node's taints (NoSchedule) are still satisfied by pods' tolerations.
func checkPodsSatisfyTolerations(pod *v1.Pod, node *v1.Node) bool {
tolerations := pod.Spec.Tolerations
taints := node.Spec.Taints
if len(taints) == 0 {
return true
}
noScheduleTaints := getNoScheduleTaints(taints)
if !allTaintsTolerated(noScheduleTaints, tolerations) {
klog.V(2).Infof("Not all taints are tolerated after update for Pod %v on node %v", pod.Name, node.Name)
return false
}
return true
}

// getNoScheduleTaints return a slice of NoSchedule taints from the a slice of taints that it receives.
func getNoScheduleTaints(taints []v1.Taint) []v1.Taint {
result := []v1.Taint{}
for i := range taints {
if taints[i].Effect == v1.TaintEffectNoSchedule {
result = append(result, taints[i])
}
}
return result
}

//toleratesTaint returns true if a toleration tolerates a taint, or false otherwise
func toleratesTaint(toleration *v1.Toleration, taint *v1.Taint) bool {

if (len(toleration.Key) > 0 && toleration.Key != taint.Key) ||
(len(toleration.Effect) > 0 && toleration.Effect != taint.Effect) {
return false
}
switch toleration.Operator {
// empty operator means Equal
case "", TolerationOpEqual:
return toleration.Value == taint.Value
case TolerationOpExists:
return true
default:
return false
}
}

// allTaintsTolerated returns true if all are tolerated, or false otherwise.
func allTaintsTolerated(taints []v1.Taint, tolerations []v1.Toleration) bool {
if len(taints) == 0 {
return true
}
if len(tolerations) == 0 && len(taints) > 0 {
return false
}
for i := range taints {
tolerated := false
for j := range tolerations {
if toleratesTaint(&tolerations[j], &taints[i]) {
tolerated = true
break
}
}
if !tolerated {
return false
}
}
return true
}
33 changes: 7 additions & 26 deletions pkg/descheduler/strategies/node_taint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func TestToleratesTaint(t *testing.T) {
description: "toleration and taint have the same key and effect, and operator is Exists, and taint has no value, expect tolerated",
toleration: v1.Toleration{
Key: "foo",
Operator: TolerationOpExists,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
taint: v1.Taint{
Expand All @@ -201,7 +201,7 @@ func TestToleratesTaint(t *testing.T) {
description: "toleration and taint have the same key and effect, and operator is Exists, and taint has some value, expect tolerated",
toleration: v1.Toleration{
Key: "foo",
Operator: TolerationOpExists,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
taint: v1.Taint{
Expand All @@ -215,7 +215,7 @@ func TestToleratesTaint(t *testing.T) {
description: "toleration and taint have the same effect, toleration has empty key and operator is Exists, means match all taints, expect tolerated",
toleration: v1.Toleration{
Key: "",
Operator: TolerationOpExists,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
taint: v1.Taint{
Expand All @@ -229,7 +229,7 @@ func TestToleratesTaint(t *testing.T) {
description: "toleration and taint have the same key, effect and value, and operator is Equal, expect tolerated",
toleration: v1.Toleration{
Key: "foo",
Operator: TolerationOpEqual,
Operator: v1.TolerationOpEqual,
Value: "bar",
Effect: v1.TaintEffectNoSchedule,
},
Expand All @@ -244,7 +244,7 @@ func TestToleratesTaint(t *testing.T) {
description: "toleration and taint have the same key and effect, but different values, and operator is Equal, expect not tolerated",
toleration: v1.Toleration{
Key: "foo",
Operator: TolerationOpEqual,
Operator: v1.TolerationOpEqual,
Value: "value1",
Effect: v1.TaintEffectNoSchedule,
},
Expand All @@ -259,7 +259,7 @@ func TestToleratesTaint(t *testing.T) {
description: "toleration and taint have the same key and value, but different effects, and operator is Equal, expect not tolerated",
toleration: v1.Toleration{
Key: "foo",
Operator: TolerationOpEqual,
Operator: v1.TolerationOpEqual,
Value: "bar",
Effect: v1.TaintEffectNoSchedule,
},
Expand All @@ -272,27 +272,8 @@ func TestToleratesTaint(t *testing.T) {
},
}
for _, tc := range testCases {
if tolerated := toleratesTaint(&tc.toleration, &tc.taint); tc.expectTolerated != tolerated {
if tolerated := tc.toleration.ToleratesTaint(&tc.taint); tc.expectTolerated != tolerated {
t.Errorf("[%s] expect %v, got %v: toleration %+v, taint %s", tc.description, tc.expectTolerated, tolerated, tc.toleration, tc.taint.ToString())
}
}
}

func TestFilterNoExecuteTaints(t *testing.T) {
taints := []v1.Taint{
{
Key: "one",
Value: "one",
Effect: v1.TaintEffectNoExecute,
},
{
Key: "two",
Value: "two",
Effect: v1.TaintEffectNoSchedule,
},
}
taints = getNoScheduleTaints(taints)
if len(taints) != 1 || taints[0].Key != "two" {
t.Errorf("Filtering doesn't work. Got %v", taints)
}
}

0 comments on commit 5911b2f

Please sign in to comment.