Merge pull request #25520 from Honny1/fix-hc-inf-log

Fix HealthCheck log destination, count, and size defaults
This commit is contained in:
openshift-merge-bot[bot]
2025-03-13 18:59:34 +00:00
committed by GitHub
18 changed files with 138 additions and 134 deletions

View File

@ -1313,6 +1313,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

@ -418,13 +418,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

@ -2970,11 +2970,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 {
@ -2982,7 +2982,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

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

View File

@ -1549,7 +1549,7 @@ func WithHealthCheckLogDestination(destination string) CtrCreateOption {
if err != nil {
return err
}
ctr.config.HealthLogDestination = dest
ctr.config.HealthLogDestination = &dest
return nil
}
}
@ -1560,7 +1560,7 @@ func WithHealthCheckMaxLogCount(maxLogCount uint) CtrCreateOption {
if ctr.valid {
return define.ErrCtrFinalized
}
ctr.config.HealthMaxLogCount = maxLogCount
ctr.config.HealthMaxLogCount = &maxLogCount
return nil
}
}
@ -1571,7 +1571,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

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

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

@ -604,14 +604,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"