From 17c1bb5c21da254ab4b02061b2162f14adda6b33 Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Fri, 27 Feb 2026 21:28:11 +1000 Subject: [PATCH] feat: add WaitContainerHealthy client method and health check utilities --- pkg/api/client.go | 12 ++- pkg/api/container.go | 73 +++++++++++++++++ pkg/api/container_exec.go | 37 --------- pkg/api/container_health.go | 11 --- pkg/client/container.go | 151 ++++++++++++++++++++++++++++++++++++ 5 files changed, 234 insertions(+), 50 deletions(-) delete mode 100644 pkg/api/container_exec.go delete mode 100644 pkg/api/container_health.go diff --git a/pkg/api/client.go b/pkg/api/client.go index 8bd920dd..0d752fa2 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -21,11 +21,14 @@ type ContainerClient interface { CreateContainer( ctx context.Context, serviceID string, spec ServiceSpec, machineID string, ) (container.CreateResponse, error) + ExecContainer(ctx context.Context, serviceNameOrID, containerNameOrID string, config ExecOptions) (int, error) InspectContainer(ctx context.Context, serviceNameOrID, containerNameOrID string) (MachineServiceContainer, error) - RemoveContainer(ctx context.Context, serviceNameOrID, containerNameOrID string, opts container.RemoveOptions) error StartContainer(ctx context.Context, serviceNameOrID, containerNameOrID string) error StopContainer(ctx context.Context, serviceNameOrID, containerNameOrID string, opts container.StopOptions) error - ExecContainer(ctx context.Context, serviceNameOrID, containerNameOrID string, config ExecOptions) (int, error) + RemoveContainer(ctx context.Context, serviceNameOrID, containerNameOrID string, opts container.RemoveOptions) error + WaitContainerHealthy( + ctx context.Context, serviceNameOrID, containerNameOrID string, opts WaitContainerHealthyOptions, + ) error } type DNSClient interface { @@ -57,3 +60,8 @@ type VolumeClient interface { ListVolumes(ctx context.Context, filter *VolumeFilter) ([]MachineVolume, error) RemoveVolume(ctx context.Context, machineNameOrID, volumeName string, force bool) error } + +// AsPtr returns a pointer to the given value. Useful for optional fields in API structs. +func AsPtr[T any](v T) *T { + return &v +} diff --git a/pkg/api/container.go b/pkg/api/container.go index e386cfe3..90aa64f4 100644 --- a/pkg/api/container.go +++ b/pkg/api/container.go @@ -3,7 +3,10 @@ package api import ( "encoding/json" "fmt" + "io" "net/netip" + "os" + "strconv" "strings" "time" @@ -41,6 +44,19 @@ func (c *Container) CreatedTime() time.Time { return c.created } +// HasHealthcheck returns true if the container has a health check configured. +func (c *Container) HasHealthcheck() bool { + hc := c.Config.Healthcheck + if hc == nil { + return false + } + + if len(hc.Test) > 0 && hc.Test[0] == "NONE" { + return false + } + return true +} + // Healthy determines if the container is running and healthy. // A running container with no health check configured is considered healthy. func (c *Container) Healthy() bool { @@ -244,3 +260,60 @@ func (c *ServiceContainer) UnmarshalJSON(data []byte) error { return nil } + +// DefaultHealthMonitorPeriod is the default duration to wait before checking that the container is still running +// and not restarting. Can be overridden with the UNCLOUD_DEFAULT_HEALTH_MONITOR_PERIOD_MS environment variable. +var DefaultHealthMonitorPeriod = defaultHealthMonitorPeriod() + +func defaultHealthMonitorPeriod() time.Duration { + if v, ok := os.LookupEnv("UNCLOUD_DEFAULT_HEALTH_MONITOR_PERIOD_MS"); ok { + if ms, err := strconv.Atoi(v); err == nil { + return time.Duration(ms) * time.Millisecond + } + } + + return 5 * time.Second +} + +// WaitContainerHealthyOptions configures the behaviour of WaitContainerHealthy. +type WaitContainerHealthyOptions struct { + // MonitorPeriod is how long to wait before checking that the container is still running and not restarting. + // Containers with a health check that become healthy before the period ends succeed early. + // nil means use the default DefaultHealthMonitorPeriod. + // Zero skips the monitoring and checks the container's health immediately after starting. + MonitorPeriod *time.Duration +} + +// ExecOptions contains configuration for executing a command in a container. +type ExecOptions struct { + // Command is the command to run in the container. + Command []string + // AttachStdin attaches the stdin stream to the exec session. + AttachStdin bool + // AttachStdout attaches the stdout stream to the exec session. + AttachStdout bool + // AttachStderr attaches the stderr stream to the exec session. + AttachStderr bool + // Tty allocates a pseudo-TTY for the exec session. + Tty bool + // Detach runs the command in the background without attaching to streams. + Detach bool + + //// Not yet implemented fields + // User specifies the user to run the command as. + User string + // Privileged runs the command in privileged mode. + Privileged bool + // WorkingDir sets the working directory for the command. + WorkingDir string + // Env sets environment variables for the command. + Env []string + + // Client-side only fields (not serialized, not sent to server) + // Stdin is the input stream. Defaults to os.Stdin if nil. + Stdin io.Reader `json:"-"` + // Stdout is the output stream. Defaults to os.Stdout if nil. + Stdout io.Writer `json:"-"` + // Stderr is the error stream. Defaults to os.Stderr if nil. + Stderr io.Writer `json:"-"` +} diff --git a/pkg/api/container_exec.go b/pkg/api/container_exec.go deleted file mode 100644 index c55f90e1..00000000 --- a/pkg/api/container_exec.go +++ /dev/null @@ -1,37 +0,0 @@ -package api - -import "io" - -// ExecOptions contains configuration for executing a command in a container. -type ExecOptions struct { - // Command is the command to run in the container. - Command []string - // AttachStdin attaches the stdin stream to the exec session. - AttachStdin bool - // AttachStdout attaches the stdout stream to the exec session. - AttachStdout bool - // AttachStderr attaches the stderr stream to the exec session. - AttachStderr bool - // Tty allocates a pseudo-TTY for the exec session. - Tty bool - // Detach runs the command in the background without attaching to streams. - Detach bool - - //// Not yet implemented fields - // User specifies the user to run the command as. - User string - // Privileged runs the command in privileged mode. - Privileged bool - // WorkingDir sets the working directory for the command. - WorkingDir string - // Env sets environment variables for the command. - Env []string - - // Client-side only fields (not serialized, not sent to server) - // Stdin is the input stream. Defaults to os.Stdin if nil. - Stdin io.Reader `json:"-"` - // Stdout is the output stream. Defaults to os.Stdout if nil. - Stdout io.Writer `json:"-"` - // Stderr is the error stream. Defaults to os.Stderr if nil. - Stderr io.Writer `json:"-"` -} diff --git a/pkg/api/container_health.go b/pkg/api/container_health.go deleted file mode 100644 index b4818840..00000000 --- a/pkg/api/container_health.go +++ /dev/null @@ -1,11 +0,0 @@ -package api - -import "time" - -const ( - // defaultDockerHealthcheckInterval is the default Docker interval between health check runs. - defaultDockerHealthcheckInterval = 30 * time.Second - // defaultDockerHealthcheckRetries is the default Docker number of consecutive failures needed - // to consider the container unhealthy. - defaultDockerHealthcheckRetries = 3 -) diff --git a/pkg/client/container.go b/pkg/client/container.go index 7c05c58e..8ea074d2 100644 --- a/pkg/client/container.go +++ b/pkg/client/container.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/containerd/errdefs" "github.com/docker/compose/v2/pkg/progress" @@ -17,6 +18,9 @@ import ( "google.golang.org/grpc/status" ) +// TODO: format container and machine IDs in 'Container %s on %s' events as bold. +// Consider formatting containers as /. + // CreateContainer creates a new container for the given service on the specified machine. func (cli *Client) CreateContainer( ctx context.Context, serviceID string, spec api.ServiceSpec, machineID string, @@ -354,3 +358,150 @@ func (cli *Client) ExecContainer( return exitCode, nil } + +// WaitContainerHealthy polls the container until it is considered running and healthy. +// +// For containers without a health check, it waits for the monitor period and then verifies the container +// is still running and not restarting. +// +// For containers with a health check, it waits until Docker reports healthy or unhealthy. During the monitor period, +// unhealthy status is treated as retryable (the container may be recovering from a transient crash). +// After the monitor period, unhealthy becomes a permanent failure. +func (cli *Client) WaitContainerHealthy( + ctx context.Context, serviceNameOrID, containerNameOrID string, opts api.WaitContainerHealthyOptions, +) error { + // First inspect to get container info, machine name, and health check config. + mc, err := cli.InspectContainer(ctx, serviceNameOrID, containerNameOrID) + if err != nil { + return fmt.Errorf("inspect container: %w", err) + } + + machine, err := cli.InspectMachine(ctx, mc.MachineID) + if err != nil { + return fmt.Errorf("inspect machine '%s': %w", mc.MachineID, err) + } + + pw := progress.ContextWriter(ctx) + eventID := fmt.Sprintf("Container %s on %s", mc.Container.Name, machine.Machine.Name) + + var monitor time.Duration + if opts.MonitorPeriod == nil { + monitor = api.DefaultHealthMonitorPeriod + } else { + monitor = *opts.MonitorPeriod + } + pw.Event(progress.NewEvent(eventID, progress.Working, fmt.Sprintf("Monitoring (%s)", monitor))) + + // For containers without a health check, just wait for the monitor period and then check the container + // is still running and not restarting. + if !mc.Container.HasHealthcheck() { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(monitor): + } + + mc, err := cli.InspectContainer(ctx, serviceNameOrID, containerNameOrID) + if err != nil { + return fmt.Errorf("inspect container: %w", err) + } + + if mc.Container.Healthy() { + pw.Event(progress.RunningEvent(eventID)) + return nil + } + + humanState, _ := mc.Container.HumanState() + pw.Event(progress.ErrorMessageEvent(eventID, fmt.Sprintf("Unhealthy (%s)", humanState))) + + if mc.Container.State.Restarting { + return fmt.Errorf("container is restarting after monitor period (%s): exit_code=%d", + monitor, mc.Container.State.ExitCode) + } + return fmt.Errorf("container is unhealthy after monitor period (%s): %s", monitor, humanState) + } + + // For containers with a health check, wait until Docker reports healthy or unhealthy. + mctx := proxyToMachine(ctx, machine.Machine) + mctx, cancel := context.WithTimeout(mctx, healthcheckTimeout(mc.Container.Config.Healthcheck)) + defer cancel() + ticker := time.NewTicker(1 * time.Second) + defer ticker.Stop() + monitorDeadline := time.Now().Add(monitor) + + for { + select { + case <-mctx.Done(): + return mctx.Err() + case <-ticker.C: + ctr, err := cli.Docker.InspectServiceContainer(mctx, mc.Container.ID) + if err != nil { + pw.Event(progress.NewEvent(eventID, progress.Working, + fmt.Sprintf("Health checking (failed to inspect container: %v)", err))) + continue + } + + // Reset the event status if previous inspect failed. + eventStatus := fmt.Sprintf("Monitoring (%s)", monitor) + if time.Now().After(monitorDeadline) { + // TODO: provide more details about running checks or waiting so the user can see what's going on. + eventStatus = "Health checking" + } + pw.Event(progress.NewEvent(eventID, progress.Working, eventStatus)) + + if ctr.Healthy() { + pw.Event(progress.Healthy(eventID)) + return nil + } + if time.Now().Before(monitorDeadline) { + continue + } + + if ctr.State.Health.Status == container.Unhealthy { + humanState, _ := ctr.HumanState() + pw.Event(progress.ErrorMessageEvent(eventID, fmt.Sprintf("Unhealthy (%s)", humanState))) + + if ctr.State.Restarting { + return fmt.Errorf("container is restarting after monitor period (%s): exit_code=%d", + monitor, ctr.State.ExitCode) + } + return fmt.Errorf("container is unhealthy after monitor period (%s): %s", monitor, humanState) + } + } + } +} + +const ( + // defaultDockerHealthcheckInterval is the default Docker interval between health check runs. + defaultDockerHealthcheckInterval = 30 * time.Second + // defaultDockerHealthcheckTimeout is the default Docker timeout for each health check run. + defaultDockerHealthcheckTimeout = 30 * time.Second + // defaultDockerHealthcheckRetries is the default Docker number of consecutive failures needed + // to consider the container unhealthy. + defaultDockerHealthcheckRetries = 3 +) + +// healthcheckTimeout computes the maximum time to wait for a container to become healthy based on +// its health check config. This is the worst case timeout to stop polling in case something goes wrong and Docker +// doesn't report the container as unhealthy after it should. +func healthcheckTimeout(hc *container.HealthConfig) time.Duration { + if hc == nil { + return 0 + } + + interval := hc.Interval + if interval <= 0 { + interval = defaultDockerHealthcheckInterval + } + timeout := hc.Timeout + if timeout <= 0 { + timeout = defaultDockerHealthcheckTimeout + } + retries := hc.Retries + if retries <= 0 { + retries = defaultDockerHealthcheckRetries + } + + // 5s is a buffer to account for scheduling delays. + return hc.StartPeriod + time.Duration(retries)*(interval+timeout) + 5*time.Second +}