Merge pull request #6304 from baude/v2remotehctests

Fix remote integration for healthchecks

7 files changed, 71 insertions, 74 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 909ad9851..43e873bd6 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1169,7 +1169,7 @@ func (c *Container) start() error {
 	c.state.State = define.ContainerStateRunning
 
 	if c.config.HealthCheckConfig != nil {
-		if err := c.updateHealthStatus(HealthCheckStarting); err != nil {
+		if err := c.updateHealthStatus(define.HealthCheckStarting); err != nil {
 			logrus.Error(err)
 		}
 		if err := c.startTimer(); err != nil {
diff --git a/libpod/define/healthchecks.go b/libpod/define/healthchecks.go
new file mode 100644
index 000000000..4114262b6
--- /dev/null
+++ b/libpod/define/healthchecks.go
@@ -0,0 +1,36 @@
+package define
+
+const (
+	// HealthCheckHealthy describes a healthy container
+	HealthCheckHealthy string = "healthy"
+	// HealthCheckUnhealthy describes an unhealthy container
+	HealthCheckUnhealthy string = "unhealthy"
+	// HealthCheckStarting describes the time between when the container starts
+	// and the start-period (time allowed for the container to start and application
+	// to be running) expires.
+	HealthCheckStarting string = "starting"
+)
+
+// HealthCheckStatus represents the current state of a container
+type HealthCheckStatus int
+
+const (
+	// HealthCheckSuccess means the health worked
+	HealthCheckSuccess HealthCheckStatus = iota
+	// HealthCheckFailure means the health ran and failed
+	HealthCheckFailure HealthCheckStatus = iota
+	// HealthCheckContainerStopped means the health check cannot
+	// be run because the container is stopped
+	HealthCheckContainerStopped HealthCheckStatus = iota
+	// HealthCheckContainerNotFound means the container could
+	// not be found in local store
+	HealthCheckContainerNotFound HealthCheckStatus = iota
+	// HealthCheckNotDefined means the container has no health
+	// check defined in it
+	HealthCheckNotDefined HealthCheckStatus = iota
+	// HealthCheckInternalError means some something failed obtaining or running
+	// a given health check
+	HealthCheckInternalError HealthCheckStatus = iota
+	// HealthCheckDefined means the healthcheck was found on the container
+	HealthCheckDefined HealthCheckStatus = iota
+)
diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go
index aec5fa4e0..0006b7c06 100644
--- a/libpod/healthcheck.go
+++ b/libpod/healthcheck.go
@@ -14,43 +14,12 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// HealthCheckStatus represents the current state of a container
-type HealthCheckStatus int
-
 const (
-	// HealthCheckSuccess means the health worked
-	HealthCheckSuccess HealthCheckStatus = iota
-	// HealthCheckFailure means the health ran and failed
-	HealthCheckFailure HealthCheckStatus = iota
-	// HealthCheckContainerStopped means the health check cannot
-	// be run because the container is stopped
-	HealthCheckContainerStopped HealthCheckStatus = iota
-	// HealthCheckContainerNotFound means the container could
-	// not be found in local store
-	HealthCheckContainerNotFound HealthCheckStatus = iota
-	// HealthCheckNotDefined means the container has no health
-	// check defined in it
-	HealthCheckNotDefined HealthCheckStatus = iota
-	// HealthCheckInternalError means some something failed obtaining or running
-	// a given health check
-	HealthCheckInternalError HealthCheckStatus = iota
-	// HealthCheckDefined means the healthcheck was found on the container
-	HealthCheckDefined HealthCheckStatus = iota
-
 	// MaxHealthCheckNumberLogs is the maximum number of attempts we keep
 	// in the healthcheck history file
 	MaxHealthCheckNumberLogs int = 5
 	// MaxHealthCheckLogLength in characters
 	MaxHealthCheckLogLength = 500
-
-	// HealthCheckHealthy describes a healthy container
-	HealthCheckHealthy string = "healthy"
-	// HealthCheckUnhealthy describes an unhealthy container
-	HealthCheckUnhealthy string = "unhealthy"
-	// HealthCheckStarting describes the time between when the container starts
-	// and the start-period (time allowed for the container to start and application
-	// to be running) expires.
-	HealthCheckStarting string = "starting"
 )
 
 // hcWriteCloser allows us to use bufio as a WriteCloser
@@ -65,10 +34,10 @@ func (hcwc hcWriteCloser) Close() error {
 
 // HealthCheck verifies the state and validity of the healthcheck configuration
 // on the container and then executes the healthcheck
-func (r *Runtime) HealthCheck(name string) (HealthCheckStatus, error) {
+func (r *Runtime) HealthCheck(name string) (define.HealthCheckStatus, error) {
 	container, err := r.LookupContainer(name)
 	if err != nil {
-		return HealthCheckContainerNotFound, errors.Wrapf(err, "unable to lookup %s to perform a health check", name)
+		return define.HealthCheckContainerNotFound, errors.Wrapf(err, "unable to lookup %s to perform a health check", name)
 	}
 	hcStatus, err := checkHealthCheckCanBeRun(container)
 	if err == nil {
@@ -78,7 +47,7 @@ func (r *Runtime) HealthCheck(name string) (HealthCheckStatus, error) {
 }
 
 // runHealthCheck runs the health check as defined by the container
-func (c *Container) runHealthCheck() (HealthCheckStatus, error) {
+func (c *Container) runHealthCheck() (define.HealthCheckStatus, error) {
 	var (
 		newCommand    []string
 		returnCode    int
@@ -87,11 +56,11 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) {
 	)
 	hcCommand := c.HealthCheckConfig().Test
 	if len(hcCommand) < 1 {
-		return HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
+		return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
 	}
 	switch hcCommand[0] {
 	case "", "NONE":
-		return HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
+		return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
 	case "CMD":
 		newCommand = hcCommand[1:]
 	case "CMD-SHELL":
@@ -102,7 +71,7 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) {
 		newCommand = hcCommand
 	}
 	if len(newCommand) < 1 || newCommand[0] == "" {
-		return HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
+		return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
 	}
 	captureBuffer := bufio.NewWriter(&capture)
 	hcw := hcWriteCloser{
@@ -120,13 +89,13 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) {
 
 	logrus.Debugf("executing health check command %s for %s", strings.Join(newCommand, " "), c.ID())
 	timeStart := time.Now()
-	hcResult := HealthCheckSuccess
+	hcResult := define.HealthCheckSuccess
 	config := new(ExecConfig)
 	config.Command = newCommand
 	_, hcErr := c.Exec(config, streams, nil)
 	if hcErr != nil {
 		errCause := errors.Cause(hcErr)
-		hcResult = HealthCheckFailure
+		hcResult = define.HealthCheckFailure
 		if errCause == define.ErrOCIRuntimeNotFound ||
 			errCause == define.ErrOCIRuntimePermissionDenied ||
 			errCause == define.ErrOCIRuntime {
@@ -154,7 +123,7 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) {
 
 	if timeEnd.Sub(timeStart) > c.HealthCheckConfig().Timeout {
 		returnCode = -1
-		hcResult = HealthCheckFailure
+		hcResult = define.HealthCheckFailure
 		hcErr = errors.Errorf("healthcheck command exceeded timeout of %s", c.HealthCheckConfig().Timeout.String())
 	}
 	hcl := newHealthCheckLog(timeStart, timeEnd, returnCode, eventLog)
@@ -164,18 +133,18 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) {
 	return hcResult, hcErr
 }
 
-func checkHealthCheckCanBeRun(c *Container) (HealthCheckStatus, error) {
+func checkHealthCheckCanBeRun(c *Container) (define.HealthCheckStatus, error) {
 	cstate, err := c.State()
 	if err != nil {
-		return HealthCheckInternalError, err
+		return define.HealthCheckInternalError, err
 	}
 	if cstate != define.ContainerStateRunning {
-		return HealthCheckContainerStopped, errors.Errorf("container %s is not running", c.ID())
+		return define.HealthCheckContainerStopped, errors.Errorf("container %s is not running", c.ID())
 	}
 	if !c.HasHealthCheck() {
-		return HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
+		return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID())
 	}
-	return HealthCheckDefined, nil
+	return define.HealthCheckDefined, nil
 }
 
 func newHealthCheckLog(start, end time.Time, exitCode int, log string) define.HealthCheckLog {
@@ -210,18 +179,18 @@ func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, inStartPerio
 	}
 	if hcl.ExitCode == 0 {
 		//	set status to healthy, reset failing state to 0
-		healthCheck.Status = HealthCheckHealthy
+		healthCheck.Status = define.HealthCheckHealthy
 		healthCheck.FailingStreak = 0
 	} else {
 		if len(healthCheck.Status) < 1 {
-			healthCheck.Status = HealthCheckHealthy
+			healthCheck.Status = define.HealthCheckHealthy
 		}
 		if !inStartPeriod {
 			// increment failing streak
 			healthCheck.FailingStreak += 1
 			// if failing streak > retries, then status to unhealthy
 			if healthCheck.FailingStreak >= c.HealthCheckConfig().Retries {
-				healthCheck.Status = HealthCheckUnhealthy
+				healthCheck.Status = define.HealthCheckUnhealthy
 			}
 		}
 	}
diff --git a/pkg/api/handlers/libpod/healthcheck.go b/pkg/api/handlers/libpod/healthcheck.go
index 6eb2ab0e3..0ca3574b7 100644
--- a/pkg/api/handlers/libpod/healthcheck.go
+++ b/pkg/api/handlers/libpod/healthcheck.go
@@ -4,6 +4,7 @@ import (
 	"net/http"
 
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/api/handlers/utils"
 )
 
@@ -12,32 +13,27 @@ func RunHealthCheck(w http.ResponseWriter, r *http.Request) {
 	name := utils.GetName(r)
 	status, err := runtime.HealthCheck(name)
 	if err != nil {
-		if status == libpod.HealthCheckContainerNotFound {
+		if status == define.HealthCheckContainerNotFound {
 			utils.ContainerNotFound(w, name, err)
 			return
 		}
-		if status == libpod.HealthCheckNotDefined {
+		if status == define.HealthCheckNotDefined {
 			utils.Error(w, "no healthcheck defined", http.StatusConflict, err)
 			return
 		}
-		if status == libpod.HealthCheckContainerStopped {
+		if status == define.HealthCheckContainerStopped {
 			utils.Error(w, "container not running", http.StatusConflict, err)
 			return
 		}
 		utils.InternalServerError(w, err)
 		return
 	}
-	ctr, err := runtime.LookupContainer(name)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
+	hcStatus := define.HealthCheckUnhealthy
+	if status == define.HealthCheckSuccess {
+		hcStatus = define.HealthCheckHealthy
 	}
-
-	hcLog, err := ctr.GetHealthCheckLog()
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
+	report := define.HealthCheckResults{
+		Status: hcStatus,
 	}
-
-	utils.WriteResponse(w, http.StatusOK, hcLog)
+	utils.WriteResponse(w, http.StatusOK, report)
 }
diff --git a/pkg/domain/infra/abi/healthcheck.go b/pkg/domain/infra/abi/healthcheck.go
index 351bf4f7e..4e925ef56 100644
--- a/pkg/domain/infra/abi/healthcheck.go
+++ b/pkg/domain/infra/abi/healthcheck.go
@@ -3,7 +3,6 @@ package abi
 import (
 	"context"
 
-	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/domain/entities"
 )
@@ -13,9 +12,9 @@ func (ic *ContainerEngine) HealthCheckRun(ctx context.Context, nameOrId string,
 	if err != nil {
 		return nil, err
 	}
-	hcStatus := "unhealthy"
-	if status == libpod.HealthCheckSuccess {
-		hcStatus = "healthy"
+	hcStatus := define.HealthCheckUnhealthy
+	if status == define.HealthCheckSuccess {
+		hcStatus = define.HealthCheckHealthy
 	}
 	report := define.HealthCheckResults{
 		Status: hcStatus,
diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go
index 258cb8652..291353cad 100644
--- a/pkg/varlinkapi/containers.go
+++ b/pkg/varlinkapi/containers.go
@@ -901,12 +901,12 @@ func (i *VarlinkAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.Exec
 // HealthCheckRun executes defined container's healthcheck command and returns the container's health status.
 func (i *VarlinkAPI) HealthCheckRun(call iopodman.VarlinkCall, nameOrID string) error {
 	hcStatus, err := i.Runtime.HealthCheck(nameOrID)
-	if err != nil && hcStatus != libpod.HealthCheckFailure {
+	if err != nil && hcStatus != define.HealthCheckFailure {
 		return call.ReplyErrorOccurred(err.Error())
 	}
-	status := libpod.HealthCheckUnhealthy
-	if hcStatus == libpod.HealthCheckSuccess {
-		status = libpod.HealthCheckHealthy
+	status := define.HealthCheckUnhealthy
+	if hcStatus == define.HealthCheckSuccess {
+		status = define.HealthCheckHealthy
 	}
 	return call.ReplyHealthCheckRun(status)
 }
diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go
index f434836d3..8e63d9f4c 100644
--- a/test/e2e/healthcheck_run_test.go
+++ b/test/e2e/healthcheck_run_test.go
@@ -83,7 +83,6 @@ var _ = Describe("Podman healthcheck run", func() {
 	})
 
 	It("podman healthcheck that should fail", func() {
-		Skip(v2remotefail)
 		session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "docker.io/libpod/badhealthcheck:latest"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
@@ -122,7 +121,6 @@ var _ = Describe("Podman healthcheck run", func() {
 	})
 
 	It("podman healthcheck failed checks in start-period should not change status", func() {
-		Skip(v2remotefail)
 		session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-start-period", "2m", "--health-retries", "2", "--health-cmd", "ls /foo || exit 1", ALPINE, "top"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
@@ -144,7 +142,6 @@ var _ = Describe("Podman healthcheck run", func() {
 	})
 
 	It("podman healthcheck failed checks must reach retries before unhealthy ", func() {
-		Skip(v2remotefail)
 		session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-retries", "2", "--health-cmd", "ls /foo || exit 1", ALPINE, "top"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))