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

React to health_status events #1447

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Conversation

martinpitt
Copy link
Member

We previously didn't react to health_status events as they were broken, and only reacted to exec_died instead. With the upcoming podman release, health_status events are reliable, and they will also not be accompanied by an exec_died event any more. So start updating the container status on them.

Still keep listening to exec_died to support older podman releases.

Fixes #1443

@martinpitt
Copy link
Member Author

I triggered two additional /podman-next scenarios.

@martinpitt
Copy link
Member Author

martinpitt commented Oct 11, 2023

I apparently fixed the follow-up failure (see #1443), and HealthcheckUser seems stable now. But now there is a new failure in HealthcheckSystem only, which also reproduces locally.

Indeed healthcheck events are completely broken now for unhealthy system containers.

@martinpitt martinpitt removed the request for review from jelly October 11, 2023 17:10
@martinpitt martinpitt marked this pull request as draft October 11, 2023 17:34
@martinpitt
Copy link
Member Author

I filed containers/podman#20342 about the healthcheck regression. Let's land this to at least make test failures more predictable, and this is a good and necessary bug fix.

We previously didn't react to `health_status` events as they were
broken, and only reacted to `exec_died` instead. With the upcoming
podman release, `health_status` events are reliable, and they will also
not be accompanied by an `exec_died` event any more. So start updating
the container status on them.

Still keep listening to `exec_died` to support older podman releases.

Fixes cockpit-project#1443
Copy link
Member

@jelly jelly left a comment

Choose a reason for hiding this comment

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

Thanks for debugging this!

@jelly jelly merged commit 8e243ad into cockpit-project:main Oct 12, 2023
19 checks passed
@martinpitt martinpitt deleted the fix-healthcheck branch October 12, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nightly tests did not succeed on fedora-38/podman-next: testHealthcheckUser timeout
2 participants