From dd0ea08cefce43b4ca5e468a4bafb909f877cd58 Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Tue, 9 Jul 2019 20:19:20 +0300 Subject: healthcheck: improve command list parser - remove duplicate check, already called in HealthCheck() - reject zero-length command list and empty command string as errorneous - support all Docker command list keywords: NONE, CMD or CMD-SHELL - use Docker default "/bin/sh -c" for CMD-SHELL Fixes #3507 Signed-off-by: Stefan Becker --- libpod/healthcheck.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index f4ea6c694..e9bf984e2 100644 --- a/libpod/healthcheck.go +++ b/libpod/healthcheck.go @@ -107,14 +107,20 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) { capture bytes.Buffer inStartPeriod bool ) - hcStatus, err := checkHealthCheckCanBeRun(c) - if err != nil { - return hcStatus, err - } hcCommand := c.HealthCheckConfig().Test - if len(hcCommand) > 0 && hcCommand[0] == "CMD-SHELL" { - newCommand = []string{"sh", "-c", strings.Join(hcCommand[1:], " ")} - } else { + if len(hcCommand) < 1 { + return 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()) + case "CMD": + newCommand = hcCommand[1:] + case "CMD-SHELL": + // TODO: SHELL command from image not available in Container - use Docker default + newCommand = []string{"/bin/sh", "-c", strings.Join(hcCommand[1:], " ")} + default: + // command supplied on command line - pass as-is newCommand = hcCommand } captureBuffer := bufio.NewWriter(&capture) -- cgit v1.2.3-54-g00ecf From 33001a9028d0ad12d2992dda295fd570714ca237 Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Tue, 9 Jul 2019 21:14:36 +0300 Subject: create: apply defaults on image healthcheck options If the image doesn't provide any options, e.g. interval, timeout, etc., then apply the Docker defaults when creating the container. Otherwise the defaults will be left 0 and podman doesn't schedule the healtcheck service & timer for the container or incorrectly reports unhealthy state when the check is executed. Fixes #3525 Signed-off-by: Stefan Becker --- cmd/podman/shared/create.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index 17455c4c9..9ce20047b 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -115,6 +115,24 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod. if err != nil { return nil, nil, errors.Wrapf(err, "unable to get healthcheck for %s", c.InputArgs[0]) } + + if healthCheck != nil { + // apply defaults if image doesn't override them + if healthCheck.Interval == 0 { + healthCheck.Interval = 30 * time.Second + } + if healthCheck.Timeout == 0 { + healthCheck.Timeout = 30 * time.Second + } + /* Docker default is 0s, so the following would be a no-op + if healthCheck.StartPeriod == 0 { + healthCheck.StartPeriod = 0 * time.Second + } + */ + if healthCheck.Retries == 0 { + healthCheck.Retries = 3 + } + } } } } -- cgit v1.2.3-54-g00ecf From 5caf218b1162d6b24ac898b0e8ff730f5dcafe23 Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Wed, 10 Jul 2019 08:40:58 +0300 Subject: create: ignore check if image has HEALTHCHECK NONE If the image was built with "HEALTHCHECK NONE" then we should create a container without healthcheck configuration. Otherwise executing the healthcheck on the container will return "unhealthy" instead of the correct error message that the container doesn't have a healthcheck. We also ignore the healthcheck configuration if the command list is empty or the command string is empty. Fixes #3525 Signed-off-by: Stefan Becker --- cmd/podman/shared/create.go | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index 9ce20047b..fd319e215 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -117,20 +117,26 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod. } if healthCheck != nil { - // apply defaults if image doesn't override them - if healthCheck.Interval == 0 { - healthCheck.Interval = 30 * time.Second - } - if healthCheck.Timeout == 0 { - healthCheck.Timeout = 30 * time.Second - } - /* Docker default is 0s, so the following would be a no-op - if healthCheck.StartPeriod == 0 { - healthCheck.StartPeriod = 0 * time.Second - } - */ - if healthCheck.Retries == 0 { - healthCheck.Retries = 3 + hcCommand := healthCheck.Test + if len(hcCommand) < 1 || hcCommand[0] == "" || hcCommand[0] == "NONE" { + // disable health check + healthCheck = nil + } else { + // apply defaults if image doesn't override them + if healthCheck.Interval == 0 { + healthCheck.Interval = 30 * time.Second + } + if healthCheck.Timeout == 0 { + healthCheck.Timeout = 30 * time.Second + } + /* Docker default is 0s, so the following would be a no-op + if healthCheck.StartPeriod == 0 { + healthCheck.StartPeriod = 0 * time.Second + } + */ + if healthCheck.Retries == 0 { + healthCheck.Retries = 3 + } } } } -- cgit v1.2.3-54-g00ecf From 5ed2de158ff8a234f0d5d8380bd44a41c616225c Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Thu, 11 Jul 2019 20:18:56 +0300 Subject: healthcheck: reject empty commands An image with "HEALTHCHECK CMD ['']" is valid but as there is no command defined the healthcheck will fail. Reject such a configuration. Fixes #3507 Signed-off-by: Stefan Becker --- libpod/healthcheck.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index e9bf984e2..8ed2b12e1 100644 --- a/libpod/healthcheck.go +++ b/libpod/healthcheck.go @@ -123,6 +123,9 @@ func (c *Container) runHealthCheck() (HealthCheckStatus, error) { // command supplied on command line - pass as-is newCommand = hcCommand } + if len(newCommand) < 1 || newCommand[0] == "" { + return HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID()) + } captureBuffer := bufio.NewWriter(&capture) hcw := hcWriteCloser{ captureBuffer, -- cgit v1.2.3-54-g00ecf