Inspecting Containers without health checks return health nil

[rhatdan]

Fixed: #18792

This will match Docker behaviour.
`

Does this PR introduce a user-facing change?

Inspecting containers without healthchecks will return nil for heath field.

[Luap99]

This is part of InspectContainerData which is exposed in pkg/bindings.
We should not break these types as they must be considered stable.

I wonder why omitempty is not working already, AFAIK it doesn't need to be a pointer for that?

[Luap99]

Maybe this is better:

diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go
index 0309a8dde..c015349fa 100644
--- a/libpod/define/container_inspect.go
+++ b/libpod/define/container_inspect.go
@@ -239,11 +239,11 @@ func (s *InspectContainerState) Healthcheck() HealthCheckResults {
 // HealthCheckResults describes the results/logs from a healthcheck
 type HealthCheckResults struct {
        // Status starting, healthy or unhealthy
-       Status string `json:"Status"`
+       Status string `json:"Status,omitempty"`
        // FailingStreak is the number of consecutive failed healthchecks
-       FailingStreak int `json:"FailingStreak"`
+       FailingStreak int `json:"FailingStreak,omitempty"`
        // Log describes healthcheck attempts and results
-       Log []HealthCheckLog `json:"Log"`
+       Log []HealthCheckLog `json:"Log,omitempty"`
 }
 
 // HealthCheckLog describes the results of a single healthcheck

Of course this would still show as "Health": {}, in the inspect json but I think that is good enough? In any case don't think we should break this type.

[vrothberg]

LGTM
@Luap99 PTAL

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, vrothberg

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 [rhatdan,vrothberg]

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

[Luap99]

ah, I think this might not be a good idea then after all.
If we hide the FailingStreak 0 even when there is a health check we could break parsers.

[Luap99]

omitzero is not needed, please remove it. Only the toml parser uses omitzero.

[Luap99]

Let's wait for comments on #18792, I am not sure if this would actually help the user in its current form.

[Luap99]

Based on the comments in #18792 I don't think it makes sense to move forward with this. It will not fix the issue reporters problem and likely cause unwanted side effect for the FailingStreak field.
Your original patch of changing this to a pointer makes sense but should wait for podman 5.0 IMO.