Skip to content

Add support for Docker HealthConfig.StartInterval (v25.0.0+)#2345

Merged
mtrmac merged 1 commit intocontainers:mainfrom
migesok:feature/docker-healthcheck-start-interval
Mar 18, 2024
Merged

Add support for Docker HealthConfig.StartInterval (v25.0.0+)#2345
mtrmac merged 1 commit intocontainers:mainfrom
migesok:feature/docker-healthcheck-start-interval

Conversation

@migesok
Copy link
Contributor

@migesok migesok commented Mar 15, 2024

I'm trying to add support for Dockerfile HEALTHCHECK --start-interval param which has been added in Docker v25.0.0.
So I figured out I have to update this library as well.
The change is quite trivial but I couldn't say that I understand completely what I'm doing so feedback is appreciated.

Signed-off-by: Mikhail Sokolov <msokolov@evolution.com>
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks!

This looks about right, and it might be sufficient for some purposes.

For other purposes:

  • Docker now uses, and interprets, the fields also in OCI images, not just schema2. So we should be able to decode and use those fields in that situation (… arguably there might be some benefit in intentionally refusing to support that, but that’s probably not ultimately helpful to users)
  • And to support both formats, the general c/image abstractions should allow doing that without an image consumer having to specifically support individual manifest types. That would probably mean adding corresponding fields to types.ImageInspectInfo, and updating Inspect to populate them.

But all of that can come later, this is already valuable as is.

@migesok
Copy link
Contributor Author

migesok commented Mar 18, 2024

@mtrmac thank you! I can add the fields in other places if you think it should be done. But I'm a bit out of my depth here - I haven't found anything "health" related in types.ImageInspectInfo.
Either I need a bit more guidance or it's better if someone else does that.

@mtrmac mtrmac merged commit f16cb7c into containers:main Mar 18, 2024
@mtrmac
Copy link
Collaborator

mtrmac commented Mar 18, 2024

Yes, ImageInspectInfo does not currently contain the health fields.

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