containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
23.06k stars 2.35k forks source link

RFE: Healthcheck restart policy should support `--requires` containers #15806

Open cevich opened 2 years ago

cevich commented 2 years ago

/kind feature

Description

When a container which depends on another (via --requires) fails it's healthcheck with a restart policy, it should honor a single or a chain of --requires container restarts as well.

Steps to reproduce the issue:

  1. Run a container which has a dependency on another. Either or both should have healthchecks enabled with restart policies.

  2. Cause a healthcheck to fail

Describe the results you received:

The single or chain of dependent containers does not restart.

Describe the results you expected:

The single or chain of dependent containers does restart.

Additional information you deem important (e.g. issue happens only occasionally):

If there are a lot of dependencies, or the relationships are complex, perhaps we should provide an opt-out flag.

github-actions[bot] commented 1 year ago

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

rhatdan commented 1 year ago

@vrothberg @mheon WDYT?

vrothberg commented 1 year ago

A use case would help. In theory it sounds independent of health checks but more related to the restart logic. If a container gets restarted manually by the user or by an on-failure action should not matter.

cevich commented 1 year ago

A use case could be:

  1. Container A provides a network-namespace. On startup it does some shenanigans with a VPN to punch through a corporate. firewall. It has health-checks to verify connectivity.
  2. Container B provides a user-namespace, it consumes the network-namespace from container A. On startup it queries, connects, or synchronizes with a remote IAM server. It has health-checks to confirm user/group queries are functional.
  3. Container C incorporates container A (to proxy other resources behind the firewall) and Container B (to manage access) and provides some web app. It runs health-checks to confirm the web app is responsive.

A failing health-check on container C could easily be the fault of B or A. Since it uses --requires, it "knows" to restart A and B due to a failing health check (assuming their own health-checks didn't fire first). Whereas without --requires, it would simply endlessly/uselessly restart itself until B health-check failures corrected the fault (which could be never).

github-actions[bot] commented 1 year ago

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

rhatdan commented 1 year ago

@vrothberg followup on this one?

vrothberg commented 1 year ago

A use case would help. In theory it sounds independent of health checks but more related to the restart logic. If a container gets restarted manually by the user or by an on-failure action should not matter.

I still think that's more a restart thing than a health check one. The health-check action just restarts the container.

cevich commented 1 year ago

Yes, definitely a restart thing, not health-check.

vrothberg commented 1 year ago

@mheon WDYT about restarting dependent containers?

Code-wise I see some challenges regarding batch restarts where we need to prevent a container from being restarted multiple times.

mheon commented 1 year ago

Could use the graph traversal code in container_graph.go - guaranteed to go in the correct order and only hit each node once, and because it was originally written for podman pod restart it already handles pods.

mheon commented 1 year ago

Sorry, it already handles restarting not pods. Need more coffee...

github-actions[bot] commented 1 year ago

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

rhatdan commented 1 year ago

@mheon @vrothberg @cevich What should we do with this one?

vrothberg commented 1 year ago

Either close it or leave it open until someone does the work. As mentioned above, it's unrelated to health-check actions but rather a restart-logic feature.

cevich commented 1 year ago

Playing my own devil's advocate here - We could close it with an excuse like: If anyone needs this kind of fancy-stuff, they should use kubernetes not podman.