Skip to content

Commit

Permalink
driver/daemonset: Align node selection behavior with Kubernetes sched…
Browse files Browse the repository at this point in the history 
…uler (#1958)

* driver/daemonset: Perform AND with nodeSelector and nodeAffinity

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: #1957
Signed-off-by: nonylene <nonylene@gmail.com>

* driver/daemonset: Fix evaluation of multiple matchExpressions

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 #1957

Signed-off-by: nonylene <nonylene@gmail.com>

---------

Signed-off-by: nonylene <nonylene@gmail.com>
Co-authored-by: Peter Grant <117960526+franknstyle@users.noreply.github.com>
  • Loading branch information
@nonylene @franknstyle
nonylene and franknstyle committed Sep 17, 2024
1 parent 18bb79d commit 7700f31
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 64 deletions.
82 changes: 48 additions & 34 deletions pkg/plugin/driver/daemonset/daemonset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,30 @@ func (p *Plugin) filterByNodeSelector(nodes []v1.Node) []v1.Node {
}
ls = ls.Add(reqs...)

if ls.Empty() && nodeSelector == nil {
logrus.Trace("Filtering by nodes had no requirements, returning all nodes")
return nodes
}

retNodes := []v1.Node{}
for _, node := range nodes {
logrus.Tracef("Filtering by labelSelector, checking node.GetLabels(): %v against %v", node, node.GetLabels())

// Accept only if both of nodeSelctor and nodeAffinity match the node label
// Split checks up to clarify logging/debugging.
if !ls.Empty() && ls.Matches(labels.Set(node.GetLabels())) {
logrus.Tracef("Matched labelSelctors")
retNodes = append(retNodes, node)
continue
ignored := false

if ls.Empty() || ls.Matches(labels.Set(node.GetLabels())) {
logrus.Tracef("Passed labelSelectors")
} else {
logrus.Tracef("Did not match labelSelctors")
logrus.Tracef("Did not match labelSelectors")
ignored = true
}
if nodeMatchesNodeSelector(&node, nodeSelector) {
logrus.Tracef("Matched affinity")
retNodes = append(retNodes, node)
continue

if nodeSelector == nil || nodeMatchesNodeSelector(&node, nodeSelector) {
logrus.Tracef("Passed nodeAffinity")
} else {
logrus.Tracef("Did not match labelSelctors")
logrus.Tracef("Did not match labelSelectors")
ignored = true
}

if !ignored {
retNodes = append(retNodes, node)
}
}
return retNodes
Expand Down Expand Up @@ -432,28 +434,40 @@ func nodeMatchesNodeSelector(node *v1.Node, sel *v1.NodeSelector) bool {
}
for _, term := range sel.NodeSelectorTerms {
// We only support MatchExpressions at this time.
// All expressions in a NodeSelectorTerm must be satisfied
matched := true
for _, exp := range term.MatchExpressions {
switch exp.Operator {
case v1.NodeSelectorOpExists:
if _, ok := node.Labels[exp.Key]; ok {
return true
}
case v1.NodeSelectorOpDoesNotExist:
if _, ok := node.Labels[exp.Key]; !ok {
return true
}
case v1.NodeSelectorOpIn:
if val, ok := node.Labels[exp.Key]; ok && stringInList(exp.Values, val) {
return true
}
case v1.NodeSelectorOpNotIn:
if val, ok := node.Labels[exp.Key]; !ok || !stringInList(exp.Values, val) {
return true
}
default:
continue
if !labelsMatchesNodeSelectorRequirement(node.Labels, exp) {
matched = false
break
}
}

if matched {
return true
}
}
return false
}

func labelsMatchesNodeSelectorRequirement(labels map[string]string, req v1.NodeSelectorRequirement) bool {
switch req.Operator {
case v1.NodeSelectorOpExists:
if _, ok := labels[req.Key]; ok {
return true
}
case v1.NodeSelectorOpDoesNotExist:
if _, ok := labels[req.Key]; !ok {
return true
}
case v1.NodeSelectorOpIn:
if val, ok := labels[req.Key]; ok && stringInList(req.Values, val) {
return true
}
case v1.NodeSelectorOpNotIn:
if val, ok := labels[req.Key]; !ok || !stringInList(req.Values, val) {
return true
}
}
return false
}
Expand Down
97 changes: 67 additions & 30 deletions pkg/plugin/driver/daemonset/daemonset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,25 +549,26 @@ func TestExpectedResults(t *testing.T) {
{ObjectMeta: metav1.ObjectMeta{Name: "node4", Labels: map[string]string{"foo": "bar2"}}},
}

pluginWithAffinity := func(reqs []corev1.NodeSelectorRequirement) *Plugin {
pluginWithNodeFilters := func(nodeSelector map[string]string, nodeAffinityReqs []corev1.NodeSelectorRequirement) *Plugin {
p := &Plugin{
Base: driver.Base{
Definition: manifest.Manifest{
SonobuoyConfig: manifest.SonobuoyConfig{PluginName: "myPlugin"},
PodSpec: &manifest.PodSpec{
PodSpec: corev1.PodSpec{
NodeSelector: nodeSelector,
},
},
},
},
}
if len(reqs) > 0 {
p.Base.Definition.PodSpec = &manifest.PodSpec{
PodSpec: corev1.PodSpec{
Affinity: &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
{
MatchExpressions: reqs,
},
},
if len(nodeAffinityReqs) > 0 {
p.Base.Definition.PodSpec.Affinity = &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
{
MatchExpressions: nodeAffinityReqs,
},
},
},
Expand All @@ -577,18 +578,25 @@ func TestExpectedResults(t *testing.T) {
return p
}

pluginWithNodeSelector := func(key, val string) *Plugin {
pluginWithNodeAffinity := func(nodeAffinitySelector *corev1.NodeSelector) *Plugin {
p := &Plugin{
Base: driver.Base{
Definition: manifest.Manifest{
SonobuoyConfig: manifest.SonobuoyConfig{PluginName: "myPlugin"},
PodSpec: &manifest.PodSpec{
PodSpec: corev1.PodSpec{},
},
},
},
}
if len(key) > 0 {
if nodeAffinitySelector != nil {
p.Base.Definition.PodSpec = &manifest.PodSpec{
PodSpec: corev1.PodSpec{
NodeSelector: map[string]string{key: val},
Affinity: &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: nodeAffinitySelector,
},
},
},
}
}
Expand All @@ -608,60 +616,89 @@ func TestExpectedResults(t *testing.T) {
{NodeName: "node3", ResultType: "myPlugin"},
{NodeName: "node4", ResultType: "myPlugin"},
},
p: pluginWithAffinity(nil),
p: pluginWithNodeFilters(nil, nil),
}, {
desc: "Filters for label exists",
desc: "Affinity: Filters for label exists",
expect: []plugin.ExpectedResult{
{NodeName: "node2", ResultType: "myPlugin"},
{NodeName: "node3", ResultType: "myPlugin"},
{NodeName: "node4", ResultType: "myPlugin"},
},
p: pluginWithAffinity([]corev1.NodeSelectorRequirement{
p: pluginWithNodeFilters(nil, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpExists},
}),
}, {
desc: "Filters for label does not exist",
desc: "Affinity: Filters for label does not exist",
expect: []plugin.ExpectedResult{
{NodeName: "node1", ResultType: "myPlugin"},
},
p: pluginWithAffinity([]corev1.NodeSelectorRequirement{
p: pluginWithNodeFilters(nil, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpDoesNotExist},
}),
}, {
desc: "Filters for label value in",
desc: "Affinity: Filters for label value in",
expect: []plugin.ExpectedResult{
{NodeName: "node2", ResultType: "myPlugin"},
{NodeName: "node3", ResultType: "myPlugin"},
},
p: pluginWithAffinity([]corev1.NodeSelectorRequirement{
p: pluginWithNodeFilters(nil, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpIn, Values: []string{"bar", "baz"}},
}),
}, {
desc: "Filters for label value not in",
desc: "Affinity: Filters for label value not in",
expect: []plugin.ExpectedResult{
{NodeName: "node1", ResultType: "myPlugin"},
{NodeName: "node4", ResultType: "myPlugin"},
},
p: pluginWithAffinity([]corev1.NodeSelectorRequirement{
p: pluginWithNodeFilters(nil, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpNotIn, Values: []string{"bar", "baz"}},
}),
}, {
desc: "Can combine filters as union",
desc: "Affinity: Can combine selctor terms as union",
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"}},
p: pluginWithNodeAffinity(&corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
{MatchExpressions: []corev1.NodeSelectorRequirement{{Key: "foo", Operator: corev1.NodeSelectorOpNotIn, Values: []string{"bar", "baz"}}}},
{MatchExpressions: []corev1.NodeSelectorRequirement{{Key: "foo", Operator: corev1.NodeSelectorOpIn, Values: []string{"bar"}}}},
},
}),
}, {
desc: "Affinity: Expressions in a single MatchExpressions are ANDed",
expect: []plugin.ExpectedResult{
{NodeName: "node3", ResultType: "myPlugin"},
},
p: pluginWithNodeAffinity(&corev1.NodeSelector{
NodeSelectorTerms: []corev1.NodeSelectorTerm{
{MatchExpressions: []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpIn, Values: []string{"bar", "baz"}},
{Key: "foo", Operator: corev1.NodeSelectorOpIn, Values: []string{"baz", "bar2"}},
}},
},
}),
}, {
desc: "Can use nodeSelector field",
desc: "Selector: Can use nodeSelector field",
expect: []plugin.ExpectedResult{
{NodeName: "node2", ResultType: "myPlugin"},
},
p: pluginWithNodeFilters(map[string]string{"foo": "bar"}, nil),
}, {
desc: "Selector and Affinity: ANDed if both specified",
expect: []plugin.ExpectedResult{
{NodeName: "node2", ResultType: "myPlugin"},
},
p: pluginWithNodeSelector("foo", "bar"),
p: pluginWithNodeFilters(map[string]string{"foo": "bar"}, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpExists},
}),
}, {
desc: "Selector and Affinity: ANDed if both specified (negative affinity)",
expect: []plugin.ExpectedResult{},
p: pluginWithNodeFilters(map[string]string{"foo": "bar"}, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpDoesNotExist},
}),
},
}
for _, tc := range testCases {
Expand Down

0 comments on commit 7700f31

Please sign in to comment.