From 141b34f6bec792e136c82f1e2d56e848f04d0f32 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Wed, 20 May 2020 14:43:01 -0500 Subject: Fix remote integration for healthchecks the one remaining test that is still skipped do to missing exec function Signed-off-by: Brent Baude --- libpod/container_internal.go | 2 +- libpod/define/healthchecks.go | 36 +++++++++++++++++++ libpod/healthcheck.go | 65 +++++++++------------------------- pkg/api/handlers/libpod/healthcheck.go | 24 ++++++------- pkg/domain/infra/abi/healthcheck.go | 7 ++-- pkg/varlinkapi/containers.go | 8 ++--- test/e2e/healthcheck_run_test.go | 3 -- 7 files changed, 71 insertions(+), 74 deletions(-) create mode 100644 libpod/define/healthchecks.go 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)) -- cgit v1.2.3-54-g00ecf