Analyse collected support bundle

cmd/troubleshootv2/cli/collect.go

@@ -9,12 +9,15 @@ import (
  "sync"
  "time"
 
- "github.com/replicatedhq/troubleshoot/internal/bundleimpl"
+ "github.com/replicatedhq/troubleshoot/internal/tsbundle"
  "github.com/replicatedhq/troubleshoot/internal/util"
  "github.com/replicatedhq/troubleshoot/pkg/bundle"
+ "github.com/replicatedhq/troubleshoot/pkg/constants"
  "github.com/replicatedhq/troubleshoot/pkg/loader"
  "github.com/replicatedhq/troubleshoot/pkg/logger"
+ "github.com/replicatedhq/troubleshoot/pkg/supportbundle"
  "github.com/spf13/cobra"
+ "go.opentelemetry.io/otel"
  "k8s.io/klog/v2"
 )
 
@@ -24,11 +27,9 @@ func CollectCmd() *cobra.Command {
  Short: "Collect bundle from a cluster or host",
  Long: "Collect bundle from a cluster or host",
  RunE: func(cmd *cobra.Command, args []string) error {
- ctx := cmd.Context()
-
- err := doRun(ctx, args)
+ err := doRun(cmd.Context(), args)
  if err != nil {
- klog.Errorf("Failed to run: %v", err)
+ klog.Errorf("Failure collecting support bundle: %v", err)
  return err
  }
 
@@ -58,17 +59,22 @@ func doRun(ctx context.Context, args []string) error {
  go func() {
  defer wg.Done()
  for msg := range progressChan {
+ // TODO: Expect error or string types
  klog.Infof("Collecting bundle: %v", msg)
  }
  }()
+ ctxWrap, root := otel.Tracer(constants.LIB_TRACER_NAME).Start(
+ ctx, constants.TROUBLESHOOT_ROOT_SPAN_NAME,
+ )
+ defer root.End()
 
  // 1. Load troubleshoot specs from args
  // TODO: "RawSpecsFromArgs" missing the logic to load specs from the cluster
  rawSpecs, err := util.RawSpecsFromArgs(args)
  if err != nil {
  return err
  }
- kinds, err := loader.LoadSpecs(ctx, loader.LoadOptions{
+ kinds, err := loader.LoadSpecs(ctxWrap, loader.LoadOptions{
  RawSpecs: rawSpecs,
  })
  if err != nil {
@@ -82,11 +88,11 @@ func doRun(ctx context.Context, args []string) error {
  }
  defer os.RemoveAll(bundleDir)
 
- bdl := bundleimpl.NewTroubleshootBundle(bundleimpl.TroubleshootBundleOptions{
+ bdl := tsbundle.NewTroubleshootBundle(tsbundle.TroubleshootBundleOptions{
  ProgressChan: progressChan,
  })
- klog.Infof("Starting collecting bundle")
- err = bdl.Collect(ctx, bundle.CollectOptions{
+ klog.Infof("Collect support bundle")
+ err = bdl.Collect(ctxWrap, bundle.CollectOptions{
  Specs: kinds,
  BundleDir: bundleDir,
  })
@@ -96,8 +102,8 @@ func doRun(ctx context.Context, args []string) error {
 
  // 3. Analyze the support bundle
  // TODO: Add results to the support bundle
- klog.Infof("Starting to analyse bundle")
- out, err := bdl.Analyze(ctx, bundle.AnalyzeOptions{
+ klog.Infof("Analyse support bundle")
+ out, err := bdl.Analyze(ctxWrap, bundle.AnalyzeOptions{
  Specs: kinds,
  })
  if err != nil {
@@ -110,29 +116,42 @@ func doRun(ctx context.Context, args []string) error {
  if err != nil {
  return err
  }
- err = bdl.BundleData().Data().SaveResult(bundleDir, "analysis.json", bytes.NewBuffer(analysis))
+ err = bdl.BundleData().Data().SaveResult(bundleDir, supportbundle.AnalysisFilename, bytes.NewBuffer(analysis))
  if err != nil {
  return err
  }
 
  // 4. Redact the support bundle
- err = bdl.Redact(ctx, bundle.RedactOptions{
+ klog.Infof("Redact support bundle")
+ err = bdl.Redact(ctxWrap, bundle.RedactOptions{
  Specs: kinds,
  })
  if err != nil {
  return err
  }
 
  // 5. Archive the support bundle
+ klog.Infof("Archive support bundle")
  supportBundlePath := path.Join(util.HomeDir(), fmt.Sprintf("support-bundle-%s.tgz", time.Now().Format("2006-01-02T15_04_05")))
- err = bdl.Archive(ctx, bundle.ArchiveOptions{
+ err = bdl.Archive(ctxWrap, bundle.ArchiveOptions{
  ArchivePath: supportBundlePath,
  })
  if err != nil {
  return err
  }
 
- // 6. Print outro i.e. "Support bundle saved to <filename>"
+ // 6. Save the bundle to a file
+ klog.Infof("Save version info to support bundle")
+ reader, err := supportbundle.GetVersionFile()
+ if err != nil {
+ return err
+ }
+ err = bdl.BundleData().Data().SaveResult(bundleDir, constants.VERSION_FILENAME, reader)
+ if err != nil {
+ return err
+ }
+
+ // 7. Print outro i.e. "Support bundle saved to <filename>"
  // Print to screen output of bdl.Analyze i.e "analysisResults"
  fmt.Printf("Support bundle saved to %s\n", supportBundlePath)
 

internal/bundleimpl/bundle.go → internal/tsbundle/bundle.go

@@ -1,5 +1,4 @@
-// Is this naming convention correct? i.e impl is for implementation
-package bundleimpl
+package tsbundle
 
 import (
  "context"
@@ -8,6 +7,7 @@ import (
  "github.com/pkg/errors"
  "github.com/replicatedhq/troubleshoot/pkg/bundle"
  "github.com/replicatedhq/troubleshoot/pkg/collect"
+ "github.com/replicatedhq/troubleshoot/pkg/supportbundle"
 )
 
 // Bundle is the Troubleshoot implementation of the bundle.Bundler interface
@@ -17,7 +17,6 @@ type Bundle struct {
 }
 
 type TroubleshootBundleOptions struct {
- // TODO: Consider just a callback function. Channels can block if not read from
  ProgressChan chan any // a channel to write progress information to
  Namespace string // namespace to limit scope the collectors need to run in
 }
@@ -29,24 +28,50 @@ func NewTroubleshootBundle(opt TroubleshootBundleOptions) bundle.Bundler {
  }
 }
 
+func (b *Bundle) reportProgress(msg any) {
+ if b.progressChan != nil {
+ // Non-blocking write to channel.
+ // In case there is no listener drop the message.
+ select {
+ case b.progressChan <- msg:
+ default:
+ }
+ }
+}
+
 func (b *Bundle) Collect(ctx context.Context, opt bundle.CollectOptions) error {
- // TODO: Error if b.data is not nil. We do not want to overwrite existing data
  if b.data != nil {
- return fmt.Errorf("bundle already has data")
+ return fmt.Errorf("we cannot run collectors if a bundle already exists")
  }
 
- b.data = collect.NewBundleData(opt.BundleDir)
-
  results, err := b.doCollect(ctx, opt)
  if err != nil {
  return err
  }
- b.data.Data().AddResult(results)
+
+ b.data = collect.NewBundleData(collect.BundleDataOptions{
+ Data: results,
+ BundleDir: opt.BundleDir,
+ })
  return nil
 }
 
 func (b *Bundle) Analyze(ctx context.Context, opt bundle.AnalyzeOptions) (bundle.AnalyzeOutput, error) {
- return bundle.AnalyzeOutput{}, nil
+ if b.data == nil {
+ return bundle.AnalyzeOutput{}, errors.New("no bundle data to analyze")
+ }
+
+ sbSpec := supportbundle.ConcatSpecs(opt.Specs.SupportBundlesV1Beta2...)
+
+ // Run Analyzers
+ analyzeResults, err := supportbundle.AnalyzeSupportBundle(ctx, &sbSpec.Spec, b.data.BundleDir())
+ if err != nil {
+ return bundle.AnalyzeOutput{}, err
+ }
+
+ return bundle.AnalyzeOutput{
+ Results: analyzeResults,
+ }, nil
 }
 
 func (b *Bundle) BundleData() *collect.BundleData {
@@ -66,7 +91,10 @@ func (b *Bundle) Archive(ctx context.Context, opt bundle.ArchiveOptions) error {
 }
 
 func (b *Bundle) Load(ctx context.Context, opt bundle.LoadBundleOptions) error {
- // TODO: Error if b.data is not nil. We do not want to overwrite existing data
+ if b.data != nil {
+ return fmt.Errorf("we cannot run collectors if a bundle already exists")
+ }
+ // TODO: Load bundle from disk or url
  return nil
 }
 

internal/bundleimpl/collect.go → internal/tsbundle/collect.go

@@ -1,4 +1,4 @@
-package bundleimpl
+package tsbundle
 
 import (
  "context"
@@ -24,27 +24,17 @@ import (
 func (b *Bundle) doCollect(
  ctx context.Context, opt bundle.CollectOptions,
 ) (collect.CollectorResult, error) {
- ctxWrap, root := otel.Tracer(constants.LIB_TRACER_NAME).Start(
- ctx, constants.TROUBLESHOOT_ROOT_SPAN_NAME,
- )
- defer func() {
- // If this function returns an error, root.End() may not be called.
- // We want to ensure this happens, so we defer it. It is safe to call
- // root.End() multiple times.
- root.End()
- }()
-
  allResults := collect.NewResult()
 
- sbSpec := concatSpecs(opt.Specs.SupportBundlesV1Beta2...)
+ sbSpec := supportbundle.ConcatSpecs(opt.Specs.SupportBundlesV1Beta2...)
 
- collectedResults, err := b.collectFromHost(ctxWrap, sbSpec.Spec.HostCollectors, opt.BundleDir, b.progressChan)
+ collectedResults, err := b.collectFromHost(ctx, sbSpec.Spec.HostCollectors, opt.BundleDir)
  if err != nil {
  return nil, errors.Wrap(err, "failed to collect bundle from host")
  }
  allResults.AddResult(collectedResults)
 
- collectedResults, err = b.collectFromCluster(ctxWrap, sbSpec.Spec.Collectors, opt.BundleDir, opt.Namespace)
+ collectedResults, err = b.collectFromCluster(ctx, sbSpec.Spec.Collectors, opt.BundleDir, opt.Namespace)
  if err != nil {
  return nil, errors.Wrap(err, "failed to collect bundle from cluster")
  }
@@ -113,7 +103,7 @@ func (b *Bundle) collectFromCluster(
  mergedCollectors, err := mergeCollector.Merge(collectors)
  if err != nil {
  msg := fmt.Sprintf("failed to merge collector: %s: %s", mergeCollector.Title(), err)
- b.progressChan <- msg
+ b.reportProgress(msg)
  }
  allCollectors = append(allCollectors, mergedCollectors...)
  } else {
@@ -124,7 +114,7 @@ func (b *Bundle) collectFromCluster(
  for _, collector := range collectors {
  for _, e := range collector.GetRBACErrors() {
  foundForbidden = true
- b.progressChan <- e
+ b.reportProgress(e)
  }
  }
  }
@@ -141,7 +131,7 @@ func (b *Bundle) collectFromCluster(
  isExcluded, _ := collector.IsExcluded()
  if isExcluded {
  msg := fmt.Sprintf("excluding %q collector", collector.Title())
- b.progressChan <- msg
+ b.reportProgress(msg)
  span.SetAttributes(attribute.Bool(constants.EXCLUDED, true))
  span.End()
  continue
@@ -151,18 +141,18 @@ func (b *Bundle) collectFromCluster(
  if collector.HasRBACErrors() {
  if _, ok := collector.(*collect.CollectClusterResources); !ok {
  msg := fmt.Sprintf("skipping collector %q with insufficient RBAC permissions", collector.Title())
- b.progressChan <- msg
+ b.reportProgress(msg)
  span.SetStatus(codes.Error, "skipping collector, insufficient RBAC permissions")
  span.End()
  continue
  }
  }
 
- b.progressChan <- collector.Title()
+ b.reportProgress(collector.Title())
  result, err := collector.Collect(b.progressChan)
  if err != nil {
  span.SetStatus(codes.Error, err.Error())
- b.progressChan <- errors.Errorf("failed to run collector: %s: %v", collector.Title(), err)
+ b.reportProgress(errors.Errorf("failed to run collector: %s: %v", collector.Title(), err))
  }
 
  for k, v := range result {
@@ -175,7 +165,7 @@ func (b *Bundle) collectFromCluster(
 }
 
 func (b *Bundle) collectFromHost(
- ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string, progressChan chan any,
+ ctx context.Context, collectSpecs []*troubleshootv1beta2.HostCollect, bundlePath string,
 ) (collect.CollectorResult, error) {
  // TODO: Duplicate of runHostCollectors from pkg/supportbundle/collect.go. DRY me up!
  collectResult := collect.NewResult()
@@ -195,18 +185,18 @@ func (b *Bundle) collectFromHost(
 
  isExcluded, _ := collector.IsExcluded()
  if isExcluded {
- progressChan <- fmt.Sprintf("[%s] Excluding host collector", collector.Title())
+ b.reportProgress(fmt.Sprintf("[%s] Excluding host collector", collector.Title()))
  span.SetAttributes(attribute.Bool(constants.EXCLUDED, true))
  span.End()
  continue
  }
 
- progressChan <- fmt.Sprintf("[%s] Running host collector...", collector.Title())
+ b.reportProgress(fmt.Sprintf("[%s] Running host collector...", collector.Title()))
  // TODO: Convert return type to CollectorResult
- result, err := collector.Collect(progressChan)
+ result, err := collector.Collect(b.progressChan)
  if err != nil {
  span.SetStatus(codes.Error, err.Error())
- progressChan <- errors.Errorf("failed to run host collector: %s: %v", collector.Title(), err)
+ b.reportProgress(errors.Errorf("failed to run host collector: %s: %v", collector.Title(), err))
  }
  span.End()
  for k, v := range result {
@@ -217,14 +207,6 @@ func (b *Bundle) collectFromHost(
  return collectResult, nil
 }
 
-func concatSpecs(specs ...troubleshootv1beta2.SupportBundle) *troubleshootv1beta2.SupportBundle {
- target := &troubleshootv1beta2.SupportBundle{}
- for _, s := range specs {
- target = supportbundle.ConcatSpec(target, &s)
- }
- return target
-}
-
 func parseTimeFlags(v *viper.Viper) (*time.Time, error) {
  // TODO: Copied from cmd/troubleshoot/cli/run.go. DRY me up!
  var (

pkg/bundle/bundler.go

@@ -65,7 +65,6 @@ type ArchiveOptions struct {
 }
 
 type AnalyzeOutput struct {
- Bundle Bundler // include bundle metadata
  // TODO: Does this need to be a slice of pointers? It's a slice and slices are already pointers
  Results []*analyze.AnalyzeResult // analysis results
 }

pkg/collect/result.go

@@ -27,10 +27,15 @@ type BundleData struct {
  bundleDir string
 }
 
-func NewBundleData(bundleDir string) *BundleData {
+type BundleDataOptions struct {
+ Data CollectorResult
+ BundleDir string
+}
+
+func NewBundleData(opt BundleDataOptions) *BundleData {
  return &BundleData{
- data: NewResult(),
- bundleDir: bundleDir,
+ data: opt.Data,
+ bundleDir: opt.BundleDir,
  }
 }
 

pkg/supportbundle/collect.go

@@ -221,7 +221,8 @@ func findFileName(basename, extension string) (string, error) {
  }
 }
 
-func getVersionFile() (io.Reader, error) {
+func GetVersionFile() (io.Reader, error) {
+ // TODO: Move to internal package
  version := troubleshootv1beta2.SupportBundleVersion{
  ApiVersion: "troubleshoot.sh/v1beta2",
  Kind: "SupportBundle",