diff --git a/go.mod b/go.mod index 8f62738a2e..d6cf006ffc 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index e58cc86bee..c7c7b5d7b1 100644 --- a/go.sum +++ b/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= diff --git a/libpod/events.go b/libpod/events.go index a9115b935f..92af63632c 100644 --- a/libpod/events.go +++ b/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 { diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index d8329724a7..8f0253a17f 100644 --- a/libpod/healthcheck.go +++ b/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 { diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 9501969d90..664737ff6a 100644 --- a/libpod/runtime_ctr.go +++ b/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 { diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go index e6843845fe..808003edae 100644 --- a/test/e2e/healthcheck_run_test.go +++ b/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"}) diff --git a/vendor/github.com/containers/common/pkg/config/config.go b/vendor/github.com/containers/common/pkg/config/config.go index d8e0646e67..c0e8319ea5 100644 --- a/vendor/github.com/containers/common/pkg/config/config.go +++ b/vendor/github.com/containers/common/pkg/config/config.go @@ -253,8 +253,6 @@ type EngineConfig struct { // and "systemd". CgroupManager string `toml:"cgroup_manager,omitempty"` - // NOTE: when changing this struct, make sure to update (*Config).Merge(). - // ConmonEnvVars are environment variables to pass to the Conmon binary // when it is launched. ConmonEnvVars attributedstring.Slice `toml:"conmon_env_vars,omitempty"` @@ -320,6 +318,13 @@ type EngineConfig struct { // graphRoot internal stores the location of the graphroot graphRoot string + // HealthcheckEvents is set to indicate whenever podman should log healthcheck events. + // With many running healthcheck on short interval Podman will spam the event log a lot. + // Because this event is optional and only useful to external consumers that may want to + // know when a healthcheck is run or failed allow users to turn it off by setting it to false. + // Default is true. + HealthcheckEvents bool `toml:"healthcheck_events,omitempty"` + // HelperBinariesDir is a list of directories which are used to search for // helper binaries. HelperBinariesDir attributedstring.Slice `toml:"helper_binaries_dir,omitempty"` diff --git a/vendor/github.com/containers/common/pkg/config/containers.conf b/vendor/github.com/containers/common/pkg/config/containers.conf index f22cd20df2..8d39c96601 100644 --- a/vendor/github.com/containers/common/pkg/config/containers.conf +++ b/vendor/github.com/containers/common/pkg/config/containers.conf @@ -529,6 +529,15 @@ default_sysctls = [ # with detailed information about the container. #events_container_create_inspect_data = false +# Whenever Podman should log healthcheck events. +# With many running healthcheck on short interval Podman will spam the event +# log a lot as it generates a event for each single healthcheck run. Because +# this event is optional and only useful to external consumers that may want +# to know when a healthcheck is run or failed allow users to turn it off by +# setting it to false. The default is true. +# +#healthcheck_events = true + # A is a list of directories which are used to search for helper binaries. # #helper_binaries_dir = [ diff --git a/vendor/github.com/containers/common/pkg/config/containers.conf-freebsd b/vendor/github.com/containers/common/pkg/config/containers.conf-freebsd index 165453b9cd..21753f4f25 100644 --- a/vendor/github.com/containers/common/pkg/config/containers.conf-freebsd +++ b/vendor/github.com/containers/common/pkg/config/containers.conf-freebsd @@ -399,6 +399,15 @@ default_sysctls = [ # #events_logger = "file" +# Whenever Podman should log healthcheck events. +# With many running healthcheck on short interval Podman will spam the event +# log a lot as it generates a event for each single healthcheck run. Because +# this event is optional and only useful to external consumers that may want +# to know when a healthcheck is run or failed allow users to turn it off by +# setting it to false. The default is true. +# +#healthcheck_events = true + # A is a list of directories which are used to search for helper binaries. # #helper_binaries_dir = [ diff --git a/vendor/github.com/containers/common/pkg/config/default.go b/vendor/github.com/containers/common/pkg/config/default.go index 5faba8c9fd..3a6d804ad1 100644 --- a/vendor/github.com/containers/common/pkg/config/default.go +++ b/vendor/github.com/containers/common/pkg/config/default.go @@ -344,6 +344,7 @@ func defaultEngineConfig() (*EngineConfig, error) { c.VolumePluginTimeout = DefaultVolumePluginTimeout c.CompressionFormat = "gzip" + c.HealthcheckEvents = true c.HelperBinariesDir.Set(defaultHelperBinariesDir) if additionalHelperBinariesDir != "" { // Prioritize additionalHelperBinariesDir over defaults. diff --git a/vendor/modules.txt b/vendor/modules.txt index a14347e3b5..e51fe07f95 100644 --- a/vendor/modules.txt +++ b/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