diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index c101f3f9371..111b22be6f7 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -65,6 +65,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. diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index a16297f2702..89d4b336a22 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -45,6 +45,7 @@ type FakeDockerClient struct { RemovedImages util.StringSet VersionInfo docker.Env Information docker.Env + ExecInspect *docker.ExecInspect } func (f *FakeDockerClient) ClearCalls() { @@ -285,6 +286,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 diff --git a/pkg/kubelet/dockertools/instrumented_docker.go b/pkg/kubelet/dockertools/instrumented_docker.go index 80efc57515a..8da521a6022 100644 --- a/pkg/kubelet/dockertools/instrumented_docker.go +++ b/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) +} diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index cea8c5e1065..71574f29ad9 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/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/capabilities" @@ -894,10 +895,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: diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 142cca1b183..0b6d2372958 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/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") + } +} diff --git a/pkg/probe/exec/exec.go b/pkg/probe/exec/exec.go index ce1858d0542..7365f98e07a 100644 --- a/pkg/probe/exec/exec.go +++ b/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 } diff --git a/pkg/probe/exec/exec_test.go b/pkg/probe/exec/exec_test.go index 7f631c9b4ef..cfa521bf46a 100644 --- a/pkg/probe/exec/exec_test.go +++ b/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 diff --git a/test/e2e/pods.go b/test/e2e/pods.go index 383472d3a93..f0414f35b97 100644 --- a/test/e2e/pods.go +++ b/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{