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.

Sign up for GitHub

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

Jump to bottom

e2e: Treat tainted nodes as unschedulable #35210

Closed
wants to merge 1 commit into from
Closed

e2e: Treat tainted nodes as unschedulable #35210

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion test/e2e/framework/util.go
View file
Open in desktop
Expand Up @@ -2350,15 +2350,27 @@ func waitListSchedulableNodesOrDie(c clientset.Interface) *api.NodeList {
return nodes
}

// isNodeTainted checks if a node is tainted and thus not schedulable to pods (without a toleration)
Copy link
Member

Choose a reason for hiding this comment

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

what does "without a toleration" mean? how would a test specify that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if you're asking for more than "what is an example of a toleration"... I suspect you are, but here is an example of a toleration anyway:

https://github.com/kubernetes/kops/blob/master/upup/models/cloudup/resources/addons/dns-controller/v1.4.0.yaml#L23

func isNodeTainted(node *api.Node) bool {
nodeTaints, err := api.GetTaintsFromNodeAnnotations(node.Annotations)
if err != nil {
Failf("error getting taints: %v", err)
}

return len(nodeTaints) > 0
Copy link
Member

Choose a reason for hiding this comment

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

you should only return true if the Effect is NoSchedule

}

// Node is schedulable if:
// 1) doesn't have "unschedulable" field set
// 2) it's Ready condition is set to true
// 3) doesn't have NetworkUnavailable condition set to true
// 4) it isn't tainted
func isNodeSchedulable(node *api.Node) bool {
nodeReady := IsNodeConditionSetAsExpected(node, api.NodeReady, true)
isTainted := isNodeTainted(node)
networkReady := IsNodeConditionUnset(node, api.NodeNetworkUnavailable) ||
IsNodeConditionSetAsExpectedSilent(node, api.NodeNetworkUnavailable, false)
return !node.Spec.Unschedulable && nodeReady && networkReady
return !node.Spec.Unschedulable && nodeReady && networkReady && !isTainted
Copy link
Member

Choose a reason for hiding this comment

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

this might happen to work now, but taints/tolerations really makes it impossible to say that a node is schedulable or unschedulable in isolation -- you have to look at the pod in question also, and say the node is unschedulable for that pod iff the node has a taint that the pod does not tolerate. For example see PodToleratesNodeTaints() in plugin/pkg/scheduler/algorithm/predicates/predicates.go. (Of course the other tests apply in addition, like nodeReady, networkReady, etc. that is earlier in this line)

}

// GetReadySchedulableNodesOrDie addresses the common use case of getting nodes you can do work on.
Expand Down