feat: add timeout to run host collector (#1435)

* feat: add timeout to run host collector

* return error on invalid timeout

* return -1 on context deadline exceeded

Author: nvanthao

config/crds/troubleshoot.sh_hostcollectors.yaml

@@ -1386,6 +1386,8 @@ spec:
                           type: object
                         outputDir:
                           type: string
+                        timeout:
+                          type: string
                       required:
                       - args
                       - command

config/crds/troubleshoot.sh_hostpreflights.yaml

@@ -1386,6 +1386,8 @@ spec:
                           type: object
                         outputDir:
                           type: string
+                        timeout:
+                          type: string
                       required:
                       - args
                       - command

config/crds/troubleshoot.sh_supportbundles.yaml

@@ -12195,6 +12195,8 @@ spec:
                           type: object
                         outputDir:
                           type: string
+                        timeout:
+                          type: string
                       required:
                       - args
                       - command

pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go

@@ -187,6 +187,7 @@ type HostRun struct {
 	Env               []string          `json:"env,omitempty" yaml:"env,omitempty"`
 	InheritEnvs       []string          `json:"inheritEnvs,omitempty" yaml:"inheritEnvs,omitempty"`
 	IgnoreParentEnvs  bool              `json:"ignoreParentEnvs,omitempty" yaml:"ignoreParentEnvs,omitempty"`
+	Timeout           string            `json:"timeout,omitempty" yaml:"timeout,omitempty"`
 }
 
 type HostCollect struct {

pkg/collect/host_run.go

@@ -2,12 +2,14 @@ package collect
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
@@ -49,7 +51,29 @@ func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]
 		collectorName = "run-host"
 	}
 
-	cmd := exec.Command(c.attemptToConvertCmdToAbsPath(), runHostCollector.Args...)
+	var (
+		timeout            time.Duration
+		cmd                *exec.Cmd
+		errInvalidDuration error
+	)
+
+	ctx := context.Background()
+	cmdPath := c.attemptToConvertCmdToAbsPath()
+
+	if runHostCollector.Timeout != "" {
+		timeout, errInvalidDuration = time.ParseDuration(runHostCollector.Timeout)
+		if errInvalidDuration != nil {
+			return nil, errors.Wrapf(errInvalidDuration, "failed to parse timeout %q", runHostCollector.Timeout)
+		}
+	}
+
+	if timeout <= time.Duration(0) {
+		cmd = exec.Command(cmdPath, runHostCollector.Args...)
+	} else {
+		timeoutCtx, cancel := context.WithTimeout(ctx, timeout)
+		defer cancel()
+		cmd = exec.CommandContext(timeoutCtx, cmdPath, runHostCollector.Args...)
+	}
 
 	klog.V(2).Infof("Run host collector command: %q", cmd.String())
 	runInfo := &HostRunInfo{
@@ -118,6 +142,9 @@ func (c *CollectHostRun) Collect(progressChan chan<- interface{}) (map[string][]
 		if werr, ok := err.(*exec.ExitError); ok {
 			runInfo.ExitCode = strings.TrimPrefix(werr.Error(), "exit status ")
 			runInfo.Error = stderr.String()
+		} else if err == context.DeadlineExceeded {
+			runInfo.ExitCode = "-1"
+			runInfo.Error = fmt.Sprintf("command timed out after %s", timeout.String())
 		} else {
 			return nil, errors.Wrap(err, "failed to run")
 		}

pkg/collect/host_run_test.go

@@ -158,3 +158,77 @@ func TestCollectHostRun_Collect(t *testing.T) {
 		}
 	}
 }
+
+func TestCollectHostRunCollectWithTimeout(t *testing.T) {
+	tests := []struct {
+		name      string
+		collector *CollectHostRun
+		wantError bool
+	}{
+		{
+			name: "no timeout",
+			collector: &CollectHostRun{
+				hostCollector: &troubleshootv1beta2.HostRun{
+					HostCollectorMeta: troubleshootv1beta2.HostCollectorMeta{
+						CollectorName: "no timeout",
+					},
+					Command: "echo",
+					Args:    []string{"1"},
+				},
+			},
+			wantError: false,
+		},
+		{
+			name: "invalid timeout",
+			collector: &CollectHostRun{
+				hostCollector: &troubleshootv1beta2.HostRun{
+					HostCollectorMeta: troubleshootv1beta2.HostCollectorMeta{
+						CollectorName: "negative timeout",
+					},
+					Command: "echo",
+					Args:    []string{"1"},
+					Timeout: "thisistheway",
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "negative timeout",
+			collector: &CollectHostRun{
+				hostCollector: &troubleshootv1beta2.HostRun{
+					HostCollectorMeta: troubleshootv1beta2.HostCollectorMeta{
+						CollectorName: "negative timeout",
+					},
+					Command: "echo",
+					Args:    []string{"1"},
+					Timeout: "-10s",
+				},
+			},
+			wantError: false,
+		},
+		{
+			name: "endless command with timeout",
+			collector: &CollectHostRun{
+				hostCollector: &troubleshootv1beta2.HostRun{
+					HostCollectorMeta: troubleshootv1beta2.HostCollectorMeta{
+						CollectorName: "endless command",
+					},
+					Command: "ping",
+					Args:    []string{"google.com"},
+					Timeout: "200ms",
+				},
+			},
+			wantError: false,
+		},
+	}
+
+	for _, test := range tests {
+		got, err := test.collector.Collect(nil)
+		if test.wantError {
+			assert.Error(t, err)
+		} else {
+			assert.NoError(t, err)
+			assert.NotNil(t, got)
+		}
+	}
+}

schemas/supportbundle-troubleshoot-v1beta2.json

@@ -11839,6 +11839,9 @@
                   },
                   "outputDir": {
                     "type": "string"
+                  },
+                  "timeout": {
+                    "type": "string"
                   }
                 }
               },