Skip to content

Commit

Permalink
Fixed Healthcheck formatting, string to []string
Browse files Browse the repository at this point in the history
Compat healthcheck tests are of the format []string but podman's were of
the format string. Converted podman's to []string at the specgen level since it has the same effect
and removed the incorrect parsing of compat healthchecks.

fixes #10617

Signed-off-by: cdoern <[email protected]>
  • Loading branch information
cdoern authored and cdoern committed Jul 26, 2021
1 parent bef1f03 commit fd1f57b
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 19 deletions.
7 changes: 6 additions & 1 deletion cmd/podman/common/create_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,12 @@ 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 + " "
}
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
31 changes: 17 additions & 14 deletions cmd/podman/common/specgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,26 +655,29 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
return nil
}

func makeHealthCheckFromCli(inCmd, interval string, retries uint, timeout, startPeriod string) (*manifest.Schema2HealthConfig, error) {
func makeHealthCheckFromCli(inCmd string, interval string, retries uint, timeout, startPeriod string) (*manifest.Schema2HealthConfig, error) {
var cmdArr []string
cmdSplitPrelim := strings.SplitN(inCmd, " ", 2)
if cmdSplitPrelim[0] == "CMD-SHELL" {
cmdArr = cmdSplitPrelim
} else if cmdSplitPrelim[0] != "CMD" && cmdSplitPrelim[0] != "none" { // if it isnt a cmd-shell or a cmd, it is a command
cmdArr = []string{"CMD-SHELL"}
cmdArr = append(cmdArr, inCmd)
} else {
cmdArr = strings.Fields(inCmd)
}
// 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}
}
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
6 changes: 3 additions & 3 deletions test/e2e/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1205,21 +1205,21 @@ USER mail`, BB)
})

It("podman run with bad healthcheck retries", func() {
session := podmanTest.Podman([]string{"run", "-dt", "--health-cmd", "[\"foo\"]", "--health-retries", "0", ALPINE, "top"})
session := podmanTest.Podman([]string{"run", "-dt", "--health-cmd", "foo", "--health-retries", "0", ALPINE, "top"})
session.Wait()
Expect(session).To(ExitWithError())
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", "--health-cmd", "[\"foo\"]", "--health-timeout", "0s", ALPINE, "top"})
session := podmanTest.Podman([]string{"run", "-dt", "--health-cmd", "foo", "--health-timeout", "0s", ALPINE, "top"})
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

0 comments on commit fd1f57b

Please sign in to comment.