From c735921b6f1ae13a985c67127efa356bdbf7dd12 Mon Sep 17 00:00:00 2001 From: Harry Zhang Date: Fri, 27 May 2016 15:57:05 +0800 Subject: [PATCH] Add no admit on node side Update generated code Refactored predicates & restore helper --- pkg/api/types.go | 7 +- pkg/api/v1/generated.proto | 4 +- pkg/api/v1/types.go | 7 +- pkg/api/v1/types_swagger_doc_generated.go | 4 +- pkg/api/validation/validation.go | 6 +- pkg/kubectl/cmd/taint.go | 2 +- .../scheduler/algorithm/predicates/error.go | 20 ++++- .../algorithm/predicates/predicates.go | 34 ++++++-- .../algorithm/predicates/predicates_test.go | 77 ++++++++++++++++--- .../algorithm/priorities/taint_toleration.go | 2 +- 10 files changed, 128 insertions(+), 35 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 8cff0e45591..e329b89ac88 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1460,7 +1460,7 @@ type Taint struct { Value string `json:"value,omitempty"` // Required. The effect of the taint on pods // that do not tolerate the taint. - // Valid effects are NoSchedule and PreferNoSchedule. + // Valid effects are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule. Effect TaintEffect `json:"effect"` } @@ -1476,12 +1476,11 @@ const ( // new pods onto the node, rather than prohibiting new pods from scheduling // onto the node entirely. Enforced by the scheduler. TaintEffectPreferNoSchedule TaintEffect = "PreferNoSchedule" - // NOT YET IMPLEMENTED. TODO: Uncomment field once it is implemented. // Do not allow new pods to schedule onto the node unless they tolerate the taint, // do not allow pods to start on Kubelet unless they tolerate the taint, // but allow all already-running pods to continue running. // Enforced by the scheduler and Kubelet. - // TaintEffectNoScheduleNoAdmit TaintEffect = "NoScheduleNoAdmit" + TaintEffectNoScheduleNoAdmit TaintEffect = "NoScheduleNoAdmit" // NOT YET IMPLEMENTED. TODO: Uncomment field once it is implemented. // Do not allow new pods to schedule onto the node unless they tolerate the taint, // do not allow pods to start on Kubelet unless they tolerate the taint, @@ -1504,7 +1503,7 @@ type Toleration struct { // If the operator is Exists, the value should be empty, otherwise just a regular string. Value string `json:"value,omitempty"` // Effect indicates the taint effect to match. Empty means match all taint effects. - // When specified, allowed values are NoSchedule and PreferNoSchedule. + // When specified, allowed values are NoSchedule,NoScheduleNoAdmit and PreferNoSchedule. Effect TaintEffect `json:"effect,omitempty"` // TODO: For forgiveness (#1574), we'd eventually add at least a grace period // here, and possibly an occurrence threshold and period. diff --git a/pkg/api/v1/generated.proto b/pkg/api/v1/generated.proto index 94ce93a8d6e..7db36a94cc8 100644 --- a/pkg/api/v1/generated.proto +++ b/pkg/api/v1/generated.proto @@ -2920,7 +2920,7 @@ message Taint { // Required. The effect of the taint on pods // that do not tolerate the taint. - // Valid effects are NoSchedule and PreferNoSchedule. + // Valid effects are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule. optional string effect = 3; } @@ -2941,7 +2941,7 @@ message Toleration { optional string value = 3; // Effect indicates the taint effect to match. Empty means match all taint effects. - // When specified, allowed values are NoSchedule and PreferNoSchedule. + // When specified, allowed values are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule. optional string effect = 4; } diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 4f0cf950fd8..955d3483bf1 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -1679,7 +1679,7 @@ type Taint struct { Value string `json:"value,omitempty" protobuf:"bytes,2,opt,name=value"` // Required. The effect of the taint on pods // that do not tolerate the taint. - // Valid effects are NoSchedule and PreferNoSchedule. + // Valid effects are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule. Effect TaintEffect `json:"effect" protobuf:"bytes,3,opt,name=effect,casttype=TaintEffect"` } @@ -1695,12 +1695,11 @@ const ( // new pods onto the node, rather than prohibiting new pods from scheduling // onto the node entirely. Enforced by the scheduler. TaintEffectPreferNoSchedule TaintEffect = "PreferNoSchedule" - // NOT YET IMPLEMENTED. TODO: Uncomment field once it is implemented. // Do not allow new pods to schedule onto the node unless they tolerate the taint, // do not allow pods to start on Kubelet unless they tolerate the taint, // but allow all already-running pods to continue running. // Enforced by the scheduler and Kubelet. - // TaintEffectNoScheduleNoAdmit TaintEffect = "NoScheduleNoAdmit" + TaintEffectNoScheduleNoAdmit TaintEffect = "NoScheduleNoAdmit" // NOT YET IMPLEMENTED. TODO: Uncomment field once it is implemented. // Do not allow new pods to schedule onto the node unless they tolerate the taint, // do not allow pods to start on Kubelet unless they tolerate the taint, @@ -1723,7 +1722,7 @@ type Toleration struct { // If the operator is Exists, the value should be empty, otherwise just a regular string. Value string `json:"value,omitempty" protobuf:"bytes,3,opt,name=value"` // Effect indicates the taint effect to match. Empty means match all taint effects. - // When specified, allowed values are NoSchedule and PreferNoSchedule. + // When specified, allowed values are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule. Effect TaintEffect `json:"effect,omitempty" protobuf:"bytes,4,opt,name=effect,casttype=TaintEffect"` // TODO: For forgiveness (#1574), we'd eventually add at least a grace period // here, and possibly an occurrence threshold and period. diff --git a/pkg/api/v1/types_swagger_doc_generated.go b/pkg/api/v1/types_swagger_doc_generated.go index 3f0ac8f0238..a440c57c1eb 100644 --- a/pkg/api/v1/types_swagger_doc_generated.go +++ b/pkg/api/v1/types_swagger_doc_generated.go @@ -1718,7 +1718,7 @@ var map_Taint = map[string]string{ "": "The node this Taint is attached to has the effect \"effect\" on any pod that that does not tolerate the Taint.", "key": "Required. The taint key to be applied to a node.", "value": "Required. The taint value corresponding to the taint key.", - "effect": "Required. The effect of the taint on pods that do not tolerate the taint. Valid effects are NoSchedule and PreferNoSchedule.", + "effect": "Required. The effect of the taint on pods that do not tolerate the taint. Valid effects are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule.", } func (Taint) SwaggerDoc() map[string]string { @@ -1730,7 +1730,7 @@ var map_Toleration = map[string]string{ "key": "Required. Key is the taint key that the toleration applies to.", "operator": "operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", "value": "Value is the taint value the toleration matches to. If the operator is Exists, the value should be empty, otherwise just a regular string.", - "effect": "Effect indicates the taint effect to match. Empty means match all taint effects. When specified, allowed values are NoSchedule and PreferNoSchedule.", + "effect": "Effect indicates the taint effect to match. Empty means match all taint effects. When specified, allowed values are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule.", } func (Toleration) SwaggerDoc() map[string]string { diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index e0d777c6b33..07dc5aa04fe 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1776,14 +1776,14 @@ func validateTaintEffect(effect *api.TaintEffect, allowEmpty bool, fldPath *fiel allErrors := field.ErrorList{} switch *effect { // TODO: Replace next line with subsequent commented-out line when implement TaintEffectNoScheduleNoAdmit, TaintEffectNoScheduleNoAdmitNoExecute. - case api.TaintEffectNoSchedule, api.TaintEffectPreferNoSchedule: - // case api.TaintEffectNoSchedule, api.TaintEffectPreferNoSchedule, api.TaintEffectNoScheduleNoAdmit, api.TaintEffectNoScheduleNoAdmitNoExecute: + case api.TaintEffectNoSchedule, api.TaintEffectPreferNoSchedule, api.TaintEffectNoScheduleNoAdmit: + // case api.TaintEffectNoSchedule, api.TaintEffectPreferNoSchedule, api.TaintEffectNoScheduleNoAdmitNoExecute: default: validValues := []string{ string(api.TaintEffectNoSchedule), string(api.TaintEffectPreferNoSchedule), + string(api.TaintEffectNoScheduleNoAdmit), // TODO: Uncomment this block when implement TaintEffectNoScheduleNoAdmit, TaintEffectNoScheduleNoAdmitNoExecute. - // string(api.TaintEffectNoScheduleNoAdmit), // string(api.TaintEffectNoScheduleNoAdmitNoExecute), } allErrors = append(allErrors, field.NotSupported(fldPath, effect, validValues)) diff --git a/pkg/kubectl/cmd/taint.go b/pkg/kubectl/cmd/taint.go index 66aa6216f91..44dac889e34 100644 --- a/pkg/kubectl/cmd/taint.go +++ b/pkg/kubectl/cmd/taint.go @@ -181,7 +181,7 @@ func parseTaints(spec []string) ([]api.Taint, []api.Taint, error) { return nil, nil, fmt.Errorf("invalid taint spec: %v, %s", taintSpec, strings.Join(errs, "; ")) } - if parts2[1] != string(api.TaintEffectNoSchedule) && parts2[1] != string(api.TaintEffectPreferNoSchedule) { + if parts2[1] != string(api.TaintEffectNoSchedule) && parts2[1] != string(api.TaintEffectPreferNoSchedule) && parts2[1] != string(api.TaintEffectNoScheduleNoAdmit) { return nil, nil, fmt.Errorf("invalid taint spec: %v, unsupported taint effect", taintSpec) } diff --git a/plugin/pkg/scheduler/algorithm/predicates/error.go b/plugin/pkg/scheduler/algorithm/predicates/error.go index a71cdb9aae7..1bb71b1ec5a 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/error.go +++ b/plugin/pkg/scheduler/algorithm/predicates/error.go @@ -29,7 +29,6 @@ var ( ErrVolumeZoneConflict = newPredicateFailureError("NoVolumeZoneConflict") ErrNodeSelectorNotMatch = newPredicateFailureError("MatchNodeSelector") ErrPodAffinityNotMatch = newPredicateFailureError("MatchInterPodAffinity") - ErrTaintsTolerationsNotMatch = newPredicateFailureError("PodToleratesNodeTaints") ErrPodNotMatchHostName = newPredicateFailureError("HostName") ErrPodNotFitsHostPorts = newPredicateFailureError("PodFitsHostPorts") ErrNodeLabelPresenceViolated = newPredicateFailureError("CheckNodeLabelPresence") @@ -42,6 +41,25 @@ var ( ErrFakePredicate = newPredicateFailureError("FakePredicateError") ) +// ErrTaintsTolerationsNotMatch is an error type that indicates with if it should be aware by kubelet. +type ErrTaintsTolerationsNotMatch struct { + SomeUntoleratedTaintIsNoAdmit bool +} + +func newErrTaintsTolerationsNotMatch(someUntoleratedTaintIsNoAdmit bool) *ErrTaintsTolerationsNotMatch { + return &ErrTaintsTolerationsNotMatch{ + SomeUntoleratedTaintIsNoAdmit: someUntoleratedTaintIsNoAdmit, + } +} + +func (e *ErrTaintsTolerationsNotMatch) Error() string { + return fmt.Sprintf("Taint Toleration unmatched with SomeUntoleratedTaintIsNoAdmit is: %v", e.SomeUntoleratedTaintIsNoAdmit) +} + +func (e *ErrTaintsTolerationsNotMatch) GetReason() string { + return fmt.Sprintf("ErrTaintsTolerationsNotMatch, and SomeUntoleratedTaintIsNoAdmit is: %v", e.SomeUntoleratedTaintIsNoAdmit) +} + // InsufficientResourceError is an error type that indicates what kind of resource limit is // hit and caused the unfitting failure. type InsufficientResourceError struct { diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index fb88a9bba81..18081030caa 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -1083,23 +1083,37 @@ func PodToleratesNodeTaints(pod *api.Pod, meta interface{}, nodeInfo *schedulerc return false, nil, err } - if tolerationsToleratesTaints(tolerations, taints) { + if tolerated, someUntoleratedTaintIsNoAdmit := tolerationsToleratesTaints(tolerations, taints); tolerated { return true, nil, nil + } else { + return false, []algorithm.PredicateFailureReason{newErrTaintsTolerationsNotMatch(someUntoleratedTaintIsNoAdmit)}, nil } - return false, []algorithm.PredicateFailureReason{ErrTaintsTolerationsNotMatch}, nil } -func tolerationsToleratesTaints(tolerations []api.Toleration, taints []api.Taint) bool { +// tolerationsToleratesTaints checks if given tolerations can live with given taints. +// It returns: +// 1. whether tolerated or not; +// 2. whether kubelet should be aware of (1). +func tolerationsToleratesTaints(tolerations []api.Toleration, taints []api.Taint) (bool, bool) { // If the taint list is nil/empty, it is tolerated by all tolerations by default. if len(taints) == 0 { - return true + return true, false } // The taint list isn't nil/empty, a nil/empty toleration list can't tolerate them. if len(tolerations) == 0 { - return false + // if there's taint has TaintEffectNoScheduleNoAdmit, kubelet should also be aware of this. + for _, taint := range taints { + if taint.Effect == api.TaintEffectNoScheduleNoAdmit { + return false, true + } + } + return false, false } + someUntoleratedTaintIsNoAdmit := false + fits := true + for i := range taints { taint := &taints[i] // skip taints that have effect PreferNoSchedule, since it is for priorities @@ -1107,12 +1121,16 @@ func tolerationsToleratesTaints(tolerations []api.Toleration, taints []api.Taint continue } - if !api.TaintToleratedByTolerations(taint, tolerations) { - return false + if tolerated := api.TaintToleratedByTolerations(taint, tolerations); !tolerated { + fits = false + if taint.Effect == api.TaintEffectNoScheduleNoAdmit { + someUntoleratedTaintIsNoAdmit = true + return fits, someUntoleratedTaintIsNoAdmit + } } } - return true + return fits, someUntoleratedTaintIsNoAdmit } // Determine if a pod is scheduled with best-effort QoS diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index cb96b4c42a3..9b4c9b6462f 100755 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -2563,10 +2563,11 @@ func TestInterPodAffinityWithMultipleNodes(t *testing.T) { func TestPodToleratesTaints(t *testing.T) { podTolerateTaintsTests := []struct { - pod *api.Pod - node api.Node - fits bool - test string + pod *api.Pod + node api.Node + fits bool + expectedFailureReasons []algorithm.PredicateFailureReason + test string }{ { pod: &api.Pod{ @@ -2587,6 +2588,7 @@ func TestPodToleratesTaints(t *testing.T) { }, }, fits: false, + expectedFailureReasons: []algorithm.PredicateFailureReason{newErrTaintsTolerationsNotMatch(false)}, test: "a pod having no tolerations can't be scheduled onto a node with nonempty taints", }, { @@ -2652,6 +2654,7 @@ func TestPodToleratesTaints(t *testing.T) { }, }, fits: false, + expectedFailureReasons: []algorithm.PredicateFailureReason{newErrTaintsTolerationsNotMatch(false)}, test: "a pod which can't be scheduled on a dedicated node assigned to user2 with effect NoSchedule", }, { @@ -2758,6 +2761,7 @@ func TestPodToleratesTaints(t *testing.T) { }, }, fits: false, + expectedFailureReasons: []algorithm.PredicateFailureReason{newErrTaintsTolerationsNotMatch(false)}, test: "a pod has a toleration that keys and values match the taint on the node, but (non-empty) effect doesn't match, " + "can't be scheduled onto the node", }, @@ -2791,7 +2795,7 @@ func TestPodToleratesTaints(t *testing.T) { }, }, fits: true, - test: "The pod has a toleration that keys and values match the taint on the node, the effect of toleration is empty, " + + test: "the pod has a toleration that keys and values match the taint on the node, the effect of toleration is empty, " + "and the effect of taint is NoSchedule. Pod can be scheduled onto the node", }, { @@ -2828,8 +2832,63 @@ func TestPodToleratesTaints(t *testing.T) { test: "The pod has a toleration that key and value don't match the taint on the node, " + "but the effect of taint on node is PreferNochedule. Pod can be scheduled onto the node", }, + { + pod: &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "podadmit1", + }, + }, + node: api.Node{ + ObjectMeta: api.ObjectMeta{ + Annotations: map[string]string{ + api.TaintsAnnotationKey: ` + [{ + "key": "dedicated", + "value": "user1", + "effect": "NoScheduleNoAdmit" + }]`, + }, + }, + }, + fits: false, + expectedFailureReasons: []algorithm.PredicateFailureReason{newErrTaintsTolerationsNotMatch(true)}, + test: "node should aware that that a pod having no tolerations can't be scheduled or started on a node with nonempty taints", + }, + { + pod: &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "podadmit2", + Annotations: map[string]string{ + api.TolerationsAnnotationKey: ` + [{ + "key": "dedicated", + "operator": "Equal", + "value": "user2", + "effect": "NoScheduleNoAdmit" + }]`, + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{{Image: "pod2:V1"}}, + }, + }, + node: api.Node{ + ObjectMeta: api.ObjectMeta{ + Annotations: map[string]string{ + api.TaintsAnnotationKey: ` + [{ + "key": "dedicated", + "value": "user1", + "effect": "NoScheduleNoAdmit" + }]`, + }, + }, + }, + fits: false, + expectedFailureReasons: []algorithm.PredicateFailureReason{newErrTaintsTolerationsNotMatch(true)}, + test: "node should aware that a pod which can't be scheduled or start on a dedicated node assgined to user2 with effect NoScheduleNoAdmit", + }, } - expectedFailureReasons := []algorithm.PredicateFailureReason{ErrTaintsTolerationsNotMatch} for _, test := range podTolerateTaintsTests { nodeInfo := schedulercache.NewNodeInfo() @@ -2838,11 +2897,11 @@ func TestPodToleratesTaints(t *testing.T) { if err != nil { t.Errorf("%s, unexpected error: %v", test.test, err) } - if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) { - t.Errorf("%s, unexpected failure reason: %v, want: %v", test.test, reasons, expectedFailureReasons) + if !fits && !reflect.DeepEqual(reasons, test.expectedFailureReasons) { + t.Errorf("%s, unexpected failure reason: %v, want: %v", test.test, reasons, test.expectedFailureReasons) } if fits != test.fits { - t.Errorf("%s, expected: %v got %v", test.test, test.fits, fits) + t.Errorf("%s,\n expected: %v got %v", test.test, test.fits, fits) } } } diff --git a/plugin/pkg/scheduler/algorithm/priorities/taint_toleration.go b/plugin/pkg/scheduler/algorithm/priorities/taint_toleration.go index b299044d212..bd31aa5877d 100644 --- a/plugin/pkg/scheduler/algorithm/priorities/taint_toleration.go +++ b/plugin/pkg/scheduler/algorithm/priorities/taint_toleration.go @@ -32,7 +32,7 @@ func countIntolerableTaintsPreferNoSchedule(taints []api.Taint, tolerations []ap continue } - if !api.TaintToleratedByTolerations(taint, tolerations) { + if tolerable := api.TaintToleratedByTolerations(taint, tolerations); !tolerable { intolerableTaints++ } }