From 2bb611cda1d55eb5369c1b61aa3d37701fe6ebe1 Mon Sep 17 00:00:00 2001
From: Evans Mungai <evans@replicated.com>
Date: Thu, 26 Sep 2024 09:25:39 -0500
Subject: [PATCH] bug: Remove duplicate results in preflights (#1626)

Change to stop re-analysing preflight results when uploadResultsTo is present leading to duplicate results

Signed-off-by: Evans Mungai <evans@replicated.com>
---
 pkg/analyze/analyzer.go     |  4 ++--
 pkg/analyze/distribution.go |  4 +++-
 pkg/preflight/analyze.go    |  8 +++++++-
 pkg/preflight/run.go        | 13 ++++++-------
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go
index 87c359a8..000e4c95 100644
--- a/pkg/analyze/analyzer.go
+++ b/pkg/analyze/analyzer.go
@@ -122,7 +122,7 @@ func Analyze(
 		return nil, errors.New("nil analyzer")
 	}
 
-	analyzerInst := getAnalyzer(analyzer)
+	analyzerInst := GetAnalyzer(analyzer)
 	if analyzerInst == nil {
 		klog.Info("Non-existent analyzer found in the spec. Please double-check the spelling and indentation of the analyzers in the spec.")
 		return nil, nil
@@ -188,7 +188,7 @@ type Analyzer interface {
 	Analyze(getFile getCollectedFileContents, findFiles getChildCollectedFileContents) ([]*AnalyzeResult, error)
 }
 
-func getAnalyzer(analyzer *troubleshootv1beta2.Analyze) Analyzer {
+func GetAnalyzer(analyzer *troubleshootv1beta2.Analyze) Analyzer {
 	switch {
 	case analyzer.ClusterVersion != nil:
 		return &AnalyzeClusterVersion{analyzer: analyzer.ClusterVersion}
diff --git a/pkg/analyze/distribution.go b/pkg/analyze/distribution.go
index fc8e0301..2417aa6b 100644
--- a/pkg/analyze/distribution.go
+++ b/pkg/analyze/distribution.go
@@ -10,6 +10,7 @@ import (
 	"github.com/replicatedhq/troubleshoot/pkg/constants"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/klog/v2"
 )
 
 type providers struct {
@@ -75,7 +76,8 @@ func (a *AnalyzeDistribution) IsExcluded() (bool, error) {
 func (a *AnalyzeDistribution) Analyze(getFile getCollectedFileContents, findFiles getChildCollectedFileContents) ([]*AnalyzeResult, error) {
 	result, err := a.analyzeDistribution(a.analyzer, getFile)
 	if err != nil {
-		return nil, err
+		klog.Errorf("failed to analyze distribution: %v", err)
+		return nil, errors.Wrapf(err, "failed to analyze distribution")
 	}
 	result.Strict = a.analyzer.Strict.BoolOrDefaultFalse()
 	return []*AnalyzeResult{result}, nil
diff --git a/pkg/preflight/analyze.go b/pkg/preflight/analyze.go
index cb090352..3deea546 100644
--- a/pkg/preflight/analyze.go
+++ b/pkg/preflight/analyze.go
@@ -103,11 +103,17 @@ func doAnalyze(
 				klog.Errorf("failed to determine if analyzer %v is strict: %s", analyzer, strictErr)
 			}
 
+			title := "Analyzer Failed"
+			analyzerInst := analyze.GetAnalyzer(analyzer)
+			if analyzerInst != nil {
+				title = analyzerInst.Title()
+			}
+
 			analyzeResult = []*analyze.AnalyzeResult{
 				{
 					Strict:  strict,
 					IsFail:  true,
-					Title:   "Analyzer Failed",
+					Title:   title,
 					Message: err.Error(),
 				},
 			}
diff --git a/pkg/preflight/run.go b/pkg/preflight/run.go
index 1da6f9ed..cceda923 100644
--- a/pkg/preflight/run.go
+++ b/pkg/preflight/run.go
@@ -31,6 +31,8 @@ import (
 	"k8s.io/klog/v2"
 )
 
+type empty struct{}
+
 func RunPreflights(interactive bool, output string, format string, args []string) error {
 	ctx, root := otel.Tracer(
 		constants.LIB_TRACER_NAME).Start(context.Background(), constants.TROUBLESHOOT_ROOT_SPAN_NAME)
@@ -113,7 +115,7 @@ func RunPreflights(interactive bool, output string, format string, args []string
 		progressCollection.Go(collectNonInteractiveProgess(ctx, progressCh))
 	}
 
-	uploadResultsMap := make(map[string][]CollectResult)
+	uploadResultsMap := make(map[string]empty)
 	collectorResults := collect.NewResult()
 	analyzers := []*troubleshootv1beta2.Analyze{}
 	hostAnalyzers := []*troubleshootv1beta2.HostAnalyze{}
@@ -130,7 +132,7 @@ func RunPreflights(interactive bool, output string, format string, args []string
 		collectorResults.AddResult(collect.CollectorResult(collectorResult.AllCollectedData))
 
 		if spec.Spec.UploadResultsTo != "" {
-			uploadResultsMap[spec.Spec.UploadResultsTo] = append(uploadResultsMap[spec.Spec.UploadResultsTo], *r)
+			uploadResultsMap[spec.Spec.UploadResultsTo] = empty{}
 			uploadCollectResults = append(collectResults, *r)
 		} else {
 			collectResults = append(collectResults, *r)
@@ -188,11 +190,8 @@ func RunPreflights(interactive bool, output string, format string, args []string
 	}
 
 	uploadAnalyzeResultsMap := make(map[string][]*analyzer.AnalyzeResult)
-	for location, results := range uploadResultsMap {
-		for _, res := range results {
-			uploadAnalyzeResultsMap[location] = append(uploadAnalyzeResultsMap[location], res.Analyze()...)
-			analyzeResults = append(analyzeResults, uploadAnalyzeResultsMap[location]...)
-		}
+	for location := range uploadResultsMap {
+		uploadAnalyzeResultsMap[location] = append(uploadAnalyzeResultsMap[location], analyzeResults...)
 	}
 
 	for k, v := range uploadAnalyzeResultsMap {