From 41591b0ec1d43a13bc33bbe13732fc16da4573ed Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 21 Aug 2026 20:29:36 +0000 Subject: [PATCH 1/2] feat: wire DEVSY_AGENT_PATH to entrypoint Signed-off-by: Samuel K --- pkg/agent/delivery/local_docker.go | 2 +- pkg/config/env.go | 11 ++- pkg/devcontainer/compose_test.go | 2 +- pkg/devcontainer/single.go | 5 +- pkg/devcontainer/single_test.go | 9 ++- pkg/docker/helper.go | 19 ++--- pkg/driver/docker/build.go | 3 +- pkg/driver/docker/lifecycle.go | 117 +++++++++++++++++----------- pkg/driver/docker/lifecycle_test.go | 2 +- 9 files changed, 104 insertions(+), 66 deletions(-) diff --git a/pkg/agent/delivery/local_docker.go b/pkg/agent/delivery/local_docker.go index fd29a1388..8b23e403c 100644 --- a/pkg/agent/delivery/local_docker.go +++ b/pkg/agent/delivery/local_docker.go @@ -66,7 +66,7 @@ func (d *LocalDockerDelivery) DeliverPreStart(ctx context.Context, opts PreStart if opts.RunOptions.Env == nil { opts.RunOptions.Env = make(map[string]string) } - opts.RunOptions.Env["DEVSY_AGENT_PATH"] = volumeMountPath + "/" + binaryName() + opts.RunOptions.Env[pkgconfig.EnvAgentPath] = volumeMountPath + "/" + binaryName() return nil } diff --git a/pkg/config/env.go b/pkg/config/env.go index 48ab7ef75..b42a69d3d 100644 --- a/pkg/config/env.go +++ b/pkg/config/env.go @@ -60,10 +60,15 @@ const ( // EnvAgentPreferDownload forces agent binary download even if a local copy exists. EnvAgentPreferDownload = "DEVSY_AGENT_PREFER_DOWNLOAD" - // EnvOS is set to the host operating system (runtime.GOOS). + // EnvAgentPath is the path to the agent binary inside the workspace + // container, set by agent delivery so the container entrypoint can locate + // it (defaults to /usr/local/bin/devsy). + EnvAgentPath = "DEVSY_AGENT_PATH" + + // EnvOS is set to the host operating system. EnvOS = "DEVSY_OS" - // EnvArch is set to the host architecture (runtime.GOARCH). + // EnvArch is set to the host architecture. EnvArch = "DEVSY_ARCH" // EnvLogLevel is set to the current log level. @@ -121,8 +126,6 @@ const ( // EnvProviderPrefix is the prefix for provider-specific option env vars (append provider name + "_"). EnvProviderPrefix = EnvPrefix + "PROVIDER_" - // --- Provider-scoped env vars (set when running provider commands) ---. - // EnvProviderWorkspaceID is the workspace identifier passed to providers. EnvProviderWorkspaceID = "WORKSPACE_ID" diff --git a/pkg/devcontainer/compose_test.go b/pkg/devcontainer/compose_test.go index 516096cad..41009c0d6 100644 --- a/pkg/devcontainer/compose_test.go +++ b/pkg/devcontainer/compose_test.go @@ -781,7 +781,7 @@ func TestBuildOverrideEntrypointAppendsUserEntrypoint(t *testing.T) { func TestBuildOverrideEntrypointKeepsDefaultEntrypointReachable(t *testing.T) { script := buildOverrideEntrypoint(&config.MergedDevContainerConfig{}, nil) body := script[2] - if !strings.Contains(body, "devsy internal agent container daemon") { + if !strings.Contains(body, "internal agent container daemon") { t.Errorf("expected default entrypoint invocation in script, got %q", body) } } diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 26936b825..38183cabd 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -44,10 +44,11 @@ func joinShellStatements(statements ...string) string { // DefaultEntrypoint waits for the devsy agent binary to become available // before handing off to the container daemon. var DefaultEntrypoint = joinShellStatements( - `while ! command -v /usr/local/bin/devsy >/dev/null 2>&1; do echo "waiting for devsy agent to be available"`, + `while ! command -v "${DEVSY_AGENT_PATH:-/usr/local/bin/devsy}" >/dev/null 2>&1`, + `do echo "waiting for devsy agent to be available"`, "sleep 1", "done", - "exec /usr/local/bin/devsy internal agent container daemon", + `exec "${DEVSY_AGENT_PATH:-/usr/local/bin/devsy}" internal agent container daemon`, ) // resolvedContainer holds the outputs that every code path through diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go index ec425b617..079fe67fe 100644 --- a/pkg/devcontainer/single_test.go +++ b/pkg/devcontainer/single_test.go @@ -209,9 +209,12 @@ func TestDefaultEntrypointSingleLine(t *testing.T) { if strings.Contains(DefaultEntrypoint, "\n") { t.Fatalf("DefaultEntrypoint must be single-line, got %q", DefaultEntrypoint) } - if !strings.Contains(DefaultEntrypoint, "devsy internal agent container daemon") { + if !strings.Contains(DefaultEntrypoint, "internal agent container daemon") { t.Errorf("DefaultEntrypoint must invoke the agent daemon, got %q", DefaultEntrypoint) } + if !strings.Contains(DefaultEntrypoint, `"${DEVSY_AGENT_PATH:-/usr/local/bin/devsy}"`) { + t.Errorf("DefaultEntrypoint must honor DEVSY_AGENT_PATH, got %q", DefaultEntrypoint) + } } func TestGetStartScriptSingleLine(t *testing.T) { @@ -227,7 +230,7 @@ func TestGetStartScriptSingleLine(t *testing.T) { if !strings.Contains(got, `exec "$@"`) { t.Fatalf("GetStartScript() must keep the shell exec passthrough, got %q", got) } - if !strings.Contains(got, "devsy internal agent container daemon") { + if !strings.Contains(got, "internal agent container daemon") { t.Fatalf("GetStartScript() must invoke the agent, got %q", got) } } @@ -245,7 +248,7 @@ func TestGetStartScriptPreservesStatementOrder(t *testing.T) { `exec "$@"`, "first-entrypoint", "second-entrypoint", - "devsy internal agent container daemon", + "internal agent container daemon", } lastIdx := -1 for _, want := range wantOrder { diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go index f9f190c19..6e54c993c 100644 --- a/pkg/docker/helper.go +++ b/pkg/docker/helper.go @@ -40,8 +40,14 @@ const ( var ( ErrContainerTerminal = errors.New("container in terminal state") - ErrContainerExited = errors.New("container exited after start") - ErrImageNotFound = errors.New("image not found") + ErrContainerExited = errors.New("container exited after start") + ErrImageNotFound = errors.New("image not found") + + // podmanMachineStartTimeout is the maximum time to wait for a Podman machine to start. + podmanMachineStartTimeout = 90 * time.Second + + // pingTimeout is the maximum time to wait for a ping to the runtime daemon. + pingTimeout = 30 * time.Second ) var imageNotFoundMarkers = []string{ @@ -158,11 +164,6 @@ func (r *DockerHelper) ClientVersion(ctx context.Context) string { return strings.TrimSpace(string(out)) } -// podmanMachineStartTimeout bounds a Podman machine boot, which spins up a VM. -var podmanMachineStartTimeout = 90 * time.Second - -var pingTimeout = 30 * time.Second - func runCmdCombined(ctx context.Context, cmd *exec.Cmd) error { var out bytes.Buffer cmd.Stdout = &out @@ -450,7 +451,7 @@ func (r *DockerHelper) WaitContainerRunning(ctx context.Context, containerID str details, err := r.InspectContainers(ctx, []string{containerID}) if err != nil { lastErr = err - log.Debugf("WaitContainerRunning: inspect error (will retry): %v", err) + log.Debugf("inspecting container %s: %v", containerID, err) return false, nil } lastErr = nil @@ -458,7 +459,7 @@ func (r *DockerHelper) WaitContainerRunning(ctx context.Context, containerID str }, ) if pollErr != nil && lastErr != nil { - return fmt.Errorf("%w (last inspect error: %v)", pollErr, lastErr) + return fmt.Errorf("waiting for container %s to be running: %w", containerID, lastErr) } return pollErr } diff --git a/pkg/driver/docker/build.go b/pkg/driver/docker/build.go index f68703bd4..34366a2b6 100644 --- a/pkg/driver/docker/build.go +++ b/pkg/driver/docker/build.go @@ -415,13 +415,14 @@ func (d *dockerDriver) executeBuild( return nil } +// createBuildInfo constructs the BuildInfo after a successful build. When pushing, +// the image may not be available locally, so ImageDetails may be nil. func (d *dockerDriver) createBuildInfo( ctx context.Context, imageName string, req driver.BuildRequest, buildOptions *build.BuildOptions, ) (*config.BuildInfo, error) { - // When pushing, image may not be available locally var imageDetails *config.ImageDetails if !buildOptions.Push { var err error diff --git a/pkg/driver/docker/lifecycle.go b/pkg/driver/docker/lifecycle.go index 0713733b1..f0b73c07f 100644 --- a/pkg/driver/docker/lifecycle.go +++ b/pkg/driver/docker/lifecycle.go @@ -15,17 +15,42 @@ import ( "k8s.io/apimachinery/pkg/util/wait" ) -const containerRestartAttempts = 3 +type containerState string + +const ( + containerStatusRunning containerState = "running" + containerStatusExited containerState = "exited" + containerStatusCreated containerState = "created" + containerStatusPaused containerState = "paused" + containerStatusRestarting containerState = "restarting" + containerStatusDead containerState = "dead" + containerStatusRemoving containerState = "removing" +) -const containerStatusRunning = "running" +var containerStates = map[string]containerState{ + "running": containerStatusRunning, + "exited": containerStatusExited, + "created": containerStatusCreated, + "paused": containerStatusPaused, + "restarting": containerStatusRestarting, + "dead": containerStatusDead, + "removing": containerStatusRemoving, +} -// snapshotImageLabel marks a committed image as a devsy workspace snapshot, -// so it's identifiable via `docker inspect`/`docker images --filter` by -// anyone who pulls or lists it outside `devsy snapshot` tooling — the -// snapshot manifest (pkg/snapshot) already carries richer sh.devsy.snapshot.* -// metadata, but that lives in a separate OCI artifact a raw image pull won't -// see. -const snapshotImageLabel = "sh.devsy.snapshot=true" +func toContainerState(s string) containerState { + if state, ok := containerStates[strings.ToLower(s)]; ok { + return state + } + return containerState(s) +} + +const ( + containerRestartAttempts = 3 + + // snapshotImageLabel marks a committed image as a devsy workspace snapshot, + // so it's identifiable via `docker inspect`/`docker images --filter`. + snapshotImageLabel = "sh.devsy.snapshot=true" +) func (d *dockerDriver) CommandDevContainer( ctx context.Context, @@ -58,75 +83,82 @@ func (d *dockerDriver) CommandDevContainer( return nil } -// ensureContainerRunning checks that the given container is running, and if -// not, attempts to start it and wait for it to be running. If the container is -// in a terminal state (dead or removing), it returns an error. +// ensureContainerRunning checks the container's state and starts it if necessary. func (d *dockerDriver) ensureContainerRunning( ctx context.Context, container *config.ContainerDetails, ) error { - status := strings.ToLower(container.State.Status) - if status == "dead" || status == "removing" { + status := toContainerState(container.State.Status) + switch status { + case containerStatusRunning: + return nil + case containerStatusDead, containerStatusRemoving: return fmt.Errorf( "%w: container %s is %q", docker.ErrContainerTerminal, container.ID, status, ) + case containerStatusExited, containerStatusCreated, + containerStatusPaused, containerStatusRestarting: + return d.restartAndWait(ctx, container) + default: + return fmt.Errorf( + "%w: container %s is in unknown state %q", + docker.ErrContainerTerminal, + container.ID, + status, + ) } - if status == containerStatusRunning { - return nil - } +} +// restartAndWait starts the container and waits for it to be running, +// retrying up to containerRestartAttempts times. It aborts immediately when +// the container enters a terminal state. +func (d *dockerDriver) restartAndWait( + ctx context.Context, + container *config.ContainerDetails, +) error { var lastErr error for attempt := 1; attempt <= containerRestartAttempts; attempt++ { if err := ctx.Err(); err != nil { return err } log.Infof( - "container %s is not running (status=%s), restarting (attempt %d/%d)", - container.ID, status, attempt, containerRestartAttempts, + "restarting container %s (status=%s, attempt=%d/%d)", + container.ID, container.State.Status, attempt, containerRestartAttempts, ) - err := d.restartAndWait(ctx, container.ID) - if err == nil { - log.Infof("container %s is now running", container.ID) + if err := d.Docker.StartContainer(ctx, container.ID); err != nil { + lastErr = fmt.Errorf("start container: %w", err) + } else if err := d.Docker.WaitContainerRunning(ctx, container.ID); err != nil { + lastErr = fmt.Errorf("wait for container to be running: %w", err) + } else { + log.Infof("container %s is running", container.ID) return nil } - if errors.Is(err, docker.ErrContainerTerminal) { - return err + if errors.Is(lastErr, docker.ErrContainerTerminal) || + errors.Is(lastErr, context.Canceled) || + errors.Is(lastErr, context.DeadlineExceeded) { + return lastErr } - lastErr = err - log.Debugf("container %s restart attempt %d failed: %v", container.ID, attempt, err) + log.Debugf("container %s restart attempt %d failed: %v", container.ID, attempt, lastErr) } return fmt.Errorf( - "%w: container %s did not stay running after %d restart attempts: %v", + "%w: container %s did not stay running after %d attempts: %w", docker.ErrContainerTerminal, container.ID, containerRestartAttempts, lastErr, ) } -func (d *dockerDriver) restartAndWait(ctx context.Context, containerID string) error { - if err := d.Docker.StartContainer(ctx, containerID); err != nil { - return fmt.Errorf("restart container: %w", err) - } - if err := d.Docker.WaitContainerRunning(ctx, containerID); err != nil { - return fmt.Errorf("wait for container to be running: %w", err) - } - return nil -} - func (d *dockerDriver) PushDevContainer(ctx context.Context, image string) error { - // push image writer := log.Writer(log.LevelInfo) defer func() { _ = writer.Close() }() - // build args args := []string{ "push", image, } - // run command log.Debugf( "running docker push command: command=%s, args=%s", d.Docker.DockerCommand, @@ -141,18 +173,15 @@ func (d *dockerDriver) PushDevContainer(ctx context.Context, image string) error } func (d *dockerDriver) TagDevContainer(ctx context.Context, image, tag string) error { - // Tag image writer := log.Writer(log.LevelInfo) defer func() { _ = writer.Close() }() - // build args args := []string{ "tag", image, tag, } - // run command log.Debugf( "running docker tag command: command=%s, args=%s", d.Docker.DockerCommand, @@ -201,7 +230,7 @@ func (d *dockerDriver) DeleteDevContainer(ctx context.Context, workspaceId strin return nil } - if strings.ToLower(container.State.Status) == containerStatusRunning { + if status := toContainerState(container.State.Status); status == containerStatusRunning { if err := d.Docker.Stop(ctx, container.ID); err != nil { log.Warnf("stop before delete failed for %s: %v", container.ID, err) } diff --git a/pkg/driver/docker/lifecycle_test.go b/pkg/driver/docker/lifecycle_test.go index ecacf8599..adfbf0788 100644 --- a/pkg/driver/docker/lifecycle_test.go +++ b/pkg/driver/docker/lifecycle_test.go @@ -203,7 +203,7 @@ func TestEnsureContainerRunning_AlreadyRunning(t *testing.T) { d := &dockerDriver{Docker: &docker.DockerHelper{DockerCommand: testDockerCmd}} container := &config.ContainerDetails{ ID: "c1", - State: config.ContainerDetailsState{Status: containerStatusRunning}, + State: config.ContainerDetailsState{Status: string(containerStatusRunning)}, } require.NoError(t, d.ensureContainerRunning(context.Background(), container)) From 0981b3598ead34dd3da56377fd980ef22a3ec2e1 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 21 Aug 2026 22:58:59 +0000 Subject: [PATCH 2/2] style: apply formatting Signed-off-by: Samuel K --- pkg/docker/helper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go index 6e54c993c..61205a1c1 100644 --- a/pkg/docker/helper.go +++ b/pkg/docker/helper.go @@ -40,8 +40,8 @@ const ( var ( ErrContainerTerminal = errors.New("container in terminal state") - ErrContainerExited = errors.New("container exited after start") - ErrImageNotFound = errors.New("image not found") + ErrContainerExited = errors.New("container exited after start") + ErrImageNotFound = errors.New("image not found") // podmanMachineStartTimeout is the maximum time to wait for a Podman machine to start. podmanMachineStartTimeout = 90 * time.Second