From 5633ef1d15c17fa2e0249710c7591da777cd7b5e Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Thu, 16 Jun 2022 15:57:01 -0400 Subject: Docker-compose disable healthcheck properly handled Previously, if a container had healthchecks disabled in the docker-compose.yml file and the user did a `podman inspect `, they would have an incorrect output: ``` "Healthcheck":{ "Test":[ "CMD-SHELL", "NONE" ], "Interval":30000000000, "Timeout":30000000000, "Retries":3 } ``` After a quick change, the correct output is now the result: ``` "Healthcheck":{ "Test":[ "NONE" ] } ``` Additionally, I extracted the hard-coded strings that were used for comparisons into constants in `libpod/define` to prevent a similar issue from recurring. Closes: #14493 Signed-off-by: Jake Correnti --- libpod/define/healthchecks.go | 10 ++++++++++ libpod/healthcheck.go | 6 +++--- pkg/specgen/generate/kube/kube.go | 4 ++-- pkg/specgenutil/specgen.go | 12 ++++++------ test/compose/disable_healthcheck/docker-compose.yml | 10 ++++++++++ test/compose/disable_healthcheck/tests.sh | 2 ++ 6 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 test/compose/disable_healthcheck/docker-compose.yml create mode 100644 test/compose/disable_healthcheck/tests.sh diff --git a/libpod/define/healthchecks.go b/libpod/define/healthchecks.go index bde449d30..f71274350 100644 --- a/libpod/define/healthchecks.go +++ b/libpod/define/healthchecks.go @@ -47,3 +47,13 @@ const ( // DefaultHealthCheckTimeout default value DefaultHealthCheckTimeout = "30s" ) + +// HealthConfig.Test options +const ( + // HealthConfigTestNone disables healthcheck + HealthConfigTestNone = "NONE" + // HealthConfigTestCmd execs arguments directly + HealthConfigTestCmd = "CMD" + // HealthConfigTestCmdShell runs commands with the system's default shell + HealthConfigTestCmdShell = "CMD-SHELL" +) diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index 95c70b60e..df6f00e7e 100644 --- a/libpod/healthcheck.go +++ b/libpod/healthcheck.go @@ -47,11 +47,11 @@ func (c *Container) runHealthCheck() (define.HealthCheckStatus, error) { return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID()) } switch hcCommand[0] { - case "", "NONE": + case "", define.HealthConfigTestNone: return define.HealthCheckNotDefined, errors.Errorf("container %s has no defined healthcheck", c.ID()) - case "CMD": + case define.HealthConfigTestCmd: newCommand = hcCommand[1:] - case "CMD-SHELL": + case define.HealthConfigTestCmdShell: // TODO: SHELL command from image not available in Container - use Docker default newCommand = []string{"/bin/sh", "-c", strings.Join(hcCommand[1:], " ")} default: diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go index 689c740f0..5b4fa532c 100644 --- a/pkg/specgen/generate/kube/kube.go +++ b/pkg/specgen/generate/kube/kube.go @@ -511,12 +511,12 @@ func makeHealthCheck(inCmd string, interval int32, retries int32, timeout int32, cmd := []string{} if inCmd == "none" { - cmd = []string{"NONE"} + cmd = []string{define.HealthConfigTestNone} } else { err := json.Unmarshal([]byte(inCmd), &cmd) if err != nil { // ...otherwise pass it to "/bin/sh -c" inside the container - cmd = []string{"CMD-SHELL"} + cmd = []string{define.HealthConfigTestCmdShell} cmd = append(cmd, strings.Split(inCmd, " ")...) } } diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 8ad0a92e7..34350579d 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -873,23 +873,23 @@ func makeHealthCheckFromCli(inCmd, interval string, retries uint, timeout, start } var concat string - if cmdArr[0] == "CMD" || cmdArr[0] == "none" { // this is for compat, we are already split properly for most compat cases + if strings.ToUpper(cmdArr[0]) == define.HealthConfigTestCmd || strings.ToUpper(cmdArr[0]) == define.HealthConfigTestNone { // this is for compat, we are already split properly for most compat cases cmdArr = strings.Fields(inCmd) - } else if cmdArr[0] != "CMD-SHELL" { // this is for podman side of things, won't contain the keywords + } else if strings.ToUpper(cmdArr[0]) != define.HealthConfigTestCmdShell { // this is for podman side of things, won't contain the keywords if isArr && len(cmdArr) > 1 { // an array of consecutive commands - cmdArr = append([]string{"CMD"}, cmdArr...) + cmdArr = append([]string{define.HealthConfigTestCmd}, cmdArr...) } else { // one singular command if len(cmdArr) == 1 { concat = cmdArr[0] } else { concat = strings.Join(cmdArr[0:], " ") } - cmdArr = append([]string{"CMD-SHELL"}, concat) + cmdArr = append([]string{define.HealthConfigTestCmdShell}, concat) } } - if cmdArr[0] == "none" { // if specified to remove healtcheck - cmdArr = []string{"NONE"} + if strings.ToUpper(cmdArr[0]) == define.HealthConfigTestNone { // if specified to remove healtcheck + cmdArr = []string{define.HealthConfigTestNone} } // healthcheck is by default an array, so we simply pass the user input diff --git a/test/compose/disable_healthcheck/docker-compose.yml b/test/compose/disable_healthcheck/docker-compose.yml new file mode 100644 index 000000000..1f608c895 --- /dev/null +++ b/test/compose/disable_healthcheck/docker-compose.yml @@ -0,0 +1,10 @@ +version: "3.7" +services: + noHc: + image: alpine + container_name: noHc + ports: + - "4000:80" + restart: unless-stopped + healthcheck: + disable: true diff --git a/test/compose/disable_healthcheck/tests.sh b/test/compose/disable_healthcheck/tests.sh new file mode 100644 index 000000000..2460a687e --- /dev/null +++ b/test/compose/disable_healthcheck/tests.sh @@ -0,0 +1,2 @@ +podman inspect --format='{{.Config.Healthcheck.Test}}' noHc +like $output "[NONE]" "$testname: healthcheck properly disabled" -- cgit v1.2.3-54-g00ecf