Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parser for --healthcheck-command #3574

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/podman/shared/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion docs/podman-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
5 changes: 4 additions & 1 deletion docs/podman-run.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/healthcheck_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ 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")
Expect(inspect[0].State.Healthcheck.Status).To(Equal("starting"))
})

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))

Expand All @@ -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))

Expand All @@ -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))

Expand All @@ -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))

Expand Down
6 changes: 3 additions & 3 deletions test/e2e/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down