Skip to content

Commit

Permalink
driver/daemonset: Fix evaluation of multiple matchExpressions
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
nonylene committed Feb 5, 2024
1 parent 2ce774f commit 41a48b9
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 23 deletions.
50 changes: 31 additions & 19 deletions pkg/plugin/driver/daemonset/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,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
48 changes: 44 additions & 4 deletions pkg/plugin/driver/daemonset/daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,31 @@ func TestExpectedResults(t *testing.T) {
return p
}

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 nodeAffinitySelector != nil {
p.Base.Definition.PodSpec = &manifest.PodSpec{
PodSpec: corev1.PodSpec{
Affinity: &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: nodeAffinitySelector,
},
},
},
}
}
return p
}

testCases := []struct {
desc string
p *Plugin
Expand Down Expand Up @@ -629,15 +654,30 @@ func TestExpectedResults(t *testing.T) {
{Key: "foo", Operator: corev1.NodeSelectorOpNotIn, Values: []string{"bar", "baz"}},
}),
}, {
desc: "Affinity: 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: pluginWithNodeFilters(nil, []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: "Selector: Can use nodeSelector field",
Expand Down

0 comments on commit 41a48b9

Please sign in to comment.