From 6a2ef1646b4b200ad580d9f96883b8fcb27b1bb1 Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 24 Jan 2017 01:33:33 +0800 Subject: [PATCH] address review comments --- pkg/api/v1/helpers.go | 22 +++++++++++----- pkg/api/v1/helpers_test.go | 52 +++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/pkg/api/v1/helpers.go b/pkg/api/v1/helpers.go index 7624223431f..da74e382152 100644 --- a/pkg/api/v1/helpers.go +++ b/pkg/api/v1/helpers.go @@ -309,20 +309,28 @@ func GetNodeTaints(node *Node) ([]Taint, error) { } // ToleratesTaint checks if the toleration tolerates the taint. +// The matching follows the rules below: +// (1) Empty toleration.effect means to match all taint effects, +// otherwise taint effect must equal to toleration.effect. +// (2) If toleration.operator is 'Exists', it means to match all taint values. +// (3) Empty toleration.key means to match all taint keys. +// If toleration.key is empty, toleration.operator must be 'Exists'; +// this combination means to match all taint values and all taint keys. +// (4) Nil toleration.tolerationSeconds means to match taints with +// any value of taint.timeAdded. +// (5) Non-nil positive toleration.tolerationSeconds means to +// match the taint for only a duration that starts at taint.timeAdded. func (t *Toleration) ToleratesTaint(taint *Taint) bool { - // empty toleration effect means match all taint effects if len(t.Effect) > 0 && t.Effect != taint.Effect { return false } - // empty toleration key means match all taint keys if len(t.Key) > 0 && t.Key != taint.Key { return false } - // nil TolerationSeconds means tolerate the taint forever if t.TolerationSeconds != nil { - // taint with no added time indicated can only be tolerated + // taint with no timeAdded indicated can only be tolerated // by toleration with no tolerationSeconds. if taint.TimeAdded.IsZero() { return false @@ -359,14 +367,14 @@ func TolerationsTolerateTaint(tolerations []Toleration, taint *Taint) bool { type taintsFilterFunc func(*Taint) bool // TolerationsTolerateTaintsWithFilter checks if given tolerations tolerates -// all the interesting taints in given taint list. -func TolerationsTolerateTaintsWithFilter(tolerations []Toleration, taints []Taint, isInterestingTaint taintsFilterFunc) bool { +// all the taints that apply to the filter in given taint list. +func TolerationsTolerateTaintsWithFilter(tolerations []Toleration, taints []Taint, applyFilter taintsFilterFunc) bool { if len(taints) == 0 { return true } for i := range taints { - if isInterestingTaint != nil && !isInterestingTaint(&taints[i]) { + if applyFilter != nil && !applyFilter(&taints[i]) { continue } diff --git a/pkg/api/v1/helpers_test.go b/pkg/api/v1/helpers_test.go index aca4da1ce46..7ad6d173a93 100644 --- a/pkg/api/v1/helpers_test.go +++ b/pkg/api/v1/helpers_test.go @@ -445,18 +445,18 @@ func TestTolerationToleratesTaint(t *testing.T) { func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { testCases := []struct { - description string - tolerations []Toleration - taints []Taint - isInterestingTaint taintsFilterFunc - expectTolerated bool + description string + tolerations []Toleration + taints []Taint + applyFilter taintsFilterFunc + expectTolerated bool }{ { - description: "empty tolerations tolerate empty taints", - tolerations: []Toleration{}, - taints: []Taint{}, - isInterestingTaint: func(t *Taint) bool { return true }, - expectTolerated: true, + description: "empty tolerations tolerate empty taints", + tolerations: []Toleration{}, + taints: []Taint{}, + applyFilter: func(t *Taint) bool { return true }, + expectTolerated: true, }, { description: "non-empty tolerations tolerate empty taints", @@ -467,9 +467,9 @@ func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { Effect: TaintEffectNoSchedule, }, }, - taints: []Taint{}, - isInterestingTaint: func(t *Taint) bool { return true }, - expectTolerated: true, + taints: []Taint{}, + applyFilter: func(t *Taint) bool { return true }, + expectTolerated: true, }, { description: "tolerations match all taints, expect tolerated", @@ -486,11 +486,11 @@ func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { Effect: TaintEffectNoSchedule, }, }, - isInterestingTaint: func(t *Taint) bool { return true }, - expectTolerated: true, + applyFilter: func(t *Taint) bool { return true }, + expectTolerated: true, }, { - description: "tolerations don't match taints, but no taint is interested, expect tolerated", + description: "tolerations don't match taints, but no taints apply to the filter, expect tolerated", tolerations: []Toleration{ { Key: "foo", @@ -504,11 +504,11 @@ func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { Effect: TaintEffectNoSchedule, }, }, - isInterestingTaint: func(t *Taint) bool { return false }, - expectTolerated: true, + applyFilter: func(t *Taint) bool { return false }, + expectTolerated: true, }, { - description: "no isInterestedTaint indicated, means all taints are interested, tolerations don't match taints, expect untolerated", + description: "no filterFunc indicated, means all taints apply to the filter, tolerations don't match taints, expect untolerated", tolerations: []Toleration{ { Key: "foo", @@ -522,11 +522,11 @@ func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { Effect: TaintEffectNoSchedule, }, }, - isInterestingTaint: nil, - expectTolerated: false, + applyFilter: nil, + expectTolerated: false, }, { - description: "tolerations match interested taints, expect tolerated", + description: "tolerations match taints, expect tolerated", tolerations: []Toleration{ { Key: "foo", @@ -544,16 +544,16 @@ func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { Effect: TaintEffectNoSchedule, }, }, - isInterestingTaint: func(t *Taint) bool { return t.Effect == TaintEffectNoExecute }, - expectTolerated: true, + applyFilter: func(t *Taint) bool { return t.Effect == TaintEffectNoExecute }, + expectTolerated: true, }, } for _, tc := range testCases { - if tc.expectTolerated != TolerationsTolerateTaintsWithFilter(tc.tolerations, tc.taints, tc.isInterestingTaint) { + if tc.expectTolerated != TolerationsTolerateTaintsWithFilter(tc.tolerations, tc.taints, tc.applyFilter) { filteredTaints := []Taint{} for _, taint := range tc.taints { - if tc.isInterestingTaint != nil && !tc.isInterestingTaint(&taint) { + if tc.applyFilter != nil && !tc.applyFilter(&taint) { continue } filteredTaints = append(filteredTaints, taint)