Merge branch 'main' into release-process-fix

pkg/client/retrieve.go

@@ -60,6 +60,7 @@ func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, <-chan
 	ec := make(chan error, 1)
 	client, err := c.Client()
 	if err != nil {
+		ec <- err
 		return nil, ec, nil
 	}
 

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 {