Fix bug which caused daemonset status to not be updated with results

The bug would cause only one of the plugin/node values in the `sonobuoy status --json` output to be updated and it would include all the results for all nodes. Instead, we want each plugin/node value in that list to report its own values for passed/failed. Fixes #1001 Signed-off-by: John Schnake <jschnake@vmware.com>
vmware-tanzu · Dec 13, 2019 · 121ba8d · 121ba8d
1 parent ee12b94
commit 121ba8d
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 31 deletions.
diff --git a/cmd/sonobuoy/app/results.go b/cmd/sonobuoy/app/results.go
@@ -167,7 +167,7 @@ func printSinglePlugin(input resultsInput, r *results.Reader) error {
 		return err
 	}
 
-	obj = getItemInTree(obj, input.node)
+	obj = obj.GetSubTreeByName(input.node)
 	if obj == nil {
 		return fmt.Errorf("node named %q not found", input.node)
 	}
@@ -189,27 +189,6 @@ func getPluginList(r *results.Reader) ([]string, error) {
 	return runInfo.LoadedPlugins, errors.Wrap(err, "finding plugin list")
 }
 
-func getItemInTree(i *results.Item, root string) *results.Item {
-	if i == nil {
-		return nil
-	}
-
-	if root == "" || i.Name == root {
-		return i
-	}
-
-	if len(i.Items) > 0 {
-		for _, v := range i.Items {
-			subItem := getItemInTree(&v, root)
-			if subItem != nil {
-				return subItem
-			}
-		}
-	}
-
-	return nil
-}
-
 func printResultsDetails(treePath []string, o *results.Item, input resultsInput) error {
 	if o == nil {
 		return nil

diff --git a/pkg/client/results/processing.go b/pkg/client/results/processing.go
@@ -95,6 +95,29 @@ func (i Item) Empty() bool {
 	return false
 }
 
+// GetSubTreeByName traverses the tree and returns a reference to the
+// subtree whose root has the given name.
+func (i *Item) GetSubTreeByName(root string) *Item {
+	if i == nil {
+		return nil
+	}
+
+	if root == "" || i.Name == root {
+		return i
+	}
+
+	if len(i.Items) > 0 {
+		for _, v := range i.Items {
+			subItem := (&v).GetSubTreeByName(root)
+			if subItem != nil {
+				return subItem
+			}
+		}
+	}
+
+	return nil
+}
+
 // aggregateStatus defines the aggregation rules for status. Failures bubble
 // up and otherwise the status is assumed to pass as long as there are >=1 result.
 // If 0 items are aggregated, StatusUnknown is returned.

diff --git a/pkg/discovery/discovery.go b/pkg/discovery/discovery.go
@@ -222,9 +222,7 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 		}
 
 		// Update the plugin status with this post-processed information.
-		statusInfo := map[string]int{}
-		statusCounts(&item, statusInfo)
-		updatePluginStatus(kubeClient, cfg.Namespace, p.GetName(), item.Status, statusInfo)
+		updatePluginStatus(kubeClient, cfg.Namespace, p.GetName(), item)
 	}
 
 	// Saving plugin definitions in their respective folders for easy reference.
@@ -357,20 +355,38 @@ func updateStatus(client kubernetes.Interface, namespace string, status string,
 	return setPodStatusAnnotation(client, namespace, podStatus)
 }
 
-func updatePluginStatus(client kubernetes.Interface, namespace string, pluginType string, pluginResultStatus string, pluginResultCounts map[string]int) error {
+func updatePluginStatus(client kubernetes.Interface, namespace string, pluginName string, item results.Item) error {
 	podStatus, err := pluginaggregation.GetStatus(client, namespace)
 	if err != nil {
 		return errors.Wrap(err, "failed to get the existing status")
 	}
 
+	integrateResultsIntoStatus(podStatus, pluginName, &item)
+
+	return setPodStatusAnnotation(client, namespace, podStatus)
+}
+
+func integrateResultsIntoStatus(podStatus *pluginaggregation.Status, pluginName string, item *results.Item) {
 	for i := range podStatus.Plugins {
-		if podStatus.Plugins[i].Plugin == pluginType {
-			podStatus.Plugins[i].ResultStatus = pluginResultStatus
-			podStatus.Plugins[i].ResultStatusCounts = pluginResultCounts
-			break
+		if podStatus.Plugins[i].Plugin != pluginName {
+			continue
+		}
+		var itemForNode *results.Item
+		if podStatus.Plugins[i].Node == plugin.GlobalResult {
+			itemForNode = item
+		} else {
+			itemForNode = item.GetSubTreeByName(podStatus.Plugins[i].Node)
+		}
+
+		if itemForNode == nil {
+			return
 		}
+
+		statusInfo := map[string]int{}
+		statusCounts(itemForNode, statusInfo)
+		podStatus.Plugins[i].ResultStatus = itemForNode.Status
+		podStatus.Plugins[i].ResultStatusCounts = statusInfo
 	}
-	return setPodStatusAnnotation(client, namespace, podStatus)
 }
 
 // setPodStatusAnnotation sets the status on the pod via an annotation. It will overwrite the

diff --git a/pkg/discovery/discovery_test.go b/pkg/discovery/discovery_test.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/vmware-tanzu/sonobuoy/pkg/client/results"
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
+	pluginaggregation "github.com/vmware-tanzu/sonobuoy/pkg/plugin/aggregation"
 
 	"github.com/kylelemons/godebug/pretty"
 )
@@ -123,3 +124,116 @@ func TestStatusCounts(t *testing.T) {
 		})
 	}
 }
+
+func TestIntegrateResultsIntoStatus(t *testing.T) {
+	testCases := []struct {
+		desc         string
+		status       *pluginaggregation.Status
+		expectStatus *pluginaggregation.Status
+		pluginName   string
+		item         *results.Item
+	}{
+		{
+			desc: "Updates correct plugin by name for global plugins",
+			status: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{
+						Plugin: "foo", Node: "global",
+					},
+				},
+			},
+			expectStatus: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{
+						Plugin: "foo", Node: "global",
+						ResultStatus:       "passed",
+						ResultStatusCounts: map[string]int{"passed": 1},
+					},
+				},
+			},
+			pluginName: "foo",
+			item:       &results.Item{Status: "passed"},
+		}, {
+			desc: "Wont update any if no match by name",
+			status: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{
+						Plugin: "foo", Node: "global",
+					},
+				},
+			},
+			expectStatus: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{
+						Plugin: "foo", Node: "global",
+					},
+				},
+			},
+			pluginName: "notfoo",
+			item:       &results.Item{Status: "passed"},
+		}, {
+			desc: "Updates each daemonsets node in item",
+			status: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{Plugin: "foo", Node: "node1"},
+					{Plugin: "foo", Node: "node2"},
+				},
+			},
+			expectStatus: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{
+						Plugin: "foo", Node: "node1",
+						ResultStatus:       "passed",
+						ResultStatusCounts: map[string]int{"passed": 1},
+					},
+					{
+						Plugin: "foo", Node: "node2",
+						ResultStatus:       "failed",
+						ResultStatusCounts: map[string]int{"failed": 1},
+					},
+				},
+			},
+			pluginName: "foo",
+			item: &results.Item{
+				Status: "failed",
+				Items: []results.Item{
+					{Name: "node1", Status: "passed"},
+					{Name: "node2", Status: "failed"},
+				},
+			},
+		}, {
+			desc: "If daemonset missing node then no change to that value",
+			status: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{Plugin: "foo", Node: "node1"},
+					{Plugin: "foo", Node: "node2"},
+				},
+			},
+			expectStatus: &pluginaggregation.Status{
+				Plugins: []pluginaggregation.PluginStatus{
+					{
+						Plugin: "foo", Node: "node1",
+						ResultStatus:       "passed",
+						ResultStatusCounts: map[string]int{"passed": 1},
+					},
+					{Plugin: "foo", Node: "node2"},
+				},
+			},
+			pluginName: "foo",
+			item: &results.Item{
+				Status: "failed",
+				Items: []results.Item{
+					{Name: "node1", Status: "passed"},
+				},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			integrateResultsIntoStatus(tc.status, tc.pluginName, tc.item)
+			if diff := pretty.Compare(tc.status, tc.expectStatus); diff != "" {
+				t.Fatalf("\n\n%s\n", diff)
+			}
+		})
+	}
+}