-
Notifications
You must be signed in to change notification settings - Fork 339
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
driver/daemonset: Align node selection behavior with Kubernetes scheduler #1958
base: main
Are you sure you want to change the base?
Conversation
Align node filter behavior with Kuberntes scheduler to avoid errors when both of nodeSelctor and nodeAffinity are set to PodSpec. > If you specify both nodeSelector and nodeAffinity, both must be satisfied for the Pod to be scheduled onto a node. > > https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/ Issue: vmware-tanzu#1957 Signed-off-by: nonylene <nonylene@gmail.com>
Align nodeAffinity matching behavior with Kubernetes schduler. > If you specify multiple expressions in a single matchExpressions field associated with a term in nodeSelectorTerms, then the Pod can be scheduled onto a node only if all the expressions are satisfied (expressions are ANDed). > > https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/ Close vmware-tanzu#1957 Signed-off-by: nonylene <nonylene@gmail.com>
@@ -119,28 +119,30 @@ func (p *Plugin) filterByNodeSelector(nodes []v1.Node) []v1.Node { | |||
} | |||
ls = ls.Add(reqs...) | |||
|
|||
if ls.Empty() && nodeSelector == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition is now covered within the for loop below.
expect: []plugin.ExpectedResult{ | ||
{NodeName: "node1", ResultType: "myPlugin"}, | ||
{NodeName: "node2", ResultType: "myPlugin"}, | ||
{NodeName: "node4", ResultType: "myPlugin"}, | ||
}, | ||
p: pluginWithAffinity([]corev1.NodeSelectorRequirement{ | ||
{Key: "foo", Operator: corev1.NodeSelectorOpNotIn, Values: []string{"bar", "baz"}}, | ||
{Key: "foo", Operator: corev1.NodeSelectorOpIn, Values: []string{"bar"}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case was inconsistent with the behavior of Kubernetes scheduler (fixed)
{Key: "foo", Operator: corev1.NodeSelectorOpExists}, | ||
}), | ||
}, { | ||
desc: "Selector and Affinity: ANDed if both specified (negative affinity)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this case since a case when positive in the selector and negative in the affinity is not tested in the last test
What this PR does / why we need it:
Align node selection behavior with Kubernetes scheduler in DaemonSet driver.
Which issue(s) this PR fixes
Special notes for your reviewer:
Release note: