diff --git a/pkg/plugin/driver/daemonset/daemonset.go b/pkg/plugin/driver/daemonset/daemonset.go index 8d4b485a3..4c9a91428 100644 --- a/pkg/plugin/driver/daemonset/daemonset.go +++ b/pkg/plugin/driver/daemonset/daemonset.go @@ -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 @@ -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 } diff --git a/pkg/plugin/driver/daemonset/daemonset_test.go b/pkg/plugin/driver/daemonset/daemonset_test.go index 870c1d0b8..66847c747 100644 --- a/pkg/plugin/driver/daemonset/daemonset_test.go +++ b/pkg/plugin/driver/daemonset/daemonset_test.go @@ -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, }, }, }, @@ -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, + }, + }, }, } } @@ -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 {