Fix overwriting the Healthcheck configuration from the image

If the --health-cmd flag is not specified, other flags such as --health-interval, --health-timeout, --health-retries, and --health-start-period are ignored if the image contains a Healthcheck. This makes it impossible to modify these Healthcheck configuration when a container is created.

Fixes: containers#20212
Fixes: https://issues.redhat.com/browse/RUN-2629

Signed-off-by: Jan Rodák <[email protected]>

Author: Honny1

cmd/podman/common/create.go

@@ -1,6 +1,8 @@
 package common
 
 import (
+	"fmt"
+
 	"github.com/containers/common/pkg/auth"
 	"github.com/containers/common/pkg/completion"
 	commonFlag "github.com/containers/common/pkg/flag"
@@ -1053,3 +1055,21 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 	)
 	_ = cmd.RegisterFlagCompletionFunc(blkioWeightDeviceFlagName, completion.AutocompleteDefault)
 }
+
+func GetHealthCheckOverrideConfig(cmd *cobra.Command, vals *entities.ContainerCreateOptions) map[string]string {
+	out := make(map[string]string)
+
+	if cmd.Flags().Changed("health-interval") {
+		out["health-interval"] = vals.HealthInterval
+	}
+	if cmd.Flags().Changed("health-retries") {
+		out["health-retries"] = fmt.Sprintf("%d", vals.HealthRetries)
+	}
+	if cmd.Flags().Changed("health-timeout") {
+		out["health-timeout"] = vals.HealthTimeout
+	}
+	if cmd.Flags().Changed("health-start-period") {
+		out["health-start-period"] = vals.HealthStartPeriod
+	}
+	return out
+}

cmd/podman/containers/create.go

@@ -174,6 +174,10 @@ func create(cmd *cobra.Command, args []string) error {
 		}
 	}
 
+	if s.HealthConfig == nil {
+		s.HealthImageOverride = common.GetHealthCheckOverrideConfig(cmd, &cliVals)
+	}
+
 	report, err := registry.ContainerEngine().ContainerCreate(registry.Context(), s)
 	if err != nil {
 		// if pod was created as part of run

cmd/podman/containers/run.go

@@ -225,6 +225,10 @@ func run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	if s.HealthConfig == nil {
+		s.HealthImageOverride = common.GetHealthCheckOverrideConfig(cmd, &cliVals)
+	}
+
 	report, err := registry.ContainerEngine().ContainerRun(registry.Context(), runOpts)
 	// report.ExitCode is set by ContainerRun even it returns an error
 	if report != nil {

docs/source/markdown/options/health-interval.md

@@ -5,3 +5,5 @@
 #### **--health-interval**=*interval*
 
 Set an interval for the healthchecks. An _interval_ of **disable** results in no automatic timer setup. The default is **30s**.
+
+Note: This parameter can overwrite the healthcheck configuration from the image.

docs/source/markdown/options/health-retries.md

@@ -5,3 +5,5 @@
 #### **--health-retries**=*retries*
 
 The number of retries allowed before a healthcheck is considered to be unhealthy. The default value is **3**.
+
+Note: This parameter can overwrite the healthcheck configuration from the image.

docs/source/markdown/options/health-start-period.md

@@ -12,3 +12,5 @@ the container's health state will be updated to `healthy`. However, if the healt
 stay as `starting` until either the health check is successful or until the `--health-start-period` time is over. If the
 health check command fails after the `--health-start-period` time is over, the health state will be updated to `unhealthy`.
 The health check command is executed periodically based on the value of `--health-interval`.
+
+Note: This parameter can overwrite the healthcheck configuration from the image.

docs/source/markdown/options/health-timeout.md

@@ -10,3 +10,5 @@ value can be expressed in a time format such as **1m22s**. The default value is
 Note: A timeout marks the healthcheck as failed but does not terminate the running process.
 This ensures that a slow but eventually successful healthcheck does not disrupt the container
 but is still accounted for in the health status.
+
+Note: This parameter can overwrite the healthcheck configuration from the image.

pkg/specgen/generate/container.go

@@ -61,6 +61,52 @@ func getImageFromSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGen
 	return image, resolvedName, inspectData, err
 }
 
+func applyHealthCheckOverrides(s *specgen.SpecGenerator) error {
+	for k, v := range s.HealthImageOverride {
+		switch k {
+		case "health-interval":
+			if v == "disable" {
+				v = "0"
+			}
+			hct, err := time.ParseDuration(v)
+			if err != nil {
+				return err
+			}
+			s.HealthConfig.Interval = hct
+		case "health-retries":
+			i, err := strconv.Atoi(v)
+			if err != nil {
+				return err
+			}
+			if i < 1 {
+				return errors.New("healthcheck-retries must be greater than 0")
+			}
+			s.HealthConfig.Retries = i
+		case "health-timeout":
+			hct, err := time.ParseDuration(v)
+			if err != nil {
+				return err
+			}
+			if hct < time.Duration(1) {
+				return errors.New("healthcheck-timeout must be at least 1 second")
+			}
+			s.HealthConfig.Timeout = hct
+		case "health-start-period":
+			hct, err := time.ParseDuration(v)
+			if err != nil {
+				return err
+			}
+			if hct < time.Duration(0) {
+				return errors.New("healthcheck-start-period must be 0 seconds or greater")
+			}
+			s.HealthConfig.StartPeriod = hct
+		default:
+			return fmt.Errorf("unknown healthcheck override key %q", k)
+		}
+	}
+	return nil
+}
+
 // Fill any missing parts of the spec generator (e.g. from the image).
 // Returns a set of warnings or any fatal error that occurred.
 func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerator) ([]string, error) {
@@ -89,6 +135,7 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 					}
 					s.HealthConfig.Interval = hct
 				}
+				applyHealthCheckOverrides(s)
 			}
 		}
 

pkg/specgen/specgen.go

@@ -615,6 +615,8 @@ type ContainerHealthCheckConfig struct {
 	// ("0" value means an infinite log length).
 	// TODO (6.0): In next major release convert it to pointer and use omitempty
 	HealthMaxLogSize uint `json:"healthMaxLogSize"`
+	// HealthImageOverride is a map of healthcheck flags that overrides healthcheck configuration from image.
+	HealthImageOverride map[string]string `json:"HealthImageOverride,omitempty"`
 }
 
 // SpecGenerator creates an OCI spec and Libpod configuration options to create

test/e2e/healthcheck_run_test.go

@@ -32,6 +32,20 @@ var _ = Describe("Podman healthcheck run", func() {
 		Expect(hc).Should(ExitWithError(125, "has no defined healthcheck"))
 	})
 
+	It("podman run/create override image healthcheck configuration", func() {
+		podmanTest.PodmanExitCleanly("run", "-dt", "--name", "hc", "--health-start-period", "10s", "--health-interval", "10s", "--health-timeout", "10s", "--health-retries", "2", HEALTHCHECK_IMAGE)
+		hc := podmanTest.PodmanExitCleanly("container", "inspect", "--format", "{{.Config.Healthcheck.StartPeriod}}--{{.Config.Healthcheck.Interval}}--{{.Config.Healthcheck.Timeout}}--{{.Config.Healthcheck.Retries}}", "hc")
+		Expect(hc.OutputToString()).To(Equal("10s--10s--10s--2"))
+
+		podmanTest.PodmanExitCleanly("create", "-q", "--name", "hc1", "--health-start-period", "10s", "--health-interval", "10s", "--health-timeout", "10s", "--health-retries", "2", "quay.io/libpod/healthcheck:config-only", "ls")
+		hc1 := podmanTest.PodmanExitCleanly("container", "inspect", "--format", "{{.Config.Healthcheck.StartPeriod}}--{{.Config.Healthcheck.Interval}}--{{.Config.Healthcheck.Timeout}}--{{.Config.Healthcheck.Retries}}", "hc1")
+		Expect(hc1.OutputToString()).To(Equal("10s--10s--10s--2"))
+
+		podmanTest.PodmanExitCleanly("run", "-dt", "--name", "hc2", "--health-start-period", "10s", "--health-interval", "disable", "--health-timeout", "10s", "--health-retries", "2", HEALTHCHECK_IMAGE)
+		hc2 := podmanTest.PodmanExitCleanly("container", "inspect", "--format", "{{.Config.Healthcheck.StartPeriod}}--{{.Config.Healthcheck.Interval}}--{{.Config.Healthcheck.Timeout}}--{{.Config.Healthcheck.Retries}}", "hc2")
+		Expect(hc2.OutputToString()).To(Equal("10s--0s--10s--2"))
+	})
+
 	It("podman disable healthcheck with --no-healthcheck must not show starting on status", func() {
 		session := podmanTest.Podman([]string{"run", "-dt", "--no-healthcheck", "--name", "hc", HEALTHCHECK_IMAGE})
 		session.WaitWithDefaultTimeout()