address review comments 3

This commit is contained in:
Kevin 2017-01-31 21:20:42 +08:00
parent 3c550c32fb
commit b410f3f4c2
2 changed files with 1 additions and 72 deletions

View File

@ -20,9 +20,7 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"strings" "strings"
"time"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
@ -316,10 +314,6 @@ func GetNodeTaints(node *Node) ([]Taint, error) {
// (3) Empty toleration.key means to match all taint keys. // (3) Empty toleration.key means to match all taint keys.
// If toleration.key is empty, toleration.operator must be 'Exists'; // If toleration.key is empty, toleration.operator must be 'Exists';
// this combination means to match all taint values and all taint keys. // this combination means to match all taint values and all taint keys.
// (4) Nil toleration.tolerationSeconds means to tolerate the taint forever.
// (5) Non-nil positive toleration.tolerationSeconds means to
// match the taint for only a duration since the taint was observed
// by the TaintManager.
func (t *Toleration) ToleratesTaint(taint *Taint) bool { func (t *Toleration) ToleratesTaint(taint *Taint) bool {
if len(t.Effect) > 0 && t.Effect != taint.Effect { if len(t.Effect) > 0 && t.Effect != taint.Effect {
return false return false
@ -329,11 +323,6 @@ func (t *Toleration) ToleratesTaint(taint *Taint) bool {
return false return false
} }
// TODO: need to take time skew into consideration, make sure toleration won't become out of age ealier than expected.
if t.TolerationSeconds != nil && metav1.Now().After(taint.TimeAdded.Add(time.Second*time.Duration(*t.TolerationSeconds))) {
return false
}
// TODO: Use proper defaulting when Toleration becomes a field of PodSpec // TODO: Use proper defaulting when Toleration becomes a field of PodSpec
switch t.Operator { switch t.Operator {
// empty operator means Equal // empty operator means Equal
@ -392,6 +381,7 @@ func DeleteTaintsByKey(taints []Taint, taintKey string) ([]Taint, bool) {
return newTaints, deleted return newTaints, deleted
} }
// DeleteTaint removes all the the taints that have the same key and effect to given taintToDelete.
func DeleteTaint(taints []Taint, taintToDelete *Taint) ([]Taint, bool) { func DeleteTaint(taints []Taint, taintToDelete *Taint) ([]Taint, bool) {
newTaints := []Taint{} newTaints := []Taint{}
deleted := false deleted := false

View File

@ -281,9 +281,6 @@ func TestMatchTaint(t *testing.T) {
} }
func TestTolerationToleratesTaint(t *testing.T) { func TestTolerationToleratesTaint(t *testing.T) {
genTolerationSeconds := func(f int64) *int64 {
return &f
}
testCases := []struct { testCases := []struct {
description string description string
@ -377,64 +374,6 @@ func TestTolerationToleratesTaint(t *testing.T) {
}, },
expectTolerated: false, expectTolerated: false,
}, },
{
description: "expect toleration with nil tolerationSeconds tolerates taint that is newly added",
toleration: Toleration{
Key: "foo",
Operator: TolerationOpExists,
Effect: TaintEffectNoExecute,
},
taint: Taint{
Key: "foo",
Effect: TaintEffectNoExecute,
TimeAdded: metav1.Now(),
},
expectTolerated: true,
},
{
description: "forgiveness toleration has not timed out, expect tolerated",
toleration: Toleration{
Key: "foo",
Operator: TolerationOpExists,
Effect: TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(300),
},
taint: Taint{
Key: "foo",
Effect: TaintEffectNoExecute,
TimeAdded: metav1.Unix(metav1.Now().Unix()-100, 0),
},
expectTolerated: true,
},
{
description: "forgiveness toleration has timed out, expect not tolerated",
toleration: Toleration{
Key: "foo",
Operator: TolerationOpExists,
Effect: TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(300),
},
taint: Taint{
Key: "foo",
Effect: TaintEffectNoExecute,
TimeAdded: metav1.Unix(metav1.Now().Unix()-1000, 0),
},
expectTolerated: false,
},
{
description: "toleration with explicit forgiveness can't tolerate taint with no added time, expect not tolerated",
toleration: Toleration{
Key: "foo",
Operator: TolerationOpExists,
Effect: TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(300),
},
taint: Taint{
Key: "foo",
Effect: TaintEffectNoExecute,
},
expectTolerated: false,
},
} }
for _, tc := range testCases { for _, tc := range testCases {
if tolerated := tc.toleration.ToleratesTaint(&tc.taint); tc.expectTolerated != tolerated { if tolerated := tc.toleration.ToleratesTaint(&tc.taint); tc.expectTolerated != tolerated {