fix(api): expose sandbox placement failure reason in error message - #3554
fix(api): expose sandbox placement failure reason in error message#3554AdaAibaby wants to merge 4 commits into
Conversation
When no node can host a sandbox, the placement algorithm now counts how many nodes were rejected at each filter stage (not-accepting, CPU incompatible, label-filtered, excluded) and includes those counts plus the build CPU constraints in the error returned to the caller. The ClientMsg in create_instance.go is updated to surface this diagnostic string, so clients see why placement failed instead of the opaque "Failed to place sandbox" message. Example new message: Failed to place sandbox: no compatible node found (38 nodes checked: 0 not-accepting, 38 cpu-incompatible, 0 label-filtered, 0 excluded); build cpu: arch=x86_64 family=6 model=207
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffeca63463
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Use errors.As to distinguish a FailedToPlaceSandboxError (which contains safe, diagnostic node-rejection counts and CPU constraints) from other placement failures (timeouts, gRPC internals). Only the former is formatted into the client-visible message; all other errors keep the generic "Failed to place sandbox" string.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a6ab5b69e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… only Node counts and CPU model details exposed in the placement error could allow any team API key to probe cluster topology that is otherwise restricted to the admin-only GET /nodes route. Revert ClientMsg to the generic "Failed to place sandbox". The detailed FailedToPlaceSandboxError (node rejection counts, build CPU constraints) remains in the internal Err field and is already recorded by telemetry.ReportError in startSandboxInternal, making it findable in server logs without exposing it to callers.
telemetry.ReportError writes to OTel spans only, so placement failures were invisible in Loki. Add an explicit logger.L().Error at the placement failure site with sandboxID, templateID, and the full FailedToPlaceSandboxError message (node rejection counts + build CPU constraints) so the cause is visible in log queries.
|
This change is already incorporated on |
3 participants
Fixes #3553
What
When
BestOfK.chooseNodefinds no eligible node it returns aFailedToPlaceSandboxError, butcreate_instance.goonly uses the internalErrfield and returns the opaque"Failed to place sandbox"to the client.This PR makes the error actionable:
BestOfK.sample()now counts how many nodes were rejected at each filter stage:not-accepting,cpu-incompatible,label-filtered,excluded.FailedToPlaceSandboxError.Error()emits a structured message with those counts plus the build CPU constraints and required labels.create_instance.gopropagateserr.Error()toClientMsgso the caller receives the full diagnosis.Before:
After:
Why this is safe to expose
The message reveals CPU generation and scheduling labels — information already visible through the admin
GET /nodesendpoint. No secrets or topology details are disclosed.Changes
packages/api/internal/orchestrator/placement/placement_best_of_K.go— addnodeRejectionCounts, thread throughsample(), populateFailedToPlaceSandboxError, updateError()packages/api/internal/orchestrator/create_instance.go—ClientMsgnow includeserr.Error()packages/api/internal/orchestrator/placement/placement_best_of_K_test.go— update assertions for new error format/cc @jakubno @dobrac @ValentaTomas @arkamar @tvi @tomassrnka Looking forward to your code review.