Skip to content

feat(worker): run instance namespace isolation from a dedicated controller - #779

Merged
thxCode merged 10 commits into
mainfrom
fix/instance-network-isolation
Oct 21, 2026
Merged

thxCode merged 10 commits into
mainfrom
fix/instance-network-isolation

Conversation

@thxCode

@thxCode thxCode commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind enhancement
/area worker

What this PR does / why we need it:

Instance pods of different tenants must not reach each other while external clients keep NodePort access. This branch ships that isolation as an opt-in feature:

  1. Namespace-isolation policy controller (pkg/worker/controllers/worker/instance_isolation.go) — a dedicated InstanceIsolationReconciler watches Namespaces (a namespace is covered from the moment it exists, before its first Instance) and the delegated settings Secret. The pod-CIDR value rides an atomic.Value cache primed at setup and refreshed only on a real content change, so a jittery read can never masquerade as an emptied Setting and collect the policies. Reserved namespaces are never isolated (kuberess.IsReservedNamespace, the same rule the LocalQueue entrance applies).

  2. Opt-in Setting instance-isolation-pod-cidrs (pkg/worker/settings/value.go) — the one source of the pod address space (chart worker.instanceIsolationPodCIDRs, env GPUSTACK_INSTANCE_ISOLATION_POD_CIDRS). Blank (default) renders no policy at all; emptying the list collects the operator-rendered policy, never a same-named one written by hand. New admissions in pkg/setting: AllowIPv4CIDRList and DisallowListWith.

  3. Instance pods carry resource.gpustack.ai/type=instances (pkg/deviceplugin/pod_labels.go) — the operator's own resource-type label selects the guarded pods, so a third-party workload stamping a generic recommended label cannot walk into the isolation rules.

  4. The policy: same-namespace in/out, external in/out via an ipBlock 0.0.0.0/0 except <pod CIDRs> rule (how NodePort traffic arrives), DNS port-scoped; fixed name gpustack-instance-isolation.

Verified on a single-node rke2 + Calico cluster: verify-matrix.sh reports ALL CHECKS PASS (9/9) — cross-namespace probes blocked in both directions, same-namespace and external access intact, and the own-node hairpin residual is closed (stronger than the spec requires). The live opt-out cycle (blank the Setting, the policies are collected; restore, they are recreated and blocking again) is exercised too.

Which issue(s) this PR links to:

Addresses #778

Special notes for your reviewer:

  • The isolation policy is off by default: the Setting must name the cluster's pod CIDRs before any policy renders. On a Calico-IPAM cluster a node's spec.podCIDRs can disagree with what the CNI actually allocates — the docs point at kubectl get ippools.crd.projectcalico.org.
  • docs/reference/settings.md states the two effect windows (Immediate / next trigger) for settings; the new setting is Immediate.

Does this PR introduce a user-facing change?

Instance pods of different tenants can be isolated at the namespace boundary through the new opt-in `instance-isolation-pod-cidrs` setting (blank by default renders no policy). Reserved namespaces are never isolated; a CNI that enforces NetworkPolicy (Calico) is required.

- add InstanceComponent to the shared pod-label contract in pod_labels.go
- stamp ComponentLabelKey=instance on Instance Pods so one NetworkPolicy
  per namespace can select every Instance Pod

Task 1 of instance-network-isolation.

Signed-off-by: thxCode <thxcode0824@gmail.com>
- render one tenant-isolation NetworkPolicy per namespace holding Instances,
  selecting the stable instance component label
- ingress/egress allow same-namespace pods and external (NodePort) traffic,
  block every pod CIDR learned from the Nodes; DNS stays open port-scoped

Task 2 of instance-network-isolation.

Signed-off-by: thxCode <thxcode0824@gmail.com>
The tenant-isolation NetworkPolicy becomes opt-in and configuration-driven:
the new instance-tenant-isolation-pod-cidrs Setting (blank by default) is the
one source of the pod address space, so a reconcile renders the desired spec
without touching Nodes or CNI CRDs, compares with semantic DeepEqual, and
collects the policy again - only its own rendering, guarded by the
resource-type note - when the Setting empties.

Instance Pods carry the operator's resource-type label instead of the generic
recommended component label, whose collision space could sweep a third-party
workload into the isolation rules.

Signed-off-by: thxCode <thxcode0824@gmail.com>
Network isolation in the Instance guide now states the Setting contract, the
CIDR discovery paths, and that emptying the Setting collects only the
operator-rendered policy; the settings reference gains the new row.

Signed-off-by: thxCode <thxcode0824@gmail.com>
…oller

The namespace-isolation policy moves out of the Instance reconcile into its
own controller, watching Namespaces - a namespace is covered from the moment
it exists, before its first Instance - and the delegated settings Secret,
whose content is the Setting an extension API shows. The Setting value rides
an atomic cache primed at setup and refreshed only on a real content change,
so a jittery read can never masquerade as an emptied Setting and collect the
policies. Reserved namespaces are never isolated, the same rule the LocalQueue
entrance applies, and the policy is named gpustack-instance-isolation.

The pod-CIDR Setting renames to instance-isolation-pod-cidrs along with the
chart value and environment variable, and its admission moves into the
setting package as AllowIPv4CIDRList next to the tracking list's
DisallowListWith.

Signed-off-by: thxCode <thxcode0824@gmail.com>
The settings reference states the two effect windows - Immediate (pushed by
the consuming controller through the delegated settings Secret) and next
trigger (the next natural event named in each row) - marks the live-read
rows Immediate, and moves node-management-manual under the instance-type
rows to match the Setting registry's order.

Signed-off-by: thxCode <thxcode0824@gmail.com>
@gpustack-code-review

gpustack-code-review Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

✅ OpenCodeReview: Review skipped: no items were selected.

Reviewed a817fb8..d7a8599 only; earlier commits in this PR were reviewed in a previous run.

…d controller

Signed-off-by: thxCode <thxcode0824@gmail.com>
@thxCode

thxCode commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Addressing the review round — 5 findings (2 inline + 3 summary-routed):

  • bug · high — the align branch rewrote a same-named foreign policy (instance_isolation.go): Fixed. The update branch now applies the same resource-type-note guard as the opt-out; a policy nobody here rendered is skipped with a V(2) log, never rewritten. Covered by the new "configured setting leaves a same-named stranger exactly as found" table case.
  • bug · medium — running pods predate the resource-type label (instance.go): Fixed. The Instance reconcile patches the label in place on drifted pods — labels are mutable metadata, so no restart is involved. Covered by TestInstanceReconciler_PatchesResourceTypeLabelOnRunningPod.
  • maintainability · low — settings-name literal duplicated (instance_isolation.go): Fixed. The controller reads settings.InstanceIsolationPodCIDRs.Name(); the literal now lives only at the registration in value.go.
  • docs wording — "every namespace that holds an Instance" (docs/modules/instances/_index.md): Already fixed on the head this review ran against — the section reads "every non-reserved namespace — created when the namespace is, updated when the Setting changes, and collected when the Setting empties".
  • maintainability · low — values.yaml regeneration + IPv4 schema constraint (deploy/gpustack-operator/chart/values.yaml): The regenerated README.md and values.schema.json are already part of this PR. Constraining the schema entries to IPv4 CIDR strings is a fair refinement — deferred to a follow-up so this PR stays reviewable.

All gates green after the fixes: go build ./pkg/..., the four touched test suites, make lint, make lint docs.

…d controller

Signed-off-by: thxCode <thxcode0824@gmail.com>
…d controller

Signed-off-by: thxCode <thxcode0824@gmail.com>
…d controller

Signed-off-by: thxCode <thxcode0824@gmail.com>
@thxCode
thxCode merged commit 25c6524 into main Oct 21, 2026
17 checks passed
@thxCode
thxCode deleted the fix/instance-network-isolation branch October 21, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant