Host preflight ux improvements

Author: emosbaugh

cmd/preflight/cli/run.go

@@ -93,6 +93,7 @@ func runPreflights(v *viper.Viper, arg string) error {
 
 	s := spin.New()
 	go func() {
+		lastMsg := ""
 		for {
 			select {
 			case msg, ok := <-progressCh:
@@ -104,6 +105,10 @@ func runPreflights(v *viper.Viper, arg string) error {
 					c := color.New(color.FgHiRed)
 					c.Println(fmt.Sprintf("%s\r * %v", cursor.ClearEntireLine(), msg))
 				case string:
+					if lastMsg == msg {
+						break
+					}
+					lastMsg = msg
 					c := color.New(color.FgCyan)
 					c.Println(fmt.Sprintf("%s\r * %s", cursor.ClearEntireLine(), msg))
 				}

pkg/analyze/analyzer.go

@@ -47,10 +47,8 @@ func HostAnalyze(hostAnalyzer *troubleshootv1beta2.HostAnalyze, getFile getColle
 		return NewAnalyzeResultError(analyzer, errors.New("invalid analyzer"))
 	}
 
-	isExcluded, err := analyzer.IsExcluded()
-	if err != nil {
-		return NewAnalyzeResultError(analyzer, errors.Wrap(err, "is excluded"))
-	} else if isExcluded {
+	isExcluded, _ := analyzer.IsExcluded()
+	if isExcluded {
 		return nil
 	}
 

pkg/analyze/host_analyzer.go

@@ -12,18 +12,20 @@ func GetHostAnalyzer(analyzer *troubleshootv1beta2.HostAnalyze) (HostAnalyzer, b
 	switch {
 	case analyzer.CPU != nil:
 		return &AnalyzeHostCPU{analyzer.CPU}, true
+	case analyzer.Memory != nil:
+		return &AnalyzeHostMemory{analyzer.Memory}, true
 	case analyzer.TCPLoadBalancer != nil:
 		return &AnalyzeHostTCPLoadBalancer{analyzer.TCPLoadBalancer}, true
 	case analyzer.HTTPLoadBalancer != nil:
 		return &AnalyzeHostHTTPLoadBalancer{analyzer.HTTPLoadBalancer}, true
 	case analyzer.DiskUsage != nil:
 		return &AnalyzeHostDiskUsage{analyzer.DiskUsage}, true
-	case analyzer.Memory != nil:
-		return &AnalyzeHostMemory{analyzer.Memory}, true
 	case analyzer.TCPPortStatus != nil:
 		return &AnalyzeHostTCPPortStatus{analyzer.TCPPortStatus}, true
 	case analyzer.HTTP != nil:
 		return &AnalyzeHostHTTP{analyzer.HTTP}, true
+	case analyzer.Time != nil:
+		return &AnalyzeHostTime{analyzer.Time}, true
 	case analyzer.BlockDevices != nil:
 		return &AnalyzeHostBlockDevices{analyzer.BlockDevices}, true
 	case analyzer.TCPConnect != nil:

pkg/collect/host_block_device.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 
 	"github.com/pkg/errors"
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 )
 
 type BlockDeviceInfo struct {
@@ -28,7 +29,19 @@ type BlockDeviceInfo struct {
 const lsblkColumns = "NAME,KNAME,PKNAME,TYPE,MAJ:MIN,SIZE,FSTYPE,MOUNTPOINT,SERIAL,RO,RM"
 const lsblkFormat = `NAME=%q KNAME=%q PKNAME=%q TYPE=%q MAJ:MIN="%d:%d" SIZE="%d" FSTYPE=%q MOUNTPOINT=%q SERIAL=%q RO="%d" RM="%d0"`
 
-func HostBlockDevices(c *HostCollector) (map[string][]byte, error) {
+type CollectHostBlockDevices struct {
+	hostCollector *troubleshootv1beta2.HostBlockDevices
+}
+
+func (c *CollectHostBlockDevices) Title() string {
+	return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, "Block Devices")
+}
+
+func (c *CollectHostBlockDevices) IsExcluded() (bool, error) {
+	return isExcluded(c.hostCollector.Exclude)
+}
+
+func (c *CollectHostBlockDevices) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
 	var devices []BlockDeviceInfo
 
 	cmd := exec.Command("lsblk", "--noheadings", "--bytes", "--pairs", "-o", lsblkColumns)

pkg/collect/host_certificate.go

@@ -6,6 +6,8 @@ import (
 	"io/ioutil"
 	"path/filepath"
 	"strings"
+
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 )
 
 const KeyPairMissing = "key-pair-missing"
@@ -15,10 +17,22 @@ const KeyPairMismatch = "key-pair-mismatch"
 const KeyPairInvalid = "key-pair-invalid"
 const KeyPairValid = "key-pair-valid"
 
-func HostCertificate(c *HostCollector) (map[string][]byte, error) {
+type CollectHostCertificate struct {
+	hostCollector *troubleshootv1beta2.Certificate
+}
+
+func (c *CollectHostCertificate) Title() string {
+	return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, "Certificate Key Pair")
+}
+
+func (c *CollectHostCertificate) IsExcluded() (bool, error) {
+	return isExcluded(c.hostCollector.Exclude)
+}
+
+func (c *CollectHostCertificate) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
 	var result = KeyPairValid
 
-	_, err := tls.LoadX509KeyPair(c.Collect.Certificate.CertificatePath, c.Collect.Certificate.KeyPath)
+	_, err := tls.LoadX509KeyPair(c.hostCollector.CertificatePath, c.hostCollector.KeyPath)
 	if err != nil {
 		if strings.Contains(err.Error(), "no such file") {
 			result = KeyPairMissing
@@ -29,7 +43,7 @@ func HostCertificate(c *HostCollector) (map[string][]byte, error) {
 		} else if strings.Contains(err.Error(), "private key does not match public key") {
 			result = KeyPairMismatch
 		} else if strings.Contains(err.Error(), "failed to parse private key") {
-			if encrypted, _ := isEncryptedKey(c.Collect.Certificate.KeyPath); encrypted {
+			if encrypted, _ := isEncryptedKey(c.hostCollector.KeyPath); encrypted {
 				result = KeyPairEncrypted
 			} else {
 				result = KeyPairInvalid
@@ -39,7 +53,7 @@ func HostCertificate(c *HostCollector) (map[string][]byte, error) {
 		}
 	}
 
-	collectorName := c.Collect.Certificate.CollectorName
+	collectorName := c.hostCollector.CollectorName
 	if collectorName == "" {
 		collectorName = "certificate"
 	}

pkg/collect/host_collector.go

@@ -1,104 +1,51 @@
 package collect
 
 import (
-	"github.com/pkg/errors"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
-	"github.com/replicatedhq/troubleshoot/pkg/multitype"
 )
 
-type HostCollector struct {
-	Collect *troubleshootv1beta2.HostCollect
+type HostCollector interface {
+	Title() string
+	IsExcluded() (bool, error)
+	Collect(progressChan chan<- interface{}) (map[string][]byte, error)
 }
 
-type HostCollectors []*HostCollector
-
-func (c *HostCollector) RunCollectorSync() (result map[string][]byte, err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			err = errors.Errorf("recovered rom panic: %v", r)
-		}
-	}()
-
-	if c.IsExcluded() {
-		return
-	}
-
-	if c.Collect.CPU != nil {
-		result, err = HostCPU(c)
-	} else if c.Collect.Memory != nil {
-		result, err = HostMemory(c)
-	} else if c.Collect.TCPLoadBalancer != nil {
-		result, err = HostTCPLoadBalancer(c)
-	} else if c.Collect.HTTPLoadBalancer != nil {
-		result, err = HostHTTPLoadBalancer(c)
-	} else if c.Collect.DiskUsage != nil {
-		result, err = HostDiskUsage(c)
-	} else if c.Collect.TCPPortStatus != nil {
-		result, err = HostTCPPortStatus(c)
-	} else if c.Collect.HTTP != nil {
-		result, err = HostHTTP(c)
-	} else if c.Collect.Time != nil {
-		result, err = HostTime(c)
-	} else if c.Collect.BlockDevices != nil {
-		result, err = HostBlockDevices(c)
-	} else if c.Collect.TCPConnect != nil {
-		result, err = HostTCPConnect(c)
-	} else if c.Collect.IPV4Interfaces != nil {
-		result, err = HostIPV4Interfaces(c)
-	} else if c.Collect.FilesystemPerformance != nil {
-		result, err = HostFilesystemPerformance(c)
-	} else if c.Collect.Certificate != nil {
-		result, err = HostCertificate(c)
-	} else {
-		err = errors.New("no spec found to run")
-		return
-	}
-	if err != nil {
-		return
+func GetHostCollector(collector *troubleshootv1beta2.HostCollect) (HostCollector, bool) {
+	switch {
+	case collector.CPU != nil:
+		return &CollectHostCPU{collector.CPU}, true
+	case collector.Memory != nil:
+		return &CollectHostMemory{collector.Memory}, true
+	case collector.TCPLoadBalancer != nil:
+		return &CollectHostTCPLoadBalancer{collector.TCPLoadBalancer}, true
+	case collector.HTTPLoadBalancer != nil:
+		return &CollectHostHTTPLoadBalancer{collector.HTTPLoadBalancer}, true
+	case collector.DiskUsage != nil:
+		return &CollectHostDiskUsage{collector.DiskUsage}, true
+	case collector.TCPPortStatus != nil:
+		return &CollectHostTCPPortStatus{collector.TCPPortStatus}, true
+	case collector.HTTP != nil:
+		return &CollectHostHTTP{collector.HTTP}, true
+	case collector.Time != nil:
+		return &CollectHostTime{collector.Time}, true
+	case collector.BlockDevices != nil:
+		return &CollectHostBlockDevices{collector.BlockDevices}, true
+	case collector.TCPConnect != nil:
+		return &CollectHostTCPConnect{collector.TCPConnect}, true
+	case collector.IPV4Interfaces != nil:
+		return &CollectHostIPV4Interfaces{collector.IPV4Interfaces}, true
+	case collector.FilesystemPerformance != nil:
+		return &CollectHostFilesystemPerformance{collector.FilesystemPerformance}, true
+	case collector.Certificate != nil:
+		return &CollectHostCertificate{collector.Certificate}, true
+	default:
+		return nil, false
 	}
-
-	return
 }
 
-func (c *HostCollector) IsExcluded() bool {
-	exclude := multitype.BoolOrString{}
-	if c.Collect.CPU != nil {
-		exclude = c.Collect.CPU.Exclude
-	} else if c.Collect.Memory != nil {
-		exclude = c.Collect.Memory.Exclude
-	} else if c.Collect.TCPLoadBalancer != nil {
-		exclude = c.Collect.TCPLoadBalancer.Exclude
-	} else if c.Collect.HTTPLoadBalancer != nil {
-		exclude = c.Collect.HTTPLoadBalancer.Exclude
-	} else if c.Collect.DiskUsage != nil {
-		exclude = c.Collect.DiskUsage.Exclude
-	} else if c.Collect.TCPPortStatus != nil {
-		exclude = c.Collect.TCPPortStatus.Exclude
-	} else if c.Collect.HTTP != nil {
-		exclude = c.Collect.HTTP.Exclude
-	} else if c.Collect.Time != nil {
-		exclude = c.Collect.Time.Exclude
-	} else if c.Collect.BlockDevices != nil {
-		exclude = c.Collect.BlockDevices.Exclude
-	} else if c.Collect.TCPConnect != nil {
-		exclude = c.Collect.TCPConnect.Exclude
-	} else if c.Collect.IPV4Interfaces != nil {
-		exclude = c.Collect.IPV4Interfaces.Exclude
-	} else if c.Collect.FilesystemPerformance != nil {
-		exclude = c.Collect.FilesystemPerformance.Exclude
-	} else if c.Collect.Certificate != nil {
-		exclude = c.Collect.Certificate.Exclude
-	} else {
-		return true
-	}
-
-	isExcludedResult, err := isExcluded(exclude)
-	if err != nil {
-		return true
+func hostCollectorTitleOrDefault(meta troubleshootv1beta2.HostCollectorMeta, defaultTitle string) string {
+	if meta.CollectorName != "" {
+		return meta.CollectorName
 	}
-	return isExcludedResult
-}
-
-func (c *HostCollector) GetDisplayName() string {
-	return c.Collect.GetName()
+	return defaultTitle
 }

pkg/collect/host_cpu.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 
 	"github.com/pkg/errors"
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 	"github.com/shirou/gopsutil/cpu"
 )
 
@@ -12,7 +13,19 @@ type CPUInfo struct {
 	PhysicalCount int `json:"physicalCount"`
 }
 
-func HostCPU(c *HostCollector) (map[string][]byte, error) {
+type CollectHostCPU struct {
+	hostCollector *troubleshootv1beta2.CPU
+}
+
+func (c *CollectHostCPU) Title() string {
+	return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, "CPU Info")
+}
+
+func (c *CollectHostCPU) IsExcluded() (bool, error) {
+	return isExcluded(c.hostCollector.Exclude)
+}
+
+func (c *CollectHostCPU) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
 	cpuInfo := CPUInfo{}
 
 	logicalCount, err := cpu.Counts(true)

pkg/collect/host_disk_usage.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 
 	"github.com/pkg/errors"
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 	"github.com/shirou/gopsutil/disk"
 )
 
@@ -15,14 +16,26 @@ type DiskUsageInfo struct {
 	UsedBytes  uint64 `json:"used_bytes"`
 }
 
-func HostDiskUsage(c *HostCollector) (map[string][]byte, error) {
+type CollectHostDiskUsage struct {
+	hostCollector *troubleshootv1beta2.DiskUsage
+}
+
+func (c *CollectHostDiskUsage) Title() string {
+	return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, fmt.Sprintf("Disk Usage %s", c.hostCollector.CollectorName))
+}
+
+func (c *CollectHostDiskUsage) IsExcluded() (bool, error) {
+	return isExcluded(c.hostCollector.Exclude)
+}
+
+func (c *CollectHostDiskUsage) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
 	result := map[string][]byte{}
 
-	if c.Collect.DiskUsage == nil {
+	if c.hostCollector == nil {
 		return result, nil
 	}
 
-	pathExists, err := traverseFiletreeDirExists(c.Collect.DiskUsage.Path)
+	pathExists, err := traverseFiletreeDirExists(c.hostCollector.Path)
 	if err != nil {
 		return result, errors.Wrap(err, "traverse file tree")
 	}
@@ -39,7 +52,7 @@ func HostDiskUsage(c *HostCollector) (map[string][]byte, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to marshal disk space info")
 	}
-	key := HostDiskUsageKey(c.Collect.DiskUsage.CollectorName)
+	key := HostDiskUsageKey(c.hostCollector.CollectorName)
 	result[key] = b
 
 	return result, nil

pkg/collect/host_filesystem_performance.go

@@ -4,12 +4,30 @@ import (
 	"math"
 	"math/rand"
 	"time"
+
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
 )
 
 func init() {
 	rand.Seed(time.Now().UnixNano())
 }
 
+type CollectHostFilesystemPerformance struct {
+	hostCollector *troubleshootv1beta2.FilesystemPerformance
+}
+
+func (c *CollectHostFilesystemPerformance) Title() string {
+	return hostCollectorTitleOrDefault(c.hostCollector.HostCollectorMeta, "Filesystem Performance")
+}
+
+func (c *CollectHostFilesystemPerformance) IsExcluded() (bool, error) {
+	return isExcluded(c.hostCollector.Exclude)
+}
+
+func (c *CollectHostFilesystemPerformance) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
+	return collectHostFilesystemPerformance(c.hostCollector)
+}
+
 type FSPerfResults struct {
 	Min     time.Duration
 	Max     time.Duration

pkg/collect/host_filesystem_performance_darwin.go

@@ -1,7 +1,10 @@
 package collect
 
-import "github.com/pkg/errors"
+import (
+	"github.com/pkg/errors"
+	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+)
 
-func HostFilesystemPerformance(c *HostCollector) (map[string][]byte, error) {
+func collectHostFilesystemPerformance(hostCollector *troubleshootv1beta2.FilesystemPerformance) (map[string][]byte, error) {
 	return nil, errors.New("Filesystem performance collector is only implemented for Linux")
 }