Sandbox related enhancement/fixes - #8
danielvallance wants to merge 13 commits into
Conversation
A few operations answer with their payload verbatim rather than inside the envelope: a file download, a stream of output. The bytes arrive with the status and headers that came with them, because a 206 says nothing without the Content-Range naming what it carries. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
An operation whose success response is octet-stream now goes through the raw transport rather than being typed as answering nothing, and a header parameter becomes a keyword argument merged over the caller's own, so a Range reads like a query filter. The platform and control-plane clients regenerate byte-identical. Each specification gains a target of its own, the sandbox plugin's among them, so one client can be regenerated without the others. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
A plugin serves an API of its own from inside an instance, and the SDK had no way to reach one. The platform proxies that API under the instance on the metro running it, a route no client knew how to build. The generated commands and filesystem clients are pointed at one instance by for_instance(), and hang off ukc.api.plugins.sandbox beside the platform and control-plane surfaces. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
Images are reported per metro, so an account-wide view means asking every metro in scope and merging what comes back, each image carrying the one that reported it. A partial answer is still an answer: the metros that did report travel with the failure. Looking an image up is a filtered listing rather than an operation of its own, which is how the CLI has it, and one metro's view is that metro's client. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
A plugin is reached through a route under the instance that runs it, which the platform resolves by UUID. A handle usually knows that UUID already, from the reference or from what it created, located or waited for, so addressing one costs a read only when it does not. Commands follow the CLI's sandbox package: an unbounded wait runs alongside the output polling, which speeds up while output arrives and backs off while it does not, and a timeout interrupts a command and then waits for it rather than abandoning it. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
The API reports a stop as two integers, a reason bitmask and a code whose meaning depends on it. The Go SDK decodes both; this one handed them over raw, so every caller decoded the platform's image-pull failure and the kernel's out-of-memory for itself. Stop, StopReason and the platform and kernel codes port the Go package, and Instance.stop reads them off an instance. The string form reads as the CLI reports a stop, e.g. "kernel crash: out of memory (ENOMEM)". Signed-off-by: Daniel Vallance <daniel@unikraft.com>
A create that waited for its instance to run raised a bare "API reported an error" when the instance stopped instead. The failed item names the instance and nothing else, so the reason survived only on the stopped instance, which every caller had to read. Instances.create now reads that instance and raises InstanceStoppedError, carrying it and its decoded stop; any other failure is unchanged. A wait that lapses raises WaitTimeoutError, as wait() already does. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
A delete answered as soon as it was under way, an instance already gone was a failure, and one the platform reported busy failed at once. A relay interface stays in use for a while after the instance using it is deleted, so callers retried and stopped by hand. delete() takes timeout_seconds for the API's own wait, missing_ok to resolve to None whether the API lost the instance or no metro held it, and retry_busy to back off while the API says EBUSY. The bulk delete takes timeout_seconds too. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
An instance set could be named only by uuid or name, one match per metro. A caller cleaning up everything a job had created had to list by tag and then delete each match itself, or guess at names. each(tags=[...]) locates every instance in scope carrying the tags and hands back the usual set, so `.delete(missing_ok=True)` covers them all. Tags that select nothing are an empty set rather than a failure, so a cleanup can run again. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
The images client lists what each metro's nodes have cached, which is not whether an image can be pulled: the cache drops images the registry keeps and keeps ones it has dropped. Callers asked the OCI registry directly, with its token exchange, to find out. Images.find and Images.exists ask the control plane, which answers from the registry itself and needs only the SDK's token. A reference may carry a tag, a digest, a registry host or a scheme, and a bare one means the latest tag. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
collect() gathered a command's output for the result, so a caller that wanted it as it arrived had to poll the command itself, and with it re-implement the interrupt and the grace a timeout gives. A finished command also stayed in the plugin until deleted. An on_output sink is handed each chunk as it is read, a coroutine it returns awaited before the next, while the result still carries everything. forget drops the plugin's record once the command has ended; a command still running is kept. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
The plugin reads a request body of two mebibytes at most, and a whole file came back in one payload. A caller moving anything larger chunked its writes by hand and held a download in memory. write() sends data longer than a chunk in appended pieces and can create the directories above the file; upload_file() streams a local file the same way. stream() and read_to() hand a file over as it arrives, so one larger than memory travels too. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
The metro proxy ends a plugin request after 60s. An unbounded wait for a command's end, and any wait longer than that, was cut off with the proxy's 504 page, so a long command could not be waited for or followed to its end. A wait is sent as waits of at most WAIT_SLICE, thirty seconds, until the command ends or the caller's timeout is used up, and the output stream waits for the end the same way. Signed-off-by: Daniel Vallance <daniel@unikraft.com>
There was a problem hiding this comment.
Copilot review overview
馃煛 Changes recommended
Timeout enforcement, chunk-size handling, route normalization, and HTTP pool ownership have unresolved correctness and reliability issues.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 7
Open (7)
Give PluginsApi ownership of its HTTP client pool 路 New Normalize /v1 suffix in API base URLs 路 New Use shared client ownership for SandboxApi resources 路 New Guard plugin route resolution in ready() 路 New Reject nonpositive chunk sizes before filesystem work 路 New Chunk or limit oversized public uploads 路 New Recognize localhost as an OCI registry host 路 New
What changed in this PR
Adds sandbox plugin support and improves reliability around file transfers, command execution, stopped instances, deletion, and image discovery.
Changes:
- Adds sandbox command/filesystem APIs with raw-byte streaming and chunked transfers.
- Adds stop diagnostics, image lookup/listing, and enhanced instance deletion.
- Extends generated clients, public exports, documentation, and tests.
| File | Description |
|---|---|
README.md |
Documents stopped-instance errors. |
Makefile |
Adds sandbox API generation. |
templates/鈥媟esources.tmpl |
Generates raw-byte and header handling. |
src/鈥媢nikraft_cloud/鈥媉_init__.py |
Exports new public APIs. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媉_init__.py |
Exposes plugin clients. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媝lugins/鈥媉_init__.py |
Groups plugin APIs. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媝lugins/鈥媉route.py |
Builds plugin routes. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媝lugins/鈥媠andbox/鈥媉_init__.py |
Groups sandbox resources. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媝lugins/鈥媠andbox/鈥媍ommands_gen.py |
Adds generated command operations. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媝lugins/鈥媠andbox/鈥媐s_gen.py |
Adds generated filesystem operations. |
src/鈥媢nikraft_cloud/鈥媋pi/鈥媝lugins/鈥媠andbox/鈥媘odels_gen.py |
Adds sandbox wire models. |
src/鈥媢nikraft_cloud/鈥媍lient.py |
Exposes image resources. |
src/鈥媢nikraft_cloud/鈥媍ore/鈥媓andle.py |
Supports absence-tolerant handles. |
src/鈥媢nikraft_cloud/鈥媍ore/鈥媓ttp.py |
Adds raw-byte responses and streaming. |
src/鈥媢nikraft_cloud/鈥媝lugins/鈥媉_init__.py |
Resolves instance plugin clients. |
src/鈥媢nikraft_cloud/鈥媝lugins/鈥媠andbox.py |
Implements sandbox commands and files. |
src/鈥媢nikraft_cloud/鈥媟esources/鈥媔mages.py |
Adds image listing and registry lookup. |
src/鈥媢nikraft_cloud/鈥媟esources/鈥媔nstances.py |
Enhances create, delete, tags, and plugins. |
src/鈥媢nikraft_cloud/鈥媟esources/鈥媠top.py |
Decodes instance stop information. |
tests/鈥媍onftest.py |
Extends asynchronous test transport helpers. |
tests/鈥媐ake_sandbox.py |
Adds an in-memory sandbox service. |
tests/鈥媡est_client.py |
Tests instance reliability enhancements. |
tests/鈥媡est_images.py |
Tests image discovery. |
tests/鈥媡est_plugins.py |
Tests plugin resolution. |
tests/鈥媡est_plugins_api.py |
Tests generated plugin plumbing. |
tests/鈥媡est_sandbox.py |
Tests sandbox workflows and transfers. |
tests/鈥媡est_stop.py |
Tests stop decoding. |
tests/鈥媡est_transport.py |
Tests raw-byte transport behavior. |
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def __init__(self, config: ApiClientConfig) -> None: | ||
| #: The sandbox plugin: commands and files inside an instance. | ||
| self.sandbox = SandboxApi(config) |
| return ( | ||
| f"{base_url.rstrip('/')}/v1/instances/{quote(instance_uuid, safe='')}" | ||
| f"/plugins/{quote(plugin, safe='')}" | ||
| ) |
| def __init__(self, config: ApiClientConfig) -> None: | ||
| self._config = config | ||
| #: Shell commands running inside the sandbox. | ||
| self.commands = CommandsApi(config) | ||
| #: The sandbox filesystem. | ||
| self.fs = FsApi(config) |
| api = await self.api() | ||
| try: | ||
| await api.commands.list_commands() | ||
| except UnikraftCloudError: | ||
| return False |
| if parents: | ||
| await self._make_parents(path) | ||
| payload = data.encode() if isinstance(data, str) else data |
| encoding, payload = _encode(data) | ||
| api = await self._sandbox.api() | ||
| await api.fs.upload_file( | ||
| body=models.UploadFileRequest( | ||
| path=path, filename=filename, parents=parents, encoding=encoding, data=payload | ||
| ) |
| # A registry host carries a dot or a port; a namespace never does. | ||
| if segments and ("." in segments[0] or ":" in segments[0]): | ||
| segments = segments[1:] |
| @dataclass(frozen=True) | ||
| class ExecResult: | ||
| """A finished command: its exit code and everything it wrote.""" | ||
|
|
||
| uuid: str | ||
| exit_code: int | ||
| stdout: bytes | ||
| stderr: bytes | ||
| #: Whether the command was interrupted because a timeout elapsed. | ||
| interrupted: bool = False |
There was a problem hiding this comment.
Was this file generated? Asking because i think the approach we should have for this should be similar to how we're doing the SDK for plugins compared to the Go and JS SDK. For those, we have tools which generate code, see here:
js/tools/tsplugingengo/tools/sdkgen- we should have a
python/tools/sdkgen
This will make it uniform across all plugins.
Then in the case of the JS SDK (and in the future the Go SDK), it's integrated as a peer dependency package with a nice porcelain layer around the plumbing. See example here for JS: unikraft-cloud/js-sdk#24

This PR enhances the sandbox functionality, so it gives this SDK more power to drive the sandbox plugin
It also contains various fixes/improvements to improve the reliability of a system using the Python SDK to drive the sandbox plugin, including: