feat(controller): clone a reference SandboxPool into each org namespace (#288)#746
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds template-driven per-org SandboxPool provisioning. New controller flags and Helm values configure the template source, org pool name, and warm-min override; the reconciler clones the template into each org namespace when enabled. Tests, docs, and schema formatting are updated accordingly. ChangesPer-org pool template feature
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/threat-model.md`:
- Around line 1168-1178: Clarify the behavior of --org-default-pool-image in the
threat-model text: it should be described as provisioning a default SandboxPool
inside each already-existing org namespace, not as the feature that enables
per-org namespaces. Update the sentence in the affected paragraph to reflect
that --enable-org-tenancy is what gates namespace reconciliation, and keep the
explanation aligned with the existing terminology around SandboxPool, org
namespace, and isolation boundary.
In `@internal/controller/org_controller.go`:
- Around line 69-73: The org default pool and gateway default pool can drift
apart, causing unnamed creates to fail at runtime. Add a startup/config
validation or shared constant so `DefaultPoolName` in `org_controller` stays
aligned with the gateway’s `--default-pool` setting, and make the check explicit
in the controller and gateway main setup paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a1046dd4-ccc1-4f8a-b76e-49144f816a20
📥 Commits
Reviewing files that changed from the base of the PR and between 76cc54b and cebd4cedef78dfa134d215f5d06075407ad417b6.
📒 Files selected for processing (8)
cmd/controller/main.godeploy/charts/mitos/templates/controller-deployment.yamldeploy/charts/mitos/values.schema.jsondeploy/charts/mitos/values.yamldocs/threat-model.mdinternal/controller/org_builders_test.gointernal/controller/org_controller.gointernal/controller/org_envtest_test.go
…ce (#288) Per-org namespace tenancy provisioned everything EXCEPT a SandboxPool, so a create in an org namespace had no pool to fork from. This closes that gap so the hosted deployment can run per-org namespaces instead of one shared namespace (which is what removes the shared-namespace referenced-object class of findings, e.g. the bare-name secret/workspace reference). When --org-pool-template-name is set (and --enable-org-tenancy), the OrgReconciler CLONES that reference pool's full spec (image, resources, placement, snapshots, network) into every org namespace with warm.min overridden (default 0), named --org-pool-name. Cloning the full spec from one source of truth is what makes the per-org pool schedule exactly where the reference does; a bare image would not carry the KVM placement the hosted nodes require. warm.min:0 means no idle warm husks, so N per-org pools cost nothing at rest (the snapshot is content-addressed and deduped per node); the first fork per org cold-starts and the pool warms on demand. The pool is cloned once and never clobbered, so autoscaler or operator warm tuning persists. Empty template name (the default) provisions no pool, so a self-host / single-tenant install is unaffected. Chart: controller.orgTenancy.poolTemplate.{name,namespace,orgPoolName,warmMin}. Threat-model 7a notes the pool lives inside the already-isolated org namespace and grants no cross-tenant reach. TDD: builder unit test (clone + deep-copy independence) + envtest (clones from a seeded reference pool when configured, skips when not). Refs #288. Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
cebd4ce to
57a4eed
Compare
Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks> # Conflicts: # deploy/charts/mitos/values.schema.json
|
Re-assessed against current main. Still relevant and not superseded: org_controller.go has had zero conflicting commits since #410, the merge is clean, CI is green, and per-org pools remain the missing enablement for flipping hosted prod to per-org namespaces. Three small fixes before merge:
Beyond the fixes, merging this is coupled to the operational decision to flip hosted prod to per-org tenancy, so it stays parked until that call is made; the feature is off by default and safe to land ahead of the flip if preferred. |
…g pool, doc pool-name invariant Review follow-ups on the per-org default pool clone (#288): 1. TemplateRef gap: buildOrgPoolFromTemplate deep-copies the reference pool spec verbatim, so a reference pool using spec.templateRef (a bare name resolved in its OWN namespace, ADR 0007) would clone a dangling ref into every org namespace where the shared template does not exist, and every org pool would silently never build. ensureOrgPool now rejects a templateRef reference pool with actionable text (make the reference pool inline), covered by a new envtest that asserts the reconcile errors and no per-org pool is created. 2. Deletion self-heal: the reconciler Owns the namespace, quota, netpol, and rolebinding but not the cloned SandboxPool, so a deleted per-org pool only returned on the next Org event or resync despite the stack-self-heal claim. Add Owns(&v1.SandboxPool{}); create-once ensureOrgPool skips existing pools, so the recreate is prompt and idempotent. 3. Pool-name invariant: --org-pool-name help now states it must match BOTH the SDK image/pool argument and the gateway --default-pool, or a create resolves a pool with no per-org clone and the org gets a silent Pending dead end. Signed-off-by: Jannes Stubbemann <jannes@openclaw.rocks>
Thinking Path
Linked Issues or Issue Description
Refs #288 (per-org namespace tenancy). This is the enablement that lets the hosted deployment move off the single shared namespace, complementing the defense-in-depth in #739.
What Changed
internal/controller/org_controller.go:ensureOrgPool+buildOrgPoolFromTemplate. WhenPoolTemplateNameis set, CLONE that reference pool's full spec (image, resources, placement, snapshots, network) into the org namespace withwarm.minoverridden (default 0), namedOrgPoolName, org label, owner-referenced to the Org. Cloning the full spec is what makes the per-org pool schedule where the reference does (a bare image would not carry the KVM placement). Cloned once and never clobbered. No-op when unset (self-host / bring-your-own-pool).cmd/controller/main.go:--org-pool-template-name/--org-pool-template-namespace/--org-pool-name/--org-pool-warm-minflags, wired under--enable-org-tenancy, with a startup log line.controller.orgTenancy.poolTemplate.{name,namespace,orgPoolName,warmMin}values, rendered onto the controller args (gated on name being set);values.schema.jsonupdated.docs/threat-model.md7a: notes the pool lives inside the already-isolated org namespace and grants no cross-tenant reach.Verification
go build ./...eval $(setup-envtest use 1.31 -p env) && go test ./internal/controller/ -run Org(new:TestBuildOrgPoolFromTemplateincl. deep-copy independence,TestOrgReconcilerProvisionsPoolFromTemplate,TestOrgReconcilerSkipsPoolWhenUnconfigured; existing Org stack tests still green; all TDD failing-first)helm template --kube-version 1.31: pool args render only whencontroller.orgTenancy.poolTemplate.nameis set, absent otherwise; schema validatescreateonsandboxpools)Risks
Low risk. Off by default (empty image), so self-host and the current single-tenant prod are unaffected until the flag is set. The pool is created once and never overwritten, so it cannot clobber warm tuning.
warm.min:0means no idle cost. The one operational note: the cloned pool name (OrgPoolName) must match the pool name a create resolves (the SDK image/pool argument, e.g.python); documented in the values.Model Used
Claude Fable 5 (
claude-fable-5), Anthropic, via Claude Code. Extended reasoning mode.Checklist
Signed-off-bytrailerSummary by CodeRabbit
SandboxPoolby cloning a reference template, including template location, per-org pool name, andwarm.minoverride.warm.minoverride; per-org pools are created only when missing (no updates to existing).