Repository: replicatedhq/troubleshoot
Pull request #339 (2021-03-18 23:04:31)

added two parameters to the eligible block device check
whether to accept unmounted partitions (default false) and minimum acceptable device size (default 0)


Patch:
diff --git a/pkg/analyze/host_block_devices.go b/pkg/analyze/host_block_devices.go
index 099c88b2..6761d701 100644
--- a/pkg/analyze/host_block_devices.go
+++ b/pkg/analyze/host_block_devices.go
@@ -51,7 +51,7 @@ func (a *AnalyzeHostBlockDevices) Analyze(getCollectedFileContents func(string)
 				return &result, nil
 			}
 
-			isMatch, err := compareHostBlockDevicesConditionalToActual(outcome.Fail.When, devices)
+			isMatch, err := compareHostBlockDevicesConditionalToActual(outcome.Fail.When, hostAnalyzer.MinimumAcceptableSize, hostAnalyzer.IncludeUnmountedPartitions, devices)
 			if err != nil {
 				return nil, errors.Wrapf(err, "failed to compare %s", outcome.Fail.When)
 			}
@@ -72,7 +72,7 @@ func (a *AnalyzeHostBlockDevices) Analyze(getCollectedFileContents func(string)
 				return &result, nil
 			}
 
-			isMatch, err := compareHostBlockDevicesConditionalToActual(outcome.Warn.When, devices)
+			isMatch, err := compareHostBlockDevicesConditionalToActual(outcome.Warn.When, hostAnalyzer.MinimumAcceptableSize, hostAnalyzer.IncludeUnmountedPartitions, devices)
 			if err != nil {
 				return nil, errors.Wrapf(err, "failed to compare %s", outcome.Warn.When)
 			}
@@ -93,7 +93,7 @@ func (a *AnalyzeHostBlockDevices) Analyze(getCollectedFileContents func(string)
 				return &result, nil
 			}
 
-			isMatch, err := compareHostBlockDevicesConditionalToActual(outcome.Pass.When, devices)
+			isMatch, err := compareHostBlockDevicesConditionalToActual(outcome.Pass.When, hostAnalyzer.MinimumAcceptableSize, hostAnalyzer.IncludeUnmountedPartitions, devices)
 			if err != nil {
 				return nil, errors.Wrapf(err, "failed to compare %s", outcome.Pass.When)
 			}
@@ -113,7 +113,7 @@ func (a *AnalyzeHostBlockDevices) Analyze(getCollectedFileContents func(string)
 
 // <regexp> <op> <count>
 // example: sdb > 0
-func compareHostBlockDevicesConditionalToActual(conditional string, devices []collect.BlockDeviceInfo) (res bool, err error) {
+func compareHostBlockDevicesConditionalToActual(conditional string, minimumAcceptableSize uint64, includeUnmountedPartitions bool, devices []collect.BlockDeviceInfo) (res bool, err error) {
 	parts := strings.Split(conditional, " ")
 	if len(parts) != 3 {
 		return false, fmt.Errorf("Expected exactly 3 parts, got %d", len(parts))
@@ -123,7 +123,7 @@ func compareHostBlockDevicesConditionalToActual(conditional string, devices []co
 	if err != nil {
 		return false, errors.Wrapf(err, "failed to compile regex %q", parts[0])
 	}
-	count := countEligibleBlockDevices(rx, devices)
+	count := countEligibleBlockDevices(rx, minimumAcceptableSize, includeUnmountedPartitions, devices)
 
 	desiredInt, err := strconv.Atoi(parts[2])
 	if err != nil {
@@ -146,11 +146,11 @@ func compareHostBlockDevicesConditionalToActual(conditional string, devices []co
 	return false, fmt.Errorf("Unexpected operator %q", parts[1])
 }
 
-func countEligibleBlockDevices(rx *regexp.Regexp, devices []collect.BlockDeviceInfo) int {
+func countEligibleBlockDevices(rx *regexp.Regexp, minimumAcceptableSize uint64, includeUnmountedPartitions bool, devices []collect.BlockDeviceInfo) int {
 	count := 0
 
 	for _, device := range devices {
-		if isEligibleBlockDevice(rx, device, devices) {
+		if isEligibleBlockDevice(rx, minimumAcceptableSize, includeUnmountedPartitions, device, devices) {
 			count++
 		}
 	}
@@ -158,13 +158,25 @@ func countEligibleBlockDevices(rx *regexp.Regexp, devices []collect.BlockDeviceI
 	return count
 }
 
-func isEligibleBlockDevice(rx *regexp.Regexp, device collect.BlockDeviceInfo, devices []collect.BlockDeviceInfo) bool {
+func isEligibleBlockDevice(rx *regexp.Regexp, minimumAcceptableSize uint64, includeUnmountedPartitions bool, device collect.BlockDeviceInfo, devices []collect.BlockDeviceInfo) bool {
 	if !rx.MatchString(device.Name) {
 		return false
 	}
 
-	if device.Type != "disk" {
-		return false
+	if includeUnmountedPartitions {
+		if device.Type != "disk" && device.Type != "part" {
+			return false
+		}
+	} else {
+		if device.Type != "disk" {
+			return false
+		}
+	}
+
+	if minimumAcceptableSize != 0 {
+		if device.Size < minimumAcceptableSize {
+			return false
+		}
 	}
 
 	if device.Mountpoint != "" {
diff --git a/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go b/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go
index c4b06a6f..9b386afa 100644
--- a/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go
+++ b/pkg/apis/troubleshoot/v1beta2/hostanalyzer_shared.go
@@ -46,8 +46,10 @@ type TimeAnalyze struct {
 }
 
 type BlockDevicesAnalyze struct {
-	AnalyzeMeta `json:",inline" yaml:",inline"`
-	Outcomes    []*Outcome `json:"outcomes" yaml:"outcomes"`
+	AnalyzeMeta                `json:",inline" yaml:",inline"`
+	MinimumAcceptableSize      uint64     `json:"minimumAcceptableSize" yaml:"minimumAcceptableSize"`
+	IncludeUnmountedPartitions bool       `json:"includeUnmountedPartitions" yaml:"includeUnmountedPartitions"`
+	Outcomes                   []*Outcome `json:"outcomes" yaml:"outcomes"`
 }
 
 type TCPConnectAnalyze struct {
diff --git a/pkg/preflight/collect.go b/pkg/preflight/collect.go
index 416aa78e..c8daef2d 100644
--- a/pkg/preflight/collect.go
+++ b/pkg/preflight/collect.go
@@ -19,10 +19,8 @@ type CollectOpts struct {
 }
 
 type CollectProgress struct {
-	CurrentName    string
-	CurrentStatus  string
-	CompletedCount int
-	TotalCount     int
+	Name   string
+	Status string
 }
 
 type CollectResult interface {
@@ -141,46 +139,30 @@ func Collect(opts CollectOpts, p *troubleshootv1beta2.Preflight) (CollectResult,
 	}
 
 	// Run preflights collectors synchronously
-	for i, collector := range collectors {
+	for _, collector := range collectors {
 		if len(collector.RBACErrors) > 0 {
 			// don't skip clusterResources collector due to RBAC issues
 			if collector.Collect.ClusterResources == nil {
 				collectResult.isRBACAllowed = false // not failing, but going to report this
 				opts.ProgressChan <- fmt.Sprintf("skipping collector %s with insufficient RBAC permissions", collector.GetDisplayName())
-				opts.ProgressChan <- CollectProgress{
-					CurrentName:    collector.GetDisplayName(),
-					CurrentStatus:  "skipped",
-					CompletedCount: i + 1,
-					TotalCount:     len(collectors),
-				}
 				continue
 			}
 		}
 
 		opts.ProgressChan <- CollectProgress{
-			CurrentName:    collector.GetDisplayName(),
-			CurrentStatus:  "running",
-			CompletedCount: i,
-			TotalCount:     len(collectors),
+			Name:   collector.GetDisplayName(),
+			Status: "running",
 		}
 
 		result, err := collector.RunCollectorSync(nil)
 		if err != nil {
 			opts.ProgressChan <- errors.Errorf("failed to run collector %s: %v\n", collector.GetDisplayName(), err)
-			opts.ProgressChan <- CollectProgress{
-				CurrentName:    collector.GetDisplayName(),
-				CurrentStatus:  "failed",
-				CompletedCount: i + 1,
-				TotalCount:     len(collectors),
-			}
 			continue
 		}
 
 		opts.ProgressChan <- CollectProgress{
-			CurrentName:    collector.GetDisplayName(),
-			CurrentStatus:  "completed",
-			CompletedCount: i + 1,
-			TotalCount:     len(collectors),
+			Name:   collector.GetDisplayName(),
+			Status: "completed",
 		}
 
 		if result != nil {


Test patch:
diff --git a/pkg/analyze/host_block_devices_test.go b/pkg/analyze/host_block_devices_test.go
index 962b96b5..fccbcdf1 100644
--- a/pkg/analyze/host_block_devices_test.go
+++ b/pkg/analyze/host_block_devices_test.go
@@ -149,6 +149,97 @@ func TestAnalyzeBlockDevices(t *testing.T) {
 				Message: "No block device available",
 			},
 		},
+		{
+			name: ".* > 1, pass with unmounted partition",
+			devices: []collect.BlockDeviceInfo{
+				{
+					Name:       "sdb",
+					KernelName: "sdb",
+					Type:       "disk",
+					Major:      8,
+					Serial:     "disk1",
+				},
+				{
+					Name:             "sdb1",
+					KernelName:       "sdb1",
+					ParentKernelName: "sdb",
+					Type:             "part",
+					Major:            8,
+					Minor:            1,
+				},
+			},
+			hostAnalyzer: &troubleshootv1beta2.BlockDevicesAnalyze{
+				IncludeUnmountedPartitions: true,
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							When:    ".* >= 1",
+							Message: "Block device or partition available",
+						},
+					},
+				},
+			},
+			result: &AnalyzeResult{
+				Title:   "Block Devices",
+				IsPass:  true,
+				Message: "Block device or partition available",
+			},
+		},
+		{
+			name: ".* = 2, pass with two unmounted partitions/devices of at least 10gb in size",
+			devices: []collect.BlockDeviceInfo{
+				{
+					Name:       "sdb",
+					KernelName: "sdb",
+					Type:       "disk",
+					Major:      8,
+					Serial:     "disk1",
+					Size:       1024 * 1024 * 1024 * 128,
+				},
+				{
+					Name:             "sdb1",
+					KernelName:       "sdb1",
+					ParentKernelName: "sdb",
+					Type:             "part",
+					Major:            8,
+					Minor:            1,
+					Size:             1024 * 1024 * 1024 * 128,
+				},
+				{
+					Name:       "sdc",
+					KernelName: "sdc",
+					Type:       "disk",
+					Major:      8,
+					Serial:     "disk2",
+					Size:       1024 * 1024 * 1024 * 16,
+				},
+				{
+					Name:       "sdd",
+					KernelName: "sdd",
+					Type:       "disk",
+					Major:      8,
+					Serial:     "disk3",
+					Size:       1024 * 1024 * 1024 * 8,
+				},
+			},
+			hostAnalyzer: &troubleshootv1beta2.BlockDevicesAnalyze{
+				IncludeUnmountedPartitions: true,
+				MinimumAcceptableSize:      1024 * 1024 * 1024 * 10,
+				Outcomes: []*troubleshootv1beta2.Outcome{
+					{
+						Pass: &troubleshootv1beta2.SingleOutcome{
+							When:    ".* = 2",
+							Message: "Two block devices or partitions of >10gb size available",
+						},
+					},
+				},
+			},
+			result: &AnalyzeResult{
+				Title:   "Block Devices",
+				IsPass:  true,
+				Message: "Two block devices or partitions of >10gb size available",
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {