Refactor

Author: marccampbell

cmd/preflight/cli/root.go

@@ -4,7 +4,6 @@ import (
 	"os"
 	"strings"
 
-	troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -64,16 +63,3 @@ func initConfig() {
 	viper.SetEnvPrefix("PREFLIGHT")
 	viper.AutomaticEnv()
 }
-
-func ensureCollectorInList(list []*troubleshootv1beta1.Collect, collector troubleshootv1beta1.Collect) []*troubleshootv1beta1.Collect {
-	for _, inList := range list {
-		if collector.ClusterResources != nil && inList.ClusterResources != nil {
-			return list
-		}
-		if collector.ClusterInfo != nil && inList.ClusterInfo != nil {
-			return list
-		}
-	}
-
-	return append(list, &collector)
-}

cmd/preflight/cli/run.go

@@ -1,23 +1,18 @@
 package cli
 
 import (
-	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"os"
-	"path/filepath"
-	"strings"
 	"time"
 
 	cursor "github.com/ahmetalpbalkan/go-cursor"
 	"github.com/fatih/color"
 	"github.com/pkg/errors"
-	analyzerunner "github.com/replicatedhq/troubleshoot/pkg/analyze"
 	troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1"
 	troubleshootclientsetscheme "github.com/replicatedhq/troubleshoot/pkg/client/troubleshootclientset/scheme"
-	"github.com/replicatedhq/troubleshoot/pkg/collect"
+	"github.com/replicatedhq/troubleshoot/pkg/preflight"
 	"github.com/spf13/viper"
 	spin "github.com/tj/go-spin"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -66,7 +61,7 @@ func runPreflights(v *viper.Viper, arg string) error {
 		return errors.Wrapf(err, "failed to parse %s", arg)
 	}
 
-	preflight := obj.(*troubleshootv1beta1.Preflight)
+	preflightSpec := obj.(*troubleshootv1beta1.Preflight)
 
 	s := spin.New()
 	finishedCh := make(chan bool, 1)
@@ -98,48 +93,34 @@ func runPreflights(v *viper.Viper, arg string) error {
 		close(finishedCh)
 	}()
 
-	allCollectedData, err := RunCollectors(v, *preflight, progressChan)
+	restConfig, err := KubernetesConfigFlags.ToRESTConfig()
 	if err != nil {
-		return err
-	}
-
-	getCollectedFileContents := func(fileName string) ([]byte, error) {
-		contents, ok := allCollectedData[fileName]
-		if !ok {
-			return nil, fmt.Errorf("file %s was not collected", fileName)
-		}
-
-		return contents, nil
+		return errors.Wrap(err, "failed to convert kube flags to rest config")
 	}
-	getChildCollectedFileContents := func(prefix string) (map[string][]byte, error) {
-		matching := make(map[string][]byte)
-		for k, v := range allCollectedData {
-			if strings.HasPrefix(k, prefix) {
-				matching[k] = v
-			}
-		}
 
-		return matching, nil
+	collectOpts := preflight.CollectOpts{
+		Namespace:              v.GetString("namespace"),
+		IgnorePermissionErrors: v.GetBool("collect-without-permissions"),
+		ProgressChan:           progressChan,
+		KubernetesRestConfig:   restConfig,
 	}
 
-	analyzeResults := []*analyzerunner.AnalyzeResult{}
-	for _, analyzer := range preflight.Spec.Analyzers {
-		analyzeResult, err := analyzerunner.Analyze(analyzer, getCollectedFileContents, getChildCollectedFileContents)
-		if err != nil {
-			analyzeResult = &analyzerunner.AnalyzeResult{
-				IsFail:  true,
-				Title:   "Analyzer Failed",
-				Message: err.Error(),
+	collectResults, err := preflight.Collect(collectOpts, preflightSpec)
+	if err != nil {
+		if !collectResults.IsRBACAllowed {
+			if preflightSpec.Spec.UploadResultsTo != "" {
+				err := uploadErrors(preflightSpec.Spec.UploadResultsTo, collectResults.Collectors)
+				if err != nil {
+					progressChan <- err
+				}
 			}
 		}
-
-		if analyzeResult != nil {
-			analyzeResults = append(analyzeResults, analyzeResult)
-		}
+		return err
 	}
 
-	if preflight.Spec.UploadResultsTo != "" {
-		err := uploadResults(preflight.Spec.UploadResultsTo, analyzeResults)
+	analyzeResults := collectResults.Analyze()
+	if preflightSpec.Spec.UploadResultsTo != "" {
+		err := uploadResults(preflightSpec.Spec.UploadResultsTo, analyzeResults)
 		if err != nil {
 			progressChan <- err
 		}
@@ -151,117 +132,8 @@ func runPreflights(v *viper.Viper, arg string) error {
 		if len(analyzeResults) == 0 {
 			return errors.New("no data has been collected")
 		}
-		return showInteractiveResults(preflight.Name, analyzeResults)
-	}
-
-	return showStdoutResults(v.GetString("format"), preflight.Name, analyzeResults)
-}
-
-func RunCollectors(v *viper.Viper, preflight troubleshootv1beta1.Preflight, progressChan chan interface{}) (map[string][]byte, error) {
-	collectSpecs := make([]*troubleshootv1beta1.Collect, 0, 0)
-	collectSpecs = append(collectSpecs, preflight.Spec.Collectors...)
-	collectSpecs = ensureCollectorInList(collectSpecs, troubleshootv1beta1.Collect{ClusterInfo: &troubleshootv1beta1.ClusterInfo{}})
-	collectSpecs = ensureCollectorInList(collectSpecs, troubleshootv1beta1.Collect{ClusterResources: &troubleshootv1beta1.ClusterResources{}})
-
-	allCollectedData := make(map[string][]byte)
-
-	config, err := KubernetesConfigFlags.ToRESTConfig()
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to convert kube flags to rest config")
-	}
-
-	var collectors collect.Collectors
-	for _, desiredCollector := range collectSpecs {
-		collector := collect.Collector{
-			Redact:       true,
-			Collect:      desiredCollector,
-			ClientConfig: config,
-			Namespace:    v.GetString("namespace"),
-		}
-		collectors = append(collectors, &collector)
-	}
-
-	if err := collectors.CheckRBAC(); err != nil {
-		return nil, errors.Wrap(err, "failed to check RBAC for collectors")
-	}
-
-	foundForbidden := false
-	for _, c := range collectors {
-		for _, e := range c.RBACErrors {
-			foundForbidden = true
-			progressChan <- e
-		}
-	}
-
-	if foundForbidden && !v.GetBool("collect-without-permissions") {
-		if preflight.Spec.UploadResultsTo != "" {
-			err := uploadErrors(preflight.Spec.UploadResultsTo, collectors)
-			if err != nil {
-				progressChan <- err
-			}
-		}
-		return nil, errors.New("insufficient permissions to run all collectors")
-	}
-
-	// Run preflights collectors synchronously
-	for _, collector := range collectors {
-		if len(collector.RBACErrors) > 0 {
-			// don't skip clusterResources collector due to RBAC issues
-			if collector.Collect.ClusterResources == nil {
-				progressChan <- fmt.Sprintf("skipping collector %s with insufficient RBAC permissions", collector.GetDisplayName())
-				continue
-			}
-		}
-
-		result, err := collector.RunCollectorSync()
-		if err != nil {
-			progressChan <- errors.Errorf("failed to run collector %s: %v\n", collector.GetDisplayName(), err)
-			continue
-		}
-
-		if result != nil {
-			output, err := parseCollectorOutput(string(result))
-			if err != nil {
-				progressChan <- errors.Errorf("failed to parse collector output %s: %v\n", collector.GetDisplayName(), err)
-				continue
-			}
-			for k, v := range output {
-				allCollectedData[k] = v
-			}
-		}
-	}
-
-	return allCollectedData, nil
-}
-
-func parseCollectorOutput(output string) (map[string][]byte, error) {
-	input := make(map[string]interface{})
-	files := make(map[string][]byte)
-	if err := json.Unmarshal([]byte(output), &input); err != nil {
-		return nil, err
-	}
-
-	for filename, maybeContents := range input {
-		fileDir, fileName := filepath.Split(filename)
-
-		switch maybeContents.(type) {
-		case string:
-			decoded, err := base64.StdEncoding.DecodeString(maybeContents.(string))
-			if err != nil {
-				return nil, err
-			}
-			files[filepath.Join(fileDir, fileName)] = decoded
-
-		case map[string]interface{}:
-			for k, v := range maybeContents.(map[string]interface{}) {
-				decoded, err := base64.StdEncoding.DecodeString(v.(string))
-				if err != nil {
-					return nil, err
-				}
-				files[filepath.Join(fileDir, fileName, k)] = decoded
-			}
-		}
+		return showInteractiveResults(preflightSpec.Name, analyzeResults)
 	}
 
-	return files, nil
+	return showStdoutResults(v.GetString("format"), preflightSpec.Name, analyzeResults)
 }

pkg/preflight/analyze.go

@@ -0,0 +1,48 @@
+package preflight
+
+import (
+	"fmt"
+	"strings"
+
+	analyze "github.com/replicatedhq/troubleshoot/pkg/analyze"
+)
+
+// Analyze runs the analyze phase of preflight checks
+func (c CollectResult) Analyze() []*analyze.AnalyzeResult {
+	getCollectedFileContents := func(fileName string) ([]byte, error) {
+		contents, ok := c.AllCollectedData[fileName]
+		if !ok {
+			return nil, fmt.Errorf("file %s was not collected", fileName)
+		}
+
+		return contents, nil
+	}
+	getChildCollectedFileContents := func(prefix string) (map[string][]byte, error) {
+		matching := make(map[string][]byte)
+		for k, v := range c.AllCollectedData {
+			if strings.HasPrefix(k, prefix) {
+				matching[k] = v
+			}
+		}
+
+		return matching, nil
+	}
+
+	analyzeResults := []*analyze.AnalyzeResult{}
+	for _, analyzer := range c.Spec.Spec.Analyzers {
+		analyzeResult, err := analyze.Analyze(analyzer, getCollectedFileContents, getChildCollectedFileContents)
+		if err != nil {
+			analyzeResult = &analyze.AnalyzeResult{
+				IsFail:  true,
+				Title:   "Analyzer Failed",
+				Message: err.Error(),
+			}
+		}
+
+		if analyzeResult != nil {
+			analyzeResults = append(analyzeResults, analyzeResult)
+		}
+	}
+
+	return analyzeResults
+}