add healthcheck orchestration logic

coderbirju · coderbirju · commit 2c6cf8048511 · 2025-08-08T17:04:55.000Z
Signed-off-by: Arjun Raja Yogidas &lt;arjunry@amazon.com&gt;
diff --git a/cmd/nerdctl/container/container_create.go b/cmd/nerdctl/container/container_create.go
@@ -279,10 +279,6 @@ 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
diff --git a/cmd/nerdctl/container/container_run.go b/cmd/nerdctl/container/container_run.go
@@ -37,6 +37,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
 	"github.com/containerd/nerdctl/v2/pkg/defaults"
 	"github.com/containerd/nerdctl/v2/pkg/errutil"
+	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/logging"
 	"github.com/containerd/nerdctl/v2/pkg/netutil"
@@ -240,7 +241,6 @@ func setCreateFlags(cmd *cobra.Command) {
 	cmd.Flags().Duration("health-timeout", 0, "Maximum time to allow one check to run (default: 30s)")
 	cmd.Flags().Int("health-retries", 0, "Consecutive failures needed to report unhealthy (default: 3)")
 	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 checks during the start period")
 	cmd.Flags().Bool("no-healthcheck", false, "Disable any container-specified HEALTHCHECK")
 
 	// #region env flags
@@ -439,6 +439,14 @@ func runAction(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	// Setup container healthchecks.
+	if err := healthcheck.CreateTimer(ctx, c); err != nil {
+		return fmt.Errorf("failed to create healthcheck timer: %w", err)
+	}
+	if err := healthcheck.StartTimer(ctx, c); err != nil {
+		return fmt.Errorf("failed to start healthcheck timer: %w", err)
+	}
+
 	if createOpt.Detach {
 		fmt.Fprintln(createOpt.Stdout, id)
 		return nil
diff --git a/cmd/nerdctl/container/container_run_test.go b/cmd/nerdctl/container/container_run_test.go
@@ -841,6 +841,9 @@ func TestRunDomainname(t *testing.T) {
 }
 
 func TestRunHealthcheckFlags(t *testing.T) {
+	if rootlessutil.IsRootless() {
+		t.Skip("healthcheck tests are skipped in rootless environment")
+	}
 	testCase := nerdtest.Setup()
 
 	testCases := []struct {
@@ -990,6 +993,9 @@ func TestRunHealthcheckFlags(t *testing.T) {
 }
 
 func TestRunHealthcheckFromImage(t *testing.T) {
+	if rootlessutil.IsRootless() {
+		t.Skip("healthcheck tests are skipped in rootless environment")
+	}
 	nerdtest.Setup()
 
 	dockerfile := fmt.Sprintf(`FROM %s
diff --git a/cmd/nerdctl/helpers/flagutil.go b/cmd/nerdctl/helpers/flagutil.go
@@ -52,8 +52,7 @@ func ValidateHealthcheckFlags(options types.ContainerCreateOptions) error {
 		options.HealthInterval != 0 ||
 			options.HealthTimeout != 0 ||
 			options.HealthRetries != 0 ||
-			options.HealthStartPeriod != 0 ||
-			options.HealthStartInterval != 0
+			options.HealthStartPeriod != 0
 
 	if options.NoHealthcheck {
 		if options.HealthCmd != "" || healthFlagsSet {
@@ -74,9 +73,6 @@ 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/go.mod b/go.mod
@@ -120,7 +120,7 @@ require (
 	github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 // indirect
 	github.com/sasha-s/go-deadlock v0.3.5 // indirect
 	//gomodjail:unconfined
-	github.com/sirupsen/logrus v1.9.3 // indirect
+	github.com/sirupsen/logrus v1.9.3
 	github.com/smallstep/pkcs7 v0.1.1 // indirect
 	github.com/spaolacci/murmur3 v1.1.0 // indirect
 	github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 // indirect
diff --git a/pkg/api/types/container_types.go b/pkg/api/types/container_types.go
@@ -285,13 +285,12 @@ type ContainerCreateOptions struct {
 	ImagePullOpt ImagePullOptions
 
 	// Healthcheck related fields
-	HealthCmd           string
-	HealthInterval      time.Duration
-	HealthTimeout       time.Duration
-	HealthRetries       int
-	HealthStartPeriod   time.Duration
-	HealthStartInterval time.Duration
-	NoHealthcheck       bool
+	HealthCmd         string
+	HealthInterval    time.Duration
+	HealthTimeout     time.Duration
+	HealthRetries     int
+	HealthStartPeriod 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
@@ -891,9 +891,6 @@ func withHealthcheck(options types.ContainerCreateOptions, ensuredImage *imgutil
 	if options.HealthStartPeriod != 0 {
 		hc.StartPeriod = options.HealthStartPeriod
 	}
-	if options.HealthStartInterval != 0 {
-		hc.StartInterval = options.HealthStartInterval
-	}
 
 	// If no healthcheck config is set (via CLI or image), return empty string so we skip adding to container config.
 	if reflect.DeepEqual(hc, &healthcheck.Healthcheck{}) {
diff --git a/pkg/cmd/container/health_check.go b/pkg/cmd/container/health_check.go
@@ -59,7 +59,6 @@ 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.DefaultStartInterval)
 	if hcConfig.Retries == 0 {
 		hcConfig.Retries = healthcheck.DefaultProbeRetries
 	}
diff --git a/pkg/cmd/container/kill.go b/pkg/cmd/container/kill.go
@@ -35,6 +35,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/clientutil"
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
+	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/netutil"
@@ -111,6 +112,11 @@ func killContainer(ctx context.Context, container containerd.Container, signal s
 		return err
 	}
 
+	// Clean up healthcheck systemd units
+	if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
+		log.G(ctx).Warnf("failed to clean up healthcheck units for container %s: %s", container.ID(), err)
+	}
+
 	// signal will be sent once resume is finished
 	if paused {
 		if err := task.Resume(ctx); err != nil {
diff --git a/pkg/cmd/container/remove.go b/pkg/cmd/container/remove.go
@@ -34,6 +34,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/clientutil"
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
 	"github.com/containerd/nerdctl/v2/pkg/dnsutil/hostsstore"
+	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
 	"github.com/containerd/nerdctl/v2/pkg/ipcutil"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
@@ -179,6 +180,11 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions
 		// Otherwise, nil the error so that we do not write the error label on the container
 		retErr = nil
 
+		// Clean up healthcheck systemd units
+		if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, c); err != nil {
+			log.G(ctx).WithError(err).Warnf("failed to clean up healthcheck units for container %q", id)
+		}
+
 		// Now, delete the actual container
 		var delOpts []containerd.DeleteOpts
 		if _, err := c.Image(ctx); err == nil {
diff --git a/pkg/cmd/container/stop.go b/pkg/cmd/container/stop.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
+	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
 )
 
@@ -39,6 +40,9 @@ func Stop(ctx context.Context, client *containerd.Client, reqs []string, opt typ
 			if err := cleanupNetwork(ctx, found.Container, opt.GOptions); err != nil {
 				return fmt.Errorf("unable to cleanup network for container: %s", found.Req)
 			}
+			if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, found.Container); err != nil {
+				return fmt.Errorf("unable to cleanup healthcheck timer for container: %s: %w", found.Req, err)
+			}
 			if err := containerutil.Stop(ctx, found.Container, opt.Timeout, opt.Signal); err != nil {
 				if errdefs.IsNotFound(err) {
 					fmt.Fprintf(opt.Stderr, "No such container: %s\n", found.Req)
diff --git a/pkg/containerutil/containerutil.go b/pkg/containerutil/containerutil.go
@@ -48,6 +48,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/consoleutil"
 	"github.com/containerd/nerdctl/v2/pkg/errutil"
 	"github.com/containerd/nerdctl/v2/pkg/formatter"
+	"github.com/containerd/nerdctl/v2/pkg/healthcheck"
 	"github.com/containerd/nerdctl/v2/pkg/ipcutil"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/labels/k8slabels"
@@ -283,6 +284,15 @@ func Start(ctx context.Context, container containerd.Container, isAttach bool, i
 	if err := task.Start(ctx); err != nil {
 		return err
 	}
+
+	// If container has health checks configured, create and start systemd timer/service files.
+	if err := healthcheck.CreateTimer(ctx, container); err != nil {
+		return fmt.Errorf("failed to create healthcheck timer: %w", err)
+	}
+	if err := healthcheck.StartTimer(ctx, container); err != nil {
+		return fmt.Errorf("failed to start healthcheck timer: %w", err)
+	}
+
 	if !isAttach {
 		return nil
 	}
@@ -351,6 +361,11 @@ func Stop(ctx context.Context, container containerd.Container, timeout *time.Dur
 		}
 	}()
 
+	// Clean up healthcheck units if configured.
+	// if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
+	// 	return fmt.Errorf("failed to clean up healthcheck units for container %s", container.ID())
+	// }
+
 	if timeout == nil {
 		t, ok := l[labels.StopTimeout]
 		if !ok {
@@ -489,6 +504,11 @@ func Pause(ctx context.Context, client *containerd.Client, id string) error {
 		return err
 	}
 
+	// Clean up healthcheck units if configured.
+	// if err := healthcheck.RemoveTransientHealthCheckFiles(ctx, container); err != nil {
+	// 	return fmt.Errorf("failed to clean up healthcheck units for container %s", container.ID())
+	// }
+
 	switch status.Status {
 	case containerd.Paused:
 		return fmt.Errorf("container %s is already paused", id)
@@ -516,6 +536,14 @@ func Unpause(ctx context.Context, client *containerd.Client, id string) error {
 		return err
 	}
 
+	// Recreate healthcheck related systemd timer/service files.
+	if err := healthcheck.CreateTimer(ctx, container); err != nil {
+		return fmt.Errorf("failed to create healthcheck timer: %w", err)
+	}
+	if err := healthcheck.StartTimer(ctx, container); err != nil {
+		return fmt.Errorf("failed to start healthcheck timer: %w", err)
+	}
+
 	switch status.Status {
 	case containerd.Paused:
 		return task.Resume(ctx)
diff --git a/pkg/healthcheck/executor.go b/pkg/healthcheck/executor.go
@@ -128,26 +128,42 @@ func updateHealthStatus(ctx context.Context, container containerd.Container, hcC
 		currentHealth = &HealthState{
 			Status:        Starting,
 			FailingStreak: 0,
+			StartPeriod:   hcConfig.StartPeriod > 0,
 		}
 	}
 
-	// Check if still within start period
-	startPeriod := hcConfig.StartPeriod
+	// Get container info for start period check
 	info, err := container.Info(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to get container info: %w", err)
 	}
 	containerCreated := info.CreatedAt
-	stillInStartPeriod := hcResult.Start.Sub(containerCreated) < startPeriod
-
-	// Update health status based on exit code
-	if hcResult.ExitCode == 0 {
-		currentHealth.Status = Healthy
-		currentHealth.FailingStreak = 0
-	} else if !stillInStartPeriod {
-		currentHealth.FailingStreak++
-		if currentHealth.FailingStreak >= hcConfig.Retries {
-			currentHealth.Status = Unhealthy
+
+	// Check if we're in start period workflow
+	inStartPeriodTime := hcResult.Start.Sub(containerCreated) < hcConfig.StartPeriod
+	inStartPeriodState := currentHealth.StartPeriod
+
+	if inStartPeriodTime && inStartPeriodState {
+		// Start Period Workflow
+		if hcResult.ExitCode == 0 {
+			// First healthy result transitions us out of start period
+			currentHealth.Status = Healthy
+			currentHealth.FailingStreak = 0
+			currentHealth.StartPeriod = false
+		}
+		// Ignore unhealthy results during start period
+	} else {
+		// Health Interval Workflow
+		if hcResult.ExitCode == 0 {
+			if currentHealth.Status != Healthy {
+				currentHealth.Status = Healthy
+				currentHealth.FailingStreak = 0
+			}
+		} else {
+			currentHealth.FailingStreak++
+			if currentHealth.FailingStreak >= hcConfig.Retries && currentHealth.Status != Unhealthy {
+				currentHealth.Status = Unhealthy
+			}
 		}
 	}
 
diff --git a/pkg/healthcheck/health.go b/pkg/healthcheck/health.go
@@ -43,7 +43,6 @@ 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.
-	DefaultStartInterval   = 5 * time.Second  // Interval between checks during 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.
@@ -70,18 +69,18 @@ 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
-	StartInterval time.Duration `json:"StartInterval,omitempty"` // StartInterval is the time between health checks during the start period
+	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
 }
 
 // HealthState stores the current health state of a container
 type HealthState struct {
 	Status        HealthStatus // Status is one of [Starting], [Healthy] or [Unhealthy]
 	FailingStreak int          // FailingStreak is the number of consecutive failures
+	StartPeriod   bool         // StartPeriod indicates if we're in the start period workflow
 }
 
 // ToJSONString serializes HealthState to a JSON string for label storage
diff --git a/pkg/healthcheck/healthcheck_manager_darwin.go b/pkg/healthcheck/healthcheck_manager_darwin.go
@@ -0,0 +1,43 @@
+/*
+   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 (
+	"context"
+
+	containerd "github.com/containerd/containerd/v2/client"
+)
+
+// CreateTimer sets up the transient systemd timer and service for healthchecks.
+func CreateTimer(ctx context.Context, container containerd.Container) error {
+	return nil
+}
+
+// StartTimer starts the healthcheck timer unit.
+func StartTimer(ctx context.Context, container containerd.Container) error {
+	return nil
+}
+
+// RemoveTransientHealthCheckFiles stops and cleans up the transient timer and service.
+func RemoveTransientHealthCheckFiles(ctx context.Context, container containerd.Container) error {
+	return nil
+}
+
+// RemoveTransientHealthCheckFilesByID stops and cleans up the transient timer and service using just the container ID.
+func RemoveTransientHealthCheckFilesByID(ctx context.Context, containerID string) error {
+	return nil
+}
diff --git a/pkg/healthcheck/healthcheck_manager_freebsd.go b/pkg/healthcheck/healthcheck_manager_freebsd.go
diff --git a/pkg/healthcheck/healthcheck_manager_linux.go b/pkg/healthcheck/healthcheck_manager_linux.go
diff --git a/pkg/healthcheck/healthcheck_manager_windows.go b/pkg/healthcheck/healthcheck_manager_windows.go
diff --git a/pkg/ocihook/ocihook.go b/pkg/ocihook/ocihook.go

Original file line number	Diff line number	Diff line change
`@@ -891,9 +891,6 @@ func withHealthcheck(options types.ContainerCreateOptions, ensuredImage *imgutil`
`891`	`891`	`if options.HealthStartPeriod != 0 {`
`892`	`892`	`hc.StartPeriod = options.HealthStartPeriod`
`893`	`893`	`}`
`894`		`- if options.HealthStartInterval != 0 {`
`895`		`- hc.StartInterval = options.HealthStartInterval`
`896`		`- }`
`897`	`894`
`898`	`895`	`// If no healthcheck config is set (via CLI or image), return empty string so we skip adding to container config.`
`899`	`896`	`if reflect.DeepEqual(hc, &healthcheck.Healthcheck{}) {`
Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,6 @@ func HealthCheck(ctx context.Context, client *containerd.Client, container conta`
`59`	`59`	`hcConfig.Interval = timeoutWithDefault(hcConfig.Interval, healthcheck.DefaultProbeInterval)`
`60`	`60`	`hcConfig.Timeout = timeoutWithDefault(hcConfig.Timeout, healthcheck.DefaultProbeTimeout)`
`61`	`61`	`hcConfig.StartPeriod = timeoutWithDefault(hcConfig.StartPeriod, healthcheck.DefaultStartPeriod)`
`62`		`- hcConfig.StartInterval = timeoutWithDefault(hcConfig.StartInterval, healthcheck.DefaultStartInterval)`
`63`	`62`	`if hcConfig.Retries == 0 {`
`64`	`63`	`hcConfig.Retries = healthcheck.DefaultProbeRetries`
`65`	`64`	`}`