feat(driver): validate provider backend before use (preflight) - #773
Conversation
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.
✅ Deploy Preview for devsydev canceled.
|
📝 WalkthroughWalkthroughThe change adds a shared driver preflight layer, exposes the ChangesDriver preflight flow
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
✅ Deploy Preview for images-devsy-sh canceled.
|
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.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/driver/microsandbox/preflight_test.go (1)
18-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert 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.Errmatchesc.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
📒 Files selected for processing (20)
cmd/workspace/up/up_flags.gopkg/apple/helper.gopkg/devcontainer/run.gopkg/docker/helper.gopkg/driver/apple/client.gopkg/driver/apple/driver.gopkg/driver/apple/lifecycle_test.gopkg/driver/apple/preflight_test.gopkg/driver/docker/docker.gopkg/driver/docker/preflight_test.gopkg/driver/kubernetes/client.gopkg/driver/kubernetes/driver.gopkg/driver/kubernetes/preflight_test.gopkg/driver/microsandbox/microsandbox.gopkg/driver/microsandbox/microsandbox_test.gopkg/driver/microsandbox/preflight_test.gopkg/driver/preflight.gopkg/driver/preflight_test.gopkg/flags/names/names.gopkg/provider/workspace.go
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
Preflightercapability interface, mirroring the existingMountCapableDriver/DriverSupportsMountTypeidiom. Drivers that don't implement it are a no-op.devcontainer.NewRunnerrunsdriver.DriverPreflight, covering every backend-touching command (up/stop/delete/logs/exec), since they all construct a runner.PreflightErroris a recognition marker (errors.As) that carries the runtime's own message rather than wrapping it in higher-level plumbing.PreflightOptions— no process-global state.Per-driver behavior
containerbinary + system service/versionreachabilitymsbbinaryAuto-start opt-out
Auto-start is default-on. Users can opt out with
--no-auto-start(travels agent-side viaCLIOptions) or by exportingDEVSY_NO_AUTOSTART.NewRunnerresolves the two intoPreflightOptions.DisableAutoStart.Notes
Preflightso it runs through the common gate and honors the opt-out.Summary by CodeRabbit
New Features
--no-auto-startoption to report stopped services without starting them.Bug Fixes