From 54f67266bb3d53818b565bb6ed259fc6c48bc754 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Wed, 5 May 2021 12:56:30 +0200 Subject: [PATCH 1/3] Define TolerationsEqual --- pkg/utils/predicates.go | 57 ++++++ pkg/utils/predicates_test.go | 337 +++++++++++++++++++++++++++++++++++ 2 files changed, 394 insertions(+) create mode 100644 pkg/utils/predicates_test.go diff --git a/pkg/utils/predicates.go b/pkg/utils/predicates.go index 126db7810..a705f4314 100644 --- a/pkg/utils/predicates.go +++ b/pkg/utils/predicates.go @@ -18,6 +18,7 @@ package utils import ( "fmt" + "sort" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -107,3 +108,59 @@ func TolerationsTolerateTaintsWithFilter(tolerations []v1.Toleration, taints []v return true } + +// sort by (key, value, effect, operand) +func uniqueSortTolerations(srcTolerations []v1.Toleration) []v1.Toleration { + tolerations := append([]v1.Toleration{}, srcTolerations...) + + if len(tolerations) < 2 { + return tolerations + } + + sort.Slice(tolerations, func(i, j int) bool { + if tolerations[i].Key < tolerations[j].Key { + return true + } + if tolerations[i].Key > tolerations[j].Key { + return false + } + if tolerations[i].Value < tolerations[j].Value { + return true + } + if tolerations[i].Value > tolerations[j].Value { + return false + } + if tolerations[i].Effect < tolerations[j].Effect { + return true + } + if tolerations[i].Effect > tolerations[j].Effect { + return false + } + return tolerations[i].Operator < tolerations[j].Operator + }) + uniqueTolerations := []v1.Toleration{tolerations[0]} + idx := 0 + for _, t := range tolerations[1:] { + if t.MatchToleration(&uniqueTolerations[idx]) { + continue + } + idx++ + uniqueTolerations = append(uniqueTolerations, t) + } + return uniqueTolerations +} + +func TolerationsEqual(t1, t2 []v1.Toleration) bool { + t1Sorted := uniqueSortTolerations(t1) + t2Sorted := uniqueSortTolerations(t2) + l1Len := len(t1Sorted) + if l1Len != len(t2Sorted) { + return false + } + for i := 0; i < l1Len; i++ { + if !t1Sorted[i].MatchToleration(&t2Sorted[i]) { + return false + } + } + return true +} diff --git a/pkg/utils/predicates_test.go b/pkg/utils/predicates_test.go new file mode 100644 index 000000000..691da26ea --- /dev/null +++ b/pkg/utils/predicates_test.go @@ -0,0 +1,337 @@ +package utils + +import ( + "reflect" + "testing" + + v1 "k8s.io/api/core/v1" +) + +func TestUniqueSortTolerations(t *testing.T) { + tests := []struct { + name string + tolerations []v1.Toleration + expectedTolerations []v1.Toleration + }{ + { + name: "sort by key", + tolerations: []v1.Toleration{ + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key3", + Operator: v1.TolerationOpEqual, + Value: "value3", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + }, + expectedTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key3", + Operator: v1.TolerationOpEqual, + Value: "value3", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + { + name: "sort by value", + tolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value3", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + }, + expectedTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value3", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + { + name: "sort by effect", + tolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoExecute, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectPreferNoSchedule, + }, + }, + expectedTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoExecute, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectPreferNoSchedule, + }, + }, + }, + { + name: "sort unique", + tolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + expectedTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resultTolerations := uniqueSortTolerations(test.tolerations) + if !reflect.DeepEqual(resultTolerations, test.expectedTolerations) { + t.Errorf("tolerations not sorted as expected, \n\tgot: %#v, \n\texpected: %#v", resultTolerations, test.expectedTolerations) + } + }) + } +} + +func TestTolerationsEqual(t *testing.T) { + tests := []struct { + name string + leftTolerations, rightTolerations []v1.Toleration + equal bool + }{ + { + name: "identical lists", + leftTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + rightTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + equal: true, + }, + { + name: "equal lists", + leftTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + rightTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + equal: true, + }, + { + name: "non-equal lists", + leftTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoSchedule, + }, + }, + rightTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoExecute, + }, + }, + equal: false, + }, + { + name: "different sizes lists", + leftTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + }, + rightTolerations: []v1.Toleration{ + { + Key: "key1", + Operator: v1.TolerationOpEqual, + Value: "value1", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "key2", + Operator: v1.TolerationOpEqual, + Value: "value2", + Effect: v1.TaintEffectNoExecute, + }, + }, + equal: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + equal := TolerationsEqual(test.leftTolerations, test.rightTolerations) + if equal != test.equal { + t.Errorf("TolerationsEqual expected to be %v, got %v", test.equal, equal) + } + }) + } +} From 4edbecc85dac29816b3991ad93eebaa550887cec Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Sun, 9 May 2021 11:56:38 +0200 Subject: [PATCH 2/3] Define NodeSelectorsEqual predicate --- pkg/utils/predicates.go | 111 +++++++ pkg/utils/predicates_test.go | 603 +++++++++++++++++++++++++++++++++++ 2 files changed, 714 insertions(+) diff --git a/pkg/utils/predicates.go b/pkg/utils/predicates.go index a705f4314..bc7b59a2f 100644 --- a/pkg/utils/predicates.go +++ b/pkg/utils/predicates.go @@ -18,6 +18,7 @@ package utils import ( "fmt" + "reflect" "sort" v1 "k8s.io/api/core/v1" @@ -81,6 +82,116 @@ func podMatchesNodeLabels(pod *v1.Pod, node *v1.Node) bool { return true } +func uniqueSortNodeSelectorTerms(srcTerms []v1.NodeSelectorTerm) []v1.NodeSelectorTerm { + terms := append([]v1.NodeSelectorTerm{}, srcTerms...) + for i := range terms { + if terms[i].MatchExpressions != nil { + terms[i].MatchExpressions = uniqueSortNodeSelectorRequirements(terms[i].MatchExpressions) + } + if terms[i].MatchFields != nil { + terms[i].MatchFields = uniqueSortNodeSelectorRequirements(terms[i].MatchFields) + } + } + + if len(terms) < 2 { + return terms + } + + lastTerm := terms[0] + uniqueTerms := append([]v1.NodeSelectorTerm{}, terms[0]) + + for _, term := range terms[1:] { + if reflect.DeepEqual(term, lastTerm) { + continue + } + lastTerm = term + uniqueTerms = append(uniqueTerms, term) + } + + return uniqueTerms +} + +// sort NodeSelectorRequirement in (key, operator, values) order +func uniqueSortNodeSelectorRequirements(srcReqs []v1.NodeSelectorRequirement) []v1.NodeSelectorRequirement { + reqs := append([]v1.NodeSelectorRequirement{}, srcReqs...) + + // unique sort Values + for i := range reqs { + sort.Strings(reqs[i].Values) + if len(reqs[i].Values) > 1 { + lastString := reqs[i].Values[0] + values := []string{lastString} + for _, val := range reqs[i].Values { + if val == lastString { + continue + } + lastString = val + values = append(values, val) + } + reqs[i].Values = values + } + } + + if len(reqs) < 2 { + return reqs + } + + // unique sort reqs + sort.Slice(reqs, func(i, j int) bool { + if reqs[i].Key < reqs[j].Key { + return true + } + if reqs[i].Key > reqs[j].Key { + return false + } + if reqs[i].Operator < reqs[j].Operator { + return true + } + if reqs[i].Operator > reqs[j].Operator { + return false + } + if len(reqs[i].Values) < len(reqs[j].Values) { + return true + } + if len(reqs[i].Values) > len(reqs[j].Values) { + return false + } + for k := range reqs[i].Values { + if reqs[i].Values[k] < reqs[j].Values[k] { + return true + } + if reqs[i].Values[k] > reqs[j].Values[k] { + return false + } + } + return true + }) + + lastReq := reqs[0] + uniqueReqs := append([]v1.NodeSelectorRequirement{}, lastReq) + for _, req := range reqs[1:] { + if reflect.DeepEqual(req, lastReq) { + continue + } + lastReq = req + uniqueReqs = append(uniqueReqs, req) + } + return uniqueReqs +} + +func NodeSelectorsEqual(n1, n2 *v1.NodeSelector) bool { + if n1 == nil && n2 == nil { + return true + } + if n1 == nil || n2 == nil { + return false + } + return reflect.DeepEqual( + uniqueSortNodeSelectorTerms(n1.NodeSelectorTerms), + uniqueSortNodeSelectorTerms(n2.NodeSelectorTerms), + ) +} + // TolerationsTolerateTaint checks if taint is tolerated by any of the tolerations. func TolerationsTolerateTaint(tolerations []v1.Toleration, taint *v1.Taint) bool { for i := range tolerations { diff --git a/pkg/utils/predicates_test.go b/pkg/utils/predicates_test.go index 691da26ea..defef78af 100644 --- a/pkg/utils/predicates_test.go +++ b/pkg/utils/predicates_test.go @@ -335,3 +335,606 @@ func TestTolerationsEqual(t *testing.T) { }) } } + +func TestUniqueSortNodeSelectorRequirements(t *testing.T) { + tests := []struct { + name string + requirements []v1.NodeSelectorRequirement + expectedRequirements []v1.NodeSelectorRequirement + }{ + { + name: "Identical requirements", + requirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + expectedRequirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + { + name: "Sorted requirements", + requirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2"}, + }, + }, + expectedRequirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2"}, + }, + }, + }, + { + name: "Sort values", + requirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2", "v1"}, + }, + }, + expectedRequirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + }, + { + name: "Sort by key", + requirements: []v1.NodeSelectorRequirement{ + { + Key: "k3", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + expectedRequirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k3", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + }, + { + name: "Sort by operator", + requirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpExists, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpGt, + Values: []string{"v1", "v2"}, + }, + }, + expectedRequirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpExists, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpGt, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + }, + { + name: "Sort by values", + requirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v6", "v5"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2", "v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v4", "v1"}, + }, + }, + expectedRequirements: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v4"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v5", "v6"}, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resultRequirements := uniqueSortNodeSelectorRequirements(test.requirements) + if !reflect.DeepEqual(resultRequirements, test.expectedRequirements) { + t.Errorf("Requirements not sorted as expected, \n\tgot: %#v, \n\texpected: %#v", resultRequirements, test.expectedRequirements) + } + }) + } +} + +func TestUniqueSortNodeSelectorTerms(t *testing.T) { + tests := []struct { + name string + terms []v1.NodeSelectorTerm + expectedTerms []v1.NodeSelectorTerm + }{ + { + name: "Identical terms", + terms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + expectedTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + { + name: "Sorted terms", + terms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + expectedTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + { + name: "Sort terms", + terms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2", "v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v3", "v1"}, + }, + }, + }, + }, + expectedTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v3"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + { + name: "Unique sort terms", + terms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2", "v1"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2", "v1"}, + }, + }, + }, + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v2", "v1"}, + }, + }, + }, + }, + expectedTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resultTerms := uniqueSortNodeSelectorTerms(test.terms) + if !reflect.DeepEqual(resultTerms, test.expectedTerms) { + t.Errorf("Terms not sorted as expected, \n\tgot: %#v, \n\texpected: %#v", resultTerms, test.expectedTerms) + } + }) + } +} + +func TestNodeSelectorTermsEqual(t *testing.T) { + tests := []struct { + name string + leftSelector, rightSelector v1.NodeSelector + equal bool + }{ + { + name: "identical selectors", + leftSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + rightSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + equal: true, + }, + { + name: "equal selectors", + leftSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + }, + }, + }, + rightSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + equal: true, + }, + { + name: "non-equal selectors in values", + leftSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + rightSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + }, + }, + }, + equal: false, + }, + { + name: "non-equal selectors in keys", + leftSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k3", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1"}, + }, + }, + }, + }, + }, + rightSelector: v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "k1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"v1", "v2"}, + }, + }, + }, + }, + }, + equal: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + equal := NodeSelectorsEqual(&test.leftSelector, &test.rightSelector) + if equal != test.equal { + t.Errorf("NodeSelectorsEqual expected to be %v, got %v", test.equal, equal) + } + }) + } +} From 5396282e3ddfc2ab8a180ae026f8c3af39b9bc13 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Wed, 5 May 2021 14:37:09 +0200 Subject: [PATCH 3/3] RemoveDuplicates: take node taints, node affinity and node selector into account when computing a number of feasible nodes for the average occurence of pods per node Nodes with taints which are not tolerated by evicted pods will never run the pods. The same holds for node affinity and node selector. So increase the number of pods per feasible nodes to decrease the number of evicted pods. --- pkg/descheduler/strategies/duplicates.go | 81 ++++++- pkg/descheduler/strategies/duplicates_test.go | 211 ++++++++++++++++++ 2 files changed, 289 insertions(+), 3 deletions(-) diff --git a/pkg/descheduler/strategies/duplicates.go b/pkg/descheduler/strategies/duplicates.go index 92de6ec3d..380979d6d 100644 --- a/pkg/descheduler/strategies/duplicates.go +++ b/pkg/descheduler/strategies/duplicates.go @@ -183,9 +183,17 @@ func RemoveDuplicatePods( } // 1. how many pods can be evicted to respect uniform placement of pods among viable nodes? - for ownerKey, nodes := range duplicatePods { - upperAvg := int(math.Ceil(float64(ownerKeyOccurence[ownerKey]) / float64(nodeCount))) - for nodeName, pods := range nodes { + for ownerKey, podNodes := range duplicatePods { + targetNodes := getTargetNodes(podNodes, nodes) + + klog.V(2).InfoS("Adjusting feasible nodes", "owner", ownerKey, "from", nodeCount, "to", len(targetNodes)) + if len(targetNodes) < 2 { + klog.V(1).InfoS("Less than two feasible nodes for duplicates to land, skipping eviction", "owner", ownerKey) + continue + } + + upperAvg := int(math.Ceil(float64(ownerKeyOccurence[ownerKey]) / float64(len(targetNodes)))) + for nodeName, pods := range podNodes { klog.V(2).InfoS("Average occurrence per node", "node", klog.KObj(nodeMap[nodeName]), "ownerKey", ownerKey, "avg", upperAvg) // list of duplicated pods does not contain the original referential pod if len(pods)+1 > upperAvg { @@ -202,6 +210,73 @@ func RemoveDuplicatePods( } } +func getNodeAffinityNodeSelector(pod *v1.Pod) *v1.NodeSelector { + if pod.Spec.Affinity == nil { + return nil + } + if pod.Spec.Affinity.NodeAffinity == nil { + return nil + } + return pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution +} + +func getTargetNodes(podNodes map[string][]*v1.Pod, nodes []*v1.Node) []*v1.Node { + // In order to reduce the number of pods processed, identify pods which have + // equal (tolerations, nodeselectors, node affinity) terms and considered them + // as identical. Identical pods wrt. (tolerations, nodeselectors, node affinity) terms + // will produce the same result when checking if a pod is feasible for a node. + // Thus, improving efficiency of processing pods marked for eviction. + + // Collect all distinct pods which differ in at least taints, node affinity or node selector terms + distinctPods := map[*v1.Pod]struct{}{} + for _, pods := range podNodes { + for _, pod := range pods { + duplicated := false + for dp := range distinctPods { + if utils.TolerationsEqual(pod.Spec.Tolerations, dp.Spec.Tolerations) && + utils.NodeSelectorsEqual(getNodeAffinityNodeSelector(pod), getNodeAffinityNodeSelector(dp)) && + reflect.DeepEqual(pod.Spec.NodeSelector, dp.Spec.NodeSelector) { + duplicated = true + continue + } + } + if duplicated { + continue + } + distinctPods[pod] = struct{}{} + } + } + + // For each distinct pod get a list of nodes where it can land + targetNodesMap := map[string]*v1.Node{} + for pod := range distinctPods { + matchingNodes := map[string]*v1.Node{} + for _, node := range nodes { + if !utils.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, node.Spec.Taints, func(taint *v1.Taint) bool { + return taint.Effect == v1.TaintEffectNoSchedule || taint.Effect == v1.TaintEffectNoExecute + }) { + continue + } + if match, err := utils.PodMatchNodeSelector(pod, node); err == nil && !match { + continue + } + matchingNodes[node.Name] = node + } + if len(matchingNodes) > 1 { + for nodeName := range matchingNodes { + targetNodesMap[nodeName] = matchingNodes[nodeName] + } + } + } + + targetNodes := []*v1.Node{} + for _, node := range targetNodesMap { + targetNodes = append(targetNodes, node) + } + + return targetNodes +} + func hasExcludedOwnerRefKind(ownerRefs []metav1.OwnerReference, strategy api.DeschedulerStrategy) bool { if strategy.Params == nil || strategy.Params.RemoveDuplicates == nil { return false diff --git a/pkg/descheduler/strategies/duplicates_test.go b/pkg/descheduler/strategies/duplicates_test.go index 8121d3a77..fe8d4dab9 100644 --- a/pkg/descheduler/strategies/duplicates_test.go +++ b/pkg/descheduler/strategies/duplicates_test.go @@ -240,6 +240,117 @@ func TestRemoveDuplicatesUniformly(t *testing.T) { } } + setTolerationsK1 := func(pod *v1.Pod) { + test.SetRSOwnerRef(pod) + pod.Spec.Tolerations = []v1.Toleration{ + { + Key: "k1", + Value: "v1", + Operator: v1.TolerationOpEqual, + Effect: v1.TaintEffectNoSchedule, + }, + } + } + setTolerationsK2 := func(pod *v1.Pod) { + test.SetRSOwnerRef(pod) + pod.Spec.Tolerations = []v1.Toleration{ + { + Key: "k2", + Value: "v2", + Operator: v1.TolerationOpEqual, + Effect: v1.TaintEffectNoSchedule, + }, + } + } + + setMasterNoScheduleTaint := func(node *v1.Node) { + node.Spec.Taints = []v1.Taint{ + { + Effect: v1.TaintEffectNoSchedule, + Key: "node-role.kubernetes.io/master", + }, + } + } + + setMasterNoScheduleLabel := func(node *v1.Node) { + if node.ObjectMeta.Labels == nil { + node.ObjectMeta.Labels = map[string]string{} + } + node.ObjectMeta.Labels["node-role.kubernetes.io/master"] = "" + } + + setWorkerLabel := func(node *v1.Node) { + if node.ObjectMeta.Labels == nil { + node.ObjectMeta.Labels = map[string]string{} + } + node.ObjectMeta.Labels["node-role.kubernetes.io/worker"] = "k1" + node.ObjectMeta.Labels["node-role.kubernetes.io/worker"] = "k2" + } + + setNotMasterNodeSelectorK1 := func(pod *v1.Pod) { + test.SetRSOwnerRef(pod) + pod.Spec.Affinity = &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "node-role.kubernetes.io/master", + Operator: v1.NodeSelectorOpDoesNotExist, + }, + { + Key: "k1", + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, + }, + }, + } + } + + setNotMasterNodeSelectorK2 := func(pod *v1.Pod) { + test.SetRSOwnerRef(pod) + pod.Spec.Affinity = &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "node-role.kubernetes.io/master", + Operator: v1.NodeSelectorOpDoesNotExist, + }, + { + Key: "k2", + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, + }, + }, + } + } + + setWorkerLabelSelectorK1 := func(pod *v1.Pod) { + test.SetRSOwnerRef(pod) + if pod.Spec.NodeSelector == nil { + pod.Spec.NodeSelector = map[string]string{} + } + pod.Spec.NodeSelector["node-role.kubernetes.io/worker"] = "k1" + } + + setWorkerLabelSelectorK2 := func(pod *v1.Pod) { + test.SetRSOwnerRef(pod) + if pod.Spec.NodeSelector == nil { + pod.Spec.NodeSelector = map[string]string{} + } + pod.Spec.NodeSelector["node-role.kubernetes.io/worker"] = "k2" + } + testCases := []struct { description string maxPodsToEvictPerNode int @@ -393,6 +504,106 @@ func TestRemoveDuplicatesUniformly(t *testing.T) { }, strategy: api.DeschedulerStrategy{}, }, + { + description: "Evict pods uniformly respecting taints", + pods: []v1.Pod{ + // (5,3,1,0,0,0) -> (3,3,3,0,0,0) -> 2 evictions + *test.BuildTestPod("p1", 100, 0, "worker1", setTolerationsK1), + *test.BuildTestPod("p2", 100, 0, "worker1", setTolerationsK2), + *test.BuildTestPod("p3", 100, 0, "worker1", setTolerationsK1), + *test.BuildTestPod("p4", 100, 0, "worker1", setTolerationsK2), + *test.BuildTestPod("p5", 100, 0, "worker1", setTolerationsK1), + *test.BuildTestPod("p6", 100, 0, "worker2", setTolerationsK2), + *test.BuildTestPod("p7", 100, 0, "worker2", setTolerationsK1), + *test.BuildTestPod("p8", 100, 0, "worker2", setTolerationsK2), + *test.BuildTestPod("p9", 100, 0, "worker3", setTolerationsK1), + }, + expectedEvictedPodCount: 2, + nodes: []*v1.Node{ + test.BuildTestNode("worker1", 2000, 3000, 10, nil), + test.BuildTestNode("worker2", 2000, 3000, 10, nil), + test.BuildTestNode("worker3", 2000, 3000, 10, nil), + test.BuildTestNode("master1", 2000, 3000, 10, setMasterNoScheduleTaint), + test.BuildTestNode("master2", 2000, 3000, 10, setMasterNoScheduleTaint), + test.BuildTestNode("master3", 2000, 3000, 10, setMasterNoScheduleTaint), + }, + strategy: api.DeschedulerStrategy{}, + }, + { + description: "Evict pods uniformly respecting RequiredDuringSchedulingIgnoredDuringExecution node affinity", + pods: []v1.Pod{ + // (5,3,1,0,0,0) -> (3,3,3,0,0,0) -> 2 evictions + *test.BuildTestPod("p1", 100, 0, "worker1", setNotMasterNodeSelectorK1), + *test.BuildTestPod("p2", 100, 0, "worker1", setNotMasterNodeSelectorK2), + *test.BuildTestPod("p3", 100, 0, "worker1", setNotMasterNodeSelectorK1), + *test.BuildTestPod("p4", 100, 0, "worker1", setNotMasterNodeSelectorK2), + *test.BuildTestPod("p5", 100, 0, "worker1", setNotMasterNodeSelectorK1), + *test.BuildTestPod("p6", 100, 0, "worker2", setNotMasterNodeSelectorK2), + *test.BuildTestPod("p7", 100, 0, "worker2", setNotMasterNodeSelectorK1), + *test.BuildTestPod("p8", 100, 0, "worker2", setNotMasterNodeSelectorK2), + *test.BuildTestPod("p9", 100, 0, "worker3", setNotMasterNodeSelectorK1), + }, + expectedEvictedPodCount: 2, + nodes: []*v1.Node{ + test.BuildTestNode("worker1", 2000, 3000, 10, nil), + test.BuildTestNode("worker2", 2000, 3000, 10, nil), + test.BuildTestNode("worker3", 2000, 3000, 10, nil), + test.BuildTestNode("master1", 2000, 3000, 10, setMasterNoScheduleLabel), + test.BuildTestNode("master2", 2000, 3000, 10, setMasterNoScheduleLabel), + test.BuildTestNode("master3", 2000, 3000, 10, setMasterNoScheduleLabel), + }, + strategy: api.DeschedulerStrategy{}, + }, + { + description: "Evict pods uniformly respecting node selector", + pods: []v1.Pod{ + // (5,3,1,0,0,0) -> (3,3,3,0,0,0) -> 2 evictions + *test.BuildTestPod("p1", 100, 0, "worker1", setWorkerLabelSelectorK1), + *test.BuildTestPod("p2", 100, 0, "worker1", setWorkerLabelSelectorK2), + *test.BuildTestPod("p3", 100, 0, "worker1", setWorkerLabelSelectorK1), + *test.BuildTestPod("p4", 100, 0, "worker1", setWorkerLabelSelectorK2), + *test.BuildTestPod("p5", 100, 0, "worker1", setWorkerLabelSelectorK1), + *test.BuildTestPod("p6", 100, 0, "worker2", setWorkerLabelSelectorK2), + *test.BuildTestPod("p7", 100, 0, "worker2", setWorkerLabelSelectorK1), + *test.BuildTestPod("p8", 100, 0, "worker2", setWorkerLabelSelectorK2), + *test.BuildTestPod("p9", 100, 0, "worker3", setWorkerLabelSelectorK1), + }, + expectedEvictedPodCount: 2, + nodes: []*v1.Node{ + test.BuildTestNode("worker1", 2000, 3000, 10, setWorkerLabel), + test.BuildTestNode("worker2", 2000, 3000, 10, setWorkerLabel), + test.BuildTestNode("worker3", 2000, 3000, 10, setWorkerLabel), + test.BuildTestNode("master1", 2000, 3000, 10, nil), + test.BuildTestNode("master2", 2000, 3000, 10, nil), + test.BuildTestNode("master3", 2000, 3000, 10, nil), + }, + strategy: api.DeschedulerStrategy{}, + }, + { + description: "Evict pods uniformly respecting node selector with zero target nodes", + pods: []v1.Pod{ + // (5,3,1,0,0,0) -> (3,3,3,0,0,0) -> 2 evictions + *test.BuildTestPod("p1", 100, 0, "worker1", setWorkerLabelSelectorK1), + *test.BuildTestPod("p2", 100, 0, "worker1", setWorkerLabelSelectorK2), + *test.BuildTestPod("p3", 100, 0, "worker1", setWorkerLabelSelectorK1), + *test.BuildTestPod("p4", 100, 0, "worker1", setWorkerLabelSelectorK2), + *test.BuildTestPod("p5", 100, 0, "worker1", setWorkerLabelSelectorK1), + *test.BuildTestPod("p6", 100, 0, "worker2", setWorkerLabelSelectorK2), + *test.BuildTestPod("p7", 100, 0, "worker2", setWorkerLabelSelectorK1), + *test.BuildTestPod("p8", 100, 0, "worker2", setWorkerLabelSelectorK2), + *test.BuildTestPod("p9", 100, 0, "worker3", setWorkerLabelSelectorK1), + }, + expectedEvictedPodCount: 0, + nodes: []*v1.Node{ + test.BuildTestNode("worker1", 2000, 3000, 10, nil), + test.BuildTestNode("worker2", 2000, 3000, 10, nil), + test.BuildTestNode("worker3", 2000, 3000, 10, nil), + test.BuildTestNode("master1", 2000, 3000, 10, nil), + test.BuildTestNode("master2", 2000, 3000, 10, nil), + test.BuildTestNode("master3", 2000, 3000, 10, nil), + }, + strategy: api.DeschedulerStrategy{}, + }, } for _, testCase := range testCases {