Skip to content

Commit df45d40

Browse files
committed
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 <[email protected]>
1 parent f144078 commit df45d40

File tree

4 files changed

+163
-3
lines changed

4 files changed

+163
-3
lines changed

cmd/nerdctl/container/container_health_check_linux_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,139 @@ func TestContainerHealthCheckBasic(t *testing.T) {
139139
testCase.Run(t)
140140
}
141141

142+
func TestContainerHealthCheckDefaults(t *testing.T) {
143+
testCase := nerdtest.Setup()
144+
145+
// Docker CLI does not provide a standalone healthcheck command.
146+
testCase.Require = require.Not(nerdtest.Docker)
147+
148+
// Skip systemd tests in rootless environment to bypass dbus permission issues
149+
if rootlessutil.IsRootless() {
150+
t.Skip("systemd healthcheck tests are skipped in rootless environment")
151+
}
152+
153+
testCase.SubTests = []*test.Case{
154+
{
155+
Description: "Health check applies default values when not explicitly set",
156+
Setup: func(data test.Data, helpers test.Helpers) {
157+
// Create container with only --health-cmd, no other health flags
158+
helpers.Ensure("run", "-d", "--name", data.Identifier(),
159+
"--health-cmd", "echo healthy",
160+
testutil.CommonImage, "sleep", nerdtest.Infinity)
161+
nerdtest.EnsureContainerStarted(helpers, data.Identifier())
162+
},
163+
Cleanup: func(data test.Data, helpers test.Helpers) {
164+
helpers.Anyhow("rm", "-f", data.Identifier())
165+
},
166+
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
167+
return helpers.Command("inspect", data.Identifier())
168+
},
169+
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
170+
return &test.Expected{
171+
ExitCode: 0,
172+
Output: expect.All(func(stdout string, t tig.T) {
173+
inspect := nerdtest.InspectContainer(helpers, data.Identifier())
174+
175+
// Parse the healthcheck config from container labels
176+
hcLabel := inspect.Config.Labels["nerdctl/healthcheck"]
177+
assert.Assert(t, hcLabel != "", "expected healthcheck label to be present")
178+
179+
var hc healthcheck.Healthcheck
180+
err := json.Unmarshal([]byte(hcLabel), &hc)
181+
assert.NilError(t, err, "failed to parse healthcheck config")
182+
183+
// Verify default values are applied
184+
assert.Equal(t, hc.Interval, 30*time.Second, "expected default interval of 30s")
185+
assert.Equal(t, hc.Timeout, 30*time.Second, "expected default timeout of 30s")
186+
assert.Equal(t, hc.Retries, 3, "expected default retries of 3")
187+
assert.Equal(t, hc.StartPeriod, 0*time.Second, "expected default start period of 0s")
188+
189+
// Verify the command was set correctly
190+
assert.DeepEqual(t, hc.Test, []string{"CMD-SHELL", "echo healthy"})
191+
}),
192+
}
193+
},
194+
},
195+
{
196+
Description: "CLI flags override default values correctly",
197+
Setup: func(data test.Data, helpers test.Helpers) {
198+
// Create container with custom health flags that override defaults
199+
helpers.Ensure("run", "-d", "--name", data.Identifier(),
200+
"--health-cmd", "echo custom",
201+
"--health-interval", "45s",
202+
"--health-timeout", "15s",
203+
"--health-retries", "5",
204+
"--health-start-period", "10s",
205+
testutil.CommonImage, "sleep", nerdtest.Infinity)
206+
nerdtest.EnsureContainerStarted(helpers, data.Identifier())
207+
},
208+
Cleanup: func(data test.Data, helpers test.Helpers) {
209+
helpers.Anyhow("rm", "-f", data.Identifier())
210+
},
211+
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
212+
return helpers.Command("inspect", data.Identifier())
213+
},
214+
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
215+
return &test.Expected{
216+
ExitCode: 0,
217+
Output: expect.All(func(stdout string, t tig.T) {
218+
inspect := nerdtest.InspectContainer(helpers, data.Identifier())
219+
220+
// Parse the healthcheck config from container labels
221+
hcLabel := inspect.Config.Labels["nerdctl/healthcheck"]
222+
assert.Assert(t, hcLabel != "", "expected healthcheck label to be present")
223+
224+
var hc healthcheck.Healthcheck
225+
err := json.Unmarshal([]byte(hcLabel), &hc)
226+
assert.NilError(t, err, "failed to parse healthcheck config")
227+
228+
// Verify CLI overrides are applied (not defaults)
229+
assert.Equal(t, hc.Interval, 45*time.Second, "expected custom interval of 45s")
230+
assert.Equal(t, hc.Timeout, 15*time.Second, "expected custom timeout of 15s")
231+
assert.Equal(t, hc.Retries, 5, "expected custom retries of 5")
232+
assert.Equal(t, hc.StartPeriod, 10*time.Second, "expected custom start period of 10s")
233+
234+
// Verify the command was set correctly
235+
assert.DeepEqual(t, hc.Test, []string{"CMD-SHELL", "echo custom"})
236+
}),
237+
}
238+
},
239+
},
240+
{
241+
Description: "No defaults applied when no healthcheck is configured",
242+
Setup: func(data test.Data, helpers test.Helpers) {
243+
// Create container without any health flags
244+
helpers.Ensure("run", "-d", "--name", data.Identifier(),
245+
testutil.CommonImage, "sleep", nerdtest.Infinity)
246+
nerdtest.EnsureContainerStarted(helpers, data.Identifier())
247+
},
248+
Cleanup: func(data test.Data, helpers test.Helpers) {
249+
helpers.Anyhow("rm", "-f", data.Identifier())
250+
},
251+
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
252+
return helpers.Command("inspect", data.Identifier())
253+
},
254+
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
255+
return &test.Expected{
256+
ExitCode: 0,
257+
Output: expect.All(func(stdout string, t tig.T) {
258+
inspect := nerdtest.InspectContainer(helpers, data.Identifier())
259+
260+
// Verify no healthcheck label is present
261+
hcLabel := inspect.Config.Labels["nerdctl/healthcheck"]
262+
assert.Equal(t, hcLabel, "", "expected no healthcheck label when no healthcheck is configured")
263+
264+
// Verify no health state
265+
assert.Assert(t, inspect.State.Health == nil, "expected no health state when no healthcheck is configured")
266+
}),
267+
}
268+
},
269+
},
270+
}
271+
272+
testCase.Run(t)
273+
}
274+
142275
func TestContainerHealthCheckAdvance(t *testing.T) {
143276
testCase := nerdtest.Setup()
144277

pkg/cmd/container/create.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,11 @@ func withHealthcheck(options types.ContainerCreateOptions, ensuredImage *imgutil
892892
hc.StartPeriod = options.HealthStartPeriod
893893
}
894894

895+
// Apply defaults for any unset values, but only if we have a healthcheck configured
896+
if len(hc.Test) > 0 && hc.Test[0] != "NONE" {
897+
hc.ApplyDefaults()
898+
}
899+
895900
// If no healthcheck config is set (via CLI or image), return empty string so we skip adding to container config.
896901
if reflect.DeepEqual(hc, &healthcheck.Healthcheck{}) {
897902
return "", nil

pkg/healthcheck/health.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,19 @@ func HealthcheckResultFromJSON(s string) (*HealthcheckResult, error) {
136136
}
137137
return &r, nil
138138
}
139+
140+
// ApplyDefaults sets default values for unset healthcheck fields
141+
func (hc *Healthcheck) ApplyDefaults() {
142+
if hc.Interval == 0 {
143+
hc.Interval = DefaultProbeInterval
144+
}
145+
if hc.Timeout == 0 {
146+
hc.Timeout = DefaultProbeTimeout
147+
}
148+
if hc.StartPeriod == 0 {
149+
hc.StartPeriod = DefaultStartPeriod
150+
}
151+
if hc.Retries == 0 {
152+
hc.Retries = DefaultProbeRetries
153+
}
154+
}

pkg/healthcheck/healthcheck_manager_linux.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,13 @@ func CreateTimer(ctx context.Context, container containerd.Container, cfg *confi
5656
// Always use health-interval for timer frequency
5757
cmdOpts = append(cmdOpts, "--unit", containerID, "--on-unit-inactive="+hc.Interval.String(), "--timer-property=AccuracySec=1s")
5858

59-
cmdOpts = append(cmdOpts, "nerdctl", "container", "healthcheck", containerID)
59+
// Get the full path to the current nerdctl binary
60+
nerdctlPath, err := os.Executable()
61+
if err != nil {
62+
return fmt.Errorf("Could not determine nerdctl executable path: %v", err)
63+
}
64+
65+
cmdOpts = append(cmdOpts, nerdctlPath, "container", "healthcheck", containerID)
6066
if log.G(ctx).Logger.IsLevelEnabled(log.DebugLevel) {
6167
cmdOpts = append(cmdOpts, "--debug")
6268
}
@@ -257,8 +263,8 @@ func shouldSkipHealthCheckSystemd(hc *Healthcheck, cfg *config.Config) bool {
257263
return true
258264
}
259265

260-
// Don't proceed if health check is nil, empty, explicitly NONE or interval is 0.
261-
if hc == nil || len(hc.Test) == 0 || hc.Test[0] == "NONE" || hc.Interval == 0 {
266+
// Don't proceed if health check is nil, empty or explicitly NONE.
267+
if hc == nil || len(hc.Test) == 0 || hc.Test[0] == "NONE" {
262268
return true
263269
}
264270
return false

0 commit comments

Comments
 (0)