Merge pull request #22443 from Luap99/podman-healthcheck-events

add containers.conf healthcheck_events support

go.mod

@@ -26,7 +26,7 @@ require (
 	github.com/checkpoint-restore/go-criu/v7 v7.1.0
 	github.com/containernetworking/plugins v1.4.0
 	github.com/containers/buildah v1.35.1-0.20240412112838-e393e57728f5
-	github.com/containers/common v0.58.1-0.20240410144442-8db59bf2fcce
+	github.com/containers/common v0.58.1-0.20240419143618-deb3eeef3b74
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/gvisor-tap-vsock v0.7.4-0.20240408151405-d744d71db363
 	github.com/containers/image/v5 v5.30.1-0.20240411200840-dc519780d39f

go.sum

@@ -76,8 +76,8 @@ github.com/containernetworking/plugins v1.4.0 h1:+w22VPYgk7nQHw7KT92lsRmuToHvb7w
 github.com/containernetworking/plugins v1.4.0/go.mod h1:UYhcOyjefnrQvKvmmyEKsUA+M9Nfn7tqULPpH0Pkcj0=
 github.com/containers/buildah v1.35.1-0.20240412112838-e393e57728f5 h1:ucOnAzlQRjgDogeTTByJ45E1fW/On2CYc1WH4XmcHkQ=
 github.com/containers/buildah v1.35.1-0.20240412112838-e393e57728f5/go.mod h1:unO5wyQXGHXcDBFu0D+W3bUXvfQrMEh1J6a8dgX8F+4=
-github.com/containers/common v0.58.1-0.20240410144442-8db59bf2fcce h1:mt7/jkY4a+q8SHLE85v7D4XoWX0KGC3tAfBZ7Mfpqos=
-github.com/containers/common v0.58.1-0.20240410144442-8db59bf2fcce/go.mod h1:wxQdMk9Wuu178UQLJonrQlBCw940zof77Xm60NmDmlI=
+github.com/containers/common v0.58.1-0.20240419143618-deb3eeef3b74 h1:3o+wybYKyr03hlrNdZGDjV8ukVTU2JXttzAVk9OmRLg=
+github.com/containers/common v0.58.1-0.20240419143618-deb3eeef3b74/go.mod h1:AnMTrXjygOD8jQKNBae4EEjKLlED9Svysh98Be+MktM=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/gvisor-tap-vsock v0.7.4-0.20240408151405-d744d71db363 h1:EqWMZeFa08y2c1GniaFkfjlO5AjegoG2foWo6NlDfUY=

libpod/events.go

@@ -28,19 +28,27 @@ func (r *Runtime) newEventer() (events.Eventer, error) {
 
 // newContainerEvent creates a new event based on a container
 func (c *Container) newContainerEvent(status events.Status) {
-	if err := c.newContainerEventWithInspectData(status, false); err != nil {
+	if err := c.newContainerEventWithInspectData(status, "", false); err != nil {
+		logrus.Errorf("Unable to write container event: %v", err)
+	}
+}
+
+// newContainerHealthCheckEvent creates a new healthcheck event with the given status
+func (c *Container) newContainerHealthCheckEvent(healthStatus string) {
+	if err := c.newContainerEventWithInspectData(events.HealthStatus, healthStatus, false); err != nil {
 		logrus.Errorf("Unable to write container event: %v", err)
 	}
 }
 
 // newContainerEventWithInspectData creates a new event and sets the
 // ContainerInspectData field if inspectData is set.
-func (c *Container) newContainerEventWithInspectData(status events.Status, inspectData bool) error {
+func (c *Container) newContainerEventWithInspectData(status events.Status, healthStatus string, inspectData bool) error {
 	e := events.NewEvent(status)
 	e.ID = c.ID()
 	e.Name = c.Name()
 	e.Image = c.config.RootfsImageName
 	e.Type = events.Container
+	e.HealthStatus = healthStatus
 
 	e.Details = events.Details{
 		PodID:      c.PodID(),
@@ -65,16 +73,6 @@ func (c *Container) newContainerEventWithInspectData(status events.Status, inspe
 		}
 	}
 
-	// if the current event is a HealthStatus event, we need to get the current
-	// status of the container to pass to the event
-	if status == events.HealthStatus {
-		containerHealthStatus, err := c.healthCheckStatus()
-		if err != nil {
-			e.HealthStatus = err.Error()
-		}
-		e.HealthStatus = containerHealthStatus
-	}
-
 	if status == events.Remove {
 		exitCode, err := c.runtime.state.GetContainerExitCode(c.ID())
 		if err == nil {

libpod/healthcheck.go

@@ -14,8 +14,6 @@ import (
 	"time"
 
 	"github.com/containers/podman/v5/libpod/define"
-	"github.com/containers/podman/v5/libpod/events"
-	"github.com/containers/storage/pkg/fileutils"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -178,7 +176,9 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define.
 	if hcResult == define.HealthCheckNotDefined || hcResult == define.HealthCheckInternalError {
 		return hcResult, logStatus, hcErr
 	}
-	c.newContainerEvent(events.HealthStatus)
+	if c.runtime.config.Engine.HealthcheckEvents {
+		c.newContainerHealthCheckEvent(logStatus)
+	}
 
 	return hcResult, logStatus, hcErr
 }
@@ -430,11 +430,12 @@ func (c *Container) healthCheckLogPath() string {
 // The caller should lock the container before this function is called.
 func (c *Container) getHealthCheckLog() (define.HealthCheckResults, error) {
 	var healthCheck define.HealthCheckResults
-	if err := fileutils.Exists(c.healthCheckLogPath()); errors.Is(err, fs.ErrNotExist) {
-		return healthCheck, nil
-	}
 	b, err := os.ReadFile(c.healthCheckLogPath())
 	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			// If the file does not exists just return empty healthcheck and no error.
+			return healthCheck, nil
+		}
 		return healthCheck, fmt.Errorf("failed to read health check log file: %w", err)
 	}
 	if err := json.Unmarshal(b, &healthCheck); err != nil {

libpod/runtime_ctr.go

@@ -585,7 +585,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 	}
 
 	if ctr.runtime.config.Engine.EventsContainerCreateInspectData {
-		if err := ctr.newContainerEventWithInspectData(events.Create, true); err != nil {
+		if err := ctr.newContainerEventWithInspectData(events.Create, "", true); err != nil {
 			return nil, err
 		}
 	} else {

test/e2e/healthcheck_run_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"time"
 
 	"github.com/containers/podman/v5/libpod/define"
@@ -215,53 +216,75 @@ var _ = Describe("Podman healthcheck run", func() {
 		Expect(hc).Should(Exit(1))
 	})
 
-	It("podman healthcheck single healthy result changes failed to healthy", func() {
-		session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-retries", "2", "--health-cmd", "ls /foo || exit 1", ALPINE, "top"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-
-		hc := podmanTest.Podman([]string{"healthcheck", "run", "hc"})
-		hc.WaitWithDefaultTimeout()
-		Expect(hc).Should(Exit(1))
+	// Run this test with and without healthcheck events, even without events
+	// podman inspect and ps should still show accurate healthcheck results.
+	for _, hcEvent := range []bool{true, false} {
+		hcEvent := hcEvent
+		testName := "hc_events=" + strconv.FormatBool(hcEvent)
+		It("podman healthcheck single healthy result changes failed to healthy "+testName, func() {
+			if !hcEvent {
+				path := filepath.Join(podmanTest.TempDir, "containers.conf")
+				err := os.WriteFile(path, []byte("[engine]\nhealthcheck_events=false\n"), 0o644)
+				Expect(err).ToNot(HaveOccurred())
+				err = os.Setenv("CONTAINERS_CONF_OVERRIDE", path)
+				Expect(err).ToNot(HaveOccurred())
+				if IsRemote() {
+					podmanTest.StopRemoteService()
+					podmanTest.StartRemoteService()
+				}
+			}
 
-		inspect := podmanTest.InspectContainer("hc")
-		Expect(inspect[0].State.Health).To(HaveField("Status", "starting"))
+			session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-retries", "2", "--health-cmd", "ls /foo || exit 1", ALPINE, "top"})
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitCleanly())
 
-		hc = podmanTest.Podman([]string{"healthcheck", "run", "hc"})
-		hc.WaitWithDefaultTimeout()
-		Expect(hc).Should(Exit(1))
+			hc := podmanTest.Podman([]string{"healthcheck", "run", "hc"})
+			hc.WaitWithDefaultTimeout()
+			Expect(hc).Should(Exit(1))
 
-		inspect = podmanTest.InspectContainer("hc")
-		Expect(inspect[0].State.Health).To(HaveField("Status", define.HealthCheckUnhealthy))
+			inspect := podmanTest.InspectContainer("hc")
+			Expect(inspect[0].State.Health).To(HaveField("Status", "starting"))
 
-		foo := podmanTest.Podman([]string{"exec", "hc", "touch", "/foo"})
-		foo.WaitWithDefaultTimeout()
-		Expect(foo).Should(ExitCleanly())
+			hc = podmanTest.Podman([]string{"healthcheck", "run", "hc"})
+			hc.WaitWithDefaultTimeout()
+			Expect(hc).Should(Exit(1))
 
-		hc = podmanTest.Podman([]string{"healthcheck", "run", "hc"})
-		hc.WaitWithDefaultTimeout()
-		Expect(hc).Should(ExitCleanly())
+			inspect = podmanTest.InspectContainer("hc")
+			Expect(inspect[0].State.Health).To(HaveField("Status", define.HealthCheckUnhealthy))
 
-		inspect = podmanTest.InspectContainer("hc")
-		Expect(inspect[0].State.Health).To(HaveField("Status", define.HealthCheckHealthy))
+			foo := podmanTest.Podman([]string{"exec", "hc", "touch", "/foo"})
+			foo.WaitWithDefaultTimeout()
+			Expect(foo).Should(ExitCleanly())
 
-		// Test that events generated have correct status (#19237)
-		events := podmanTest.Podman([]string{"events", "--stream=false", "--filter", "event=health_status", "--since", "1m"})
-		events.WaitWithDefaultTimeout()
-		Expect(events).Should(ExitCleanly())
-		eventsOut := events.OutputToStringArray()
-		Expect(eventsOut).To(HaveLen(3))
-		Expect(eventsOut[0]).To(ContainSubstring("health_status=starting"))
-		Expect(eventsOut[1]).To(ContainSubstring("health_status=unhealthy"))
-		Expect(eventsOut[2]).To(ContainSubstring("health_status=healthy"))
+			hc = podmanTest.Podman([]string{"healthcheck", "run", "hc"})
+			hc.WaitWithDefaultTimeout()
+			Expect(hc).Should(ExitCleanly())
+
+			inspect = podmanTest.InspectContainer("hc")
+			Expect(inspect[0].State.Health).To(HaveField("Status", define.HealthCheckHealthy))
+
+			// Test that events generated have correct status (#19237)
+			events := podmanTest.Podman([]string{"events", "--stream=false", "--filter", "event=health_status", "--since", "1m"})
+			events.WaitWithDefaultTimeout()
+			Expect(events).Should(ExitCleanly())
+			if hcEvent {
+				eventsOut := events.OutputToStringArray()
+				Expect(eventsOut).To(HaveLen(3))
+				Expect(eventsOut[0]).To(ContainSubstring("health_status=starting"))
+				Expect(eventsOut[1]).To(ContainSubstring("health_status=unhealthy"))
+				Expect(eventsOut[2]).To(ContainSubstring("health_status=healthy"))
+			} else {
+				Expect(events.OutputToString()).To(BeEmpty())
+			}
 
-		// Test podman ps --filter health is working (#11687)
-		ps := podmanTest.Podman([]string{"ps", "--filter", "health=healthy"})
-		ps.WaitWithDefaultTimeout()
-		Expect(ps).Should(ExitCleanly())
-		Expect(ps.OutputToStringArray()).To(HaveLen(2))
-		Expect(ps.OutputToString()).To(ContainSubstring("hc"))
-	})
+			// Test podman ps --filter health is working (#11687)
+			ps := podmanTest.Podman([]string{"ps", "--filter", "health=healthy"})
+			ps.WaitWithDefaultTimeout()
+			Expect(ps).Should(ExitCleanly())
+			Expect(ps.OutputToStringArray()).To(HaveLen(2))
+			Expect(ps.OutputToString()).To(ContainSubstring("hc"))
+		})
+	}
 
 	It("hc logs do not include exec events", func() {
 		session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-cmd", "true", "--health-interval", "5s", "alpine", "sleep", "60"})

vendor/modules.txt

@@ -171,7 +171,7 @@ github.com/containers/buildah/pkg/sshagent
 github.com/containers/buildah/pkg/util
 github.com/containers/buildah/pkg/volumes
 github.com/containers/buildah/util
-# github.com/containers/common v0.58.1-0.20240410144442-8db59bf2fcce
+# github.com/containers/common v0.58.1-0.20240419143618-deb3eeef3b74
 ## explicit; go 1.20
 github.com/containers/common/internal
 github.com/containers/common/internal/attributedstring