Skip to content

Commit

Permalink
driver/daemonset: Perform AND with nodeSelector and nodeAffinity
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@nonylene
nonylene committed Feb 5, 2024
1 parent 6f9e27f commit 2ce774f
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 58 deletions.
32 changes: 17 additions & 15 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
83 changes: 40 additions & 43 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,24 +578,6 @@ func TestExpectedResults(t *testing.T) {
return p
}

pluginWithNodeSelector := func(key, val string) *Plugin {
p := &Plugin{
Base: driver.Base{
Definition: manifest.Manifest{
SonobuoyConfig: manifest.SonobuoyConfig{PluginName: "myPlugin"},
},
},
}
if len(key) > 0 {
p.Base.Definition.PodSpec = &manifest.PodSpec{
PodSpec: corev1.PodSpec{
NodeSelector: map[string]string{key: val},
},
}
}
return p
}

testCases := []struct {
desc string
p *Plugin
Expand All @@ -608,60 +591,74 @@ 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 filters as union",
expect: []plugin.ExpectedResult{
{NodeName: "node1", ResultType: "myPlugin"},
{NodeName: "node2", ResultType: "myPlugin"},
{NodeName: "node4", ResultType: "myPlugin"},
},
p: pluginWithAffinity([]corev1.NodeSelectorRequirement{
p: pluginWithNodeFilters(nil, []corev1.NodeSelectorRequirement{
{Key: "foo", Operator: corev1.NodeSelectorOpNotIn, Values: []string{"bar", "baz"}},
{Key: "foo", Operator: corev1.NodeSelectorOpIn, Values: []string{"bar"}},
}),
}, {
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 2ce774f

Please sign in to comment.