Merge pull request #63332 from zhouhaibing089/exec-timeout

Automatic merge from submit-queue (batch tested with PRs 63792, 63495, 63742, 63332, 63779). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

add timeout for exec interface

This should get us away from situations like https://github.com/kubernetes/kubernetes/issues/63331.

A little bit more context, the `os/exec` package starts to accept `context.Context` in golang 1.7. We should leverage that so we can have a more predictable behavior, then. 

```release-note
NONE
```

Godeps/Godeps.json

@@ -3331,15 +3331,15 @@
 		},
 		{
 			"ImportPath": "k8s.io/utils/clock",
-			"Rev": "aedf551cdb8b0119df3a19c65fde413a13b34997"
+			"Rev": "258e2a2fa64568210fbd6267cf1d8fd87c3cb86e"
 		},
 		{
 			"ImportPath": "k8s.io/utils/exec",
-			"Rev": "aedf551cdb8b0119df3a19c65fde413a13b34997"
+			"Rev": "258e2a2fa64568210fbd6267cf1d8fd87c3cb86e"
 		},
 		{
 			"ImportPath": "k8s.io/utils/exec/testing",
-			"Rev": "aedf551cdb8b0119df3a19c65fde413a13b34997"
+			"Rev": "258e2a2fa64568210fbd6267cf1d8fd87c3cb86e"
 		},
 		{
 			"ImportPath": "vbom.ml/util/sortorder",

pkg/util/iptables/iptables.go

@@ -18,10 +18,12 @@ package iptables
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"regexp"
 	"strings"
 	"sync"
+	"time"
 
 	godbus "github.com/godbus/dbus"
 	"github.com/golang/glog"
@@ -413,11 +415,18 @@ func iptablesCommand(protocol Protocol) string {
 }
 
 func (runner *runner) run(op operation, args []string) ([]byte, error) {
+	return runner.runContext(nil, op, args)
+}
+
+func (runner *runner) runContext(ctx context.Context, op operation, args []string) ([]byte, error) {
 	iptablesCmd := iptablesCommand(runner.protocol)
 	fullArgs := append(runner.waitFlag, string(op))
 	fullArgs = append(fullArgs, args...)
 	glog.V(5).Infof("running iptables %s %v", string(op), args)
-	return runner.exec.Command(iptablesCmd, fullArgs...).CombinedOutput()
+	if ctx == nil {
+		return runner.exec.Command(iptablesCmd, fullArgs...).CombinedOutput()
+	}
+	return runner.exec.CommandContext(ctx, iptablesCmd, fullArgs...).CombinedOutput()
 	// Don't log err here - callers might not think it is an error.
 }
 
@@ -426,9 +435,8 @@ func (runner *runner) run(op operation, args []string) ([]byte, error) {
 func (runner *runner) checkRule(table Table, chain Chain, args ...string) (bool, error) {
 	if runner.hasCheck {
 		return runner.checkRuleUsingCheck(makeFullArgs(table, chain, args...))
-	} else {
-		return runner.checkRuleWithoutCheck(table, chain, args...)
 	}
+	return runner.checkRuleWithoutCheck(table, chain, args...)
 }
 
 var hexnumRE = regexp.MustCompile("0x0+([0-9])")
@@ -489,7 +497,13 @@ func (runner *runner) checkRuleWithoutCheck(table Table, chain Chain, args ...st
 
 // Executes the rule check using the "-C" flag
 func (runner *runner) checkRuleUsingCheck(args []string) (bool, error) {
-	out, err := runner.run(opCheckRule, args)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	out, err := runner.runContext(ctx, opCheckRule, args)
+	if ctx.Err() == context.DeadlineExceeded {
+		return false, fmt.Errorf("timed out while checking rules")
+	}
 	if err == nil {
 		return true, nil
 	}

vendor/k8s.io/utils/exec/exec.go

@@ -17,6 +17,7 @@ limitations under the License.
 package exec
 
 import (
+	"context"
 	"io"
 	osexec "os/exec"
 	"syscall"
@@ -33,6 +34,13 @@ type Interface interface {
 	// This follows the pattern of package os/exec.
 	Command(cmd string, args ...string) Cmd
 
+	// CommandContext returns a Cmd instance which can be used to run a single command.
+	//
+	// The provided context is used to kill the process if the context becomes done
+	// before the command completes on its own. For example, a timeout can be set in
+	// the context.
+	CommandContext(ctx context.Context, cmd string, args ...string) Cmd
+
 	// LookPath wraps os/exec.LookPath
 	LookPath(file string) (string, error)
 }
@@ -82,6 +90,11 @@ func (executor *executor) Command(cmd string, args ...string) Cmd {
 	return (*cmdWrapper)(osexec.Command(cmd, args...))
 }
 
+// CommandContext is part of the Interface interface.
+func (executor *executor) CommandContext(ctx context.Context, cmd string, args ...string) Cmd {
+	return (*cmdWrapper)(osexec.CommandContext(ctx, cmd, args...))
+}
+
 // LookPath is part of the Interface interface
 func (executor *executor) LookPath(file string) (string, error) {
 	return osexec.LookPath(file)
@@ -110,52 +123,52 @@ func (cmd *cmdWrapper) SetStderr(out io.Writer) {
 
 // Run is part of the Cmd interface.
 func (cmd *cmdWrapper) Run() error {
-	return (*osexec.Cmd)(cmd).Run()
+	err := (*osexec.Cmd)(cmd).Run()
+	return handleError(err)
 }
 
 // CombinedOutput is part of the Cmd interface.
 func (cmd *cmdWrapper) CombinedOutput() ([]byte, error) {
 	out, err := (*osexec.Cmd)(cmd).CombinedOutput()
-	if err != nil {
+	return out, handleError(err)
-		return out, handleError(err)
-	}
-	return out, nil
 }
 
 func (cmd *cmdWrapper) Output() ([]byte, error) {
 	out, err := (*osexec.Cmd)(cmd).Output()
-	if err != nil {
+	return out, handleError(err)
-		return out, handleError(err)
-	}
-	return out, nil
 }
 
 // Stop is part of the Cmd interface.
 func (cmd *cmdWrapper) Stop() {
 	c := (*osexec.Cmd)(cmd)
-	if c.ProcessState.Exited() {
+
+	if c.Process == nil {
 		return
 	}
+
 	c.Process.Signal(syscall.SIGTERM)
+
 	time.AfterFunc(10*time.Second, func() {
-		if c.ProcessState.Exited() {
+		if !c.ProcessState.Exited() {
-			return
+			c.Process.Signal(syscall.SIGKILL)
 		}
-		c.Process.Signal(syscall.SIGKILL)
 	})
 }
 
 func handleError(err error) error {
-	if ee, ok := err.(*osexec.ExitError); ok {
+	if err == nil {
-		// Force a compile fail if exitErrorWrapper can't convert to ExitError.
+		return nil
-		var x ExitError = &ExitErrorWrapper{ee}
-		return x
 	}
-	if ee, ok := err.(*osexec.Error); ok {
+
-		if ee.Err == osexec.ErrNotFound {
+	switch e := err.(type) {
+	case *osexec.ExitError:
+		return &ExitErrorWrapper{e}
+	case *osexec.Error:
+		if e.Err == osexec.ErrNotFound {
 			return ErrExecutableNotFound
 		}
 	}
+
 	return err
 }
 
@@ -165,7 +178,7 @@ type ExitErrorWrapper struct {
 	*osexec.ExitError
 }
 
-var _ ExitError = ExitErrorWrapper{}
+var _ ExitError = &ExitErrorWrapper{}
 
 // ExitStatus is part of the ExitError interface.
 func (eew ExitErrorWrapper) ExitStatus() int {

vendor/k8s.io/utils/exec/testing/fake_exec.go

@@ -17,6 +17,7 @@ limitations under the License.
 package testingexec
 
 import (
+	"context"
 	"fmt"
 	"io"
 
@@ -30,6 +31,8 @@ type FakeExec struct {
 	LookPathFunc  func(string) (string, error)
 }
 
+var _ exec.Interface = &FakeExec{}
+
 type FakeCommandAction func(cmd string, args ...string) exec.Cmd
 
 func (fake *FakeExec) Command(cmd string, args ...string) exec.Cmd {
@@ -41,6 +44,10 @@ func (fake *FakeExec) Command(cmd string, args ...string) exec.Cmd {
 	return fake.CommandScript[i](cmd, args...)
 }
 
+func (fake *FakeExec) CommandContext(ctx context.Context, cmd string, args ...string) exec.Cmd {
+	return fake.Command(cmd, args...)
+}
+
 func (fake *FakeExec) LookPath(file string) (string, error) {
 	return fake.LookPathFunc(file)
 }