Skip to content

Export container healthcheck as LivenessProbe in generate kube#28664

Open
givensuman wants to merge 1 commit into
containers:mainfrom
givensuman:fix/generate-kube-healthcheck
Open

Export container healthcheck as LivenessProbe in generate kube#28664
givensuman wants to merge 1 commit into
containers:mainfrom
givensuman:fix/generate-kube-healthcheck

Conversation

@givensuman
Copy link
Copy Markdown
Contributor

@givensuman givensuman commented May 7, 2026

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

None

This PR aims to supersede stale PR #28415 and close #22095, implementing maintainer comments.
If approved I plan to squash down the commits and add myself as co-signer.

@lsm5
Copy link
Copy Markdown
Member

lsm5 commented May 7, 2026

@givensuman (not a review) rebase on main to get past Mac OS arm64 issue.

@baude
Copy link
Copy Markdown
Member

baude commented May 7, 2026

also, please consider squashing your commits and and running make validatepr to take care of some of the basic linting, etc.

Comment thread test/e2e/generate_kube_test.go Outdated
"--health-interval", "10s",
"--health-timeout", "5s",
"--health-retries", "3",
"--health-start-period", "2s",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't be generated, so do we need to include it?

Copy link
Copy Markdown
Contributor Author

@givensuman givensuman May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but now the tests are combined it makes sense to include for both.

Comment thread test/e2e/generate_kube_test.go
@givensuman givensuman force-pushed the fix/generate-kube-healthcheck branch from c821c01 to 409d5c8 Compare May 8, 2026 20:07
…kube

Co-authored-by: givensuman <givensuman@duck.com>

Signed-off-by: Devesh B <98201065+DeveshB-1@users.noreply.github.com>
Signed-off-by: givensuman <givensuman@duck.com>
@givensuman givensuman force-pushed the fix/generate-kube-healthcheck branch from 409d5c8 to b7e885a Compare May 8, 2026 20:09
Copy link
Copy Markdown
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some comments. Also, don't forget to add a release note and Fixes: to commit msg.

probe := pod.Spec.Containers[0].LivenessProbe
Expect(probe).ToNot(BeNil(), "LivenessProbe should be set when container has a healthcheck")
Expect(probe.Exec).ToNot(BeNil())
Expect(probe.Exec.Command).To(ContainElement(ctr.healthCmdExpect))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Expect(probe.Exec.Command).To(ContainElement(ctr.healthCmdExpect))
Expect(probe.Exec.Command).To(Equal([]string{"/bin/sh", "-c", "/bin/true"}))

testCases := []struct {
name string
healthCmd string
healthCmdExpect string
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The healthCmdExpect should be []string. See my suggestion of assert. (It is not correct.)

Comment thread libpod/kube.go
FailureThreshold: int32(hc.Retries),
}

if hc.Test[0] == define.HealthConfigTestCmdShell {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use switch something like this:

switch hc.Test[0] {
case define.HealthConfigTestCmd:
    cmd = hc.Test[1:]
case define.HealthConfigTestCmdShell:
    cmd = append([]string{"/bin/sh", "-c"}, hc.Test[1:]...)
default:
    return nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kube playbook doesn't contain healthchecks

6 participants