From e1de9e71933db7acfd721369524694cd0afb5844 Mon Sep 17 00:00:00 2001 From: Arjun Raja Yogidas Date: Tue, 14 Oct 2025 23:16:34 +0000 Subject: [PATCH] healthcheck: fix path issues and add default config values - Fixed PATH resolution by using explicit nerdctl binary path in systemd service files, eliminating 'nerdctl' not found errors - Added default values for unspecified healthcheck flags to prevent silent failures Signed-off-by: Arjun Raja Yogidas --- .../container_health_check_linux_test.go | 133 ++++++++++++++++++ pkg/cmd/container/create.go | 5 + pkg/healthcheck/health.go | 16 +++ pkg/healthcheck/healthcheck_manager_linux.go | 12 +- 4 files changed, 163 insertions(+), 3 deletions(-) diff --git a/cmd/nerdctl/container/container_health_check_linux_test.go b/cmd/nerdctl/container/container_health_check_linux_test.go index 9ce502f1523..cda5cec1cb7 100644 --- a/cmd/nerdctl/container/container_health_check_linux_test.go +++ b/cmd/nerdctl/container/container_health_check_linux_test.go @@ -139,6 +139,139 @@ func TestContainerHealthCheckBasic(t *testing.T) { testCase.Run(t) } +func TestContainerHealthCheckDefaults(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 applies default values when not explicitly set", + Setup: func(data test.Data, helpers test.Helpers) { + // Create container with only --health-cmd, no other health flags + helpers.Ensure("run", "-d", "--name", data.Identifier(), + "--health-cmd", "echo healthy", + 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 { + 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()) + + // Parse the healthcheck config from container labels + hcLabel := inspect.Config.Labels["nerdctl/healthcheck"] + assert.Assert(t, hcLabel != "", "expected healthcheck label to be present") + + var hc healthcheck.Healthcheck + err := json.Unmarshal([]byte(hcLabel), &hc) + assert.NilError(t, err, "failed to parse healthcheck config") + + // Verify default values are applied + assert.Equal(t, hc.Interval, 30*time.Second, "expected default interval of 30s") + 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") + + // Verify the command was set correctly + assert.DeepEqual(t, hc.Test, []string{"CMD-SHELL", "echo healthy"}) + }), + } + }, + }, + { + Description: "CLI flags override default values correctly", + Setup: func(data test.Data, helpers test.Helpers) { + // Create container with custom health flags that override defaults + helpers.Ensure("run", "-d", "--name", data.Identifier(), + "--health-cmd", "echo custom", + "--health-interval", "45s", + "--health-timeout", "15s", + "--health-retries", "5", + "--health-start-period", "10s", + 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 { + 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()) + + // Parse the healthcheck config from container labels + hcLabel := inspect.Config.Labels["nerdctl/healthcheck"] + assert.Assert(t, hcLabel != "", "expected healthcheck label to be present") + + var hc healthcheck.Healthcheck + err := json.Unmarshal([]byte(hcLabel), &hc) + assert.NilError(t, err, "failed to parse healthcheck config") + + // Verify CLI overrides are applied (not defaults) + assert.Equal(t, hc.Interval, 45*time.Second, "expected custom interval of 45s") + 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") + + // Verify the command was set correctly + assert.DeepEqual(t, hc.Test, []string{"CMD-SHELL", "echo custom"}) + }), + } + }, + }, + { + Description: "No defaults applied when no healthcheck is configured", + Setup: func(data test.Data, helpers test.Helpers) { + // Create container without any health flags + helpers.Ensure("run", "-d", "--name", data.Identifier(), + 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 { + 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()) + + // Verify no healthcheck label is present + hcLabel := inspect.Config.Labels["nerdctl/healthcheck"] + assert.Equal(t, hcLabel, "", "expected no healthcheck label when no healthcheck is configured") + + // Verify no health state + assert.Assert(t, inspect.State.Health == nil, "expected no health state when no healthcheck is configured") + }), + } + }, + }, + } + + testCase.Run(t) +} + func TestContainerHealthCheckAdvance(t *testing.T) { testCase := nerdtest.Setup() diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 232d8a27b77..69e6919ccd3 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -892,6 +892,11 @@ func withHealthcheck(options types.ContainerCreateOptions, ensuredImage *imgutil hc.StartPeriod = options.HealthStartPeriod } + // Apply defaults for any unset values, but only if we have a healthcheck configured + if len(hc.Test) > 0 && hc.Test[0] != "NONE" { + hc.ApplyDefaults() + } + // 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{}) { return "", nil diff --git a/pkg/healthcheck/health.go b/pkg/healthcheck/health.go index c074c15c413..70104187e29 100644 --- a/pkg/healthcheck/health.go +++ b/pkg/healthcheck/health.go @@ -136,3 +136,19 @@ func HealthcheckResultFromJSON(s string) (*HealthcheckResult, error) { } return &r, nil } + +// ApplyDefaults sets default values for unset healthcheck fields +func (hc *Healthcheck) ApplyDefaults() { + if hc.Interval == 0 { + hc.Interval = DefaultProbeInterval + } + if hc.Timeout == 0 { + hc.Timeout = DefaultProbeTimeout + } + if hc.StartPeriod == 0 { + hc.StartPeriod = DefaultStartPeriod + } + if hc.Retries == 0 { + hc.Retries = DefaultProbeRetries + } +} diff --git a/pkg/healthcheck/healthcheck_manager_linux.go b/pkg/healthcheck/healthcheck_manager_linux.go index e043b5c2d37..4cd33b77c01 100644 --- a/pkg/healthcheck/healthcheck_manager_linux.go +++ b/pkg/healthcheck/healthcheck_manager_linux.go @@ -56,7 +56,13 @@ func CreateTimer(ctx context.Context, container containerd.Container, cfg *confi // Always use health-interval for timer frequency cmdOpts = append(cmdOpts, "--unit", containerID, "--on-unit-inactive="+hc.Interval.String(), "--timer-property=AccuracySec=1s") - cmdOpts = append(cmdOpts, "nerdctl", "container", "healthcheck", containerID) + // Get the full path to the current nerdctl binary + nerdctlPath, err := os.Executable() + if err != nil { + return fmt.Errorf("could not determine nerdctl executable path: %v", err) + } + + cmdOpts = append(cmdOpts, nerdctlPath, "container", "healthcheck", containerID) if log.G(ctx).Logger.IsLevelEnabled(log.DebugLevel) { cmdOpts = append(cmdOpts, "--debug") } @@ -257,8 +263,8 @@ func shouldSkipHealthCheckSystemd(hc *Healthcheck, cfg *config.Config) bool { return true } - // Don't proceed if health check is nil, empty, explicitly NONE or interval is 0. - if hc == nil || len(hc.Test) == 0 || hc.Test[0] == "NONE" || hc.Interval == 0 { + // Don't proceed if health check is nil, empty or explicitly NONE. + if hc == nil || len(hc.Test) == 0 || hc.Test[0] == "NONE" { return true } return false