Merge pull request #8011 from brendandburns/exec

Switch exec to look at exit code not output status.

pkg/kubelet/dockertools/docker.go

@@ -66,6 +66,7 @@ type DockerInterface interface {
 	Info() (*docker.Env, error)
 	CreateExec(docker.CreateExecOptions) (*docker.Exec, error)
 	StartExec(string, docker.StartExecOptions) error
+	InspectExec(id string) (*docker.ExecInspect, error)
 }
 
 // KubeletContainerName encapsulates a pod name and a Kubernetes container name.

pkg/kubelet/dockertools/fake_docker_client.go

@@ -47,6 +47,7 @@ type FakeDockerClient struct {
 	RemovedImages       util.StringSet
 	VersionInfo         docker.Env
 	Information         docker.Env
+	ExecInspect         *docker.ExecInspect
 }
 
 func (f *FakeDockerClient) ClearCalls() {
@@ -288,6 +289,10 @@ func (f *FakeDockerClient) StartExec(_ string, _ docker.StartExecOptions) error
 	return nil
 }
 
+func (f *FakeDockerClient) InspectExec(id string) (*docker.ExecInspect, error) {
+	return f.ExecInspect, f.popError("inspect_exec")
+}
+
 func (f *FakeDockerClient) ListImages(opts docker.ListImagesOptions) ([]docker.APIImages, error) {
 	err := f.popError("list_images")
 	return f.Images, err

pkg/kubelet/dockertools/instrumented_docker.go

@@ -153,3 +153,11 @@ func (in instrumentedDockerInterface) StartExec(startExec string, opts docker.St
 	}()
 	return in.client.StartExec(startExec, opts)
 }
+
+func (in instrumentedDockerInterface) InspectExec(id string) (*docker.ExecInspect, error) {
+	start := time.Now()
+	defer func() {
+		metrics.DockerOperationsLatency.WithLabelValues("inspect_exec").Observe(metrics.SinceInMicroseconds(start))
+	}()
+	return in.client.InspectExec(id)
+}

pkg/kubelet/dockertools/manager.go

@@ -28,6 +28,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/record"
@@ -874,10 +875,47 @@ func (dm *DockerManager) RunInContainer(containerID string, cmd []string) ([]byt
 		RawTerminal:  false,
 	}
 	err = dm.client.StartExec(execObj.ID, startOpts)
+	if err != nil {
+		return nil, err
+	}
+	tick := time.Tick(2 * time.Second)
+	for {
+		inspect, err2 := dm.client.InspectExec(execObj.ID)
+		if err2 != nil {
+			return buf.Bytes(), err2
+		}
+		if !inspect.Running {
+			if inspect.ExitCode != 0 {
+				err = &dockerExitError{inspect}
+			}
+			break
+		}
+		<-tick
+	}
 
 	return buf.Bytes(), err
 }
 
+type dockerExitError struct {
+	Inspect *docker.ExecInspect
+}
+
+func (d *dockerExitError) String() string {
+	return d.Error()
+}
+
+func (d *dockerExitError) Error() string {
+	return fmt.Sprintf("Error executing in Docker Container: %d", d.Inspect.ExitCode)
+}
+
+func (d *dockerExitError) Exited() bool {
+	return !d.Inspect.Running
+}
+
+func (d *dockerExitError) ExitStatus() int {
+	return d.Inspect.ExitCode
+}
+
 // ExecInContainer uses nsenter to run the command inside the container identified by containerID.
 //
 // TODO:

pkg/kubelet/dockertools/manager_test.go

@@ -619,3 +619,12 @@ func TestProbeContainer(t *testing.T) {
 		}
 	}
 }
+
+func TestIsAExitError(t *testing.T) {
+	var err error
+	err = &dockerExitError{nil}
+	_, ok := err.(uexec.ExitError)
+	if !ok {
+		t.Error("couldn't cast dockerExitError to exec.ExitError")
+	}
+}

pkg/probe/exec/exec.go

@@ -17,34 +17,35 @@ limitations under the License.
 package exec
 
 import (
-	"strings"
-
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/probe"
-	uexec "github.com/GoogleCloudPlatform/kubernetes/pkg/util/exec"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/exec"
 
 	"github.com/golang/glog"
 )
 
-const defaultHealthyOutput = "ok"
-
 func New() ExecProber {
 	return execProber{}
 }
 
 type ExecProber interface {
-	Probe(e uexec.Cmd) (probe.Result, error)
+	Probe(e exec.Cmd) (probe.Result, error)
 }
 
 type execProber struct{}
 
-func (pr execProber) Probe(e uexec.Cmd) (probe.Result, error) {
+func (pr execProber) Probe(e exec.Cmd) (probe.Result, error) {
 	data, err := e.CombinedOutput()
 	glog.V(4).Infof("health check response: %s", string(data))
 	if err != nil {
+		exit, ok := err.(exec.ExitError)
+		if ok {
+			if exit.ExitStatus() == 0 {
+				return probe.Success, nil
+			} else {
+				return probe.Failure, nil
+			}
+		}
 		return probe.Unknown, err
 	}
-	if !strings.HasPrefix(strings.ToLower(string(data)), defaultHealthyOutput) {
-		return probe.Failure, nil
-	}
 	return probe.Success, nil
 }

pkg/probe/exec/exec_test.go

@@ -41,16 +41,40 @@ type healthCheckTest struct {
 	err            error
 }
 
+type fakeExitError struct {
+	exited     bool
+	statusCode int
+}
+
+func (f *fakeExitError) String() string {
+	return f.Error()
+}
+
+func (f *fakeExitError) Error() string {
+	return "fake exit"
+}
+
+func (f *fakeExitError) Exited() bool {
+	return f.exited
+}
+
+func (f *fakeExitError) ExitStatus() int {
+	return f.statusCode
+}
+
 func TestExec(t *testing.T) {
 	prober := New()
 	fake := FakeCmd{}
+
 	tests := []healthCheckTest{
 		// Ok
 		{probe.Success, false, []byte("OK"), nil},
+		// Ok
+		{probe.Success, false, []byte("OK"), &fakeExitError{true, 0}},
 		// Run returns error
 		{probe.Unknown, true, []byte("OK, NOT"), fmt.Errorf("test error")},
 		// Unhealthy
-		{probe.Failure, false, []byte("Fail"), nil},
+		{probe.Failure, false, []byte("Fail"), &fakeExitError{true, 1}},
 	}
 	for _, test := range tests {
 		fake.out = test.output

test/e2e/pods.go

@@ -421,7 +421,7 @@ var _ = Describe("Pods", func() {
 					{
 						Name:    "liveness",
 						Image:   "gcr.io/google_containers/busybox",
-						Command: []string{"/bin/sh", "-c", "echo ok >/tmp/health; sleep 10; echo fail >/tmp/health; sleep 600"},
+						Command: []string{"/bin/sh", "-c", "echo ok >/tmp/health; sleep 10; rm -rf /tmp/health; sleep 600"},
 						LivenessProbe: &api.Probe{
 							Handler: api.Handler{
 								Exec: &api.ExecAction{