feat: Optionally save preflight bundles to disk (#1612)

* feat: Optionally save preflight bundles to disk

Signed-off-by: Evans Mungai <evans@replicated.com>

* Add e2e test of saving preflight bundle

Signed-off-by: Evans Mungai <evans@replicated.com>

* Update cli docs

Signed-off-by: Evans Mungai <evans@replicated.com>

* Expose GetVersionFile function publicly

Signed-off-by: Evans Mungai <evans@replicated.com>

* Store analysis.json file in preflight bundle

Signed-off-by: Evans Mungai <evans@replicated.com>

* Run go fmt when running lint fixers

Signed-off-by: Evans Mungai <evans@replicated.com>

* Always generate a preflight bundle in CLI

Signed-off-by: Evans Mungai <evans@replicated.com>

* Print saving bundle message to stderr

Signed-off-by: Evans Mungai <evans@replicated.com>

* Revert changes in docs directory

Signed-off-by: Evans Mungai <evans@replicated.com>

* Use NewResult constructor

Signed-off-by: Evans Mungai <evans@replicated.com>

* Log always when preflight bundle is saved to disk

Signed-off-by: Evans Mungai <evans@replicated.com>

---------

Signed-off-by: Evans Mungai <evans@replicated.com>

Makefile

@@ -239,7 +239,7 @@ scan:
 lint:
 	golangci-lint run --new -c .golangci.yaml ${BUILDPATHS}
 
-.PHONY: lint-and-fix
+.PHONY: fmt lint-and-fix
 lint-and-fix:
 	golangci-lint run --new --fix -c .golangci.yaml ${BUILDPATHS}
 

cmd/preflight/cli/root.go

@@ -50,7 +50,7 @@ that a cluster meets the requirements to run an application.`,
 
 			err = preflight.RunPreflights(v.GetBool("interactive"), v.GetString("output"), v.GetString("format"), args)
 			if !v.GetBool("dry-run") && (v.GetBool("debug") || v.IsSet("v")) {
-				fmt.Printf("\n%s", traces.GetExporterInstance().GetSummary())
+				fmt.Fprintf(os.Stderr, "\n%s", traces.GetExporterInstance().GetSummary())
 			}
 
 			return err

cmd/troubleshoot/cli/redact.go

@@ -64,7 +64,7 @@ For more information on redactors visit https://troubleshoot.sh/docs/redact/
 			if output == "" {
 				output = fmt.Sprintf("redacted-support-bundle-%s.tar.gz", time.Now().Format("2006-01-02T15_04_05"))
 			}
-			err = collectorResult.ArchiveSupportBundle(bundleDir, output)
+			err = collectorResult.ArchiveBundle(bundleDir, output)
 			if err != nil {
 				return errors.Wrap(err, "failed to create support bundle archive")
 			}

cmd/troubleshoot/cli/root.go

@@ -54,7 +54,7 @@ If no arguments are provided, specs are automatically loaded from the cluster by
 
 			err = runTroubleshoot(v, args)
 			if !v.IsSet("dry-run") && (v.GetBool("debug") || v.IsSet("v")) {
-				fmt.Printf("\n%s", traces.GetExporterInstance().GetSummary())
+				fmt.Fprintf(os.Stderr, "\n%s", traces.GetExporterInstance().GetSummary())
 			}
 
 			return err

pkg/collect/result.go

@@ -270,7 +270,14 @@ func (r CollectorResult) CloseWriter(bundlePath string, relativePath string, wri
 	return errors.Errorf("cannot close writer of type %T", writer)
 }
 
+// ArchiveSupportBundle creates an archive of the files in the bundle directory
+// Deprecated: Use better named ArchiveBundle since this method is used to archive any directory
 func (r CollectorResult) ArchiveSupportBundle(bundlePath string, outputFilename string) error {
+	return r.ArchiveBundle(bundlePath, outputFilename)
+}
+
+// ArchiveBundle creates an archive of the files in the bundle directory
+func (r CollectorResult) ArchiveBundle(bundlePath string, outputFilename string) error {
 	fileWriter, err := os.Create(outputFilename)
 	if err != nil {
 		return errors.Wrap(err, "failed to create output file")
@@ -404,5 +411,5 @@ func CollectorResultFromBundle(bundleDir string) (CollectorResult, error) {
 // Deprecated: Remove in a future version (v1.0)
 func TarSupportBundleDir(bundlePath string, input CollectorResult, outputFilename string) error {
 	// Is this used anywhere external anyway?
-	return input.ArchiveSupportBundle(bundlePath, outputFilename)
+	return input.ArchiveBundle(bundlePath, outputFilename)
 }

pkg/constants/constants.go

@@ -20,6 +20,7 @@ const (
 	LIB_TRACER_NAME             = "github.com/replicatedhq/troubleshoot"
 	TROUBLESHOOT_ROOT_SPAN_NAME = "ReplicatedTroubleshootRootSpan"
 	EXCLUDED                    = "excluded"
+	ANALYSIS_FILENAME           = "analysis.json"
 
 	// Cluster Resources Collector Directories
 	CLUSTER_RESOURCES_DIR                         = "cluster-resources"

pkg/preflight/collect.go

@@ -30,6 +30,9 @@ type CollectOpts struct {
 	LabelSelector          string
 	Timeout                time.Duration
 	ProgressChan           chan interface{}
+
+	// Optional path to the bundle directory to store the collected data
+	BundlePath string
 }
 
 type CollectProgress struct {
@@ -96,7 +99,7 @@ func CollectHost(opts CollectOpts, p *troubleshootv1beta2.HostPreflight) (Collec
 func CollectHostWithContext(
 	ctx context.Context, opts CollectOpts, p *troubleshootv1beta2.HostPreflight,
 ) (CollectResult, error) {
-	collectSpecs := make([]*troubleshootv1beta2.HostCollect, 0, 0)
+	collectSpecs := make([]*troubleshootv1beta2.HostCollect, 0)
 	if p != nil && p.Spec.Collectors != nil {
 		collectSpecs = append(collectSpecs, p.Spec.Collectors...)
 	}
@@ -105,7 +108,7 @@ func CollectHostWithContext(
 
 	var collectors []collect.HostCollector
 	for _, desiredCollector := range collectSpecs {
-		collector, ok := collect.GetHostCollector(desiredCollector, "")
+		collector, ok := collect.GetHostCollector(desiredCollector, opts.BundlePath)
 		if ok {
 			collectors = append(collectors, collector)
 		}
@@ -140,6 +143,7 @@ func CollectHostWithContext(
 		span.End()
 	}
 
+	// The values of map entries will contain the collected data in bytes if the data was not stored to disk
 	collectResult.AllCollectedData = allCollectedData
 
 	return collectResult, nil
@@ -154,7 +158,7 @@ func CollectWithContext(ctx context.Context, opts CollectOpts, p *troubleshootv1
 	var allCollectors []collect.Collector
 	var foundForbidden bool
 
-	collectSpecs := make([]*troubleshootv1beta2.Collect, 0, 0)
+	collectSpecs := make([]*troubleshootv1beta2.Collect, 0)
 	if p != nil && p.Spec.Collectors != nil {
 		collectSpecs = append(collectSpecs, p.Spec.Collectors...)
 	}
@@ -180,7 +184,7 @@ func CollectWithContext(ctx context.Context, opts CollectOpts, p *troubleshootv1
 	allCollectedData := make(map[string][]byte)
 
 	for _, desiredCollector := range collectSpecs {
-		if collectorInterface, ok := collect.GetCollector(desiredCollector, "", opts.Namespace, opts.KubernetesRestConfig, k8sClient, nil); ok {
+		if collectorInterface, ok := collect.GetCollector(desiredCollector, opts.BundlePath, opts.Namespace, opts.KubernetesRestConfig, k8sClient, nil); ok {
 			if collector, ok := collectorInterface.(collect.Collector); ok {
 				err := collector.CheckRBAC(ctx, collector, desiredCollector, opts.KubernetesRestConfig, opts.Namespace)
 				if err != nil {
@@ -305,6 +309,7 @@ func CollectWithContext(ctx context.Context, opts CollectOpts, p *troubleshootv1
 		span.End()
 	}
 
+	// The values of map entries will contain the collected data in bytes if the data was not stored to disk
 	collectResult.AllCollectedData = allCollectedData
 
 	return collectResult, nil

pkg/preflight/run.go

@@ -1,10 +1,13 @@
 package preflight
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"time"
 
 	cursor "github.com/ahmetalpbalkan/go-cursor"
@@ -13,15 +16,19 @@ import (
 	"github.com/replicatedhq/troubleshoot/internal/util"
 	analyzer "github.com/replicatedhq/troubleshoot/pkg/analyze"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/replicatedhq/troubleshoot/pkg/collect"
 	"github.com/replicatedhq/troubleshoot/pkg/constants"
+	"github.com/replicatedhq/troubleshoot/pkg/convert"
 	"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
 	"github.com/replicatedhq/troubleshoot/pkg/types"
+	"github.com/replicatedhq/troubleshoot/pkg/version"
 	"github.com/spf13/viper"
 	spin "github.com/tj/go-spin"
 	"go.opentelemetry.io/otel"
 	"golang.org/x/sync/errgroup"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/klog/v2"
 )
 
 func RunPreflights(interactive bool, output string, format string, args []string) error {
@@ -77,6 +84,21 @@ func RunPreflights(interactive bool, output string, format string, args []string
 	var uploadCollectResults []CollectResult
 	preflightSpecName := ""
 
+	// Create a temporary directory to save the preflight bundle
+	tmpDir, err := os.MkdirTemp("", "preflightbundle-")
+	if err != nil {
+		return errors.Wrap(err, "create temp dir for preflightbundle")
+	}
+	defer os.RemoveAll(tmpDir)
+	bundleFileName := fmt.Sprintf("preflightbundle-%s", time.Now().Format("2006-01-02T15_04_05"))
+	bundlePath := filepath.Join(tmpDir, bundleFileName)
+	if err := os.MkdirAll(bundlePath, 0777); err != nil {
+		return errors.Wrap(err, "failed to create preflight bundle dir")
+	}
+
+	archivePath := fmt.Sprintf("%s.tar.gz", bundleFileName)
+	klog.V(2).Infof("Preflight data collected in temporary directory: %s", tmpDir)
+
 	progressCh := make(chan interface{})
 	defer close(progressCh)
 
@@ -92,12 +114,21 @@ func RunPreflights(interactive bool, output string, format string, args []string
 	}
 
 	uploadResultsMap := make(map[string][]CollectResult)
+	collectorResults := collect.NewResult()
+	analyzers := []*troubleshootv1beta2.Analyze{}
+	hostAnalyzers := []*troubleshootv1beta2.HostAnalyze{}
 
 	for _, spec := range specs.PreflightsV1Beta2 {
-		r, err := collectInCluster(ctx, &spec, progressCh)
+		r, err := collectInCluster(ctx, &spec, progressCh, bundlePath)
 		if err != nil {
 			return types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, errors.Wrap(err, "failed to collect in cluster"))
 		}
+		collectorResult, ok := (*r).(ClusterCollectResult)
+		if !ok {
+			return errors.Errorf("unexpected result type: %T", collectResults)
+		}
+		collectorResults.AddResult(collect.CollectorResult(collectorResult.AllCollectedData))
+
 		if spec.Spec.UploadResultsTo != "" {
 			uploadResultsMap[spec.Spec.UploadResultsTo] = append(uploadResultsMap[spec.Spec.UploadResultsTo], *r)
 			uploadCollectResults = append(collectResults, *r)
@@ -106,33 +137,54 @@ func RunPreflights(interactive bool, output string, format string, args []string
 		}
 		// TODO: This spec name will be overwritten by the next spec. Is this intentional?
 		preflightSpecName = spec.Name
+		analyzers = append(analyzers, spec.Spec.Analyzers...)
 	}
 
 	for _, spec := range specs.HostPreflightsV1Beta2 {
 		if len(spec.Spec.Collectors) > 0 {
-			r, err := collectHost(ctx, &spec, progressCh)
+			r, err := collectHost(ctx, &spec, progressCh, bundlePath)
 			if err != nil {
 				return types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, errors.Wrap(err, "failed to collect from host"))
 			}
 			collectResults = append(collectResults, *r)
+			collectorResult, ok := (*r).(HostCollectResult)
+			if !ok {
+				return errors.Errorf("unexpected result type: %T", collectResults)
+			}
+			collectorResults.AddResult(collect.CollectorResult(collectorResult.AllCollectedData))
 		}
 		if len(spec.Spec.RemoteCollectors) > 0 {
 			r, err := collectRemote(ctx, &spec, progressCh)
 			if err != nil {
 				return types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, errors.Wrap(err, "failed to collect remotely"))
 			}
 			collectResults = append(collectResults, *r)
+			collectorResult, ok := (*r).(RemoteCollectResult)
+			if !ok {
+				return errors.Errorf("unexpected result type: %T", collectResults)
+			}
+			collectorResults.AddResult(collect.CollectorResult(collectorResult.AllCollectedData))
 		}
 		preflightSpecName = spec.Name
+		hostAnalyzers = append(hostAnalyzers, spec.Spec.Analyzers...)
 	}
 
 	if len(collectResults) == 0 && len(uploadCollectResults) == 0 {
 		return types.NewExitCodeError(constants.EXIT_CODE_CATCH_ALL, errors.New("no data was collected"))
 	}
 
-	analyzeResults := []*analyzer.AnalyzeResult{}
-	for _, res := range collectResults {
-		analyzeResults = append(analyzeResults, res.Analyze()...)
+	err = saveTSVersionToBundle(collectorResults, bundlePath)
+	if err != nil {
+		return errors.Wrap(err, "failed to save version file")
+	}
+
+	analyzeResults, err := analyzer.AnalyzeLocal(ctx, bundlePath, analyzers, hostAnalyzers)
+	if err != nil {
+		return errors.Wrap(err, "failed to analyze support bundle")
+	}
+	err = saveAnalysisResultsToBundle(collectorResults, analyzeResults, bundlePath)
+	if err != nil {
+		return errors.Wrap(err, "failed to save analysis results to bundle")
 	}
 
 	uploadAnalyzeResultsMap := make(map[string][]*analyzer.AnalyzeResult)
@@ -150,6 +202,12 @@ func RunPreflights(interactive bool, output string, format string, args []string
 		}
 	}
 
+	// Archive preflight bundle
+	if err := collectorResults.ArchiveBundle(bundlePath, archivePath); err != nil {
+		return errors.Wrapf(err, "failed to create %s archive", archivePath)
+	}
+	defer fmt.Fprintf(os.Stderr, "\nSaving preflight bundle to %s\n", archivePath)
+
 	stopProgressCollection()
 	progressCollection.Wait()
 
@@ -176,6 +234,37 @@ func RunPreflights(interactive bool, output string, format string, args []string
 	return types.NewExitCodeError(exitCode, errors.New("preflights failed with warnings or errors"))
 }
 
+func saveAnalysisResultsToBundle(
+	results collect.CollectorResult, analyzeResults []*analyzer.AnalyzeResult, bundlePath string,
+) error {
+	data := convert.FromAnalyzerResult(analyzeResults)
+	analysis, err := json.MarshalIndent(data, "", "    ")
+	if err != nil {
+		return err
+	}
+
+	err = results.SaveResult(bundlePath, "analysis.json", bytes.NewBuffer(analysis))
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func saveTSVersionToBundle(results collect.CollectorResult, bundlePath string) error {
+	version, err := version.GetVersionFile()
+	if err != nil {
+		return err
+	}
+
+	err = results.SaveResult(bundlePath, constants.VERSION_FILENAME, bytes.NewBuffer([]byte(version)))
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // Determine if any preflight checks passed vs failed vs warned
 // If all checks passed: 0
 // If 1 or more checks failed: 3
@@ -250,7 +339,9 @@ func collectNonInteractiveProgess(ctx context.Context, progressCh <-chan interfa
 	}
 }
 
-func collectInCluster(ctx context.Context, preflightSpec *troubleshootv1beta2.Preflight, progressCh chan interface{}) (*CollectResult, error) {
+func collectInCluster(
+	ctx context.Context, preflightSpec *troubleshootv1beta2.Preflight, progressCh chan interface{}, bundlePath string,
+) (*CollectResult, error) {
 	v := viper.GetViper()
 
 	restConfig, err := k8sutil.GetRESTConfig()
@@ -263,6 +354,7 @@ func collectInCluster(ctx context.Context, preflightSpec *troubleshootv1beta2.Pr
 		IgnorePermissionErrors: v.GetBool("collect-without-permissions"),
 		ProgressChan:           progressCh,
 		KubernetesRestConfig:   restConfig,
+		BundlePath:             bundlePath,
 	}
 
 	if v.GetString("since") != "" || v.GetString("since-time") != "" {
@@ -289,7 +381,7 @@ func collectInCluster(ctx context.Context, preflightSpec *troubleshootv1beta2.Pr
 	return &collectResults, nil
 }
 
-func collectRemote(ctx context.Context, preflightSpec *troubleshootv1beta2.HostPreflight, progressCh chan interface{}) (*CollectResult, error) {
+func collectRemote(_ context.Context, preflightSpec *troubleshootv1beta2.HostPreflight, progressCh chan interface{}) (*CollectResult, error) {
 	v := viper.GetViper()
 
 	restConfig, err := k8sutil.GetRESTConfig()
@@ -331,9 +423,12 @@ func collectRemote(ctx context.Context, preflightSpec *troubleshootv1beta2.HostP
 	return &collectResults, nil
 }
 
-func collectHost(ctx context.Context, hostPreflightSpec *troubleshootv1beta2.HostPreflight, progressCh chan interface{}) (*CollectResult, error) {
+func collectHost(
+	_ context.Context, hostPreflightSpec *troubleshootv1beta2.HostPreflight, progressCh chan interface{}, bundlePath string,
+) (*CollectResult, error) {
 	collectOpts := CollectOpts{
 		ProgressChan: progressCh,
+		BundlePath:   bundlePath,
 	}
 
 	collectResults, err := CollectHost(collectOpts, hostPreflightSpec)

pkg/supportbundle/collect.go

@@ -19,7 +19,6 @@ import (
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
-	"gopkg.in/yaml.v2"
 	"k8s.io/client-go/kubernetes"
 )
 
@@ -221,24 +220,6 @@ func findFileName(basename, extension string) (string, error) {
 	}
 }
 
-func getVersionFile() (io.Reader, error) {
-	version := troubleshootv1beta2.SupportBundleVersion{
-		ApiVersion: "troubleshoot.sh/v1beta2",
-		Kind:       "SupportBundle",
-		Spec: troubleshootv1beta2.SupportBundleVersionSpec{
-			VersionNumber: version.Version(),
-		},
-	}
-	b, err := yaml.Marshal(version)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to marshal version data")
-	}
-
-	return bytes.NewBuffer(b), nil
-}
-
-const AnalysisFilename = "analysis.json"
-
 func getAnalysisFile(analyzeResults []*analyze.AnalyzeResult) (io.Reader, error) {
 	data := convert.FromAnalyzerResult(analyzeResults)
 	analysis, err := json.MarshalIndent(data, "", "    ")