Skip to content

feat(sandbox): support storage-preserving sandbox configuration updates - #4873

Open
grs wants to merge 9 commits into
NVIDIA:mainfrom
grs:feat/4623-sandbox-update/grs_sjenning
Open

grs wants to merge 9 commits into
NVIDIA:mainfrom
grs:feat/4623-sandbox-update/grs_sjenning

Conversation

@grs

@grs grs commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Add support for updating a stopped sandbox’s image and configuration while preserving its identity and persistent storage. A new image may require different environment variables or startup commands, so these fields can be updated together before the next start. Labels and annotations are included for completeness and consistency with the sandbox resource.

Related Issue

Closes #4623

Changes

  • Add SandboxUpdates.UpdateSandbox with optional image, command, environment, label, and annotation updates.
  • Implement storage-preserving updates for the Kubernetes driver.
  • Require the sandbox to be stopped and leave it stopped after updating.
  • Preserve omitted fields, replace supplied commands and maps, and protect system metadata.
  • Use conditional resource versions and durable operation tracking to handle conflicts and interrupted updates.
  • Gate updates behind explicit activation after a compatible gateway rollout.
  • Add raw SDK access, documentation, and unit and Kubernetes E2E coverage.

Release Notes

Kubernetes sandboxes can now update their image, command, environment, and metadata without recreating the sandbox or losing persistent storage. Updates require operator activation and a stopped sandbox.

Testing

  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Relevant crate READMEs and published documentation updated

sjenning and others added 9 commits October 23, 2026 12:26
Add image-only UpdateSandbox with conditional versions, durable replay, fenced pending intent, and Kubernetes storage retention. Enable non-destructive image repair after completed timeout cleanup and exercise both Kubernetes controller API lanes.

Closes NVIDIA#4623

Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
OpenClaw-Publication: b2036b09-b697-4d65-aba9-5c96a3198f9c
Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
Move image updates to a separate authenticated service and retain the baseline public sandbox status. Persist image intent and revisions independently with atomic baseline-known lifecycle fencing and explicit post-upgrade activation.

Preserve concurrent provisioning clocks, fence replaced attempts and delayed callbacks, and verify private-state transactions plus both Kubernetes controller API lanes.

Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
Move image updates to a separate optional compute-driver service, preserving the baseline generated trait and streaming associated type. Add source and TCP/UDS transport regressions, capability negotiation coverage, and a default-unsupported in-process extension.

Keep Go compatibility tests outside generated output and update production reflection expectations for the separate sandbox update service.

Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
Restore the original in-process driver trait object and existing instance enum. Register optional image updates through a default factory hook without requiring changes to existing drivers or factories, and require registration plus capability before mutation.

Accept OpenShell-only descriptors while retaining strict validation of present update services. Cover baseline registration, missing-updater no-write behavior, descriptor validation, and both live Kubernetes retention lanes.

Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
Drop the task-level fallback superseded by main's runner defaults and explicit e2e-conformance-only selector. Preserve current upstream task definitions and recovery waiver unchanged.

Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
Adapt non-launch image updates to the upstream remote admission API while retaining resource-policy verification before forwarding. Cover both negotiated extensions and preservation of backend pins and persisted launch identity.

Signed-off-by: sjenning <493891+sjenning@users.noreply.github.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@sjenning

Copy link
Copy Markdown
Collaborator

/ok to test 42ae543

@sjenning sjenning added test:e2e-kubernetes Requires Kubernetes end-to-end coverage test:e2e Requires end-to-end coverage labels Oct 26, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e-kubernetes applied for 42ae543. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute Kubernetes HA and credential-driver E2E after building the required gateway, sandbox, 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

Copy link
Copy Markdown

Label test:e2e applied for 42ae543. 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, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@mrunalp mrunalp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

This adds a new SandboxUpdates.UpdateSandbox RPC. It changes a stopped sandbox's image, command, environment, labels and annotations, and keeps its ID, custom resource and PVC. The sandbox stays stopped. Only the Kubernetes driver implements it, and it is off by default behind enable_sandbox_updates.

The core mechanics look sound:

  • Accept and complete are each a single transaction, with resource-version checks.
  • The stopped state is re-checked inside the transaction.
  • request_id payloads are fingerprinted.
  • Private state is keyed by sandbox UUID and removed on delete.
  • validate_only writes nothing.
  • PVC and volume templates are untouched.
  • The supervisor command and OPENSHELL_* env are protected.
  • Older drivers and gateways still negotiate correctly.

The concerns below are mostly about recovery and operability.

Concerns

1. A sandbox can become impossible to start, stop or delete.

Before this PR, an operator could always delete a sandbox once its driver work had settled. With this PR, an accepted update blocks Start, Stop, Delete and the GC/TTL sweeps (compute/mod.rs:1900, 2103, 3097, 3253). The only way out is retrying UpdateSandbox with exactly the same fields, and nothing clears the intent in the background.

That retry is refused before the pending path is reached (compute/image_update.rs:58-87) in any of these cases:

  • the operator turns the flag back off;
  • the Postgres check in (3) fails;
  • the driver stops advertising the capability.

A driver error that repeats on every retry has the same effect, for example a precondition failure after an out-of-band change to the custom resource. The only fix left is editing the database by hand.

Suggestion: let completion of an existing intent skip the activation gates, and add an abort or force-delete path.

2. Recovery needs state the client can't see.

The retry must match the stored intent's field selection, including whether each optional field was present or absent (image_update.rs:124-133). That intent is private, and the accept step has already written the target values into the public spec, so a different client, or one that lost the original request, can't rebuild it.

For example, sending image + command with identical values when the original sent only image fails with "another sandbox update is pending". Reusing the original request_id returns REQUEST_OUTCOME_UNCERTAIN. At minimum, the error should include the pending selection.

3. The Postgres activation check blocks updates in common deployments.

persistence/image_activation.rs:41: any client backend on the same database without the openshell-image-update-v2 marker refuses every update, including pending retries. That covers psql sessions, pg_dump, postgres_exporter and managed-DB agents. Two more ways the check misfires:

  • Rows with backend_type IS NULL count against it. Those are probably the sessions the gateway role can't fully see, which is common on managed Postgres without pg_read_all_stats.
  • The marker is set with set_config after connecting (postgres.rs:33), so each fresh gateway connection briefly looks unmarked.

The marker also unconditionally overwrites any application_name the operator set in the DSN, even with updates disabled, which affects their database monitoring.

Combined with (1), a stray admin session can freeze a pending sandbox. Suggestion: set application_name through PgConnectOptions, and make the check advisory or run it only at activation.

4. Updates are refused (HTTP 403) in the documented split gateway/workspace Helm setup.

The driver now lists every Pod in the namespace (driver-kubernetes/src/image_update.rs:235-245). The openshell-workspace Role grants pods: [get] only (deploy/helm/openshell-workspace/templates/role.yaml:39-45). In shared mode with workspaceResources.enabled=false, every UpdateSandbox call fails as an internal error, the validate_only preflight included. CI uses the default chart, so it doesn't catch this.

Listing by label plus get_opt on recorded pod names would avoid needing list. It would also cut the full-namespace scan, which can hit the 30s API timeout in large shared namespaces.

5. Annotation updates can turn an ephemeral sandbox persistent, and some user keys get locked.

Before this PR, user metadata was only set at create. Now:

  • An annotations update that leaves out openshell.nvidia.com/retention=ephemeral drops it, because that key isn't treated as system metadata (core/src/sandbox_update.rs:10, read at compute/mod.rs:5698). The sandbox silently stops being auto-deleted.
  • Create only reserves internal.openshell.ai/ (grpc/validation.rs:485), but update protects all of *.openshell.ai/ and *.agents.x-k8s.io/. User keys created under those domains can never be changed or removed.

6. Label-only or annotation-only updates go through the driver and the lock.

These are gateway-only metadata and don't touch pod metadata, yet they still require the configuration capability and a no-op driver RPC, and they set the pending flag (image_update.rs:46-54). So a metadata-only edit returns UNIMPLEMENTED on an image-only driver, and it's exposed to (1). It could be a plain conditional write on the sandbox record.

7. Docs and code describe a "protocol 2" that replaces something never released.

main has no image-update code at all, yet the docs, skills and server README talk about "protocol 2", "gateways that previously supported image-only updates" and "legacy image-only intents". Operators will look for a v1 that doesn't exist, and the compatibility branches are dead code. Collapsing to a single protocol and a single driver RPC (the configuration request already embeds the image request) would simplify both.

8. Bundled lifecycle change.

Stop on a timed-out Error sandbox whose cleanup has finished now writes Stopped directly, without a driver stop. Driver Deleted events no longer prune timed-out Error/Stopped records (compute/mod.rs:1916-1969, 5734-5741). This may be intended for image repair, but it's a separate behavior change and deserves its own description or PR. The heading rewrite in configuration.mdx:1507 is unrelated too.

9. E2E cost.

Adding openshell-sdk and openshell-bootstrap to the e2e-kubernetes feature pulls about 900 lockfile lines (proto codegen, OIDC, tunnel deps) into every Kubernetes e2e binary. The test also re-implements gateway discovery and mTLS loading that tests/testkit already provides, so it would fit better as a #[testkit::test] suite. It pulls images from docker.io, so Docker Hub rate limits apply.

Smaller items

  • Two HA replicas retrying at the same time: the one that loses the row lock reports a conflict even though the update succeeded (sandbox_image.rs:228).
  • The UNIMPLEMENTED message says "image updates" even when the missing capability is the configuration one (image_update.rs:79).
  • UpdateSandboxConfiguration skips the RPC tracer (driver-kubernetes/src/grpc.rs:333).
  • Label maps are checked against the annotation entry limit (grpc/sandbox.rs:405).
  • metadata.uid in the Kubernetes merge patch isn't a real precondition; resourceVersion does the fencing, so the README's "UID-fenced" claim overstates it.
  • The protocol 2 is hard-coded in two places (gateway_members.rs, image_activation.rs:98).

This branch has not been deployed

No deployments
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.

feat: update sandbox workload images without replacing persistent storage

3 participants