Skip to content

Conversation

@thielepaul
Copy link
Contributor

I wanted to use the new health check status added in #3640 and noticed two issues that are fixed in this PR.

  • The state of the health status is never updated, instead it is read only once (at container start or at cadvisor start)
  • Containers without health check and unhealthy containers both have the value 0 in the metric (I changed it to -1)

@thielepaul
Copy link
Contributor Author

@dims can you take a look, if these changes make sense to you?

@dims
Copy link
Collaborator

dims commented Dec 4, 2025

@thielepaul do you mind adding tests?

@thielepaul thielepaul force-pushed the fix-docker-health-state branch from c45ecb2 to 12eb997 Compare December 4, 2025 15:44
@thielepaul
Copy link
Contributor Author

@thielepaul do you mind adding tests?

I added a test for the value conversion in prometheus.go, however, I am unsure how to write a good test for the change in handler.go. If you think another test makes sense, please give me some pointers on where to add it.

@dims
Copy link
Collaborator

dims commented Dec 4, 2025

@thielepaul thielepaul force-pushed the fix-docker-health-state branch from 0618aaa to 9611ef6 Compare December 5, 2025 12:49
@thielepaul
Copy link
Contributor Author

here? https://github.com/google/cadvisor/tree/master/integration/tests/api

Thank you! I added an integration test.
Do you have any other comments?

@thielepaul
Copy link
Contributor Author

@dims can you please trigger the tests again? I added a tag to the gcr.io/k8s-staging-test-infra/bootstrap image, so the same version that was previously is used and the tests hopefully pass.

@dims dims merged commit a310a2c into google:master Dec 5, 2025
7 checks passed
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.

2 participants