Inspecting Containers without health checks return health nil

Fixed: containers#18792 This will match Docker behaviour. ` Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan · Jun 6, 2023 · 206e548 · 206e548
1 parent 8e3ecc8
commit 206e548
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 31 deletions.
diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go
@@ -206,33 +206,33 @@ type InspectMount struct {
 // Docker, but here we see more fields that are unused (nonsensical in the
 // context of Libpod).
 type InspectContainerState struct {
-	OciVersion     string             `json:"OciVersion"`
-	Status         string             `json:"Status"`
-	Running        bool               `json:"Running"`
-	Paused         bool               `json:"Paused"`
-	Restarting     bool               `json:"Restarting"` // TODO
-	OOMKilled      bool               `json:"OOMKilled"`
-	Dead           bool               `json:"Dead"`
-	Pid            int                `json:"Pid"`
-	ConmonPid      int                `json:"ConmonPid,omitempty"`
-	ExitCode       int32              `json:"ExitCode"`
-	Error          string             `json:"Error"` // TODO
-	StartedAt      time.Time          `json:"StartedAt"`
-	FinishedAt     time.Time          `json:"FinishedAt"`
-	Health         HealthCheckResults `json:"Health,omitempty"`
-	Checkpointed   bool               `json:"Checkpointed,omitempty"`
-	CgroupPath     string             `json:"CgroupPath,omitempty"`
-	CheckpointedAt time.Time          `json:"CheckpointedAt,omitempty"`
-	RestoredAt     time.Time          `json:"RestoredAt,omitempty"`
-	CheckpointLog  string             `json:"CheckpointLog,omitempty"`
-	CheckpointPath string             `json:"CheckpointPath,omitempty"`
-	RestoreLog     string             `json:"RestoreLog,omitempty"`
-	Restored       bool               `json:"Restored,omitempty"`
+	OciVersion     string              `json:"OciVersion"`
+	Status         string              `json:"Status"`
+	Running        bool                `json:"Running"`
+	Paused         bool                `json:"Paused"`
+	Restarting     bool                `json:"Restarting"` // TODO
+	OOMKilled      bool                `json:"OOMKilled"`
+	Dead           bool                `json:"Dead"`
+	Pid            int                 `json:"Pid"`
+	ConmonPid      int                 `json:"ConmonPid,omitempty"`
+	ExitCode       int32               `json:"ExitCode"`
+	Error          string              `json:"Error"` // TODO
+	StartedAt      time.Time           `json:"StartedAt"`
+	FinishedAt     time.Time           `json:"FinishedAt"`
+	Health         *HealthCheckResults `json:"Health,omitempty"`
+	Checkpointed   bool                `json:"Checkpointed,omitempty"`
+	CgroupPath     string              `json:"CgroupPath,omitempty"`
+	CheckpointedAt time.Time           `json:"CheckpointedAt,omitempty"`
+	RestoredAt     time.Time           `json:"RestoredAt,omitempty"`
+	CheckpointLog  string              `json:"CheckpointLog,omitempty"`
+	CheckpointPath string              `json:"CheckpointPath,omitempty"`
+	RestoreLog     string              `json:"RestoreLog,omitempty"`
+	Restored       bool                `json:"Restored,omitempty"`
 }
 
 // Healthcheck returns the HealthCheckResults. This is used for old podman compat
 // to make the "Healthcheck" key available in the go template.
-func (s *InspectContainerState) Healthcheck() HealthCheckResults {
+func (s *InspectContainerState) Healthcheck() *HealthCheckResults {
 	return s.Health
 }
 

diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go
@@ -408,19 +408,19 @@ func (c *Container) healthCheckLogPath() string {
 // health check log file.  If the health check log file does not exist, then
 // an empty healthcheck struct is returned
 // The caller should lock the container before this function is called.
-func (c *Container) getHealthCheckLog() (define.HealthCheckResults, error) {
-	var healthCheck define.HealthCheckResults
+func (c *Container) getHealthCheckLog() (*define.HealthCheckResults, error) {
 	if _, err := os.Stat(c.healthCheckLogPath()); os.IsNotExist(err) {
-		return healthCheck, nil
+		return nil, nil
 	}
 	b, err := os.ReadFile(c.healthCheckLogPath())
 	if err != nil {
-		return healthCheck, fmt.Errorf("failed to read health check log file: %w", err)
+		return nil, fmt.Errorf("failed to read health check log file: %w", err)
 	}
+	var healthCheck define.HealthCheckResults
 	if err := json.Unmarshal(b, &healthCheck); err != nil {
-		return healthCheck, fmt.Errorf("failed to unmarshal existing healthcheck results in %s: %w", c.healthCheckLogPath(), err)
+		return nil, fmt.Errorf("failed to unmarshal existing healthcheck results in %s: %w", c.healthCheckLogPath(), err)
 	}
-	return healthCheck, nil
+	return &healthCheck, nil
 }
 
 // HealthCheckStatus returns the current state of a container with a healthcheck.

diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go
@@ -34,10 +34,10 @@ var _ = Describe("Podman healthcheck run", func() {
 		session := podmanTest.Podman([]string{"run", "-dt", "--no-healthcheck", "--name", "hc", HEALTHCHECK_IMAGE})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
-		hc := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.State.Health.Status}}", "hc"})
+		hc := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.State.Health}}", "hc"})
 		hc.WaitWithDefaultTimeout()
 		Expect(hc).Should(Exit(0))
-		Expect(hc.OutputToString()).To(Not(ContainSubstring("starting")))
+		Expect(hc.OutputToString()).To(Equal("<nil>"))
 	})
 
 	It("podman run healthcheck and logs should contain healthcheck output", func() {