From c57bfa974988d6a8210e8ac1e4327009b7d9d698 Mon Sep 17 00:00:00 2001 From: Akash Kumar <116457960+akashchamp@users.noreply.github.com> Date: Thu, 24 Sep 2026 03:45:08 +0530 Subject: [PATCH 1/2] feat(healthcheck): add --health-start-interval for run/create Docker's `--health-start-interval` lets a container probe more frequently while it is still inside `--health-start-period`, then fall back to the regular `--health-interval` cadence once the start period has elapsed. nerdctl accepted `--health-start-period` but had no way to probe faster during it, and docs/command-reference.md still listed `--health-start-interval` as unimplemented. This wires the flag through run/create exactly like the existing `--health-start-period` plumbing (CLI flag, ContainerCreateOptions, validation, the Healthcheck struct that gets stored in the container label and read back on inspect), defaults it to Docker's 5s when unset, and threads it through `nerdctl compose` via `healthcheck.start_interval`. The one real behavioral change is probe scheduling. nerdctl schedules probes with a single systemd timer created once per container, whose tick cadence can't safely be changed while the very unit it triggers is still running (that unit is what invokes `nerdctl container healthcheck`). So the timer now ticks at the faster of `--health-interval` and `--health-start-interval`, and `ExecuteHealthCheck` skips ticks that arrive before the interval that actually applies (start-interval while in the start period, interval afterward) has elapsed since the last probe that really ran. This keeps the systemd side untouched for the container's lifetime while still producing the Docker-compatible effective cadence. Verified with a real container against rootful containerd + systemd: with --health-interval=20s --health-start-period=6s --health-start-interval=1s, probes ran roughly every ~1.3-1.8s during the start period, then the next probe landed ~21.7s after the last one (matching --health-interval, not --health-start-interval) once the start period ended - while `systemctl show` confirmed the timer's OnUnitInactiveUSec stayed fixed at 1s throughout, i.e. the cadence switch is purely the new software-side throttle, not a timer edit. Signed-off-by: Akash Kumar <116457960+akashchamp@users.noreply.github.com> --- cmd/nerdctl/container/container_create.go | 4 + .../container_health_check_linux_test.go | 84 ++++++++++++++ cmd/nerdctl/container/container_run.go | 1 + cmd/nerdctl/container/container_run_test.go | 40 ++++--- cmd/nerdctl/helpers/flagutil.go | 6 +- docs/command-reference.md | 4 +- docs/compose.md | 1 - docs/healthchecks.md | 15 ++- pkg/api/types/container_types.go | 13 ++- pkg/cmd/container/create.go | 3 + pkg/cmd/container/health_check.go | 1 + pkg/composer/serviceparser/serviceparser.go | 5 +- .../serviceparser/serviceparser_test.go | 2 + pkg/healthcheck/executor.go | 57 ++++++++- pkg/healthcheck/executor_test.go | 108 ++++++++++++++++++ pkg/healthcheck/health.go | 32 +++--- pkg/healthcheck/healthcheck_manager_linux.go | 15 ++- 17 files changed, 348 insertions(+), 43 deletions(-) create mode 100644 pkg/healthcheck/executor_test.go diff --git a/cmd/nerdctl/container/container_create.go b/cmd/nerdctl/container/container_create.go index e30d529481a..fc5cbdd97c8 100644 --- a/cmd/nerdctl/container/container_create.go +++ b/cmd/nerdctl/container/container_create.go @@ -279,6 +279,10 @@ func createOptions(cmd *cobra.Command) (types.ContainerCreateOptions, error) { if err != nil { return opt, err } + opt.HealthStartInterval, err = cmd.Flags().GetDuration("health-start-interval") + if err != nil { + return opt, err + } opt.NoHealthcheck, err = cmd.Flags().GetBool("no-healthcheck") if err != nil { return opt, err diff --git a/cmd/nerdctl/container/container_health_check_linux_test.go b/cmd/nerdctl/container/container_health_check_linux_test.go index 15c012df5ee..b8b63e92f19 100644 --- a/cmd/nerdctl/container/container_health_check_linux_test.go +++ b/cmd/nerdctl/container/container_health_check_linux_test.go @@ -189,6 +189,7 @@ func TestContainerHealthCheckDefaults(t *testing.T) { assert.Equal(t, hc.Timeout, 30*time.Second, "expected default timeout of 30s") assert.Equal(t, hc.Retries, 3, "expected default retries of 3") assert.Equal(t, hc.StartPeriod, 0*time.Second, "expected default start period of 0s") + assert.Equal(t, hc.StartInterval, 5*time.Second, "expected default start interval of 5s") // Verify the command was set correctly assert.DeepEqual(t, hc.Test, []string{"CMD-SHELL", "echo healthy"}) @@ -206,6 +207,7 @@ func TestContainerHealthCheckDefaults(t *testing.T) { "--health-timeout", "15s", "--health-retries", "5", "--health-start-period", "10s", + "--health-start-interval", "3s", testutil.CommonImage, "sleep", nerdtest.Infinity) nerdtest.EnsureContainerStarted(helpers, data.Identifier()) }, @@ -234,6 +236,7 @@ func TestContainerHealthCheckDefaults(t *testing.T) { assert.Equal(t, hc.Timeout, 15*time.Second, "expected custom timeout of 15s") assert.Equal(t, hc.Retries, 5, "expected custom retries of 5") assert.Equal(t, hc.StartPeriod, 10*time.Second, "expected custom start period of 10s") + assert.Equal(t, hc.StartInterval, 3*time.Second, "expected custom start interval of 3s") // Verify the command was set correctly assert.DeepEqual(t, hc.Test, []string{"CMD-SHELL", "echo custom"}) @@ -389,6 +392,87 @@ func TestContainerHealthCheckAdvance(t *testing.T) { } }, }, + { + Description: "Health check probes at start-interval cadence within the start period", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--name", data.Identifier(), + "--health-cmd", "exit 1", + "--health-interval", "60s", + "--health-start-period", "30s", + "--health-start-interval", "1s", + testutil.CommonImage, "sleep", nerdtest.Infinity) + nerdtest.EnsureContainerStarted(helpers, data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + helpers.Ensure("container", "healthcheck", data.Identifier()) + // Longer than --health-start-interval (1s) but much shorter than + // --health-interval (60s): this tick must still run because we are + // still within --health-start-period. + time.Sleep(2 * time.Second) + helpers.Ensure("container", "healthcheck", data.Identifier()) + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: 0, + Output: expect.All(func(stdout string, t tig.T) { + inspect := nerdtest.InspectContainer(helpers, data.Identifier()) + h := inspect.State.Health + debug, _ := json.MarshalIndent(h, "", " ") + t.Log(string(debug)) + assert.Assert(t, h != nil, "expected health state") + // health-cmd always fails, so unhealthy results are ignored and we + // remain in the start period workflow throughout. + assert.Equal(t, h.Status, healthcheck.Starting) + assert.Equal(t, len(h.Log), 2, + "expected both ticks to run: each was spaced beyond --health-start-interval") + }), + } + }, + }, + { + Description: "Health check falls back to health-interval cadence once the start period ends", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--name", data.Identifier(), + "--health-cmd", "exit 0", + "--health-interval", "60s", + "--health-start-period", "5s", + "--health-start-interval", "1s", + testutil.CommonImage, "sleep", nerdtest.Infinity) + nerdtest.EnsureContainerStarted(helpers, data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + // First tick always runs and, since health-cmd succeeds, immediately + // exits the start period (first healthy result). + helpers.Ensure("container", "healthcheck", data.Identifier()) + // Second tick arrives well within --health-start-interval (1s), but the + // start period already ended, so --health-interval (60s) now applies and + // this tick must be skipped. + helpers.Ensure("container", "healthcheck", data.Identifier()) + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: 0, + Output: expect.All(func(stdout string, t tig.T) { + inspect := nerdtest.InspectContainer(helpers, data.Identifier()) + h := inspect.State.Health + debug, _ := json.MarshalIndent(h, "", " ") + t.Log(string(debug)) + assert.Assert(t, h != nil, "expected health state") + assert.Equal(t, h.Status, healthcheck.Healthy) + assert.Equal(t, len(h.Log), 1, + "expected the second tick to be throttled by --health-interval after the start period ended") + }), + } + }, + }, { Description: "Health check with invalid command", Setup: func(data test.Data, helpers test.Helpers) { diff --git a/cmd/nerdctl/container/container_run.go b/cmd/nerdctl/container/container_run.go index 47f63aac2b7..8ad38f12b3d 100644 --- a/cmd/nerdctl/container/container_run.go +++ b/cmd/nerdctl/container/container_run.go @@ -256,6 +256,7 @@ func setCreateFlags(cmd *cobra.Command) { cmd.Flags().Duration("health-timeout", 0, "Maximum time to allow one check to run; 0 uses the image value or 30s when unset there too") cmd.Flags().Int("health-retries", 0, "Consecutive failures needed to report unhealthy; 0 uses the image value or 3 when unset there too") cmd.Flags().Duration("health-start-period", 0, "Start period for the container to initialize before starting health-retries countdown") + cmd.Flags().Duration("health-start-interval", 0, "Time between running the check during the start period; 0 uses the image value or 5s when unset there too") cmd.Flags().Bool("no-healthcheck", false, "Disable any container-specified HEALTHCHECK") // #region env flags diff --git a/cmd/nerdctl/container/container_run_test.go b/cmd/nerdctl/container/container_run_test.go index 4f002cfeb9e..d7fedf18f9e 100644 --- a/cmd/nerdctl/container/container_run_test.go +++ b/cmd/nerdctl/container/container_run_test.go @@ -937,14 +937,15 @@ func TestRunHealthcheckFlags(t *testing.T) { testCase.Require = require.Not(nerdtest.Rootless) testCases := []struct { - name string - args []string - shouldFail bool - expectTest []string - expectRetries int - expectInterval time.Duration - expectTimeout time.Duration - expectStartPeriod time.Duration + name string + args []string + shouldFail bool + expectTest []string + expectRetries int + expectInterval time.Duration + expectTimeout time.Duration + expectStartPeriod time.Duration + expectStartInterval time.Duration }{ { name: "Valid_full_config", @@ -954,12 +955,14 @@ func TestRunHealthcheckFlags(t *testing.T) { "--health-timeout", "5s", "--health-retries", "3", "--health-start-period", "2s", + "--health-start-interval", "1s", }, - expectTest: []string{"CMD-SHELL", "curl -f http://localhost || exit 1"}, - expectInterval: 30 * time.Second, - expectTimeout: 5 * time.Second, - expectRetries: 3, - expectStartPeriod: 2 * time.Second, + expectTest: []string{"CMD-SHELL", "curl -f http://localhost || exit 1"}, + expectInterval: 30 * time.Second, + expectTimeout: 5 * time.Second, + expectRetries: 3, + expectStartPeriod: 2 * time.Second, + expectStartInterval: 1 * time.Second, }, { name: "No_healthcheck", @@ -996,6 +999,14 @@ func TestRunHealthcheckFlags(t *testing.T) { }, shouldFail: true, }, + { + name: "Negative_start_interval", + args: []string{ + "--health-cmd", "true", + "--health-start-interval", "-1s", + }, + shouldFail: true, + }, { name: "Invalid_timeout_format", args: []string{ @@ -1067,6 +1078,9 @@ func TestRunHealthcheckFlags(t *testing.T) { if tc.expectStartPeriod > 0 { assert.Equal(t, hc.StartPeriod, tc.expectStartPeriod) } + if tc.expectStartInterval > 0 { + assert.Equal(t, hc.StartInterval, tc.expectStartInterval) + } }, ), } diff --git a/cmd/nerdctl/helpers/flagutil.go b/cmd/nerdctl/helpers/flagutil.go index 514651d3235..d8359b74361 100644 --- a/cmd/nerdctl/helpers/flagutil.go +++ b/cmd/nerdctl/helpers/flagutil.go @@ -52,7 +52,8 @@ func ValidateHealthcheckFlags(options types.ContainerCreateOptions) error { options.HealthInterval != 0 || options.HealthTimeout != 0 || options.HealthRetries != 0 || - options.HealthStartPeriod != 0 + options.HealthStartPeriod != 0 || + options.HealthStartInterval != 0 if options.NoHealthcheck { if options.HealthCmd != "" || healthFlagsSet { @@ -73,6 +74,9 @@ func ValidateHealthcheckFlags(options types.ContainerCreateOptions) error { if options.HealthStartPeriod < 0 { return fmt.Errorf("--health-start-period cannot be negative") } + if options.HealthStartInterval < 0 { + return fmt.Errorf("--health-start-interval cannot be negative") + } return nil } diff --git a/docs/command-reference.md b/docs/command-reference.md index bf6ceb2a0ed..130496bee40 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -365,6 +365,7 @@ Health check flags: - :whale: `--health-timeout`: Time to wait before considering the check failed (e.g., 5s) - :whale: `--health-retries`: Number of failures before container is considered unhealthy - :whale: `--health-start-period`: Start period for the container to initialize before starting health-retries countdown +- :whale: `--health-start-interval`: Time between running the check during the start period (e.g., 5s) - :whale: `--no-healthcheck`: Disable any health checks defined by image or CLI Logging flags: @@ -475,8 +476,7 @@ IPFS flags: Unimplemented `docker run` flags: `--device-cgroup-rule`, `--disable-content-trust`, - `--health-start-interval`, `--link*`, `--storage-opt`, - `--volume-driver` + `--link*`, `--storage-opt`, `--volume-driver` ### :whale: nerdctl exec diff --git a/docs/compose.md b/docs/compose.md index c48639a612e..508d5f8b078 100644 --- a/docs/compose.md +++ b/docs/compose.md @@ -28,7 +28,6 @@ which was derived from [Docker Compose file version 3 specification](https://doc - `services..deploy.resources.reservations` - `services..deploy.placement` - `services..deploy.endpoint_mode` -- `services..healthcheck.start_interval` - `services..stop_grace_period` - `services..stop_signal` - `configs..external` diff --git a/docs/healthchecks.md b/docs/healthchecks.md index 628a8710a29..eeda7ccbe01 100644 --- a/docs/healthchecks.md +++ b/docs/healthchecks.md @@ -14,12 +14,11 @@ Health checks can be configured in multiple ways: - `--health-timeout`: Maximum time to allow one check to run (default: 30s) - `--health-retries`: Consecutive failures needed to report unhealthy (default: 3) - `--health-start-period`: Start period for the container to initialize before starting health-retries countdown + - `--health-start-interval`: Time between running the check during the start period (default: 5s) - `--no-healthcheck`: Disable any container-specified HEALTHCHECK 2. At image build time using HEALTHCHECK in a Dockerfile -**Note:** The `--health-start-interval` option is currently not supported by nerdctl. - ## Configuration Priority When a container is created, nerdctl determines the health check configuration based on this priority: @@ -90,7 +89,17 @@ nerdctl run -d --name app \ myapp ``` -3. Disable health checks: +3. Health check that probes more frequently while starting up: +```bash +nerdctl run -d --name app \ + --health-cmd="./health-check.sh" \ + --health-interval=30s \ + --health-start-period=60s \ + --health-start-interval=5s \ + myapp +``` + +4. Disable health checks: ```bash nerdctl run --no-healthcheck myapp ``` diff --git a/pkg/api/types/container_types.go b/pkg/api/types/container_types.go index 8dd099eb53d..6191ef49c85 100644 --- a/pkg/api/types/container_types.go +++ b/pkg/api/types/container_types.go @@ -300,12 +300,13 @@ type ContainerCreateOptions struct { ImagePullOpt ImagePullOptions // Healthcheck related fields - HealthCmd string - HealthInterval time.Duration - HealthTimeout time.Duration - HealthRetries int - HealthStartPeriod time.Duration - NoHealthcheck bool + HealthCmd string + HealthInterval time.Duration + HealthTimeout time.Duration + HealthRetries int + HealthStartPeriod time.Duration + HealthStartInterval time.Duration + NoHealthcheck bool // UserNS name for user namespace mapping of container UserNS string diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 6e13deb5562..323643c56b9 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -1084,6 +1084,9 @@ func withHealthcheck(options types.ContainerCreateOptions, ensuredImage *imgutil if options.HealthStartPeriod != 0 { hc.StartPeriod = options.HealthStartPeriod } + if options.HealthStartInterval != 0 { + hc.StartInterval = options.HealthStartInterval + } // Apply defaults for any unset values, but only if we have a healthcheck configured if len(hc.Test) > 0 && hc.Test[0] != "NONE" { diff --git a/pkg/cmd/container/health_check.go b/pkg/cmd/container/health_check.go index 1a96028eb50..63a6e41f1b2 100644 --- a/pkg/cmd/container/health_check.go +++ b/pkg/cmd/container/health_check.go @@ -70,6 +70,7 @@ func HealthCheck(ctx context.Context, client *containerd.Client, container conta hcConfig.Interval = timeoutWithDefault(hcConfig.Interval, healthcheck.DefaultProbeInterval) hcConfig.Timeout = timeoutWithDefault(hcConfig.Timeout, healthcheck.DefaultProbeTimeout) hcConfig.StartPeriod = timeoutWithDefault(hcConfig.StartPeriod, healthcheck.DefaultStartPeriod) + hcConfig.StartInterval = timeoutWithDefault(hcConfig.StartInterval, healthcheck.DefaultProbeStartInterval) if hcConfig.Retries == 0 { hcConfig.Retries = healthcheck.DefaultProbeRetries } diff --git a/pkg/composer/serviceparser/serviceparser.go b/pkg/composer/serviceparser/serviceparser.go index 58b9393dbd4..dd5d6687ae0 100644 --- a/pkg/composer/serviceparser/serviceparser.go +++ b/pkg/composer/serviceparser/serviceparser.go @@ -130,9 +130,9 @@ func warnUnknownFields(svc types.ServiceConfig) { "Interval", "Retries", "StartPeriod", + "StartInterval", "Disable", "Extensions", - // TODO: add support 'StartInterval' ); len(unknown) > 0 { log.L.Warnf("Ignoring: service %s: healthcheck: %+v", svc.Name, unknown) } @@ -833,6 +833,9 @@ func newContainer(project *types.Project, parsed *Service, i int) (*Container, e if hc.StartPeriod != nil { c.RunArgs = append(c.RunArgs, fmt.Sprintf("--health-start-period=%s", time.Duration(*hc.StartPeriod).String())) } + if hc.StartInterval != nil { + c.RunArgs = append(c.RunArgs, fmt.Sprintf("--health-start-interval=%s", time.Duration(*hc.StartInterval).String())) + } } } diff --git a/pkg/composer/serviceparser/serviceparser_test.go b/pkg/composer/serviceparser/serviceparser_test.go index ec23cdf6142..71c0b653be2 100644 --- a/pkg/composer/serviceparser/serviceparser_test.go +++ b/pkg/composer/serviceparser/serviceparser_test.go @@ -936,6 +936,7 @@ services: timeout: 10s retries: 3 start_period: 5s + start_interval: 2s cmd_exec: image: alpine:3.14 healthcheck: @@ -965,6 +966,7 @@ services: assert.Assert(t, in(c.RunArgs, "--health-timeout=10s")) assert.Assert(t, in(c.RunArgs, "--health-retries=3")) assert.Assert(t, in(c.RunArgs, "--health-start-period=5s")) + assert.Assert(t, in(c.RunArgs, "--health-start-interval=2s")) c = getContainersFromService(t, project, "cmd_exec")[0] assert.Assert(t, in(c.RunArgs, "--health-cmd=curl -f http://localhost")) diff --git a/pkg/healthcheck/executor.go b/pkg/healthcheck/executor.go index e86c65e7f4c..543fa170eb8 100644 --- a/pkg/healthcheck/executor.go +++ b/pkg/healthcheck/executor.go @@ -34,6 +34,15 @@ import ( // ExecuteHealthCheck executes the health check command for a container func ExecuteHealthCheck(ctx context.Context, task containerd.Task, container containerd.Container, hc *Healthcheck) error { + run, err := shouldRunProbe(ctx, container, hc) + if err != nil { + return err + } + if !run { + log.G(ctx).Debugf("skipping health check tick for %s: next probe is not due yet", container.ID()) + return nil + } + // Prepare process spec for health check command processSpec, err := prepareProcessSpec(ctx, container, hc) if err != nil { @@ -63,6 +72,50 @@ func ExecuteHealthCheck(ctx context.Context, task containerd.Task, container con return nil } +// shouldRunProbe reports whether a probe should actually execute on this tick. +// +// The systemd timer that drives ticks (see CreateTimer) runs at a single fixed cadence for the +// whole container lifetime, sized to the faster of --health-interval and --health-start-interval +// so it can be responsive during the start period. This function is what makes the cadence +// actually vary: while still within --health-start-period, a probe is due every +// --health-start-interval; once that period has elapsed (or ended early via a healthy result), +// ticks that arrive before a full --health-interval has passed since the last probe are skipped, +// so the effective probe cadence matches the configured --health-interval. +func shouldRunProbe(ctx context.Context, container containerd.Container, hc *Healthcheck) (bool, error) { + state, err := readHealthStateFromLabels(ctx, container) + if err != nil { + return false, fmt.Errorf("failed to read health state from labels: %w", err) + } + // No prior state recorded: this is the first tick for this container, always run it. + if state == nil { + return true, nil + } + + info, err := container.Info(ctx) + if err != nil { + return false, fmt.Errorf("failed to get container info: %w", err) + } + + return shouldRunProbeAt(time.Now(), info.CreatedAt, state.LastProbeAt, state.InStartPeriod, hc), nil +} + +// shouldRunProbeAt is the pure decision behind shouldRunProbe: given "now", it decides whether a +// probe is due, based on when the container was created, when a probe last actually ran, whether +// we're still tracking the container as being within its start period, and the health check +// configuration currently in effect. +func shouldRunProbeAt(now, containerCreated, lastProbeAt time.Time, inStartPeriod bool, hc *Healthcheck) bool { + // No prior probe recorded: always run the first one. + if lastProbeAt.IsZero() { + return true + } + + target := hc.Interval + if hc.StartPeriod > 0 && inStartPeriod && now.Sub(containerCreated) < hc.StartPeriod { + target = hc.StartInterval + } + return now.Sub(lastProbeAt) >= target +} + // probeHealthCheck executes the health check command inside the container context func probeHealthCheck(ctx context.Context, task containerd.Task, hc *Healthcheck, processSpec *specs.Process) (*HealthcheckResult, error) { execID := "health-check-" + idgen.TruncateID(idgen.GenerateID()) @@ -174,7 +227,9 @@ func updateHealthStatus(ctx context.Context, container containerd.Container, hcC } } - // Write updated health state back to labels + // Record when this probe ran so the next tick can tell whether it is due yet + // (see shouldRunProbe), and update the label with new health state. + currentHealth.LastProbeAt = hcResult.Start if err := writeHealthStateToLabels(ctx, container, currentHealth); err != nil { return fmt.Errorf("failed to write health state to labels: %w", err) } diff --git a/pkg/healthcheck/executor_test.go b/pkg/healthcheck/executor_test.go new file mode 100644 index 00000000000..044da566b51 --- /dev/null +++ b/pkg/healthcheck/executor_test.go @@ -0,0 +1,108 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package healthcheck + +import ( + "testing" + "time" + + "gotest.tools/v3/assert" +) + +func TestShouldRunProbeAt(t *testing.T) { + created := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + + hc := &Healthcheck{ + Interval: 60 * time.Second, + StartPeriod: 30 * time.Second, + StartInterval: 5 * time.Second, + } + + tests := []struct { + description string + now time.Time + lastProbeAt time.Time + inStartPeriod bool + hc *Healthcheck + want bool + }{ + { + description: "always runs the first ever probe", + now: created, + lastProbeAt: time.Time{}, + hc: hc, + want: true, + }, + { + description: "runs again once start-interval has elapsed, inside the start period", + now: created.Add(10 * time.Second), + lastProbeAt: created.Add(5 * time.Second), + inStartPeriod: true, + hc: hc, + want: true, // 5s elapsed >= 5s start-interval + }, + { + description: "skips a tick that arrives before start-interval has elapsed", + now: created.Add(8 * time.Second), + lastProbeAt: created.Add(5 * time.Second), + inStartPeriod: true, + hc: hc, + want: false, // 3s elapsed < 5s start-interval + }, + { + description: "skips a tick within health-interval once the start period has elapsed", + now: created.Add(35 * time.Second), + lastProbeAt: created.Add(31 * time.Second), + inStartPeriod: false, + hc: hc, + want: false, // 4s elapsed < 60s health-interval, no longer in start period + }, + { + description: "skips a tick within health-interval once InStartPeriod flips off, even if still before start-period elapses", + now: created.Add(12 * time.Second), + lastProbeAt: created.Add(10 * time.Second), + inStartPeriod: false, // e.g. an earlier healthy result already ended the start period + hc: hc, + want: false, // 2s elapsed < 60s health-interval + }, + { + description: "runs again once health-interval has elapsed after the start period", + now: created.Add(95 * time.Second), + lastProbeAt: created.Add(35 * time.Second), + inStartPeriod: false, + hc: hc, + want: true, // 60s elapsed >= 60s health-interval + }, + { + description: "treats a zero start period as never in the start-interval phase", + now: created.Add(2 * time.Second), + lastProbeAt: created.Add(1 * time.Second), + inStartPeriod: true, + hc: &Healthcheck{ + Interval: 60 * time.Second, + StartPeriod: 0, + StartInterval: 5 * time.Second, + }, + want: false, // StartPeriod == 0 means health-interval always applies + }, + } + + for _, tc := range tests { + got := shouldRunProbeAt(tc.now, created, tc.lastProbeAt, tc.inStartPeriod, tc.hc) + assert.Equal(t, got, tc.want, tc.description) + } +} diff --git a/pkg/healthcheck/health.go b/pkg/healthcheck/health.go index 70104187e29..e4407f80504 100644 --- a/pkg/healthcheck/health.go +++ b/pkg/healthcheck/health.go @@ -40,14 +40,15 @@ const ( ) const ( - DefaultProbeInterval = 30 * time.Second // Default interval between probe runs. Also applies before the first probe. - DefaultProbeTimeout = 30 * time.Second // Max duration a single probe run may take before it's considered failed. - DefaultStartPeriod = 0 * time.Second // Grace period for container startup before health checks count as failures. - DefaultProbeRetries = 3 // Number of consecutive failures before marking container as unhealthy. - MaxLogEntries = 5 // Maximum number of health check log entries to keep. - MaxOutputLenForInspect = 4096 // Max output length (in bytes) stored in health check logs during inspect. Longer outputs are truncated. - MaxOutputLen = 1 * 1024 * 1024 // Max output size for health check logs: 1MB limit (prevents excessive memory usage) - HealthLogFilename = "health.json" // HealthLogFilename is the name of the file used to persist health check status for a container. + DefaultProbeInterval = 30 * time.Second // Default interval between probe runs. Also applies before the first probe. + DefaultProbeTimeout = 30 * time.Second // Max duration a single probe run may take before it's considered failed. + DefaultStartPeriod = 0 * time.Second // Grace period for container startup before health checks count as failures. + DefaultProbeStartInterval = 5 * time.Second // Default interval between probe runs while still within the start period. + DefaultProbeRetries = 3 // Number of consecutive failures before marking container as unhealthy. + MaxLogEntries = 5 // Maximum number of health check log entries to keep. + MaxOutputLenForInspect = 4096 // Max output length (in bytes) stored in health check logs during inspect. Longer outputs are truncated. + MaxOutputLen = 1 * 1024 * 1024 // Max output size for health check logs: 1MB limit (prevents excessive memory usage) + HealthLogFilename = "health.json" // HealthLogFilename is the name of the file used to persist health check status for a container. ) // NOTE: Health, HealthcheckResult and Healthcheck types are kept Docker-compatible. @@ -69,11 +70,12 @@ type HealthcheckResult struct { // Healthcheck represents the health check configuration type Healthcheck struct { - Test []string `json:"Test,omitempty"` // Test is the check to perform that the container is healthy - Interval time.Duration `json:"Interval,omitempty"` // Interval is the time to wait between checks - Timeout time.Duration `json:"Timeout,omitempty"` // Timeout is the time to wait before considering the check to have hung - Retries int `json:"Retries,omitempty"` // Retries is the number of consecutive failures needed to consider a container as unhealthy - StartPeriod time.Duration `json:"StartPeriod,omitempty"` // StartPeriod is the period for the container to initialize before the health check starts + Test []string `json:"Test,omitempty"` // Test is the check to perform that the container is healthy + Interval time.Duration `json:"Interval,omitempty"` // Interval is the time to wait between checks + Timeout time.Duration `json:"Timeout,omitempty"` // Timeout is the time to wait before considering the check to have hung + Retries int `json:"Retries,omitempty"` // Retries is the number of consecutive failures needed to consider a container as unhealthy + StartPeriod time.Duration `json:"StartPeriod,omitempty"` // StartPeriod is the period for the container to initialize before the health check starts + StartInterval time.Duration `json:"StartInterval,omitempty"` // StartInterval is the time to wait between checks while still within the start period } // HealthState stores the current health state of a container @@ -81,6 +83,7 @@ type HealthState struct { Status HealthStatus // Status is one of [Starting], [Healthy] or [Unhealthy] FailingStreak int // FailingStreak is the number of consecutive failures InStartPeriod bool // InStartPeriod indicates if we're in the start period workflow + LastProbeAt time.Time `json:",omitempty"` // LastProbeAt is the start time of the most recently executed (non-skipped) probe } // ToJSONString serializes HealthState to a JSON string for label storage @@ -148,6 +151,9 @@ func (hc *Healthcheck) ApplyDefaults() { if hc.StartPeriod == 0 { hc.StartPeriod = DefaultStartPeriod } + if hc.StartInterval == 0 { + hc.StartInterval = DefaultProbeStartInterval + } if hc.Retries == 0 { hc.Retries = DefaultProbeRetries } diff --git a/pkg/healthcheck/healthcheck_manager_linux.go b/pkg/healthcheck/healthcheck_manager_linux.go index 5fdd69e7e2f..7e56824c66c 100644 --- a/pkg/healthcheck/healthcheck_manager_linux.go +++ b/pkg/healthcheck/healthcheck_manager_linux.go @@ -62,14 +62,25 @@ func CreateTimer(ctx context.Context, container containerd.Container, cfg *confi cmdOpts = append(cmdOpts, "--setenv=BUILDKIT_HOST="+buildKitHost) } - // Always use health-interval for timer frequency + // The timer ticks at a single fixed cadence for the whole container lifetime: systemd timer + // properties can't safely be changed while the unit they trigger is running (which is the + // case here, since this very process is what the timer just triggered). To still honor + // --health-start-interval, tick at the faster of health-interval and health-start-interval so + // ticks are frequent enough during the start period; ExecuteHealthCheck (via shouldRunProbe) + // then skips ticks that arrive before the interval that actually applies has elapsed, so the + // effective probe cadence matches --health-start-interval during the start period and + // --health-interval afterward. + tickInterval := hc.Interval + if hc.StartPeriod > 0 && hc.StartInterval > 0 && hc.StartInterval < tickInterval { + tickInterval = hc.StartInterval + } // // --collect: // Even when the healthcheck fails with the error "container is not running" after the container has // stopped, and the transient service unit enters a failed state, it will still be subject to garbage // collection due to the --collect option. Without this option, `systemctl reset-failed` would explicitly be needed. // See: https://www.freedesktop.org/software/systemd/man/latest/systemd-run.html#-G - cmdOpts = append(cmdOpts, "--unit", containerID, "--on-unit-inactive="+hc.Interval.String(), "--timer-property=AccuracySec=1s", "--collect") + cmdOpts = append(cmdOpts, "--unit", containerID, "--on-unit-inactive="+tickInterval.String(), "--timer-property=AccuracySec=1s", "--collect") cmdOpts = append(cmdOpts, nerdctlCmd) cmdOpts = append(cmdOpts, nerdctlArgs...) From 396e3ac05c644207b0be0b7c9ea0660f33a026dd Mon Sep 17 00:00:00 2001 From: Akash Kumar <116457960+akashchamp@users.noreply.github.com> Date: Fri, 25 Sep 2026 01:29:05 +0530 Subject: [PATCH 2/2] fix(healthcheck): correct start-interval test assumptions and split oversized test func Three CI failures on this branch, all confined to tests this PR itself touches or added: - "Health status transitions from healthy to unhealthy after retries" (pre-existing test) now fails because probes are throttled to run no more than once per --health-interval (see shouldRunProbe). That test doesn't set --health-interval, so the 30s default applied while the test drives 4 manual "container healthcheck" calls only 2s apart - only the first one actually ran, so FailingStreak never reached the configured 3 retries. Pass --health-interval 1s explicitly so each manual call is spaced beyond the throttle window, matching the pre-existing intent of the test. - "Health check probes at start-interval cadence within the start period" (new in this PR) asserted exactly 2 log entries after 2 manual "container healthcheck" invocations. With --health-start-interval 1s, the container's own background systemd timer also ticks every 1s during the start period and can legitimately land a probe in the ~2s window between the two manual calls, producing a 3rd log entry. Relax the assertion to a lower bound (>= 2) with a comment explaining why, since the property under test - that manual ticks aren't skipped - doesn't require an exact count. - golangci-lint (revive function-length, max 500 lines) failed because the two new start-interval subtests pushed TestContainerHealthCheckAdvance to 553 lines. Move them into a new TestContainerHealthCheckStartInterval function (479 and 103 lines respectively after the split). The EL/almalinux-8 rootful/rootless failures (TestComposeCopy, TestCopyToContainer, TestCopyFromContainer) come from workflow-flaky.yml, this repo's own known-flaky/experimental job bucket, and don't touch any file this PR changes. The gomodjail rootless (TestRunRmTime, a hard wall-clock deadline) and rootless-port-slirp4netns (TestLoadQuiet) failures are likewise in files untouched by this diff. All three are left alone as external. Verified: go build ./..., go vet ./pkg/healthcheck/... ./cmd/nerdctl/container/..., go test ./pkg/healthcheck/..., gofmt/goimports, and golangci-lint on the touched packages; also confirmed by direct line count that both split functions are under the function-length limit. Signed-off-by: Akash Kumar <116457960+akashchamp@users.noreply.github.com> --- .../container_health_check_linux_test.go | 194 ++++++++++-------- 1 file changed, 113 insertions(+), 81 deletions(-) diff --git a/cmd/nerdctl/container/container_health_check_linux_test.go b/cmd/nerdctl/container/container_health_check_linux_test.go index b8b63e92f19..d898da40da3 100644 --- a/cmd/nerdctl/container/container_health_check_linux_test.go +++ b/cmd/nerdctl/container/container_health_check_linux_test.go @@ -392,87 +392,6 @@ func TestContainerHealthCheckAdvance(t *testing.T) { } }, }, - { - Description: "Health check probes at start-interval cadence within the start period", - Setup: func(data test.Data, helpers test.Helpers) { - helpers.Ensure("run", "-d", "--name", data.Identifier(), - "--health-cmd", "exit 1", - "--health-interval", "60s", - "--health-start-period", "30s", - "--health-start-interval", "1s", - testutil.CommonImage, "sleep", nerdtest.Infinity) - nerdtest.EnsureContainerStarted(helpers, data.Identifier()) - }, - Cleanup: func(data test.Data, helpers test.Helpers) { - helpers.Anyhow("rm", "-f", data.Identifier()) - }, - Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - helpers.Ensure("container", "healthcheck", data.Identifier()) - // Longer than --health-start-interval (1s) but much shorter than - // --health-interval (60s): this tick must still run because we are - // still within --health-start-period. - time.Sleep(2 * time.Second) - helpers.Ensure("container", "healthcheck", data.Identifier()) - return helpers.Command("inspect", data.Identifier()) - }, - Expected: func(data test.Data, helpers test.Helpers) *test.Expected { - return &test.Expected{ - ExitCode: 0, - Output: expect.All(func(stdout string, t tig.T) { - inspect := nerdtest.InspectContainer(helpers, data.Identifier()) - h := inspect.State.Health - debug, _ := json.MarshalIndent(h, "", " ") - t.Log(string(debug)) - assert.Assert(t, h != nil, "expected health state") - // health-cmd always fails, so unhealthy results are ignored and we - // remain in the start period workflow throughout. - assert.Equal(t, h.Status, healthcheck.Starting) - assert.Equal(t, len(h.Log), 2, - "expected both ticks to run: each was spaced beyond --health-start-interval") - }), - } - }, - }, - { - Description: "Health check falls back to health-interval cadence once the start period ends", - Setup: func(data test.Data, helpers test.Helpers) { - helpers.Ensure("run", "-d", "--name", data.Identifier(), - "--health-cmd", "exit 0", - "--health-interval", "60s", - "--health-start-period", "5s", - "--health-start-interval", "1s", - testutil.CommonImage, "sleep", nerdtest.Infinity) - nerdtest.EnsureContainerStarted(helpers, data.Identifier()) - }, - Cleanup: func(data test.Data, helpers test.Helpers) { - helpers.Anyhow("rm", "-f", data.Identifier()) - }, - Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - // First tick always runs and, since health-cmd succeeds, immediately - // exits the start period (first healthy result). - helpers.Ensure("container", "healthcheck", data.Identifier()) - // Second tick arrives well within --health-start-interval (1s), but the - // start period already ended, so --health-interval (60s) now applies and - // this tick must be skipped. - helpers.Ensure("container", "healthcheck", data.Identifier()) - return helpers.Command("inspect", data.Identifier()) - }, - Expected: func(data test.Data, helpers test.Helpers) *test.Expected { - return &test.Expected{ - ExitCode: 0, - Output: expect.All(func(stdout string, t tig.T) { - inspect := nerdtest.InspectContainer(helpers, data.Identifier()) - h := inspect.State.Health - debug, _ := json.MarshalIndent(h, "", " ") - t.Log(string(debug)) - assert.Assert(t, h != nil, "expected health state") - assert.Equal(t, h.Status, healthcheck.Healthy) - assert.Equal(t, len(h.Log), 1, - "expected the second tick to be throttled by --health-interval after the start period ended") - }), - } - }, - }, { Description: "Health check with invalid command", Setup: func(data test.Data, helpers test.Helpers) { @@ -736,6 +655,11 @@ func TestContainerHealthCheckAdvance(t *testing.T) { "--health-cmd", "exit 1", "--health-timeout", "10s", "--health-retries", "3", + // Probes are now throttled to run no more often than --health-interval + // (see shouldRunProbe), which otherwise defaults to 30s: shorter than + // this test's manual invocations are apart, so each of them would + // otherwise be skipped as "not due yet" instead of actually probing. + "--health-interval", "1s", testutil.CommonImage, "sleep", nerdtest.Infinity) nerdtest.EnsureContainerStarted(helpers, data.Identifier()) }, @@ -835,6 +759,114 @@ func TestContainerHealthCheckAdvance(t *testing.T) { testCase.Run(t) } +// TestContainerHealthCheckStartInterval covers --health-start-interval cadence behavior: +// probes ticking at the faster start-interval cadence while inside the start period, and +// falling back to the regular health-interval cadence once the start period ends. Kept +// separate from TestContainerHealthCheckAdvance to stay under the function-length lint limit. +func TestContainerHealthCheckStartInterval(t *testing.T) { + testCase := nerdtest.Setup() + + // Docker CLI does not provide a standalone healthcheck command. + testCase.Require = require.Not(nerdtest.Docker) + + // Skip systemd tests in rootless environment to bypass dbus permission issues + if rootlessutil.IsRootless() { + t.Skip("systemd healthcheck tests are skipped in rootless environment") + } + + testCase.SubTests = []*test.Case{ + { + Description: "Health check probes at start-interval cadence within the start period", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--name", data.Identifier(), + "--health-cmd", "exit 1", + "--health-interval", "60s", + "--health-start-period", "30s", + "--health-start-interval", "1s", + testutil.CommonImage, "sleep", nerdtest.Infinity) + nerdtest.EnsureContainerStarted(helpers, data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + helpers.Ensure("container", "healthcheck", data.Identifier()) + // Longer than --health-start-interval (1s) but much shorter than + // --health-interval (60s): this tick must still run because we are + // still within --health-start-period. + time.Sleep(2 * time.Second) + helpers.Ensure("container", "healthcheck", data.Identifier()) + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: 0, + Output: expect.All(func(stdout string, t tig.T) { + inspect := nerdtest.InspectContainer(helpers, data.Identifier()) + h := inspect.State.Health + debug, _ := json.MarshalIndent(h, "", " ") + t.Log(string(debug)) + assert.Assert(t, h != nil, "expected health state") + // health-cmd always fails, so unhealthy results are ignored and we + // remain in the start period workflow throughout. + assert.Equal(t, h.Status, healthcheck.Starting) + // At least our two manual ticks must have run, since each was + // spaced beyond --health-start-interval. We can only assert a + // lower bound: the container's own background timer also ticks + // at the --health-start-interval cadence during the start + // period, and may legitimately fire once more inside this same + // window, adding an extra probe beyond the two we triggered. + assert.Assert(t, len(h.Log) >= 2, + "expected both manual ticks to run: each was spaced beyond --health-start-interval") + }), + } + }, + }, + { + Description: "Health check falls back to health-interval cadence once the start period ends", + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "-d", "--name", data.Identifier(), + "--health-cmd", "exit 0", + "--health-interval", "60s", + "--health-start-period", "5s", + "--health-start-interval", "1s", + testutil.CommonImage, "sleep", nerdtest.Infinity) + nerdtest.EnsureContainerStarted(helpers, data.Identifier()) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + // First tick always runs and, since health-cmd succeeds, immediately + // exits the start period (first healthy result). + helpers.Ensure("container", "healthcheck", data.Identifier()) + // Second tick arrives well within --health-start-interval (1s), but the + // start period already ended, so --health-interval (60s) now applies and + // this tick must be skipped. + helpers.Ensure("container", "healthcheck", data.Identifier()) + return helpers.Command("inspect", data.Identifier()) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: 0, + Output: expect.All(func(stdout string, t tig.T) { + inspect := nerdtest.InspectContainer(helpers, data.Identifier()) + h := inspect.State.Health + debug, _ := json.MarshalIndent(h, "", " ") + t.Log(string(debug)) + assert.Assert(t, h != nil, "expected health state") + assert.Equal(t, h.Status, healthcheck.Healthy) + assert.Equal(t, len(h.Log), 1, + "expected the second tick to be throttled by --health-interval after the start period ended") + }), + } + }, + }, + } + + testCase.Run(t) +} + func TestHealthCheck_SystemdIntegration_Basic(t *testing.T) { testCase := nerdtest.Setup() testCase.Require = require.Not(nerdtest.Docker)