feat: /workloads phase 1 — owner-bound workloads with a Context by moonlight16 · Pull Request #482 · rossoctl/moca

moonlight16 · 2026-10-09T20:52:27Z

Part of the work split out of #455 (the original context-delegation PR). Builds on #469 (stopped calling Context Service sandbox pools) and rossoctl/context-service#52 (upload, freeze and bundle export).

A workload now belongs to the user who created it and owns one Context in Context Service. The user creates the workload, uploads a context bundle directly to Context Service with a one-time grant, and activates it to freeze that revision. Deleting the workload deletes the Context. docs/context-service.md has the flow diagram.

Off by default: MOCA_CONTEXT_WORKLOADS_ENABLED=1 and CONTEXT_SERVICE_URL turn it on. Every route requires the user's control-plane API token.
Workload names are scoped per user, the first half of /workloads: authorize caller-named PVCs per subject, and scope workload names #358 (per-user PVC authorization and workload names). Two users can use the same name, and another user's workload reads as not found.
Per-user quota, a storage size ceiling, and a StorageClass allowlist. Lifecycle changes are atomic, so a racing activate can't undo a delete, and quota is released exactly once. Deleted records expire after 24 hours.
Runs that name a workload still answer 501. Delivering the context to sandboxes is the next PR.
mocactl support for workloads comes in a separate PR; until then, call the API directly.

Phase 2, where each workload gets its own sandboxes and PVC, is #476 (phase 2: per-workload sandboxes and storage).

🤖 Generated with Claude Code

ingpaolodettori-dev

This needs a rebase before it can be reviewed for merge. The branch forks from 22d891e (#469) and is 10 commits behind main (efd925f). GitHub reports it as conflicting, so only DCO ran: ci, security-scans and k8s-e2e never ran on this head. A local merge conflicts in docs/context-service.md and packages/knative-server/src/server.ts.

The lifecycle logic itself looks solid. The CAS transitions on (generation, status), releasing quota exactly once, per-reservation Context names, and the race tests against real Redis all check out. I didn't find a correctness bug in workloads.ts. The must-fixes come from reconciling with main:

Redis key collision with the #477 upgrade note. Main's docs/context-service.md tells operators to run redis-cli --scan --pattern 'sh:workload:*' | xargs -r redis-cli del. The new records live at sh:workload:<ownerScope>:<name>, so after this lands that command deletes live workloads. It leaves their sh:workload-quota:* counters and their Contexts behind, so users are stuck at quota with nothing to delete. Please give the new records a prefix the legacy pattern can't match, or narrow the command to legacy keys only. Keep the Upgrading section when you resolve the conflict.
docs/api/harness-openapi.yaml (#474) still documents /v1/runs + workloadId → 501 workloads_unavailable (around lines 165 and 213), and its header says /workloads is "currently 501-refused". This PR renames the runs code to workload_runs_unavailable. Update the client contract, or keep the old code for runs.
packages/knative-server/src/routes.ts (#474) lists only POST/GET/DELETE /workloads…, with "(currently 501 workloads_unavailable)" summaries. Please add POST /workloads/{id}/uploads and POST /workloads/{id}/activate and update the summaries.

Once it's rebased and CI is green, this should be close to an approve.

ingpaolodettori-dev · 2026-10-10T03:26:41Z

+
+const sha256 = (value: string) => createHash('sha256').update(value).digest('hex');
+const ownerScope = (owner: string) => sha256(owner).slice(0, 32);
+const recordKey = (owner: string, name: string) => `sh:workload:${ownerScope(owner)}:${name}`;


must-fix: main's upgrade note (#477, docs/context-service.md → Upgrading) tells operators to clear legacy records with redis-cli --scan --pattern 'sh:workload:*' | xargs -r redis-cli del. These new keys match that pattern. Run after this lands, it deletes live owner-bound workloads, leaves sh:workload-quota:<owner> at its count (no record left to release it), and orphans the Contexts in Context Service. Please use a prefix the legacy pattern can't match (e.g. sh:wl:; note that sh:workload:v2: would still match), or narrow the doc command to legacy keys.

Fixed in 39ed214: workloads are now stored under sh:wl:<owner>:<name> (quota sh:wl-quota:<owner>), which sh:workload:* can't match. Kept the Upgrading section and noted that current workloads use sh:wl:.

ingpaolodettori-dev · 2026-10-10T03:26:41Z

-function rejectWorkloads(res: ServerResponse): void {
-  res.writeHead(501, JSON_HEADERS).end(JSON.stringify({ error: 'workloads_unavailable' }));
+function rejectWorkloadRun(res: ServerResponse): void {
+  res.writeHead(501, JSON_HEADERS).end(JSON.stringify({ error: 'workload_runs_unavailable' }));


must-fix (on rebase): main's docs/api/harness-openapi.yaml (#474) is the client contract for /v1/runs. Its 501 response and description still say workloads_unavailable for a run naming a workloadId, and its header comment says /workloads is "currently 501-refused". Please update it for the rename, or keep workloads_unavailable for runs. Also add POST /workloads/{id}/uploads and /activate to src/routes.ts, and update the "(currently 501 …)" summaries there.

Fixed in 39ed214: kept the workload_runs_unavailable rename and updated docs/api/harness-openapi.yaml (header, /v1/runs description, 501 response). routes.ts now lists POST /workloads/{id}/uploads and /activate, with the "currently 501" summaries replaced.

ingpaolodettori-dev · 2026-10-10T03:26:41Z

+    let cleaned = true;
+    await deleteContext(reserved.contextName, reserved.owner).catch((err: unknown) => {
+      cleaned = false;
+      console.error('workload create rollback could not delete its Context:', err);


suggestion: suppose this delete fails and the reservation has already been superseded (a concurrent DELETE marked it deleted). Then the transition below is a no-op, and the Context exists with no record left for a retry to act on. That's rare, but when it happens the only trace is this log line, which doesn't name the Context. Could you log reserved.contextName (it doesn't reveal the owner) so an operator can clean it up?

Done in 39ed214: the rollback log line now names the Context (reserved.contextName).

ingpaolodettori-dev · 2026-10-10T03:26:41Z

+    chunks.push(chunk);
+  }
+  try {
+    return JSON.parse(Buffer.concat(chunks).toString());


nit: the docs say every create field is optional (name is generated if omitted), but a body-less POST /workloads gets 400 invalid_json. Consider treating an empty body as {}.

Done in 39ed214: a body-less POST /workloads is treated as {}, with a test.

A workload belongs to the user who created it and owns one Context in Context Service. The user creates the workload, uploads a context bundle directly to Context Service with a one-time grant, and activates it to freeze that revision. Deleting the workload deletes the Context. Off by default (MOCA_CONTEXT_WORKLOADS_ENABLED=1 and CONTEXT_SERVICE_URL). Every route requires the user's control-plane API token. Names are scoped per user (first half of rossoctl#358). Per-user quota, a storage size ceiling, a StorageClass allowlist, and atomic lifecycle transitions. Runs that name a workload still answer 501 until context delivery lands. Part of the work split out of rossoctl#455. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>

- Store workloads under sh:wl: keys so the upgrade note's sh:workload:* cleanup cannot match them. - Document workload_runs_unavailable in the harness OpenAPI and list the uploads and activate routes in routes.ts. - Log the Context name when a create rollback cannot delete it. - Treat a body-less POST /workloads as all defaults. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>

deploy/k8s/base/supervisor.yaml gained an egress rule to Context Service for the optional /workloads routes; the base-supervisor manifest test pins the full egress list. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Jeremy Cohn <Jeremy.Cohn@ibm.com>

ingpaolodettori-dev

Thanks. I checked each fix in 39ed214 against the code, and all four items from the 528c718 review are resolved: workloads/quota moved to sh:wl:/sh:wl-quota: (no longer matched by the upgrade note's sh:workload:* cleanup), harness-openapi.yaml and routes.ts updated for workload_runs_unavailable and the uploads/activate routes, the rollback log names the Context, and a body-less create defaults to {}. The rebase onto main kept the Upgrading section and main's server.ts changes. fea45db's test expectation matches the new Context Service egress rule in deploy/k8s/base/supervisor.yaml exactly, and CI is green. One optional nit inline.

ingpaolodettori-dev · 2026-10-12T01:19:14Z

-# surface. The handler also serves the workload lifecycle (/workloads — currently 501-refused,
-# #455) and the pre-/v1 wire paths (/turn, /runs, /run-leaf*) — all listed in src/routes.ts as
+# surface. The handler also serves the workload lifecycle (/workloads — off unless the operator
+# enables it, see docs/context-service.md) and the pre-/v1 wire paths (/turn, /runs, /run-leaf*) — all listed in src/routes.ts as


nit: this header comment line is now 131 chars; the rest of the header wraps at ~100. Consider rewrapping.

moonlight16 requested review from cwiklik and pdettori as code owners October 9, 2026 20:52

moonlight16 requested review from evaline-ju and maia-iyer October 9, 2026 20:52

pdettori added the ready-for-ai-review label Oct 10, 2026

ingpaolodettori-dev requested changes Oct 10, 2026

View reviewed changes

moonlight16 added 2 commits October 11, 2026 11:13

moonlight16 force-pushed the feat/workloads-phase1 branch from 528c718 to 39ed214 Compare October 11, 2026 22:19

ingpaolodettori-dev approved these changes Oct 12, 2026

View reviewed changes

pdettori merged commit a38271c into rossoctl:main Oct 12, 2026
14 checks passed

moonlight16 mentioned this pull request Oct 12, 2026

feat: run tasks on a workload's context in shared sandboxes #494

Merged

Navigation Menu

Repository navigation

feat: /workloads phase 1 — owner-bound workloads with a Context - #482

feat: /workloads phase 1 — owner-bound workloads with a Context#482
pdettori merged 3 commits into
rossoctl:mainfrom
moonlight16:feat/workloads-phase1

moonlight16 commented Oct 9, 2026

Uh oh!

ingpaolodettori-dev left a comment

Uh oh!

ingpaolodettori-dev Oct 10, 2026

Uh oh!

moonlight16 Oct 11, 2026

Uh oh!

ingpaolodettori-dev Oct 10, 2026

Uh oh!

moonlight16 Oct 11, 2026

Uh oh!

ingpaolodettori-dev Oct 10, 2026

Uh oh!

moonlight16 Oct 11, 2026

Uh oh!

ingpaolodettori-dev Oct 10, 2026

Uh oh!

moonlight16 Oct 11, 2026

Uh oh!

ingpaolodettori-dev left a comment

Uh oh!

ingpaolodettori-dev Oct 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

moonlight16 commented Oct 9, 2026

Uh oh!

ingpaolodettori-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ingpaolodettori-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants