driver/daemonset: Align node selection behavior with Kubernetes scheduler

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
 }

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 {