Skip to content

[healthcheck] optimize: reduce unnecessary function calls #5048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ningmingxiao wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[healthcheck] optimize: reduce unnecessary function calls #5048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions cmd/nerdctl/container/container_run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,11 +459,6 @@ func runAction(cmd *cobra.Command, args []string) error {
return err
}

statusC, err := task.Wait(ctx)
if err != nil {
return err
}

Comment thread
ningmingxiao marked this conversation as resolved.
if err := task.Start(ctx); err != nil {
return err
}
Expand All @@ -480,12 +475,14 @@ func runAction(cmd *cobra.Command, args []string) error {
return err
}

// Setup container healthchecks.
if err := healthcheck.CreateTimer(ctx, c, (*config.Config)(&createOpt.GOptions), createOpt.NerdctlCmd, createOpt.NerdctlArgs); err != nil {
return fmt.Errorf("failed to create healthcheck timer: %w", err)
}
if err := healthcheck.StartTimer(ctx, c, (*config.Config)(&createOpt.GOptions)); err != nil {
return fmt.Errorf("failed to start healthcheck timer: %w", err)
if hcStr, ok := lab[labels.HealthCheck]; ok && hcStr != "" {
// Setup container healthchecks.
if err := healthcheck.CreateTimer(ctx, c, (*config.Config)(&createOpt.GOptions), createOpt.NerdctlCmd, createOpt.NerdctlArgs, lab); err != nil {
return fmt.Errorf("failed to create healthcheck timer: %w", err)
}
if err := healthcheck.StartTimer(ctx, c, (*config.Config)(&createOpt.GOptions), lab); err != nil {
return fmt.Errorf("failed to start healthcheck timer: %w", err)
}
}

if createOpt.Detach {
Expand All @@ -503,6 +500,11 @@ func runAction(cmd *cobra.Command, args []string) error {
}
}

statusC, err := task.Wait(ctx)
if err != nil {
return err
}
Comment on lines +503 to +506

@haytok haytok Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please See https://github.com/containerd/containerd/blob/ba015360dd6857dbd0f0b36912d89a5946220057/docs/getting-started.md#task-wait-and-start

@ningmingxiao ningmingxiao Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can see
https://github.com/containerd/containerd/blob/v2.3.3/cmd/ctr/commands/run/run.go#L249
@haytok
here we don't need to wait container exited

@ChengyuZhu6 ChengyuZhu6 Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should move back the code before task.Start

@ningmingxiao ningmingxiao Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not necessary to move back, it's not a bug if user use -d we don't need to wait it. @ChengyuZhu6

@AkihiroSuda AkihiroSuda Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@haytok Was your concern resolved?


select {
// io.Wait() would return when either 1) the user detaches from the container OR 2) the container is about to exit.
//
Expand Down
35 changes: 22 additions & 13 deletions pkg/containerutil/containerutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,16 @@ func Start(ctx context.Context, container containerd.Container, isAttach bool, i
return err
}

if hcStr, ok := lab[labels.HealthCheck]; ok && hcStr != "" {
// If container has health checks configured, create and start systemd timer/service files.
if err := healthcheck.CreateTimer(ctx, container, cfg, nerdctlCmd, nerdctlArgs, lab); err != nil {
return fmt.Errorf("failed to create healthcheck timer: %w", err)
}
Comment on lines +295 to +299
if err := healthcheck.StartTimer(ctx, container, cfg, lab); err != nil {
return fmt.Errorf("failed to start healthcheck timer: %w", err)
}
}

// Set status label running should call after task is started.
_, restartPolicyExist := lab[restart.PolicyLabel]
if restartPolicyExist {
Expand All @@ -303,14 +313,6 @@ func Start(ctx context.Context, container containerd.Container, isAttach bool, i
return err
}

// If container has health checks configured, create and start systemd timer/service files.
if err := healthcheck.CreateTimer(ctx, container, cfg, nerdctlCmd, nerdctlArgs); err != nil {
return fmt.Errorf("failed to create healthcheck timer: %w", err)
}
if err := healthcheck.StartTimer(ctx, container, cfg); err != nil {
return fmt.Errorf("failed to start healthcheck timer: %w", err)
}

if !isAttach {
return nil
}
Expand Down Expand Up @@ -542,12 +544,19 @@ func Unpause(ctx context.Context, client *containerd.Client, id string, cfg *con
return err
}

// Recreate healthcheck related systemd timer/service files.
if err := healthcheck.CreateTimer(ctx, container, cfg, nerdctlCmd, nerdctlArgs); err != nil {
return fmt.Errorf("failed to create healthcheck timer: %w", err)
label, err := container.Labels(ctx)
if err != nil {
return err
}
if err := healthcheck.StartTimer(ctx, container, cfg); err != nil {
return fmt.Errorf("failed to start healthcheck timer: %w", err)

if hcStr, ok := label[labels.HealthCheck]; ok && hcStr != "" {
// Recreate healthcheck related systemd timer/service files.
if err := healthcheck.CreateTimer(ctx, container, cfg, nerdctlCmd, nerdctlArgs, label); err != nil {
return fmt.Errorf("failed to create healthcheck timer: %w", err)
}
if err := healthcheck.StartTimer(ctx, container, cfg, label); err != nil {
return fmt.Errorf("failed to start healthcheck timer: %w", err)
}
}

switch status.Status {
Expand Down
4 changes: 2 additions & 2 deletions pkg/healthcheck/healthcheck_manager_darwin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (
)

// CreateTimer sets up the transient systemd timer and service for healthchecks.
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string) error {
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string, label map[string]string) error {
return nil
}

// StartTimer starts the healthcheck timer unit.
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config) error {
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config, label map[string]string) error {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/healthcheck/healthcheck_manager_freebsd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (
)

// CreateTimer sets up the transient systemd timer and service for healthchecks.
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string) error {
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string, label map[string]string) error {
return nil
}

// StartTimer starts the healthcheck timer unit.
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config) error {
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config, label map[string]string) error {
return nil
}

Expand Down
26 changes: 16 additions & 10 deletions pkg/healthcheck/healthcheck_manager_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ import (
)

// CreateTimer sets up the transient systemd timer and service for healthchecks.
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string) error {
hc := extractHealthcheck(ctx, container)
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string, label map[string]string) error {
hc := extractHealthcheck(ctx, container, label)
if hc == nil {
return nil
}
Expand Down Expand Up @@ -106,8 +106,8 @@ func createDbusConn(ctx context.Context) (*dbus.Conn, error) {
}

// StartTimer starts the healthcheck timer unit.
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config) error {
hc := extractHealthcheck(ctx, container)
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config, label map[string]string) error {
hc := extractHealthcheck(ctx, container, label)
if hc == nil {
return nil
}
Expand Down Expand Up @@ -135,7 +135,7 @@ func StartTimer(ctx context.Context, container containerd.Container, cfg *config

// RemoveTransientHealthCheckFiles stops and cleans up the transient timer and service.
func RemoveTransientHealthCheckFiles(ctx context.Context, container containerd.Container) error {
hc := extractHealthcheck(ctx, container)
hc := extractHealthcheck(ctx, container, nil)
if hc == nil {
return nil
}
Expand Down Expand Up @@ -254,11 +254,17 @@ func ForceRemoveTransientHealthCheckFiles(ctx context.Context, containerID strin
return nil
}

func extractHealthcheck(ctx context.Context, container containerd.Container) *Healthcheck {
l, err := container.Labels(ctx)
if err != nil {
log.G(ctx).WithError(err).Debugf("could not get labels for container %s", container.ID())
return nil
func extractHealthcheck(ctx context.Context, container containerd.Container, label map[string]string) *Healthcheck {
var l map[string]string
var err error
if label == nil {
l, err = container.Labels(ctx)
if err != nil {
log.G(ctx).WithError(err).Debugf("could not get labels for container %s", container.ID())
return nil
}
} else {
l = label
}
hcStr, ok := l[labels.HealthCheck]
if !ok || hcStr == "" {
Expand Down
4 changes: 2 additions & 2 deletions pkg/healthcheck/healthcheck_manager_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (
)

// CreateTimer sets up the transient systemd timer and service for healthchecks.
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string) error {
func CreateTimer(ctx context.Context, container containerd.Container, cfg *config.Config, nerdctlCmd string, nerdctlArgs []string, label map[string]string) error {
return nil
}

// StartTimer starts the healthcheck timer unit.
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config) error {
func StartTimer(ctx context.Context, container containerd.Container, cfg *config.Config, label map[string]string) error {
return nil
}

Expand Down
Loading