Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add kubelet awareness to taint tolerant match caculator. #26501

Merged
merged 3 commits into from
Oct 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -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,
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/v1/generated.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions pkg/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -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,
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/v1/types_swagger_doc_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions pkg/generated/openapi/zz_generated.openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -12451,7 +12451,7 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
},
"effect": {
SchemaProps: spec.SchemaProps{
Description: "Required. The effect of the taint on pods that do not tolerate the taint. Valid effects are NoSchedule and PreferNoSchedule.",
Description: "Required. The effect of the taint on pods that do not tolerate the taint. Valid effects are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule.",
Type: []string{"string"},
Format: "",
},
Expand Down Expand Up @@ -12549,7 +12549,7 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
},
"effect": {
SchemaProps: spec.SchemaProps{
Description: "Effect indicates the taint effect to match. Empty means match all taint effects. When specified, allowed values are NoSchedule and PreferNoSchedule.",
Description: "Effect indicates the taint effect to match. Empty means match all taint effects. When specified, allowed values are NoSchedule, NoScheduleNoAdmit and PreferNoSchedule.",
Type: []string{"string"},
Format: "",
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/taint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -2208,6 +2208,7 @@ func (kl *Kubelet) canAdmitPod(pods []*api.Pod, pod *api.Pod) (bool, string, str
return false, result.Reason, result.Message
}
}

// TODO: When disk space scheduling is implemented (#11976), remove the out-of-disk check here and
// add the disk space predicate to predicates.GeneralPredicates.
if kl.isOutOfDisk() {
Expand Down
47 changes: 47 additions & 0 deletions pkg/kubelet/lifecycle/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,53 @@ func (w *predicateAdmitHandler) Admit(attrs *PodAdmitAttributes) PodAdmitResult
Message: message,
}
}

// Check toleration against taints
// NOTE(harryz) consider move PodToleratesNodeTaints to GeneralPredicates to eliminate duplicate code here
fit, reasons, err = predicates.PodToleratesNodeTaints(pod, nil, nodeInfo)
if err != nil {
message := fmt.Sprintf("PodToleratesNodeTaints failed due to %v, which is unexpected.", err)
glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
return PodAdmitResult{
Admit: fit,
Reason: "UnexpectedError",
Message: message,
}

}
if !fit {
var reason string
var message string
if len(reasons) == 0 {
message = fmt.Sprint("PodToleratesNodeTaints failed due to unknown reason, which is unexpected.")
glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
return PodAdmitResult{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without me popping through the node code, are the events log'd higher up the call chain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see event will higher it up, more clues?

Admit: fit,
Reason: "UnknownReason",
Message: message,
}
}
r := reasons[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If multiple reasons are returned, why only output [0]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caller: canAdmitPod returns bool value with one reason, so it's enough to use the first reason.

switch re := r.(type) {
case *predicates.ErrTaintsTolerationsNotMatch:
// if kubelet should not care this unfit
if !re.SomeUntoleratedTaintIsNoAdmit {
return PodAdmitResult{
Admit: true,
}
}
reason = "PodToleratesNodeTaints"
message = re.Error()
glog.Warningf("Failed to admit pod %v - %s", format.Pod(pod), message)
}

return PodAdmitResult{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment re: event.

Admit: fit,
Reason: reason,
Message: message,
}
}

return PodAdmitResult{
Admit: true,
}
Expand Down
20 changes: 19 additions & 1 deletion plugin/pkg/scheduler/algorithm/predicates/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 {
Expand Down
34 changes: 26 additions & 8 deletions plugin/pkg/scheduler/algorithm/predicates/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -1083,36 +1083,54 @@ 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 if it's unfit.
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
if taint.Effect == api.TaintEffectPreferNoSchedule {
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
Expand Down