Repository navigation
fix: stop asking Context Service to create sandbox pools - #469
Conversation
Moca's /workloads asked Context Service to create sandbox pools (/v1/sandbox-pools). Moca will own its workloads and sandboxes, and Context Service will only store context (rossoctl/context-service#46). Remove Moca's use of the sandbox-pool API. Until Moca provisions workloads itself, the /workloads routes, and any /runs request that names a workloadId, answer 501 workloads_unavailable. Runs without a workload are unchanged. Split out of rossoctl#455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
ingpaolodettori-dev
left a comment
ingpaolodettori-dev
left a comment
There was a problem hiding this comment.
Summary
A clean removal. The Context Service client, the workload store and handlers, authenticateSubject, and their tests are gone. /workloads[/{id}] (any method) and any /runs that names a workloadId (sync and async) now answer 501 workloads_unavailable before touching a sandbox or the queue. A run that names a workload is refused, not silently run on the default pool. That's the right call, and the tests pin it on both paths. I merged the branch into current main (it's 22 commits behind, no conflicts) and grepped for leftovers. Nothing outside docs/specs still references CONTEXT_SERVICE_*, authenticateSubject, /v1/sandbox-pools, or the removed error codes, and /runs still strips a caller-supplied sandboxPoolSelector. Two non-blocking comments are inline, both on docs.
FYI, not for this PR: LeafEnvelope.sandboxPoolSelector in harness/src/run-leaf.ts can no longer be set by any external caller (/runs deletes it and nothing re-adds it). Leaving it in place for the follow-up that provisions workloads makes sense.
Author: moonlight16 (MEMBER — maintainer)
Areas reviewed: TypeScript (knative-server), tests, docs
Agent/IDE config (.claude/.vscode): none
Commits: 1 commit, all signed-off: yes
CI status: passing
| authorize one API caller relative to another. | ||
| A `/runs` request that names a `workloadId` gets the same `501` instead of running on the default | ||
| sandbox pool. Requests without a `workloadId` are unchanged. `CONTEXT_SERVICE_URL` and | ||
| `CONTEXT_SERVICE_TIMEOUT_MS` are no longer read. |
There was a problem hiding this comment.
suggestion: Add an upgrade note for deployments that used the old routes. Workload records were written to Redis as sh:workload:<id> with no TTL (saveWorkload passed no EX). The Context Service pools behind them were created via POST /workloads, and with this PR Moca no longer has a way to release them: DELETE is now a 501. One or two sentences would cover it. Release any existing pools in Context Service directly (before or after upgrading), and the sh:workload:* keys can be deleted.
| sandbox-pool configuration behave as before. | ||
| MOCA no longer asks Context Service to allocate sandbox pools. Context Service is moving to context | ||
| storage only (rossoctl/context-service#46), and MOCA will provision workloads itself | ||
| (rossoctl/moca#455). |
There was a problem hiding this comment.
nit: This line and the comment at packages/knative-server/src/server.ts:453 cite #455 as where Moca will provision workloads. #455 is a PR (the one this was split out of), and your description says #358 stays open for the dynamic workload work. A tracking issue will outlive a PR that may be split or closed, so consider pointing both at #358 (or a new issue).
…#473) Resolve server.ts in main's favour for the workload lifecycle — every /workloads route and any run naming a workloadId now answers 501 workloads_unavailable (rossoctl#455) — keeping this branch's error-code changes on top. The spec follows: the /v1/runs 404 loses workload_not_found, the 501 gains workloads_unavailable, and the sandboxPoolSelector note drops the workload-resolver clause. routes.ts workload summaries note the 501. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Context Service created Moca's execution environments through /v1/sandbox-pools: Sandboxes, sandbox profiles, and WarmPool claims. Moca now owns its sandboxes, and rossoctl/moca#469 removed its last call to this API. Remove the sandbox-pool API, the Sandbox, profile and WarmPool code, contextctl sandbox-pool, CS_SANDBOX_IMAGE, and the matching RBAC. Context storage is unchanged. Closes #46. Part of the work split out of rossoctl/moca#455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
Workloads created through the old /workloads routes are no longer released by Moca or Context Service. Document how to delete their Sandboxes, PVCs, and sh:workload:* Redis keys, and point the docs at #476 instead of #455. Follow-up to #469. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>
First of the smaller PRs split out of #455.
Moca's
/workloadsasked Context Service to create sandbox pools (/v1/sandbox-pools). We're moving that responsibility: Moca will own its workloads and sandboxes, and Context Service will only store context (rossoctl/context-service#46).This PR removes Moca's use of the sandbox-pool API. Until Moca provisions workloads itself, the
/workloadsroutes, and any/runsrequest that names aworkloadId, answer501 workloads_unavailable. Runs without a workload are unchanged.This unblocks rossoctl/context-service#46. #358 stays open for the dynamic workload work.
🤖 Generated with Claude Code