Skip to content

feat(driver): validate provider backend before use (preflight)#773

Merged
skevetter merged 3 commits into
mainfrom
feat/provider-preflight
Jul 27, 2026
Merged

feat(driver): validate provider backend before use (preflight)#773
skevetter merged 3 commits into
mainfrom
feat/provider-preflight

Conversation

@skevetter

@skevetter skevetter commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a preflight step that validates a provider's backend (binary installed, daemon/machine reachable, optionally auto-started) early — at a single gate in NewRunner, right after the driver is created — instead of failing deep inside a container operation.

Motivating case: a stopped Podman machine previously surfaced as start workspace: find dev container: docker ps: Cannot connect to Podman .... Now it's caught up front, and the machine is auto-started.

Design

  • Drivers opt in via an optional Preflighter capability interface, mirroring the existing MountCapableDriver/DriverSupportsMountType idiom. Drivers that don't implement it are a no-op.
  • One choke point: devcontainer.NewRunner runs driver.DriverPreflight, covering every backend-touching command (up/stop/delete/logs/exec), since they all construct a runner.
  • PreflightError is a recognition marker (errors.As) that carries the runtime's own message rather than wrapping it in higher-level plumbing.
  • Per-invocation settings are passed explicitly via PreflightOptions — no process-global state.

Per-driver behavior

Driver Check Auto-start
docker / podman binary on PATH + daemon ping Podman machine (re-verified after start)
apple container binary + system service system service
kubernetes API server /version reachability — (remote)
microsandbox msb binary
custom no-op

Auto-start opt-out

Auto-start is default-on. Users can opt out with --no-auto-start (travels agent-side via CLIOptions) or by exporting DEVSY_NO_AUTOSTART. NewRunner resolves the two into PreflightOptions.DisableAutoStart.

Notes

  • Apple and microsandbox previously validated in their constructors; that logic moved into Preflight so it runs through the common gate and honors the opt-out.
  • Reviewed by independent architecture and idiomatic-Go passes; the podman auto-start branch is unit-tested via injected function values (no broad interface abstraction).

Summary by CodeRabbit

  • New Features

    • Added preflight checks to verify required runtimes and services are available before starting a workspace.
    • Added automatic Podman machine startup when needed.
    • Added the --no-auto-start option to report stopped services without starting them.
    • Added support for disabling auto-start through an environment setting.
    • Added clearer provider-specific errors when preflight checks fail.
  • Bug Fixes

    • Improved detection and reporting of unavailable container and Kubernetes services.

Add a preflight step so provider backends are validated early rather than
failing deep inside a container operation (e.g. "find dev container: docker
ps: Cannot connect to Podman").

Drivers opt in via the optional Preflighter capability interface, mirroring
the existing MountCapableDriver idiom. A single gate in NewRunner runs the
check right after the driver is created, so every backend-touching command
(up/stop/delete/logs/exec) is covered.

- docker/podman: binary on PATH + daemon reachability; a stopped Podman
  machine is auto-started (then re-verified) unless disabled
- apple: `container` binary + system service (auto-start)
- kubernetes: API server reachability (no auto-start)
- microsandbox: `msb` binary
- custom: no-op

Auto-start is default-on with a --no-auto-start flag / DEVSY_NO_AUTOSTART
env opt-out, passed explicitly via PreflightOptions (no process-global
state). PreflightError is a recognition marker carrying the runtime's own
message.
@netlify

netlify Bot commented Jul 27, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 9044793
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a66fc84af4f910008dacfac

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds a shared driver preflight layer, exposes the --no-auto-start workspace option, and moves provider readiness checks into driver preflight implementations for Apple, Docker/Podman, Kubernetes, and microsandbox.

Changes

Driver preflight flow

Layer / File(s) Summary
Preflight contracts and workspace wiring
pkg/driver/preflight.go, pkg/driver/preflight_test.go, pkg/flags/names/names.go, pkg/provider/workspace.go, cmd/workspace/up/up_flags.go, pkg/devcontainer/run.go
Defines preflight options, environment handling, wrapped errors, the --no-auto-start configuration, and runner-level preflight dispatch with coverage for delegation and option forwarding.
Apple container system preflight
pkg/apple/helper.go, pkg/driver/apple/*
Moves Apple system readiness checks out of driver construction, exposes SystemRunning, and applies auto-start or opt-out behavior during preflight.
Docker and Podman runtime preflight
pkg/docker/helper.go, pkg/driver/docker/*
Checks runtime availability, pings Docker or Podman, optionally starts the Podman machine, and retries the runtime check.
Kubernetes API preflight
pkg/driver/kubernetes/*
Adds a timeout-bound /version API probe and wraps connectivity failures in PreflightError.
Microsandbox installation preflight
pkg/driver/microsandbox/*
Moves runtime installation validation from construction into Preflight and tests installed and missing-runtime cases.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Workspace
  participant NewRunner
  participant DriverPreflight
  participant ProviderDriver
  participant Runtime
  Workspace->>NewRunner: Start workspace with NoAutoStart
  NewRunner->>DriverPreflight: Build and dispatch PreflightOptions
  DriverPreflight->>ProviderDriver: Preflight(ctx, opts)
  ProviderDriver->>Runtime: Probe or check runtime readiness
  Runtime-->>ProviderDriver: Readiness result
  ProviderDriver-->>NewRunner: PreflightError or success
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding driver/backend preflight validation before use.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Jul 27, 2026

Copy link
Copy Markdown

Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 9044793
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6a66fc8424d0a500088828d1

Ping used `info --format {{.ServerVersion}}`, a docker-only info field.
Podman/nerdctl have no .ServerVersion (type *define.Info), so the template
errored and preflight wrongly treated the daemon as unreachable — then tried
`podman machine start`, failing every podman (and podman-backed) up.

Run a bare `info` and judge reachability by exit status instead.
@skevetter
skevetter marked this pull request as ready for review July 27, 2026 07:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pkg/driver/microsandbox/preflight_test.go (1)

18-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert preservation of the underlying install error.

This test only checks the provider, so it would pass even if the runtime failure were replaced with a generic error. Verify perr.Err matches c.failInstall.

Suggested assertion
  if perr.Provider != "microsandbox" {
  	t.Fatalf("Provider = %q, want microsandbox", perr.Provider)
  }
+ if !errors.Is(perr.Err, c.failInstall) {
+ 	t.Fatalf("Err = %v, want %v", perr.Err, c.failInstall)
+ }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/driver/microsandbox/preflight_test.go` around lines 18 - 30, Update
TestPreflightNotInstalled to assert that perr.Err preserves the underlying
c.failInstall error, using errors.Is or an equivalent error comparison after
validating the provider. Keep the existing PreflightError type and provider
assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/driver/microsandbox/preflight_test.go`:
- Around line 18-30: Update TestPreflightNotInstalled to assert that perr.Err
preserves the underlying c.failInstall error, using errors.Is or an equivalent
error comparison after validating the provider. Keep the existing PreflightError
type and provider assertions unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dfe9c0b2-6c7a-4286-a066-bea8ec4e76a3

📥 Commits

Reviewing files that changed from the base of the PR and between 9776e9d and 9044793.

📒 Files selected for processing (20)
  • cmd/workspace/up/up_flags.go
  • pkg/apple/helper.go
  • pkg/devcontainer/run.go
  • pkg/docker/helper.go
  • pkg/driver/apple/client.go
  • pkg/driver/apple/driver.go
  • pkg/driver/apple/lifecycle_test.go
  • pkg/driver/apple/preflight_test.go
  • pkg/driver/docker/docker.go
  • pkg/driver/docker/preflight_test.go
  • pkg/driver/kubernetes/client.go
  • pkg/driver/kubernetes/driver.go
  • pkg/driver/kubernetes/preflight_test.go
  • pkg/driver/microsandbox/microsandbox.go
  • pkg/driver/microsandbox/microsandbox_test.go
  • pkg/driver/microsandbox/preflight_test.go
  • pkg/driver/preflight.go
  • pkg/driver/preflight_test.go
  • pkg/flags/names/names.go
  • pkg/provider/workspace.go

@skevetter
skevetter merged commit 6858022 into main Jul 27, 2026
66 checks passed
@skevetter
skevetter deleted the feat/provider-preflight branch July 27, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant