Merge pull request containers#21388 from ashley-cui/healthcheck

Return nil health when inspecting containers without healthchecks
mtrmac · Jan 31, 2024 · 1698fa0 · 1698fa0
2 parents 0655bf3 + a1c47f0
commit 1698fa0
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 44 deletions.
diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go
@@ -189,15 +189,20 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
 		data.OCIConfigPath = c.state.ConfigPath
 	}
 
-	if c.config.HealthCheckConfig != nil {
+	// Check if healthcheck is not nil and --no-healthcheck option is not set.
+	// If --no-healthcheck is set Test will be always set to `[NONE]`, so the
+	// inspect status should be set to nil.
+	if c.config.HealthCheckConfig != nil && !(len(c.config.HealthCheckConfig.Test) == 1 && c.config.HealthCheckConfig.Test[0] == "NONE") {
 		// This container has a healthcheck defined in it; we need to add its state
 		healthCheckState, err := c.getHealthCheckLog()
 		if err != nil {
 			// An error here is not considered fatal; no health state will be displayed
 			logrus.Error(err)
 		} else {
-			data.State.Health = healthCheckState
+			data.State.Health = &healthCheckState
 		}
+	} else {
+		data.State.Health = nil
 	}
 
 	networkConfig, err := c.getContainerNetworkInfo()

diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go
@@ -206,34 +206,34 @@ 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"`
-	StoppedByUser  bool               `json:"StoppedByUser,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"`
+	StoppedByUser  bool                `json:"StoppedByUser,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/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go
@@ -442,22 +442,28 @@ func LibpodToContainerJSON(l *libpod.Container, sz bool) (*types.ContainerJSON,
 	}
 
 	if l.HasHealthCheck() && state.Status != "created" {
-		state.Health = &types.Health{
-			Status:        inspect.State.Health.Status,
-			FailingStreak: inspect.State.Health.FailingStreak,
-		}
-
-		log := inspect.State.Health.Log
-
-		for _, item := range log {
-			res := &types.HealthcheckResult{}
-			s, _ := time.Parse(time.RFC3339Nano, item.Start)
-			e, _ := time.Parse(time.RFC3339Nano, item.End)
-			res.Start = s
-			res.End = e
-			res.ExitCode = item.ExitCode
-			res.Output = item.Output
-			state.Health.Log = append(state.Health.Log, res)
+		state.Health = &types.Health{}
+		if inspect.State.Health != nil {
+			state.Health.Status = inspect.State.Health.Status
+			state.Health.FailingStreak = inspect.State.Health.FailingStreak
+			log := inspect.State.Health.Log
+
+			for _, item := range log {
+				res := &types.HealthcheckResult{}
+				s, err := time.Parse(time.RFC3339Nano, item.Start)
+				if err != nil {
+					return nil, err
+				}
+				e, err := time.Parse(time.RFC3339Nano, item.End)
+				if err != nil {
+					return nil, err
+				}
+				res.Start = s
+				res.End = e
+				res.ExitCode = item.ExitCode
+				res.Output = item.Output
+				state.Health.Log = append(state.Health.Log, res)
+			}
 		}
 	}
 

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(ExitCleanly())
-		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(ExitCleanly())
-		Expect(hc.OutputToString()).To(Not(ContainSubstring("starting")))
+		Expect(hc.OutputToString()).To(Equal("<nil>"))
 	})
 
 	It("podman run healthcheck and logs should contain healthcheck output", func() {