From e4cba7d36a49353febfefcb25d2d820bc89e9d1b Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Sat, 13 Jul 2019 18:22:41 +0300 Subject: create: improve parser for --healthcheck-command Fix Docker CLI compatibility issue: the "--healthcheck-command" option value should not be split but instead be passed as single string to "CMD-SHELL", i.e. "/bin/sh -c ". On the other hand implement the same extension as is already available for "--entrypoint", i.e. allow the option value to be a JSON array of strings. This will make life easier for tools like podman-compose. Updated "--healthcheck-command" option values in tests accordingly. Continuation of #3455 & #3507 Signed-off-by: Stefan Becker --- cmd/podman/shared/create.go | 8 +++++--- docs/podman-create.1.md | 5 ++++- docs/podman-run.1.md | 5 ++++- test/e2e/common_test.go | 2 +- test/e2e/healthcheck_run_test.go | 10 +++++----- test/e2e/run_test.go | 6 +++--- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index be1a731cc..17455c4c9 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -26,7 +26,6 @@ import ( "github.com/docker/docker/pkg/signal" "github.com/docker/go-connections/nat" "github.com/docker/go-units" - "github.com/google/shlex" "github.com/opencontainers/selinux/go-selinux/label" "github.com/opentracing/opentracing-go" "github.com/pkg/errors" @@ -788,9 +787,12 @@ func makeHealthCheckFromCli(c *GenericCLIResults) (*manifest.Schema2HealthConfig return nil, errors.New("Must define a healthcheck command for all healthchecks") } - cmd, err := shlex.Split(inCommand) + // first try to parse option value as JSON array of strings... + cmd := []string{} + err := json.Unmarshal([]byte(inCommand), &cmd) if err != nil { - return nil, errors.Wrap(err, "failed to parse healthcheck command") + // ...otherwise pass it to "/bin/sh -c" inside the container + cmd = []string{"CMD-SHELL", inCommand} } hc := manifest.Schema2HealthConfig{ Test: cmd, diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md index 87e18dbb9..ea01a346c 100644 --- a/docs/podman-create.1.md +++ b/docs/podman-create.1.md @@ -272,12 +272,15 @@ The following example maps uids 0-2000 in the container to the uids 30000-31999 Add additional groups to run as -**--healthcheck-command**=*command* +**--healthcheck-command**=*"command"* | *'["command", "arg1", ...]'* Set or alter a healthcheck command for a container. The command is a command to be executed inside your container that determines your container health. The command is required for other healthcheck options to be applied. A value of `none` disables existing healthchecks. +Multiple options can be passed in the form of a JSON array; otherwise, the command will be interpreted +as an argument to `/bin/sh -c`. + **--healthcheck-interval**=*interval* Set an interval for the healthchecks (a value of `disable` results in no automatic timer setup) (default "30s") diff --git a/docs/podman-run.1.md b/docs/podman-run.1.md index 95499edd6..e3dca9300 100644 --- a/docs/podman-run.1.md +++ b/docs/podman-run.1.md @@ -279,12 +279,15 @@ The example maps gids 0-2000 in the container to the gids 30000-31999 on the hos Add additional groups to run as -**--healthcheck-command**=*command* +**--healthcheck-command**=*"command"* | *'["command", "arg1", ...]'* Set or alter a healthcheck command for a container. The command is a command to be executed inside your container that determines your container health. The command is required for other healthcheck options to be applied. A value of `none` disables existing healthchecks. +Multiple options can be passed in the form of a JSON array; otherwise, the command will be interpreted +as an argument to `/bin/sh -c`. + **--healthcheck-interval**=*interval* Set an interval for the healthchecks (a value of `disable` results in no automatic timer setup) (default "30s") diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 21afc4b84..b233a772a 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -375,7 +375,7 @@ func (p *PodmanTestIntegration) RunNginxWithHealthCheck(name string) (*PodmanSes if name != "" { podmanArgs = append(podmanArgs, "--name", name) } - podmanArgs = append(podmanArgs, "-dt", "-P", "--healthcheck-command", "CMD-SHELL curl http://localhost/", nginx) + podmanArgs = append(podmanArgs, "-dt", "-P", "--healthcheck-command", "curl http://localhost/", nginx) session := p.Podman(podmanArgs) session.WaitWithDefaultTimeout() return session, session.OutputToString() diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go index 125002bf9..dee86f3d9 100644 --- a/test/e2e/healthcheck_run_test.go +++ b/test/e2e/healthcheck_run_test.go @@ -95,7 +95,7 @@ var _ = Describe("Podman healthcheck run", func() { }) It("podman healthcheck should be starting", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-retries", "2", "--healthcheck-command", "\"CMD-SHELL ls /foo || exit 1\"", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-retries", "2", "--healthcheck-command", "ls /foo || exit 1", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) inspect := podmanTest.InspectContainer("hc") @@ -103,7 +103,7 @@ var _ = Describe("Podman healthcheck run", func() { }) It("podman healthcheck failed checks in start-period should not change status", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-start-period", "2m", "--healthcheck-retries", "2", "--healthcheck-command", "\"CMD-SHELL ls /foo || exit 1\"", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-start-period", "2m", "--healthcheck-retries", "2", "--healthcheck-command", "ls /foo || exit 1", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -124,7 +124,7 @@ var _ = Describe("Podman healthcheck run", func() { }) It("podman healthcheck failed checks must reach retries before unhealthy ", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-retries", "2", "--healthcheck-command", "\"CMD-SHELL ls /foo || exit 1\"", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-retries", "2", "--healthcheck-command", "ls /foo || exit 1", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -145,7 +145,7 @@ var _ = Describe("Podman healthcheck run", func() { }) It("podman healthcheck good check results in healthy even in start-period", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-start-period", "2m", "--healthcheck-retries", "2", "--healthcheck-command", "\"CMD-SHELL\" \"ls\" \"||\" \"exit\" \"1\"", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-start-period", "2m", "--healthcheck-retries", "2", "--healthcheck-command", "ls || exit 1", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -158,7 +158,7 @@ var _ = Describe("Podman healthcheck run", func() { }) It("podman healthcheck single healthy result changes failed to healthy", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-retries", "2", "--healthcheck-command", "\"CMD-SHELL\" \"ls\" \"/foo\" \"||\" \"exit\" \"1\"", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--healthcheck-retries", "2", "--healthcheck-command", "ls /foo || exit 1", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 8c7830204..886e3e7a9 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -750,21 +750,21 @@ USER mail` }) It("podman run with bad healthcheck retries", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--healthcheck-command", "foo", "--healthcheck-retries", "0", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--healthcheck-command", "[\"foo\"]", "--healthcheck-retries", "0", ALPINE, "top"}) session.Wait() Expect(session.ExitCode()).ToNot(Equal(0)) Expect(session.ErrorToString()).To(ContainSubstring("healthcheck-retries must be greater than 0")) }) It("podman run with bad healthcheck timeout", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--healthcheck-command", "foo", "--healthcheck-timeout", "0s", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--healthcheck-command", "[\"foo\"]", "--healthcheck-timeout", "0s", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).ToNot(Equal(0)) Expect(session.ErrorToString()).To(ContainSubstring("healthcheck-timeout must be at least 1 second")) }) It("podman run with bad healthcheck start-period", func() { - session := podmanTest.Podman([]string{"run", "-dt", "--healthcheck-command", "foo", "--healthcheck-start-period", "-1s", ALPINE, "top"}) + session := podmanTest.Podman([]string{"run", "-dt", "--healthcheck-command", "[\"foo\"]", "--healthcheck-start-period", "-1s", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).ToNot(Equal(0)) Expect(session.ErrorToString()).To(ContainSubstring("healthcheck-start-period must be 0 seconds or greater")) -- cgit v1.2.3-54-g00ecf