From cecdca7c4e581dd02c58dbfca5f4831d261afbe3 Mon Sep 17 00:00:00 2001 From: Harish Karumuthil Date: Wed, 18 Sep 2024 17:55:33 +0530 Subject: [PATCH 1/2] Fix: Do not emit health_status event for each health check attempt. Emit event only if there is a change in health_status Fixes #24003 Resolves https://github.com/containers/podman/pull/24005#discussion_r1766421227 Pass additional isChanged flag to event creation function Fix health check events for docker api Signed-off-by: Harish Karumuthil --- libpod/events.go | 11 ++++++----- libpod/events/config.go | 11 +++++++++++ libpod/events/journal_linux.go | 2 ++ libpod/healthcheck.go | 16 +++++++++------- libpod/runtime_ctr.go | 2 +- pkg/api/handlers/compat/events.go | 4 ++++ 6 files changed, 33 insertions(+), 13 deletions(-) diff --git a/libpod/events.go b/libpod/events.go index 92af63632c..4a06c4bad0 100644 --- a/libpod/events.go +++ b/libpod/events.go @@ -28,31 +28,32 @@ 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, events.EventMetadata{}, 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 { +func (c *Container) newContainerHealthCheckEvent(healthStatus string, isHcStatusChanged bool) { + if err := c.newContainerEventWithInspectData(events.HealthStatus, events.EventMetadata{HealthStatus: healthStatus, IsHealthStatusChanged: isHcStatusChanged}, 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, healthStatus string, inspectData bool) error { +func (c *Container) newContainerEventWithInspectData(status events.Status, metadata events.EventMetadata, 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.HealthStatus = metadata.HealthStatus e.Details = events.Details{ PodID: c.PodID(), Attributes: c.Labels(), + IsHealthStatusChanged: metadata.IsHealthStatusChanged, } if inspectData { diff --git a/libpod/events/config.go b/libpod/events/config.go index d0ab5d45f0..86236ed355 100644 --- a/libpod/events/config.go +++ b/libpod/events/config.go @@ -59,6 +59,8 @@ type Details struct { // Attributes can be used to describe specifics about the event // in the case of a container event, labels for example Attributes map[string]string + // Whether state change happened for HealthStatus or not + IsHealthStatusChanged bool } // EventerOptions describe options that need to be passed to create @@ -105,6 +107,15 @@ type Type string // Status describes the actual event action (stop, start, create, kill) type Status string +// A general purpose datatype to store additional information to be passed while creating an event +// more fields can be added as required +type EventMetadata struct { + // Actual health status string ( healthy / unhealthy) + HealthStatus string + // Whether health status of container is toggeled or not + IsHealthStatusChanged bool +} + // When updating this list below please also update the shell completion list in // cmd/podman/common/completion.go and the StringToXXX function in events.go. const ( diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index 2ee94090f8..97adbfea85 100644 --- a/libpod/events/journal_linux.go +++ b/libpod/events/journal_linux.go @@ -66,6 +66,7 @@ func (e EventJournalD) Write(ee Event) error { m["PODMAN_LABELS"] = string(b) } m["PODMAN_HEALTH_STATUS"] = ee.HealthStatus + m["PODMAN_HEALTH_STATUS_CHANGED"] = strconv.FormatBool( ee.Details.IsHealthStatusChanged ) if len(ee.Details.ContainerInspectData) > 0 { m["PODMAN_CONTAINER_INSPECT_DATA"] = ee.Details.ContainerInspectData @@ -225,6 +226,7 @@ func newEventFromJournalEntry(entry *sdjournal.JournalEntry) (*Event, error) { } } newEvent.HealthStatus = entry.Fields["PODMAN_HEALTH_STATUS"] + newEvent.Details.IsHealthStatusChanged, _ = strconv.ParseBool( entry.Fields["PODMAN_HEALTH_STATUS_CHANGED"] ) newEvent.Details.ContainerInspectData = entry.Fields["PODMAN_CONTAINER_INSPECT_DATA"] case Network: newEvent.ID = entry.Fields["PODMAN_ID"] diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index 95f20f2fd1..e1d9551300 100644 --- a/libpod/healthcheck.go +++ b/libpod/healthcheck.go @@ -154,7 +154,7 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define. } hcl := newHealthCheckLog(timeStart, timeEnd, returnCode, eventLog) - logStatus, err := c.updateHealthCheckLog(hcl, inStartPeriod, isStartup) + logStatus, isHCStausChanged, err := c.updateHealthCheckLog(hcl, inStartPeriod, isStartup) if err != nil { return hcResult, "", fmt.Errorf("unable to update health check log %s for %s: %w", c.healthCheckLogPath(), c.ID(), err) } @@ -165,7 +165,7 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define. return hcResult, logStatus, hcErr } if c.runtime.config.Engine.HealthcheckEvents { - c.newContainerHealthCheckEvent(logStatus) + c.newContainerHealthCheckEvent(logStatus, isHCStausChanged) } return hcResult, logStatus, hcErr @@ -365,7 +365,7 @@ func (c *Container) isUnhealthy() (bool, error) { } // UpdateHealthCheckLog parses the health check results and writes the log -func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, inStartPeriod, isStartup bool) (string, error) { +func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, inStartPeriod, isStartup bool) (string, bool, error) { c.lock.Lock() defer c.lock.Unlock() @@ -373,12 +373,13 @@ func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, inStartPerio // both failing and succeeding cases to match kube behavior. // So don't update the health check log till the start period is over if _, ok := c.config.Spec.Annotations[define.KubeHealthCheckAnnotation]; ok && inStartPeriod && !isStartup { - return "", nil + return "", false, nil } healthCheck, err := c.getHealthCheckLog() + oldHCStatus := healthCheck.Status if err != nil { - return "", err + return "", false, err } if hcl.ExitCode == 0 { // set status to healthy, reset failing state to 0 @@ -397,15 +398,16 @@ func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, inStartPerio } } } + isHCStausChanged := oldHCStatus != healthCheck.Status healthCheck.Log = append(healthCheck.Log, hcl) if len(healthCheck.Log) > MaxHealthCheckNumberLogs { healthCheck.Log = healthCheck.Log[1:] } newResults, err := json.Marshal(healthCheck) if err != nil { - return "", fmt.Errorf("unable to marshall healthchecks for writing: %w", err) + return "", isHCStausChanged, fmt.Errorf("unable to marshall healthchecks for writing: %w", err) } - return healthCheck.Status, os.WriteFile(c.healthCheckLogPath(), newResults, 0700) + return healthCheck.Status, isHCStausChanged, os.WriteFile(c.healthCheckLogPath(), newResults, 0700) } // HealthCheckLogPath returns the path for where the health check log is diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index a3f26cc527..2c3d512e30 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -576,7 +576,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, events.EventMetadata{}, true); err != nil { return nil, err } } else { diff --git a/pkg/api/handlers/compat/events.go b/pkg/api/handlers/compat/events.go index e449aa0cab..1ebc3a7aa3 100644 --- a/pkg/api/handlers/compat/events.go +++ b/pkg/api/handlers/compat/events.go @@ -93,6 +93,10 @@ func GetEvents(w http.ResponseWriter, r *http.Request) { if evt == nil { continue } + if evt.Status == events.HealthStatus && !evt.IsHealthStatusChanged{ + // Docker will only emit event when there is a state change + continue + } e := entities.ConvertToEntitiesEvent(*evt) // Some events differ between Libpod and Docker endpoints. From 4597408a4c4eeda07eb96562143921f9c807ec97 Mon Sep 17 00:00:00 2001 From: Harish Karumuthil Date: Sat, 21 Sep 2024 00:50:29 +0530 Subject: [PATCH 2/2] Fix Go fmt Signed-off-by: Harish Karumuthil --- libpod/events/journal_linux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index 97adbfea85..0096f3309e 100644 --- a/libpod/events/journal_linux.go +++ b/libpod/events/journal_linux.go @@ -66,7 +66,7 @@ func (e EventJournalD) Write(ee Event) error { m["PODMAN_LABELS"] = string(b) } m["PODMAN_HEALTH_STATUS"] = ee.HealthStatus - m["PODMAN_HEALTH_STATUS_CHANGED"] = strconv.FormatBool( ee.Details.IsHealthStatusChanged ) + m["PODMAN_HEALTH_STATUS_CHANGED"] = strconv.FormatBool(ee.Details.IsHealthStatusChanged) if len(ee.Details.ContainerInspectData) > 0 { m["PODMAN_CONTAINER_INSPECT_DATA"] = ee.Details.ContainerInspectData @@ -226,7 +226,7 @@ func newEventFromJournalEntry(entry *sdjournal.JournalEntry) (*Event, error) { } } newEvent.HealthStatus = entry.Fields["PODMAN_HEALTH_STATUS"] - newEvent.Details.IsHealthStatusChanged, _ = strconv.ParseBool( entry.Fields["PODMAN_HEALTH_STATUS_CHANGED"] ) + newEvent.Details.IsHealthStatusChanged, _ = strconv.ParseBool(entry.Fields["PODMAN_HEALTH_STATUS_CHANGED"]) newEvent.Details.ContainerInspectData = entry.Fields["PODMAN_CONTAINER_INSPECT_DATA"] case Network: newEvent.ID = entry.Fields["PODMAN_ID"]