Skip to content

refactor(compute): make sandbox readiness gateway-owned across all drivers#2153

Open
elezar wants to merge 2 commits into
mainfrom
refactor/1951-gateway-owned-sandbox-readiness/elezar
Open

refactor(compute): make sandbox readiness gateway-owned across all drivers#2153
elezar wants to merge 2 commits into
mainfrom
refactor/1951-gateway-owned-sandbox-readiness/elezar

Conversation

@elezar

@elezar elezar commented Jul 6, 2026

Copy link
Copy Markdown
Member

Summary

Centralises the public SandboxPhase::Ready decision in the gateway. Drivers report backend state only; the gateway composes that with supervisor session presence to produce the public phase. A structural refactor commit extracts the dispatch helpers first so the behavioral change is isolated and readable.

Related Issue

Closes #1951

Changes

  • crates/openshell-server/src/compute/mod.rs: Extract create_sandbox_record and update_sandbox_record helpers from apply_sandbox_update_locked; introduce ComposedPhase to encapsulate the four-case composition rule and own apply_readiness_conditions; add three-arm match dispatcher to make the status-less no-op path explicit
  • crates/openshell-driver-docker/src/lib.rs: Remove SupervisorReadiness trait; running containers always report BackendReady
  • crates/openshell-driver-docker/src/tests.rs: Update assertions to reflect dropped SupervisorNotConnected condition
  • crates/openshell-server/src/supervisor_session.rs: Remove impl SupervisorReadiness block
  • architecture/compute-runtimes.md: Add Sandbox Readiness Composition section documenting the four-case rule, HA note, and extension-point constraint; remove stale SupervisorNotConnected references

Deviations from Plan

None — implemented as planned.

Testing

  • mise run pre-commit passes
  • Unit tests updated

Tests updated:

  • Unit: crates/openshell-driver-docker/src/tests.rs — updated three assertions from SupervisorNotConnected to BackendReady
  • Integration: N/A
  • E2E: N/A — no changes to e2e paths

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Documentation updated:

  • architecture/compute-runtimes.md: Sandbox Readiness Composition section added

…ecord helpers

Split apply_sandbox_update_locked into named helpers to make the two
distinct paths explicit: create_sandbox_record for first-observation events
and update_sandbox_record for subsequent driver snapshots on existing sandboxes.
The dispatcher now uses a match on the existing record rather than an
early-return guard.

No behavior change.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar elezar requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 6, 2026 17:28
@elezar elezar added test:e2e Requires end-to-end coverage test:e2e-kubernetes Requires Kubernetes end-to-end coverage labels Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Label test:e2e-kubernetes applied for 84c2c32. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute Kubernetes HA E2E after building the required gateway and supervisor images once. This is an optional proof-of-life suite; failures are visible in the workflow run but do not publish a required CI gate status.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Label test:e2e applied for 84c2c32. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

if backend_phase in {Error, Deleting}: → pass through (terminal precedence)
if backend_phase == Ready && session connected: → Ready
if backend_phase == Ready && no session: → Provisioning
if backend_phase in {Provisioning, Unknown} && session: → Ready

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is it strange that a non-Ready backend phase can be promoted to Ready if a supervisor session is present?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Checked the reason: It may be that the supervisor connected event arrives before the backend ready event. In this case, the sandbox is assumed ready because it must be running if the supervisor connected.

@elezar elezar force-pushed the refactor/1951-gateway-owned-sandbox-readiness/elezar branch from 84c2c32 to 0767cc7 Compare July 6, 2026 17:45
…ivers

Introduce compute_phase_components and apply_readiness_conditions to
centralise the gateway's phase composition logic. The public SandboxPhase
is now determined by combining the backend phase reported by the driver with
supervisor session presence, independent of the driver implementation.

Remove SupervisorReadiness from the driver contract. Running containers
always report BackendReady; the gateway owns the Ready decision. Rename
the dispatcher match to three arms so that status-less events for existing
sandboxes are a documented no-op rather than a silent pass-through.

Drop backend_ready_no_session and the SupervisorNotConnected condition.
The BackendReady driver condition plus the Provisioning phase already
communicates that the backend is up but the supervisor has not connected.
The redundant condition added noise without new information.

Closes #1951

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar elezar force-pushed the refactor/1951-gateway-owned-sandbox-readiness/elezar branch from 0767cc7 to 6ea68c6 Compare July 7, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage test:e2e-kubernetes Requires Kubernetes end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: make sandbox readiness gateway-owned across compute drivers

1 participant