Fix HealthCheck log destination, count, and size defaults

GoLang sets unset values to the default value of the type. This means that the destination of the log is an empty string and the count and size are set to 0. However, this means that size and count are unbounded, and this is not the default behavior.

Fixes: containers#25473

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

Author: Honny1

libpod/container.go

@@ -1291,6 +1291,27 @@ func (c *Container) HealthCheckConfig() *manifest.Schema2HealthConfig {
 	return c.config.HealthCheckConfig
 }
 
+func (c *Container) HealthCheckLogDestination() string {
+	if c.config.HealthLogDestination == nil {
+		return define.DefaultHealthCheckLocalDestination
+	}
+	return *c.config.HealthLogDestination
+}
+
+func (c *Container) HealthCheckMaxLogCount() uint {
+	if c.config.HealthMaxLogCount == nil {
+		return define.DefaultHealthMaxLogCount
+	}
+	return *c.config.HealthMaxLogCount
+}
+
+func (c *Container) HealthCheckMaxLogSize() uint {
+	if c.config.HealthMaxLogSize == nil {
+		return define.DefaultHealthMaxLogSize
+	}
+	return *c.config.HealthMaxLogSize
+}
+
 // AutoRemove indicates whether the container will be removed after it is executed
 func (c *Container) AutoRemove() bool {
 	spec := c.config.Spec

libpod/container_config.go

@@ -416,13 +416,16 @@ type ContainerMiscConfig struct {
 	// HealthCheckOnFailureAction defines an action to take once the container turns unhealthy.
 	HealthCheckOnFailureAction define.HealthCheckOnFailureAction `json:"healthcheck_on_failure_action"`
 	// HealthLogDestination defines the destination where the log is stored
-	HealthLogDestination string `json:"healthLogDestination,omitempty"`
+	// Nil value means the default value (local).
+	HealthLogDestination *string `json:"healthLogDestination,omitempty"`
 	// HealthMaxLogCount is maximum number of attempts in the HealthCheck log file.
 	// ('0' value means an infinite number of attempts in the log file)
-	HealthMaxLogCount uint `json:"healthMaxLogCount,omitempty"`
+	// Nil value means the default value (5).
+	HealthMaxLogCount *uint `json:"healthMaxLogCount,omitempty"`
 	// HealthMaxLogSize is the maximum length in characters of stored HealthCheck log
 	// ("0" value means an infinite log length)
-	HealthMaxLogSize uint `json:"healthMaxLogSize,omitempty"`
+	// Nil value means the default value (500).
+	HealthMaxLogSize *uint `json:"healthMaxLogSize,omitempty"`
 	// StartupHealthCheckConfig is the configuration of the startup
 	// healthcheck for the container. This will run before the regular HC
 	// runs, and when it passes the regular HC will be activated.

libpod/container_inspect.go

@@ -437,11 +437,11 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp
 
 	ctrConfig.HealthcheckOnFailureAction = c.config.HealthCheckOnFailureAction.String()
 
-	ctrConfig.HealthLogDestination = c.config.HealthLogDestination
+	ctrConfig.HealthLogDestination = c.HealthCheckLogDestination()
 
-	ctrConfig.HealthMaxLogCount = c.config.HealthMaxLogCount
+	ctrConfig.HealthMaxLogCount = c.HealthCheckMaxLogCount()
 
-	ctrConfig.HealthMaxLogSize = c.config.HealthMaxLogSize
+	ctrConfig.HealthMaxLogSize = c.HealthCheckMaxLogSize()
 
 	ctrConfig.CreateCommand = c.config.CreateCommand
 

libpod/container_internal.go

@@ -2970,15 +2970,15 @@ func (c *Container) updateGlobalHealthCheckConfiguration(globalOptions define.Gl
 	}
 
 	if globalOptions.HealthMaxLogCount != nil {
-		c.config.HealthMaxLogCount = *globalOptions.HealthMaxLogCount
+		c.config.HealthMaxLogCount = globalOptions.HealthMaxLogCount
 	}
 
 	if globalOptions.HealthMaxLogSize != nil {
-		c.config.HealthMaxLogSize = *globalOptions.HealthMaxLogSize
+		c.config.HealthMaxLogSize = globalOptions.HealthMaxLogSize
 	}
 
 	if globalOptions.HealthLogDestination != nil {
-		dest, err := define.GetValidHealthCheckDestination(*globalOptions.HealthLogDestination)
+		dest, err := define.GetValidHealthCheckDestination(globalOptions.HealthLogDestination)
 		if err != nil {
 			return err
 		}

libpod/define/healthchecks.go

@@ -292,37 +292,43 @@ func (u *UpdateHealthCheckConfig) SetNewHealthCheckConfigTo(healthCheckOptions *
 	return changed
 }
 
-func GetValidHealthCheckDestination(destination string) (string, error) {
-	if destination == HealthCheckEventsLoggerDestination || destination == DefaultHealthCheckLocalDestination {
+func GetValidHealthCheckDestination(destination *string) (*string, error) {
+	if destination == nil {
+		defaultPath := DefaultHealthCheckLocalDestination
+		return &defaultPath, nil
+	}
+
+	if *destination == HealthCheckEventsLoggerDestination || *destination == DefaultHealthCheckLocalDestination {
 		return destination, nil
 	}
 
-	fileInfo, err := os.Stat(destination)
+	path := *destination
+	fileInfo, err := os.Stat(path)
 	if err != nil {
-		return "", fmt.Errorf("HealthCheck Log '%s' destination error: %w", destination, err)
+		return nil, fmt.Errorf("HealthCheck Log '%s' destination error: %w", path, err)
 	}
 	mode := fileInfo.Mode()
 	if !mode.IsDir() {
-		return "", fmt.Errorf("HealthCheck Log '%s' destination must be directory", destination)
+		return nil, fmt.Errorf("HealthCheck Log '%s' destination must be directory", path)
 	}
 
-	absPath, err := filepath.Abs(destination)
+	absPath, err := filepath.Abs(path)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	return absPath, nil
+	return &absPath, nil
 }
 
 func (u *UpdateHealthCheckConfig) GetNewGlobalHealthCheck() (GlobalHealthCheckOptions, error) {
 	globalOptions := GlobalHealthCheckOptions{}
 
 	healthLogDestination := u.HealthLogDestination
 	if u.HealthLogDestination != nil {
-		dest, err := GetValidHealthCheckDestination(*u.HealthLogDestination)
+		dest, err := GetValidHealthCheckDestination(u.HealthLogDestination)
 		if err != nil {
 			return GlobalHealthCheckOptions{}, err
 		}
-		healthLogDestination = &dest
+		healthLogDestination = dest
 	}
 	globalOptions.HealthLogDestination = healthLogDestination
 

libpod/events.go

@@ -49,7 +49,7 @@ func (c *Container) newContainerEventWithInspectData(status events.Status, healt
 	e.Image = c.config.RootfsImageName
 	e.Type = events.Container
 	e.HealthStatus = healthCheckResult.Status
-	if c.config.HealthLogDestination == define.HealthCheckEventsLoggerDestination {
+	if c.HealthCheckLogDestination() == define.HealthCheckEventsLoggerDestination {
 		if len(healthCheckResult.Log) > 0 {
 			logData, err := json.Marshal(healthCheckResult.Log[len(healthCheckResult.Log)-1])
 			if err != nil {

libpod/healthcheck.go

@@ -152,8 +152,8 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define.
 	}
 
 	eventLog := output.String()
-	if c.config.HealthMaxLogSize != 0 && len(eventLog) > int(c.config.HealthMaxLogSize) {
-		eventLog = eventLog[:c.config.HealthMaxLogSize]
+	if c.HealthCheckMaxLogSize() != 0 && len(eventLog) > int(c.HealthCheckMaxLogSize()) {
+		eventLog = eventLog[:c.HealthCheckMaxLogSize()]
 	}
 
 	if timeEnd.Sub(timeStart) > c.HealthCheckConfig().Timeout {
@@ -166,7 +166,7 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define.
 
 	healthCheckResult, err := c.updateHealthCheckLog(hcl, hcResult, inStartPeriod, isStartup)
 	if err != nil {
-		return hcResult, "", fmt.Errorf("unable to update health check log %s for %s: %w", c.config.HealthLogDestination, c.ID(), err)
+		return hcResult, "", fmt.Errorf("unable to update health check log %s for %s: %w", c.getHealthCheckLogDestination(), c.ID(), err)
 	}
 
 	// Write HC event with appropriate status as the last thing before we
@@ -399,7 +399,7 @@ func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, hcResult def
 		}
 	}
 	healthCheck.Log = append(healthCheck.Log, hcl)
-	if c.config.HealthMaxLogCount != 0 && len(healthCheck.Log) > int(c.config.HealthMaxLogCount) {
+	if c.HealthCheckMaxLogCount() != 0 && len(healthCheck.Log) > int(c.HealthCheckMaxLogCount()) {
 		healthCheck.Log = healthCheck.Log[1:]
 	}
 	return healthCheck, c.writeHealthCheckLog(healthCheck)
@@ -415,11 +415,11 @@ func (c *Container) witeToFileHealthCheckResults(path string, result define.Heal
 
 func (c *Container) getHealthCheckLogDestination() string {
 	var destination string
-	switch c.config.HealthLogDestination {
+	switch c.HealthCheckLogDestination() {
 	case define.DefaultHealthCheckLocalDestination, define.HealthCheckEventsLoggerDestination, "":
 		destination = filepath.Join(filepath.Dir(c.state.RunDir), "healthcheck.log")
 	default:
-		destination = filepath.Join(c.config.HealthLogDestination, c.ID()+"-healthcheck.log")
+		destination = filepath.Join(c.HealthCheckLogDestination(), c.ID()+"-healthcheck.log")
 	}
 	return destination
 }

libpod/options.go

@@ -1527,7 +1527,7 @@ func WithHealthCheck(healthCheck *manifest.Schema2HealthConfig) CtrCreateOption
 }
 
 // WithHealthCheckLogDestination adds the healthLogDestination to the container config
-func WithHealthCheckLogDestination(destination string) CtrCreateOption {
+func WithHealthCheckLogDestination(destination *string) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return define.ErrCtrFinalized
@@ -1542,7 +1542,7 @@ func WithHealthCheckLogDestination(destination string) CtrCreateOption {
 }
 
 // WithHealthCheckMaxLogCount adds the healthMaxLogCount to the container config
-func WithHealthCheckMaxLogCount(maxLogCount uint) CtrCreateOption {
+func WithHealthCheckMaxLogCount(maxLogCount *uint) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return define.ErrCtrFinalized
@@ -1553,7 +1553,7 @@ func WithHealthCheckMaxLogCount(maxLogCount uint) CtrCreateOption {
 }
 
 // WithHealthCheckMaxLogSize adds the healthMaxLogSize to the container config
-func WithHealthCheckMaxLogSize(maxLogSize uint) CtrCreateOption {
+func WithHealthCheckMaxLogSize(maxLogSize *uint) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return define.ErrCtrFinalized

pkg/api/handlers/libpod/containers_create.go

@@ -34,6 +34,10 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 	noHosts := conf.Containers.NoHosts
 	privileged := conf.Containers.Privileged
 
+	defaultHealthLogDestination := define.DefaultHealthCheckLocalDestination
+	defaultHealthLogCount := define.DefaultHealthMaxLogCount
+	defaultHealthLogSize := define.DefaultHealthMaxLogSize
+
 	// we have to set the default before we decode to make sure the correct default is set when the field is unset
 	sg := specgen.SpecGenerator{
 		ContainerNetworkConfig: specgen.ContainerNetworkConfig{
@@ -44,7 +48,9 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 			Privileged: &privileged,
 		},
 		ContainerHealthCheckConfig: specgen.ContainerHealthCheckConfig{
-			HealthLogDestination: define.DefaultHealthCheckLocalDestination,
+			HealthLogDestination: &defaultHealthLogDestination,
+			HealthMaxLogCount:    &defaultHealthLogCount,
+			HealthMaxLogSize:     &defaultHealthLogSize,
 		},
 	}
 

pkg/domain/entities/pods.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	commonFlag "github.com/containers/common/pkg/flag"
+	"github.com/containers/podman/v5/libpod/define"
 	"github.com/containers/podman/v5/pkg/domain/entities/types"
 	"github.com/containers/podman/v5/pkg/specgen"
 	"github.com/containers/podman/v5/pkg/util"
@@ -275,9 +276,12 @@ type ContainerCreateOptions struct {
 
 func NewInfraContainerCreateOptions() ContainerCreateOptions {
 	options := ContainerCreateOptions{
-		IsInfra:          true,
-		ImageVolume:      "anonymous",
-		MemorySwappiness: -1,
+		IsInfra:              true,
+		ImageVolume:          "anonymous",
+		MemorySwappiness:     -1,
+		HealthLogDestination: define.DefaultHealthCheckLocalDestination,
+		HealthMaxLogCount:    define.DefaultHealthMaxLogCount,
+		HealthMaxLogSize:     define.DefaultHealthMaxLogSize,
 	}
 	return options
 }

pkg/domain/infra/abi/containers.go

@@ -1765,10 +1765,6 @@ func (ic *ContainerEngine) ContainerClone(ctx context.Context, ctrCloneOpts enti
 		spec.Name = generate.CheckName(ic.Libpod, n, true)
 	}
 
-	spec.HealthLogDestination = define.DefaultHealthCheckLocalDestination
-	spec.HealthMaxLogCount = define.DefaultHealthMaxLogCount
-	spec.HealthMaxLogSize = define.DefaultHealthMaxLogSize
-
 	rtSpec, spec, opts, err := generate.MakeContainer(context.Background(), ic.Libpod, spec, true, c)
 	if err != nil {
 		return nil, err

pkg/domain/infra/abi/generate.go

@@ -59,7 +59,12 @@ func (ic *ContainerEngine) GenerateSpec(ctx context.Context, opts *entities.Gene
 	} else if p, err := ic.Libpod.LookupPod(opts.ID); err == nil {
 		pspec = &specgen.PodSpecGenerator{}
 		pspec.Name = p.Name()
-		_, err := generateUtils.PodConfigToSpec(ic.Libpod, pspec, &entities.ContainerCreateOptions{}, opts.ID)
+		_, err := generateUtils.PodConfigToSpec(ic.Libpod, pspec,
+			&entities.ContainerCreateOptions{
+				HealthLogDestination: define.DefaultHealthCheckLocalDestination,
+				HealthMaxLogCount:    define.DefaultHealthMaxLogCount,
+				HealthMaxLogSize:     define.DefaultHealthMaxLogSize,
+			}, opts.ID)
 		if err != nil {
 			return nil, err
 		}

pkg/specgen/generate/kube/kube.go

@@ -417,9 +417,6 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 	}
 
 	s.Annotations[define.KubeHealthCheckAnnotation] = "true"
-	s.HealthLogDestination = define.DefaultHealthCheckLocalDestination
-	s.HealthMaxLogCount = define.DefaultHealthMaxLogCount
-	s.HealthMaxLogSize = define.DefaultHealthMaxLogSize
 
 	// Environment Variables
 	envs := map[string]string{}

pkg/specgen/generate/pod_create.go

@@ -86,11 +86,6 @@ func MakePod(p *entities.PodSpec, rt *libpod.Runtime) (_ *libpod.Pod, finalErr e
 		p.PodSpecGen.InfraContainerSpec.ResourceLimits = nil
 		p.PodSpecGen.InfraContainerSpec.WeightDevice = nil
 
-		// Set default for HealthCheck
-		p.PodSpecGen.InfraContainerSpec.HealthLogDestination = define.DefaultHealthCheckLocalDestination
-		p.PodSpecGen.InfraContainerSpec.HealthMaxLogCount = define.DefaultHealthMaxLogCount
-		p.PodSpecGen.InfraContainerSpec.HealthMaxLogSize = define.DefaultHealthMaxLogSize
-
 		rtSpec, spec, opts, err := MakeContainer(context.Background(), rt, p.PodSpecGen.InfraContainerSpec, false, nil)
 		if err != nil {
 			return nil, err

pkg/specgen/specgen.go

@@ -602,14 +602,17 @@ type ContainerHealthCheckConfig struct {
 	// Requires that HealthConfig be set.
 	// Optional.
 	StartupHealthConfig *define.StartupHealthCheck `json:"startupHealthConfig,omitempty"`
-	// HealthLogDestination defines the destination where the log is stored
-	HealthLogDestination string `json:"healthLogDestination,omitempty"`
+	// HealthLogDestination defines the destination where the log is stored.
+	// Nil value means the default value (local).
+	HealthLogDestination *string `json:"healthLogDestination,omitempty"`
 	// HealthMaxLogCount is maximum number of attempts in the HealthCheck log file.
-	// ('0' value means an infinite number of attempts in the log file)
-	HealthMaxLogCount uint `json:"healthMaxLogCount,omitempty"`
+	// ('0' value means an infinite number of attempts in the log file).
+	// Nil value means the default value (5).
+	HealthMaxLogCount *uint `json:"healthMaxLogCount,omitempty"`
 	// HealthMaxLogSize is the maximum length in characters of stored HealthCheck log
-	// ("0" value means an infinite log length)
-	HealthMaxLogSize uint `json:"healthMaxLogSize,omitempty"`
+	// ("0" value means an infinite log length).
+	// Nil value means the default value (500).
+	HealthMaxLogSize *uint `json:"healthMaxLogSize,omitempty"`
 }
 
 // SpecGenerator creates an OCI spec and Libpod configuration options to create
@@ -682,11 +685,6 @@ func NewSpecGenerator(arg string, rootfs bool) *SpecGenerator {
 	}
 	return &SpecGenerator{
 		ContainerStorageConfig: csc,
-		ContainerHealthCheckConfig: ContainerHealthCheckConfig{
-			HealthLogDestination: define.DefaultHealthCheckLocalDestination,
-			HealthMaxLogCount:    define.DefaultHealthMaxLogCount,
-			HealthMaxLogSize:     define.DefaultHealthMaxLogSize,
-		},
 	}
 }
 
@@ -695,11 +693,6 @@ func NewSpecGeneratorWithRootfs(rootfs string) *SpecGenerator {
 	csc := ContainerStorageConfig{Rootfs: rootfs}
 	return &SpecGenerator{
 		ContainerStorageConfig: csc,
-		ContainerHealthCheckConfig: ContainerHealthCheckConfig{
-			HealthLogDestination: define.DefaultHealthCheckLocalDestination,
-			HealthMaxLogCount:    define.DefaultHealthMaxLogCount,
-			HealthMaxLogSize:     define.DefaultHealthMaxLogSize,
-		},
 	}
 }
 

pkg/specgenutil/specgen.go

@@ -370,11 +370,11 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 	}
 	s.HealthCheckOnFailureAction = onFailureAction
 
-	s.HealthLogDestination = c.HealthLogDestination
+	s.HealthLogDestination = &c.HealthLogDestination
 
-	s.HealthMaxLogCount = c.HealthMaxLogCount
+	s.HealthMaxLogCount = &c.HealthMaxLogCount
 
-	s.HealthMaxLogSize = c.HealthMaxLogSize
+	s.HealthMaxLogSize = &c.HealthMaxLogSize
 
 	if c.StartupHCCmd != "" {
 		if c.NoHealthCheck {

test/apiv2/20-containers.at

@@ -199,7 +199,10 @@ t GET    libpod/containers/${cid}/json 200 \
   .State.Running=false \
   .State.ExitCode=0 \
   .Config.Umask=0022 \
-  .Config.CreateCommand=null
+  .Config.CreateCommand=null \
+  .Config.HealthLogDestination=local \
+  .Config.HealthcheckMaxLogCount=5 \
+  .Config.HealthcheckMaxLogSize=500
 t DELETE libpod/containers/$cid 200 .[0].Id=$cid
 
 CNAME=myfoo