Merge pull request #13129 from flouthoc/healthcheck-session-read-from-pipe

healthcheck, libpod: Read healthcheck event output from os pipe

3 files changed, 46 insertions, 26 deletions

diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go
index 53bad47b4..40af9aec3 100644
--- a/libpod/healthcheck.go
+++ b/libpod/healthcheck.go
@@ -2,7 +2,6 @@ package libpod
 
 import (
 	"bufio"
-	"bytes"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -22,16 +21,6 @@ const (
 	MaxHealthCheckLogLength = 500
 )
 
-// hcWriteCloser allows us to use bufio as a WriteCloser
-type hcWriteCloser struct {
-	*bufio.Writer
-}
-
-// Used to add a closer to bufio
-func (hcwc hcWriteCloser) Close() error {
-	return nil
-}
-
 // HealthCheck verifies the state and validity of the healthcheck configuration
 // on the container and then executes the healthcheck
 func (r *Runtime) HealthCheck(name string) (define.HealthCheckStatus, error) {
@@ -51,7 +40,6 @@ func (c *Container) runHealthCheck() (define.HealthCheckStatus, error) {
 	var (
 		newCommand    []string
 		returnCode    int
-		capture       bytes.Buffer
 		inStartPeriod bool
 	)
 	hcCommand := c.HealthCheckConfig().Test
@@ -73,20 +61,30 @@ func (c *Container) runHealthCheck() (define.HealthCheckStatus, error) {
 	if len(newCommand) < 1 || newCommand[0] == "" {
 		return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
 	}
-	captureBuffer := bufio.NewWriter(&capture)
-	hcw := hcWriteCloser{
-		captureBuffer,
+	rPipe, wPipe, err := os.Pipe()
+	if err != nil {
+		return define.HealthCheckInternalError, errors.Wrapf(err, "unable to create pipe for healthcheck session")
 	}
+	defer wPipe.Close()
+	defer rPipe.Close()
+
 	streams := new(define.AttachStreams)
-	streams.OutputStream = hcw
-	streams.ErrorStream = hcw
 
 	streams.InputStream = bufio.NewReader(os.Stdin)
-
+	streams.OutputStream = wPipe
+	streams.ErrorStream = wPipe
 	streams.AttachOutput = true
 	streams.AttachError = true
 	streams.AttachInput = true
 
+	stdout := []string{}
+	go func() {
+		scanner := bufio.NewScanner(rPipe)
+		for scanner.Scan() {
+			stdout = append(stdout, scanner.Text())
+		}
+	}()
+
 	logrus.Debugf("executing health check command %s for %s", strings.Join(newCommand, " "), c.ID())
 	timeStart := time.Now()
 	hcResult := define.HealthCheckSuccess
@@ -119,7 +117,7 @@ func (c *Container) runHealthCheck() (define.HealthCheckStatus, error) {
 		}
 	}
 
-	eventLog := capture.String()
+	eventLog := strings.Join(stdout, "\n")
 	if len(eventLog) > MaxHealthCheckLogLength {
 		eventLog = eventLog[:MaxHealthCheckLogLength]
 	}
diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go
index c84488145..866edbf0e 100644
--- a/test/e2e/healthcheck_run_test.go
+++ b/test/e2e/healthcheck_run_test.go
@@ -54,6 +54,28 @@ var _ = Describe("Podman healthcheck run", func() {
 		Expect(hc).Should(Exit(125))
 	})
 
+	It("podman run healthcheck and logs should contain healthcheck output", func() {
+		session := podmanTest.Podman([]string{"run", "--name", "test-logs", "-dt", "--health-interval", "1s", "--health-cmd", "echo working", "busybox", "sleep", "3600"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		// Buy a little time to get container running
+		for i := 0; i < 5; i++ {
+			hc := podmanTest.Podman([]string{"healthcheck", "run", "test-logs"})
+			hc.WaitWithDefaultTimeout()
+			exitCode := hc.ExitCode()
+			if exitCode == 0 || i == 4 {
+				break
+			}
+			time.Sleep(1 * time.Second)
+		}
+
+		hc := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.State.Healthcheck.Log}}", "test-logs"})
+		hc.WaitWithDefaultTimeout()
+		Expect(hc).Should(Exit(0))
+		Expect(hc.OutputToString()).To(ContainSubstring("working"))
+	})
+
 	It("podman healthcheck from image's config (not container config)", func() {
 		// Regression test for #12226: a health check may be defined in
 		// the container or the container-config of an image.
diff --git a/test/system/220-healthcheck.bats b/test/system/220-healthcheck.bats
index 1d4a2ea7e..6159a2807 100644
--- a/test/system/220-healthcheck.bats
+++ b/test/system/220-healthcheck.bats
@@ -18,7 +18,7 @@ function _check_health {
         # (kludge to deal with parse_table and empty strings)
         if [ "$expect" = "''" ]; then expect=""; fi
 
-        actual=$(jq -r ".$field" <<<"$output")
+        actual=$(jq ".$field" <<<"$output")
         is "$actual" "$expect" "$testname - .State.Healthcheck.$field"
     done
 }
@@ -77,10 +77,10 @@ EOF
     is "$output" "" "output from 'podman healthcheck run'"
 
     _check_health "All healthy" "
-Status           | healthy
+Status           | \"healthy\"
 FailingStreak    | 0
 Log[-1].ExitCode | 0
-Log[-1].Output   |
+Log[-1].Output   | \"Life is Good on stdout\\\nLife is Good on stderr\"
 "
 
     # Force a failure
@@ -88,19 +88,19 @@ Log[-1].Output   |
     sleep 2
 
     _check_health "First failure" "
-Status           | healthy
+Status           | \"healthy\"
 FailingStreak    | [123]
 Log[-1].ExitCode | 1
-Log[-1].Output   |
+Log[-1].Output   | \"Uh-oh on stdout!\\\nUh-oh on stderr!\"
 "
 
     # After three successive failures, container should no longer be healthy
     sleep 5
     _check_health "Three or more failures" "
-Status           | unhealthy
+Status           | \"unhealthy\"
 FailingStreak    | [3456]
 Log[-1].ExitCode | 1
-Log[-1].Output   |
+Log[-1].Output   | \"Uh-oh on stdout!\\\nUh-oh on stderr!\"
 "
 
     # healthcheck should now fail, with exit status 1 and 'unhealthy' output