From 596cdd008c8dea2fdbc9be1c6e91279bcf9c9d67 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 23 Jun 2026 16:20:46 +0200 Subject: [PATCH] libpod: remove extra state lookup in HealthCheck() Checking the state outside of locks is not safe and does not guarantee us anything as the container could be stopped afterwards anyways. So just skip the state check and then later in the exec logic we do the same check again and return ErrCtrStateInvalid so just handle that afterwards. Signed-off-by: Paul Holzinger --- libpod/healthcheck.go | 21 ++++----------------- test/e2e/healthcheck_run_test.go | 2 +- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/libpod/healthcheck.go b/libpod/healthcheck.go index ba42954767..93837f2862 100644 --- a/libpod/healthcheck.go +++ b/libpod/healthcheck.go @@ -28,9 +28,8 @@ func (r *Runtime) HealthCheck(ctx context.Context, name string) (define.HealthCh return define.HealthCheckContainerNotFound, fmt.Errorf("unable to look up %s to perform a health check: %w", name, err) } - hcStatus, err := checkHealthCheckCanBeRun(container) - if err != nil { - return hcStatus, err + if !container.HasHealthCheck() { + return define.HealthCheckNotDefined, fmt.Errorf("container %s has no defined healthcheck", container.ID()) } isStartupHC := false @@ -119,6 +118,8 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define. if hcErr != nil { hcResult = define.HealthCheckFailure switch { + case errors.Is(hcErr, define.ErrCtrStateInvalid): + return define.HealthCheckContainerStopped, "", fmt.Errorf("container %s is not running: %w", c.ID(), hcErr) case errors.Is(hcErr, define.ErrOCIRuntimeNotFound) || errors.Is(hcErr, define.ErrOCIRuntimePermissionDenied) || errors.Is(hcErr, define.ErrOCIRuntime): @@ -219,20 +220,6 @@ func (c *Container) processHealthCheckStatus(status string) error { return nil } -func checkHealthCheckCanBeRun(c *Container) (define.HealthCheckStatus, error) { - cstate, err := c.State() - if err != nil { - return define.HealthCheckInternalError, err - } - if cstate != define.ContainerStateRunning { - return define.HealthCheckContainerStopped, fmt.Errorf("container %s is not running", c.ID()) - } - if !c.HasHealthCheck() { - return define.HealthCheckNotDefined, fmt.Errorf("container %s has no defined healthcheck", c.ID()) - } - return define.HealthCheckDefined, nil -} - // Increment the current startup healthcheck success counter. // Can stop the startup HC and start the regular HC if the startup HC has enough // consecutive successes. diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go index 8d987cefe2..673998cce9 100644 --- a/test/e2e/healthcheck_run_test.go +++ b/test/e2e/healthcheck_run_test.go @@ -147,7 +147,7 @@ var _ = Describe("Podman healthcheck run", func() { hc := podmanTest.Podman([]string{"healthcheck", "run", "hc"}) hc.WaitWithDefaultTimeout() - Expect(hc).Should(ExitWithError(125, "is not running")) + Expect(hc).Should(ExitWithError(125, "is not running: can only create exec sessions on running containers: container state improper")) }) It("podman healthcheck on container without healthcheck", func() {