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: https://github.com/containers/podman/issues/25473
Fixes: https://issues.redhat.com/browse/RHEL-83262

Signed-off-by: Jan Rodák <hony.com@seznam.cz>
This commit is contained in:
Jan Rodák
2025-03-09 17:59:53 +01:00
committed by openshift-cherrypick-robot
parent 61d703ba85
commit 64e2b91ab4
18 changed files with 138 additions and 134 deletions

View File

@ -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

View File

@ -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.

View File

@ -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

View File

@ -2845,11 +2845,11 @@ 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 {
@ -2857,7 +2857,7 @@ func (c *Container) updateGlobalHealthCheckConfiguration(globalOptions define.Gl
if err != nil {
return err
}
c.config.HealthLogDestination = dest
c.config.HealthLogDestination = &dest
}
if err := c.runtime.state.SafeRewriteContainerConfig(c, "", "", c.config); err != nil {

View File

@ -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 {

View File

@ -136,8 +136,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 {
@ -150,7 +150,7 @@ func (c *Container) runHealthCheck(ctx context.Context, isStartup bool) (define.
healthCheckResult, err := c.updateHealthCheckLog(hcl, 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
@ -404,7 +404,7 @@ func (c *Container) updateHealthCheckLog(hcl define.HealthCheckLog, inStartPerio
}
}
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)
@ -420,11 +420,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
}

View File

@ -1536,7 +1536,7 @@ func WithHealthCheckLogDestination(destination string) CtrCreateOption {
if err != nil {
return err
}
ctr.config.HealthLogDestination = dest
ctr.config.HealthLogDestination = &dest
return nil
}
}
@ -1547,7 +1547,7 @@ func WithHealthCheckMaxLogCount(maxLogCount uint) CtrCreateOption {
if ctr.valid {
return define.ErrCtrFinalized
}
ctr.config.HealthMaxLogCount = maxLogCount
ctr.config.HealthMaxLogCount = &maxLogCount
return nil
}
}
@ -1558,7 +1558,7 @@ func WithHealthCheckMaxLogSize(maxLogSize uint) CtrCreateOption {
if ctr.valid {
return define.ErrCtrFinalized
}
ctr.config.HealthMaxLogSize = maxLogSize
ctr.config.HealthMaxLogSize = &maxLogSize
return nil
}
}

View File

@ -45,6 +45,8 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
},
ContainerHealthCheckConfig: specgen.ContainerHealthCheckConfig{
HealthLogDestination: define.DefaultHealthCheckLocalDestination,
HealthMaxLogCount: define.DefaultHealthMaxLogCount,
HealthMaxLogSize: define.DefaultHealthMaxLogSize,
},
}

View File

@ -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
}

View File

@ -1760,10 +1760,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

View File

@ -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
}

View File

@ -444,9 +444,23 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, containerID
}
}
specg.HealthLogDestination = conf.HealthLogDestination
specg.HealthMaxLogCount = conf.HealthMaxLogCount
specg.HealthMaxLogSize = conf.HealthMaxLogSize
if conf.HealthLogDestination != nil {
specg.HealthLogDestination = *conf.HealthLogDestination
} else {
specg.HealthLogDestination = define.DefaultHealthCheckLocalDestination
}
if conf.HealthMaxLogCount != nil {
specg.HealthMaxLogCount = *conf.HealthMaxLogCount
} else {
specg.HealthMaxLogCount = define.DefaultHealthMaxLogCount
}
if conf.HealthMaxLogSize != nil {
specg.HealthMaxLogSize = *conf.HealthMaxLogSize
} else {
specg.HealthMaxLogSize = define.DefaultHealthMaxLogSize
}
specg.IDMappings = &conf.IDMappings
specg.ContainerCreateCommand = conf.CreateCommand

View File

@ -404,6 +404,10 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
s.Annotations[define.InspectAnnotationInit] = init
}
s.HealthLogDestination = define.DefaultHealthCheckLocalDestination
s.HealthMaxLogCount = define.DefaultHealthMaxLogCount
s.HealthMaxLogSize = define.DefaultHealthMaxLogSize
if publishAll, ok := opts.Annotations[define.InspectAnnotationPublishAll+"/"+opts.Container.Name]; ok {
if opts.IsInfra {
publishAllAsBool, err := strconv.ParseBool(publishAll)
@ -417,9 +421,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{}

View File

@ -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

View File

@ -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.
// TODO (6.0): In next major release convert it to pointer and use omitempty
HealthLogDestination string `json:"healthLogDestination"`
// 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).
// TODO (6.0): In next major release convert it to pointer and use omitempty
HealthMaxLogCount uint `json:"healthMaxLogCount"`
// 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).
// TODO (6.0): In next major release convert it to pointer and use omitempty
HealthMaxLogSize uint `json:"healthMaxLogSize"`
}
// SpecGenerator creates an OCI spec and Libpod configuration options to create

View File

@ -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

View File

@ -316,107 +316,59 @@ function _check_health_log {
assert "$count" $comparison $expect_count "Number of matching health log messages"
}
@test "podman healthcheck --health-max-log-count default value (5)" {
local msg="healthmsg-$(random_string)"
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $msg "{{.Config.HealthMaxLogCount}}" "" "5" "HealthMaxLogCount is the expected default"
@test "podman healthcheck --health-max-log-count values" {
# flag | expected value | op | log count
test="
| 5 | -eq | 5
--health-max-log-count 0 | 0 | -ge | 11
--health-max-log-count=0 | 0 | -ge | 11
--health-max-log-count 10 | 10 | -eq | 10
--health-max-log-count=10 | 10 | -eq | 10
"
for i in $(seq 1 10);
do
run_podman healthcheck run $ctrname
is "$output" "" "unexpected output from podman healthcheck run (pass $i)"
done
while read flag value op logs_count ; do
local msg="healthmsg-$(random_string)"
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $msg "{{.Config.HealthMaxLogCount}}" $flag $value "HealthMaxLogCount"
_check_health_log $ctrname $msg -eq 5
for i in $(seq 1 $((logs_count + 5)));
do
run_podman healthcheck run $ctrname
is "$output" "" "unexpected output from podman healthcheck run (pass $i)"
done
run_podman rm -t 0 -f $ctrname
_check_health_log $ctrname $msg $op $logs_count
run_podman rm -t 0 -f $ctrname
done < <(parse_table "$tests")
}
@test "podman healthcheck --health-max-log-count infinite value (0)" {
local repeat_count=10
local msg="healthmsg-$(random_string)"
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $msg "{{.Config.HealthMaxLogCount}}" "--health-max-log-count 0" "0" "HealthMaxLogCount"
# This is run one more time than repeat_count to check that the cap is working.
for i in $(seq 1 $(($repeat_count + 1)));
do
run_podman healthcheck run $ctrname
is "$output" "" "unexpected output from podman healthcheck run (pass $i)"
done
# The healthcheck is triggered by the podman when the container is started, but its execution depends on systemd.
# And since `run_podman healthcheck run` is also run manually, it will result in two runs.
_check_health_log $ctrname $msg -ge 11
run_podman rm -t 0 -f $ctrname
}
@test "podman healthcheck --health-max-log-count 10" {
local repeat_count=10
local msg="healthmsg-$(random_string)"
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $msg "{{.Config.HealthMaxLogCount}}" "--health-max-log-count $repeat_count" "$repeat_count" "HealthMaxLogCount"
# This is run one more time than repeat_count to check that the cap is working.
for i in $(seq 1 $(($repeat_count + 1)));
do
run_podman healthcheck run $ctrname
is "$output" "" "unexpected output from podman healthcheck run (pass $i)"
done
_check_health_log $ctrname $msg -eq $repeat_count
run_podman rm -t 0 -f $ctrname
}
@test "podman healthcheck --health-max-log-size 10" {
local msg="healthmsg-$(random_string)"
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $msg "{{.Config.HealthMaxLogSize}}" "--health-max-log-size 10" "10" "HealthMaxLogSize"
run_podman healthcheck run $ctrname
is "$output" "" "output from 'podman healthcheck run'"
local substr=${msg:0:10}
_check_health_log $ctrname "$substr}]\$" -eq 1
run_podman rm -t 0 -f $ctrname
}
@test "podman healthcheck --health-max-log-size infinite value (0)" {
@test "podman healthcheck --health-max-log-size values" {
local s=$(printf "healthmsg-%1000s")
local long_msg=${s// /$(random_string)}
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $long_msg "{{.Config.HealthMaxLogSize}}" "--health-max-log-size 0" "0" "HealthMaxLogSize"
run_podman healthcheck run $ctrname
is "$output" "" "output from 'podman healthcheck run'"
# flag | expected value | exp_msg
test="
| 500 | ${long_msg:0:500}}]\$
--health-max-log-size 0 | 0 | $long_msg}]\$
--health-max-log-size=0 | 0 | $long_msg}]\$
--health-max-log-size 10 | 10 | ${long_msg:0:10}}]\$
--health-max-log-size=10 | 10 | ${long_msg:0:10}}]\$
"
# The healthcheck is triggered by the podman when the container is started, but its execution depends on systemd.
# And since `run_podman healthcheck run` is also run manually, it will result in two runs.
_check_health_log $ctrname "$long_msg" -ge 1
while read flag value exp_msg ; do
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $long_msg "{{.Config.HealthMaxLogSize}}" $flag $value "HealthMaxLogSize"
run_podman rm -t 0 -f $ctrname
run_podman healthcheck run $ctrname
is "$output" "" "output from 'podman healthcheck run'"
_check_health_log $ctrname $exp_msg -eq 1
run_podman rm -t 0 -f $ctrname
done < <(parse_table "$tests")
}
@test "podman healthcheck --health-max-log-size default value (500)" {
local s=$(printf "healthmsg-%1000s")
local long_msg=${s// /$(random_string)}
local ctrname="c-h-$(safename)"
_create_container_with_health_log_settings $ctrname $long_msg "{{.Config.HealthMaxLogSize}}" "" "500" "HealthMaxLogSize is the expected default"
run_podman healthcheck run $ctrname
is "$output" "" "output from 'podman healthcheck run'"
local expect_msg="${long_msg:0:500}"
_check_health_log $ctrname "$expect_msg}]\$" -eq 1
run_podman rm -t 0 -f $ctrname
}
@test "podman healthcheck --health-log-destination file" {
local TMP_DIR_HEALTHCHECK="$PODMAN_TMPDIR/healthcheck"
mkdir $TMP_DIR_HEALTHCHECK

View File

@ -274,6 +274,11 @@ failed | exited | 17
done < <(parse_table "$tests")
}
@test "inspect - HealthCheck Defaults" {
run_podman inspect --format '{{.Config.HealthMaxLogSize}}--{{.Config.HealthMaxLogCount}}--{{.Config.HealthLogDestination}}' myrunningcontainer
assert "$output" == "500--5--local" "HealthCheck Default values of Log size, count and destination"
}
@test "network - curl" {
run -0 curl --max-time 3 -s 127.0.0.1:$HOST_PORT/index.txt
is "$output" "$RANDOM_STRING_1" "curl on running container"