Skip to content

Commit

Permalink
Revert "execwrapper: reduce general timeout to 100s"
Browse files Browse the repository at this point in the history
This reverts commit b453e5d.
  • Loading branch information
rouming committed Oct 9, 2023
1 parent 026c749 commit be2973c
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 66 deletions.
98 changes: 54 additions & 44 deletions pkg/pillar/base/execwrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,57 +17,84 @@ import (
)

const (
/*
* 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
// Will wait for 1000s max for a call instead of waiting forever
maxTimeOutLimit = 1000
timeOutLimit = 100
)

// 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
errorMonad error
command *exec.Cmd
log *LogObject
agentName string
timeout time.Duration
buffer *bytes.Buffer
ctx context.Context
}

// 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 `defaultTimeout` after which timeout error is returned
// Waits for the exec call to finish for `maxTimeOutLimit` 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 = defaultTimeout
c.timeout = maxTimeOutLimit
return c.execCommand()
}

// CombinedOutput runs the command and returns its combined standard output and standard error.
// Waits for the exec call to finish for `defaultTimeout` after which timeout error is returned
// Waits for the exec call to finish for `maxTimeOutLimit` 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 = defaultTimeout
c.timeout = maxTimeOutLimit
return c.execCommand()
}

func (c *Command) execCommand() ([]byte, error) {
if c.errorMonad != nil {
return nil, c.errorMonad
}
// 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)
return c.execCommand()
}

func (c *Command) execCommand() ([]byte, error) {
if c.log != nil {
c.log.Tracef("execCommand(%v)", c.command.Args)
}
Expand All @@ -81,7 +108,7 @@ func (c *Command) execCommand() ([]byte, error) {
stillRunning := time.NewTicker(25 * time.Second)
defer stillRunning.Stop()

waitTimer := time.NewTimer(c.timeout)
waitTimer := time.NewTimer(c.timeout * time.Second)
defer waitTimer.Stop()

if c.ctx == nil {
Expand All @@ -107,23 +134,6 @@ 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
Expand Down
18 changes: 0 additions & 18 deletions pkg/pillar/base/execwrapper_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/pillar/cmd/domainmgr/domainmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...).WithUnlimitedTimeout(15 * time.Minute).CombinedOutput()
stdoutStderr, err := base.Exec(log, cmd, args...).CombinedOutput()
if err != nil {
errStr := fmt.Sprintf("mkisofs failed: %s",
string(stdoutStderr))
Expand Down
3 changes: 1 addition & 2 deletions pkg/pillar/diskmetrics/diskmetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"os"
"strconv"
"strings"
"time"

"github.com/lf-edge/eve/pkg/pillar/base"
"github.com/lf-edge/eve/pkg/pillar/types"
Expand Down Expand Up @@ -94,7 +93,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).WithUnlimitedTimeout(15 * time.Minute).CombinedOutput()
output, err := base.Exec(log, "/usr/bin/qemu-img", args...).WithContext(ctx).CombinedOutputWithCustomTimeout(432000)
if err != nil {
errStr := fmt.Sprintf("qemu-img failed: %s, %s\n",
err, output)
Expand Down
2 changes: 1 addition & 1 deletion pkg/pillar/netmonitor/linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).CombinedOutput()
stdoutStderr, err := base.Exec(m.Log, "dhcpcd", "-U", "-4", ifName).CombinedOutputWithTimeout()
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.
Expand Down

0 comments on commit be2973c

Please sign in to comment.