execwrapper: reduce general timeout to 100s

before a timeout of 1000 seconds was used which is longer than
the watchdog waits (currently 500 seconds)

also switch to time.Duration to make time durations clearer

Signed-off-by: Christoph Ostarek <[email protected]>
(cherry picked from commit 193147f)

Author: christoph-zededa

pkg/pillar/base/execwrapper.go

@@ -17,84 +17,57 @@ import (
 )
 
 const (
-	// Will wait for 1000s max for a call instead of waiting forever
-	maxTimeOutLimit = 1000
-	timeOutLimit    = 100
+	/*
+	 * execution timeout is supposed to be less than the watchdog timeout,
+	 * as otherwise the watchdog might fire and reboot the system before
+	 * the timeout fires
+	 * exceptions are when an executable is started from a different goroutine
+	 * source of the error timeout and the watchdog timeout:
+	 * error timeout: $ grep -r errorTime pkg/pillar/cmd/ | grep time
+	 * watchdog timeout: $ grep hv_watchdog_timer pkg/grub/rootfs.cfg
+	 */
+	timeoutLimit   = 3 * time.Minute
+	defaultTimeout = 100 * time.Second
 )
 
 // Command holds the necessary data to execute command
 type Command struct {
-	command   *exec.Cmd
-	log       *LogObject
-	agentName string
-	timeout   time.Duration
-	buffer    *bytes.Buffer
-	ctx       context.Context
+	command    *exec.Cmd
+	log        *LogObject
+	agentName  string
+	timeout    time.Duration
+	buffer     *bytes.Buffer
+	ctx        context.Context
+	errorMonad error
 }
 
 // Output runs the command and returns its standard output.
 // Any returned error will usually be of type *ExitError.
-// Waits for the exec call to finish for `maxTimeOutLimit` after which timeout error is returned
+// Waits for the exec call to finish for `defaultTimeout` after which timeout error is returned
 func (c *Command) Output() ([]byte, error) {
 	var buf bytes.Buffer
 	c.command.Stdout = &buf
 	c.buffer = &buf
-	c.timeout = maxTimeOutLimit
+	c.timeout = defaultTimeout
 	return c.execCommand()
 }
 
 // CombinedOutput runs the command and returns its combined standard output and standard error.
-// Waits for the exec call to finish for `maxTimeOutLimit` after which timeout error is returned
+// Waits for the exec call to finish for `defaultTimeout` after which timeout error is returned
 func (c *Command) CombinedOutput() ([]byte, error) {
 	var buf bytes.Buffer
 	c.command.Stdout = &buf
 	c.command.Stderr = &buf
 	c.buffer = &buf
-	c.timeout = maxTimeOutLimit
-	return c.execCommand()
-}
-
-// OutputWithTimeout waits for the exec call to finish for `timeOutLimit`
-// after which timeout error is returned
-func (c *Command) OutputWithTimeout() ([]byte, error) {
-	var buf bytes.Buffer
-	c.command.Stdout = &buf
-	c.buffer = &buf
-	c.timeout = timeOutLimit
-	return c.execCommand()
-}
-
-// CombinedOutputWithTimeout waits for the exec call to finish for `timeOutLimit`
-// after which timeout error is returned
-func (c *Command) CombinedOutputWithTimeout() ([]byte, error) {
-	var buf bytes.Buffer
-	c.command.Stdout = &buf
-	c.command.Stderr = &buf
-	c.buffer = &buf
-	c.timeout = timeOutLimit
-	return c.execCommand()
-}
-
-// OutputWithCustomTimeout accepts a custom timeout limit to wait for the exec call.
-func (c *Command) OutputWithCustomTimeout(timeout uint) ([]byte, error) {
-	var buf bytes.Buffer
-	c.command.Stdout = &buf
-	c.buffer = &buf
-	c.timeout = time.Duration(timeout)
-	return c.execCommand()
-}
-
-// CombinedOutputWithCustomTimeout accepts a custom timeout limit to wait for the exec call.
-func (c *Command) CombinedOutputWithCustomTimeout(timeout uint) ([]byte, error) {
-	var buf bytes.Buffer
-	c.command.Stdout = &buf
-	c.command.Stderr = &buf
-	c.buffer = &buf
-	c.timeout = time.Duration(timeout)
+	c.timeout = defaultTimeout
 	return c.execCommand()
 }
 
 func (c *Command) execCommand() ([]byte, error) {
+	if c.errorMonad != nil {
+		return nil, c.errorMonad
+	}
+
 	if c.log != nil {
 		c.log.Tracef("execCommand(%v)", c.command.Args)
 	}
@@ -108,7 +81,7 @@ func (c *Command) execCommand() ([]byte, error) {
 	stillRunning := time.NewTicker(25 * time.Second)
 	defer stillRunning.Stop()
 
-	waitTimer := time.NewTimer(c.timeout * time.Second)
+	waitTimer := time.NewTimer(c.timeout)
 	defer waitTimer.Stop()
 
 	if c.ctx == nil {
@@ -134,6 +107,23 @@ func (c *Command) execCommand() ([]byte, error) {
 	}
 }
 
+// WithLimitedTimeout set custom timeout for command
+func (c *Command) WithLimitedTimeout(timeout time.Duration) *Command {
+	c.timeout = timeout
+	if c.timeout > timeoutLimit {
+		c.errorMonad = fmt.Errorf("custom timeout (%v) is longer than watchdog timeout (%v)", c.timeout, defaultTimeout)
+	}
+
+	return c
+}
+
+// WithUnlimitedTimeout set custom timeout for command not bound to any limits for when run in a separate goroutine
+func (c *Command) WithUnlimitedTimeout(timeout time.Duration) *Command {
+	c.timeout = timeout
+
+	return c
+}
+
 // WithContext set context for command
 func (c *Command) WithContext(ctx context.Context) *Command {
 	c.ctx = ctx

pkg/pillar/base/execwrapper_test.go

@@ -0,0 +1,18 @@
+package base_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/lf-edge/eve/pkg/pillar/base"
+)
+
+func TestTimeoutLongerThanLimit(t *testing.T) {
+	t.Parallel()
+
+	timeout := 600 * time.Second
+	_, err := base.Exec(nil, "/bin/true").WithLimitedTimeout(timeout).CombinedOutput()
+	if err == nil {
+		t.Fatalf("Execution should fail with timeout too long, but succeeded")
+	}
+}

pkg/pillar/cmd/domainmgr/domainmgr.go

@@ -2658,7 +2658,7 @@ func mkisofs(output string, dir string) error {
 		dir,
 	}
 	log.Functionf("Calling command %s %v\n", cmd, args)
-	stdoutStderr, err := base.Exec(log, cmd, args...).CombinedOutput()
+	stdoutStderr, err := base.Exec(log, cmd, args...).WithUnlimitedTimeout(15 * time.Minute).CombinedOutput()
 	if err != nil {
 		errStr := fmt.Sprintf("mkisofs failed: %s",
 			string(stdoutStderr))

pkg/pillar/diskmetrics/diskmetrics.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/lf-edge/eve/pkg/pillar/base"
 	"github.com/lf-edge/eve/pkg/pillar/types"
@@ -93,7 +94,7 @@ func RolloutImgToBlock(ctx context.Context, log *base.LogObject, diskfile, outpu
 	// writeback cache instead of default unsafe, out of order enabled, skip file creation
 	// Timeout 2 hours
 	args := []string{"convert", "--target-is-zero", "-t", "writeback", "-W", "-n", "-O", outputFormat, diskfile, outputFile}
-	output, err := base.Exec(log, "/usr/bin/qemu-img", args...).WithContext(ctx).CombinedOutputWithCustomTimeout(432000)
+	output, err := base.Exec(log, "/usr/bin/qemu-img", args...).WithContext(ctx).WithUnlimitedTimeout(15 * time.Minute).CombinedOutput()
 	if err != nil {
 		errStr := fmt.Sprintf("qemu-img failed: %s, %s\n",
 			err, output)

pkg/pillar/netmonitor/linux.go

@@ -257,7 +257,7 @@ func (m *LinuxNetworkMonitor) GetInterfaceDHCPInfo(ifIndex int) (info DHCPInfo,
 	// XXX Getting error -1 unless we add argument -4.
 	// XXX Add IPv6 support.
 	m.Log.Functionf("Calling dhcpcd -U -4 %s\n", ifName)
-	stdoutStderr, err := base.Exec(m.Log, "dhcpcd", "-U", "-4", ifName).CombinedOutputWithTimeout()
+	stdoutStderr, err := base.Exec(m.Log, "dhcpcd", "-U", "-4", ifName).CombinedOutput()
 	if err != nil {
 		if strings.Contains(string(stdoutStderr), "dhcp_dump: No such file or directory") {
 			// DHCP is not configured for this interface. Return empty DHCPInfo.