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

Fixed Healthcheck formatting, string to []string #11048

Merged
merged 2 commits into from
Jul 29, 2021
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
9 changes: 8 additions & 1 deletion cmd/podman/common/create_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,14 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c
cliOpts.OOMKillDisable = *cc.HostConfig.OomKillDisable
}
if cc.Config.Healthcheck != nil {
cliOpts.HealthCmd = strings.Join(cc.Config.Healthcheck.Test, " ")
finCmd := ""
for _, str := range cc.Config.Healthcheck.Test {
finCmd = finCmd + str + " "
}
if len(finCmd) > 1 {
finCmd = finCmd[:len(finCmd)-1]
}
cliOpts.HealthCmd = finCmd
cliOpts.HealthInterval = cc.Config.Healthcheck.Interval.String()
cliOpts.HealthRetries = uint(cc.Config.Healthcheck.Retries)
cliOpts.HealthStartPeriod = cc.Config.Healthcheck.StartPeriod.String()
Expand Down
42 changes: 28 additions & 14 deletions cmd/podman/common/specgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
if len(con) != 2 {
return fmt.Errorf("invalid --security-opt 1: %q", opt)
}

switch con[0] {
case "apparmor":
s.ContainerSecurityConfig.ApparmorProfile = con[1]
Expand Down Expand Up @@ -656,25 +655,40 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
}

func makeHealthCheckFromCli(inCmd, interval string, retries uint, timeout, startPeriod string) (*manifest.Schema2HealthConfig, error) {
cmdArr := []string{}
isArr := true
err := json.Unmarshal([]byte(inCmd), &cmdArr) // array unmarshalling
if err != nil {
cmdArr = strings.SplitN(inCmd, " ", 2) // default for compat
isArr = false
}
// Every healthcheck requires a command
if len(inCmd) == 0 {
if len(cmdArr) == 0 {
return nil, errors.New("Must define a healthcheck command for all healthchecks")
}

// first try to parse option value as JSON array of strings...
cmd := []string{}

if inCmd == "none" {
cmd = []string{"NONE"}
} else {
err := json.Unmarshal([]byte(inCmd), &cmd)
if err != nil {
// ...otherwise pass it to "/bin/sh -c" inside the container
cmd = []string{"CMD-SHELL", inCmd}
concat := ""
if cmdArr[0] == "CMD" || cmdArr[0] == "none" { // 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, wont contain the keywords
if isArr && len(cmdArr) > 1 { // an array of consecutive commands
cmdArr = append([]string{"CMD"}, cmdArr...)
} else { // one singular command
if len(cmdArr) == 1 {
concat = cmdArr[0]
} else {
concat = strings.Join(cmdArr[0:], " ")
}
cmdArr = append([]string{"CMD-SHELL"}, concat)
}
}

if cmdArr[0] == "none" { // if specified to remove healtcheck
cmdArr = []string{"NONE"}
}

// healthcheck is by default an array, so we simply pass the user input
hc := manifest.Schema2HealthConfig{
Test: cmd,
Test: cmdArr,
}

if interval == "disable" {
Expand Down
10 changes: 9 additions & 1 deletion test/apiv2/python/rest_api/test_v2_0_0_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ def test_inspect(self):
self.assertId(r.content)
_ = parse(r.json()["Created"])


r = requests.post(
self.podman_url + "/v1.40/containers/create?name=topcontainer",
json={"Cmd": ["top"], "Image": "alpine:latest"},
json={"Healthcheck": {"Test": ["CMD-SHELL", "exit 0"], "Interval":1000, "Timeout":1000, "Retries": 5}, "Cmd": ["top"], "Image": "alpine:latest"},
)
self.assertEqual(r.status_code, 201, r.text)
payload = r.json()
Expand All @@ -49,6 +50,13 @@ def test_inspect(self):
state = out["State"]["Health"]
self.assertIsInstance(state, dict)

r = requests.get(self.uri(f"/containers/{payload['Id']}/json"))
self.assertEqual(r.status_code, 200, r.text)
self.assertId(r.content)
out = r.json()
hc = out["Config"]["Healthcheck"]["Test"]
self.assertListEqual(["CMD-SHELL", "exit 0"], hc)

def test_stats(self):
r = requests.get(self.uri(self.resolve_container("/containers/{}/stats?stream=false")))
self.assertIn(r.status_code, (200, 409), r.text)
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/healthcheck_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ var _ = Describe("Podman healthcheck run", func() {
Expect(inspect[0].State.Healthcheck.Status).To(Equal("healthy"))
})

It("podman healthcheck unhealthy but valid arguments check", func() {
session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-retries", "2", "--health-cmd", "[\"ls\", \"/foo\"]", ALPINE, "top"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))

hc := podmanTest.Podman([]string{"healthcheck", "run", "hc"})
hc.WaitWithDefaultTimeout()
Expect(hc.ExitCode()).To(Equal(1))
})

It("podman healthcheck single healthy result changes failed to healthy", func() {
session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-retries", "2", "--health-cmd", "ls /foo || exit 1", ALPINE, "top"})
session.WaitWithDefaultTimeout()
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1212,14 +1212,14 @@ USER mail`, BB)
})

It("podman run with bad healthcheck timeout", func() {
session := podmanTest.Podman([]string{"run", "-dt", "--health-cmd", "[\"foo\"]", "--health-timeout", "0s", ALPINE, "top"})
session := podmanTest.Podman([]string{"run", "-dt", "--health-cmd", "foo", "--health-timeout", "0s", ALPINE, "top"})
cdoern marked this conversation as resolved.
Show resolved Hide resolved
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
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", "--health-cmd", "[\"foo\"]", "--health-start-period", "-1s", ALPINE, "top"})
session := podmanTest.Podman([]string{"run", "-dt", "--health-cmd", "foo", "--health-start-period", "-1s", ALPINE, "top"})
session.WaitWithDefaultTimeout()
Expect(session).To(ExitWithError())
Expect(session.ErrorToString()).To(ContainSubstring("healthcheck-start-period must be 0 seconds or greater"))
Expand Down