Add support for startup healthchecks #13909

mheon · 2022-04-18T14:37:06Z

TODO: Needs to be wired into podman inspect, needs tests written, needs manpages

Add support for startup healthchecks. A startup healthcheck is a healthcheck that runs before the main healthcheck, to accomodate slow-starting containers; inspiration is mostly from K8S startup probes, and we'll be using this as a backend for supporting those in podman play kube once this merges.

Containers can now have startup healthchecks, allowing a command to be run to ensure the container is fully started before the regular healthcheck is activated.

openshift-ci · 2022-04-18T14:37:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

edsantiago · 2022-04-18T18:38:03Z

@mheon this isn't even going to get to tests until you add the new options to man pages (they can be stubs) and add tests or the magic no-new-tests flag

mheon · 2022-04-18T18:52:11Z

@edsantiago I'm still trying to get it working fully working locally, will get to manpages once that's done

github-actions · 2022-05-20T00:06:25Z

A friendly reminder that this PR had no activity for 30 days.

github-actions · 2022-06-23T00:06:45Z

A friendly reminder that this PR had no activity for 30 days.

mheon · 2022-06-27T19:58:12Z

Want to add a few more tests but otherwise fundamentally complete

mheon · 2022-06-27T20:10:06Z

@edsantiago Can you tell what the documentation errors are saying? I think I've added the relevant bits to the manpages

edsantiago · 2022-06-27T20:20:00Z

docs/source/markdown/podman-run.1.md

@@ -451,6 +451,38 @@ The number of retries allowed before a healthcheck is considered to be unhealthy
 The initialization time needed for a container to bootstrap. The value can be expressed in time format like
 **2m3s**. The default value is **0s**.

+#### **-health-startup-cmd**=*"command"* | *'["command", "arg1", ...]'*


Missing dash: should be --health-etc

edsantiago · 2022-06-27T20:20:57Z

docs/source/markdown/podman-run.1.md

+A value of **0** means that any success will begin the regular healthcheck.
+The default is **0**.
+
+#### **--health-startup-timeout=*timeout*


Missing asterisks: should be timeout** (timeout, star, star, equals)

edsantiago · 2022-06-27T20:22:04Z

docs/source/markdown/podman-create.1.md

@@ -408,6 +408,38 @@ The number of retries allowed before a healthcheck is considered to be unhealthy
 The initialization time needed for a container to bootstrap. The value can be expressed in time format like
 `2m3s`. The default value is `0s`

+#### **-health-startup-cmd**=*"command"* | *'["command", "arg1", ...]'*


edsantiago · 2022-06-27T20:22:19Z

docs/source/markdown/podman-create.1.md

+A value of **0** means that any success will begin the regular healthcheck.
+The default is **0**.
+
+#### **--health-startup-timeout=*timeout*


mheon · 2022-11-16T14:37:26Z

Tests finally green. @containers/podman-maintainers PTAL

rhatdan · 2022-11-16T20:49:45Z

LGTM
@umohnani8 @vrothberg @baude @flouthoc PTAL

mheon · 2022-11-17T16:56:50Z

@vrothberg PTAL

vrothberg · 2022-11-17T17:00:55Z

@vrothberg PTAL

I only find time reviewing small PRs at the moment as I am OOO. If you want my pair of eyes, I'd be happy to take a look on Tuesday.

mheon · 2022-11-17T17:17:31Z

Fine by me

docs/source/markdown/options/health-startup-cmd.md

vrothberg

Code LGTM

Would love to see a test or two in the system tests as they are excecuted in gating.

cmd/podman/common/create.go

flouthoc · 2022-11-23T09:03:32Z

libpod/healthcheck.go

+// consecutive successes.
+func (c *Container) incrementStartupHCSuccessCounter(ctx context.Context) {
+	if !c.batched {
+		c.lock.Lock()


Question: Shouldn't we lock irrespective of if it is batched or not ? We are incrementing SuccessCounter below without any lock.

A batched command holds a single lock for multiple operations (for performance reasons... guarantees no changes to the container while we're doing a series of operations, so no need to hit the DB to check for updates). As such, locking during batching is guaranteed to deadlock as the lock is already being held.

rhatdan · 2022-11-28T11:44:17Z

@mheon Sadly this needs a rebase.

Startup healthchecks are similar to K8S startup probes, in that they are a separate check from the regular healthcheck that runs before it. If the startup healthcheck fails repeatedly, the associated container is restarted. Signed-off-by: Matthew Heon <[email protected]>

mheon · 2022-11-28T18:31:18Z

Rebased

mheon · 2022-11-28T18:31:29Z

I think this is ready. Once CI passes, at least.

rhatdan · 2022-11-30T20:18:40Z

/lgtm

Extra function arguments were added in containers#13909. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson <[email protected]>

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 18, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 18, 2022

mheon force-pushed the startup_probe branch 2 times, most recently from 69a3442 to 27b97a1 Compare April 18, 2022 18:12

mheon force-pushed the startup_probe branch from 27b97a1 to e8503df Compare April 19, 2022 14:38

github-actions bot added the stale-pr label May 20, 2022

rhatdan removed the stale-pr label May 23, 2022

github-actions bot added the stale-pr label Jun 23, 2022

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2022

mheon force-pushed the startup_probe branch from e8503df to 1e26364 Compare June 27, 2022 18:52

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2022

mheon force-pushed the startup_probe branch 4 times, most recently from adf94ab to 3551aaa Compare June 27, 2022 19:53

mheon removed the stale-pr label Jun 27, 2022

mheon changed the title ~~[WIP] Add support for startup healthchecks~~ Add support for startup healthchecks Jun 27, 2022

openshift-ci bot added do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jun 27, 2022

edsantiago reviewed Jun 27, 2022

View reviewed changes

mheon force-pushed the startup_probe branch 4 times, most recently from d0649bc to b964eee Compare November 16, 2022 04:35

openshift-ci bot added release-note and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Nov 17, 2022

TomSweeneyRedHat reviewed Nov 17, 2022

View reviewed changes

docs/source/markdown/options/health-startup-cmd.md Show resolved Hide resolved

vrothberg reviewed Nov 22, 2022

View reviewed changes

cmd/podman/common/create.go Outdated Show resolved Hide resolved

mheon force-pushed the startup_probe branch from b964eee to 12a3a6e Compare November 22, 2022 19:17

rhatdan removed the stale-pr label Nov 22, 2022

flouthoc reviewed Nov 23, 2022

View reviewed changes

mheon force-pushed the startup_probe branch from 12a3a6e to d161293 Compare November 28, 2022 18:30

openshift-ci bot assigned rhatdan Nov 30, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2022

openshift-merge-robot merged commit c00d8a2 into containers:main Nov 30, 2022

dfr added a commit to dfr/podman that referenced this pull request Dec 2, 2022

libpod: Track healthcheck API changes in healthcheck_unsupported.go

6ed8dc1

Extra function arguments were added in containers#13909. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson <[email protected]>

dfr mentioned this pull request Dec 2, 2022

libpod: Track healthcheck API changes in healthcheck_unsupported.go #16715

Merged

karta0807913 mentioned this pull request Dec 9, 2022

podman play kube support container startup probe #16794

Merged

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for startup healthchecks #13909

Add support for startup healthchecks #13909

mheon commented Apr 18, 2022 •

edited

Loading

openshift-ci bot commented Apr 18, 2022

edsantiago commented Apr 18, 2022

mheon commented Apr 18, 2022

github-actions bot commented May 20, 2022

github-actions bot commented Jun 23, 2022

mheon commented Jun 27, 2022

mheon commented Jun 27, 2022

edsantiago Jun 27, 2022

edsantiago Jun 27, 2022

edsantiago Jun 27, 2022

edsantiago Jun 27, 2022

mheon commented Nov 16, 2022

rhatdan commented Nov 16, 2022

mheon commented Nov 17, 2022

vrothberg commented Nov 17, 2022

mheon commented Nov 17, 2022

vrothberg left a comment

flouthoc Nov 23, 2022

mheon Nov 23, 2022

rhatdan commented Nov 28, 2022

mheon commented Nov 28, 2022

mheon commented Nov 28, 2022

rhatdan commented Nov 30, 2022

Add support for startup healthchecks #13909

Add support for startup healthchecks #13909

Conversation

mheon commented Apr 18, 2022 • edited Loading

openshift-ci bot commented Apr 18, 2022

edsantiago commented Apr 18, 2022

mheon commented Apr 18, 2022

github-actions bot commented May 20, 2022

github-actions bot commented Jun 23, 2022

mheon commented Jun 27, 2022

mheon commented Jun 27, 2022

edsantiago Jun 27, 2022

Choose a reason for hiding this comment

edsantiago Jun 27, 2022

Choose a reason for hiding this comment

edsantiago Jun 27, 2022

Choose a reason for hiding this comment

edsantiago Jun 27, 2022

Choose a reason for hiding this comment

mheon commented Nov 16, 2022

rhatdan commented Nov 16, 2022

mheon commented Nov 17, 2022

vrothberg commented Nov 17, 2022

mheon commented Nov 17, 2022

vrothberg left a comment

Choose a reason for hiding this comment

flouthoc Nov 23, 2022

Choose a reason for hiding this comment

mheon Nov 23, 2022

Choose a reason for hiding this comment

rhatdan commented Nov 28, 2022

mheon commented Nov 28, 2022

mheon commented Nov 28, 2022

rhatdan commented Nov 30, 2022

mheon commented Apr 18, 2022 •

edited

Loading