diff --git a/.golangci.yml b/.golangci.yml index d582d45..2fb704f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,10 @@ version: "2" run: allow-parallel-runners: true + # The e2e suite is behind this tag, so without it lint would silently skip the + # whole suite. + build-tags: + - e2e linters: default: none enable: diff --git a/Makefile b/Makefile index a31997c..f00d05d 100644 --- a/Makefile +++ b/Makefile @@ -115,7 +115,7 @@ setup-test-e2e: ## Set up a Kind cluster for e2e tests if it does not exist .PHONY: test-e2e test-e2e: setup-test-e2e manifests generate fmt vet ## Run the e2e tests. Expected an isolated environment using Kind. - KIND_CLUSTER=$(KIND_CLUSTER) go test ./test/e2e/ -v -ginkgo.v + KIND_CLUSTER=$(KIND_CLUSTER) go test -tags e2e ./test/e2e/ -v -ginkgo.v $(MAKE) cleanup-test-e2e .PHONY: cleanup-test-e2e diff --git a/README.md b/README.md index 2b541ab..be55566 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,9 @@ the standard `nvidia.com/gpu` resource limit, so scheduling and provisioning rea the same number. Do not set `nodeName` or a provider `nodeSelector` yourself — the placement controller owns those. -> `kubectl logs`/`exec` do not work against a virtual node (it doesn't serve the -> kubelet API); read workload output on the provider side. +> `kubectl logs` works, `-f` and `--tail` included: the manager serves the one kubelet +> route the API server proxies for logs. `--timestamps`/`--previous`/`--since` are +> ignored, and `kubectl exec` still does not work — use the provider's shell for that. ## Getting started diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 97f8c1a..93fe73e 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -87,91 +87,72 @@ const ( // boxes it owns — ownerReferences alone would not support a label-selector query. SandboxSetLabel = "nebula.inftyai.com/sandboxset" - // AcceleratorTypeLabel carries the requested accelerator TYPE only (e.g. - // "a100-40gb" or "h100"). The COUNT is expressed separately as a standard - // resource request/limit on the container (nvidia.com/gpu for the NVIDIA - // accelerators the wired providers serve today) — so scheduling fit and - // provisioning read the same number, and there is no bespoke count grammar. It - // is a label (not an annotation) so Pods can be selected/validated by - // accelerator type; label values forbid ":", which is exactly why the count - // could never live here. The name is provider-neutral (accelerator, not GPU) - // so it also fits non-GPU accelerators (TPUs, etc.) when such a provider lands. - // The type is matched case-insensitively against the provider catalog, so - // "a100", "A100" both resolve; the provider's canonical casing is what actually - // gets provisioned (see catalog.Base.MapAccelerator). Read type+count together + // AcceleratorTypeLabel carries the accelerator TYPE only (e.g. "a100-40gb", + // "h100"). The COUNT is a standard container resource request/limit + // (nvidia.com/gpu today), so scheduling fit and provisioning read the same number + // and there is no bespoke count grammar. A label rather than an annotation so Pods + // can be selected by accelerator type — and label values forbid ":", which is why + // the count could never live here. The name says "accelerator", not GPU, so TPUs + // and friends fit when such a provider lands. Matched case-insensitively against + // the catalog ("a100" and "A100" both resolve); the provider's canonical casing is + // what gets provisioned (see catalog.Base.MapAccelerator). Read type+count together // via util.AcceleratorRequest. AcceleratorTypeLabel = "nebula.inftyai.com/accelerator-type" - // CapacityTypeAnnotation carries the optimizer-chosen purchase tier - // (Spot/OnDemand). It is the one provisioning input that cannot be - // read off the Pod's own spec, so the placement controller writes it here - // when it ungates the Pod. The virtual kubelet — which provisions solely from - // the Pod — reads it back on CreatePod. Empty means "let the provider use its - // default" (e.g. Modal is OnDemand-only and ignores it). + // CapacityTypeAnnotation carries the chosen purchase tier (Spot/OnDemand). It is a + // provisioning input the Pod spec cannot express, so the placement controller + // writes it when it ungates the Pod and the virtual kubelet — which provisions + // solely from the Pod — reads it back on CreatePod. Empty means "use the provider's + // default" (Modal is OnDemand-only and ignores it). CapacityTypeAnnotation = "nebula.inftyai.com/capacity-type" - // RegionAnnotation carries the optimizer-chosen provider region. Like - // CapacityTypeAnnotation it is a provisioning input absent from the Pod's own - // spec, so the placement controller writes it when it ungates the Pod and the - // virtual kubelet reads it back on CreatePod into ProvisionRequest.Region. - // Empty/absent means "the provider's configured default region" — region-simple - // providers (Modal, RunPod) ignore it. + // RegionAnnotation carries the chosen provider region. Same flow as + // CapacityTypeAnnotation: stamped at ungate, read on CreatePod into + // ProvisionRequest.Region. Absent means the provider's default region; + // region-simple providers (Modal, RunPod) ignore it. RegionAnnotation = "nebula.inftyai.com/region" - // BlocklistTTLAnnotation carries the pool's FailoverPolicy.BlocklistTTL down to - // the virtual kubelet. Like the two annotations above it is a provisioning-time - // input the Pod cannot otherwise express: the TTL is a NodePool policy, but the - // VK handler (which provisions per-Pod and never sees the pool) needs it to know - // how long to blocklist a placement that just failed. The placement controller - // stamps it when it ungates the Pod; the handler reads it on a Provision failure - // to bound the block it records. Absent/unparseable means the handler's built-in - // default TTL. + // BlocklistTTLAnnotation carries the pool's FailoverPolicy.BlocklistTTL down to the + // virtual kubelet. The TTL is NodePool policy, but the VK handler provisions per-Pod + // and never sees the pool, so it needs the value here to bound the block it records + // after a Provision failure. Stamped at ungate; absent/unparseable means the + // handler's built-in default. BlocklistTTLAnnotation = "nebula.inftyai.com/blocklist-ttl" - // EndpointAnnotation carries the reachable address of the external instance (a - // public DNS name, an IP, or a URL, in the provider's own form). - // It is the ONLY way to reach the workload, so it must be visible on the Pod: - // PodIP cannot hold it because the API server validates PodIP as a literal IP - // and rejects a DNS name (the common AWS case), so the endpoint rides an - // annotation instead. Written by the virtual kubelet as soon as it knows the - // address, which is provider-dependent and NOT tied to the phase: a provider - // that mints a connect URL at create time (Modal) publishes it from CreatePod, - // before the instance is Running; one whose address only exists after boot (AWS) - // publishes it from the poll loop. Absent until then, and never cleared once - // written. Unlike the provisioning-input annotations above (which the placement - // controller stamps and VK reads), this flows the other way — VK writes it for - // operators/tooling to read. + // EndpointAnnotation carries the reachable address of the external instance (a DNS + // name, an IP, or a URL, in the provider's own form). It is the only way to reach + // the workload, and PodIP cannot hold it — the API server validates PodIP as a + // literal IP and rejects a DNS name, the common AWS case — so it rides an + // annotation. The virtual kubelet writes it as soon as it knows the address, which + // is NOT tied to the phase: a provider that mints a connect URL at create time + // (Modal) publishes from CreatePod, before Running; one whose address only exists + // after boot (AWS) publishes from the poll loop. Absent until then, never cleared. + // This one flows outward — VK writes, operators read. EndpointAnnotation = "nebula.inftyai.com/endpoint" - // TerminateInstanceFinalizer is held by every NodeClaim to guarantee teardown. - // The virtual kubelet owns the happy path (DeletePod → provider.Terminate, - // keyed on the Pod-derived claim name), but its teardown is edge-triggered and - // its instance tracking is in-memory, so a Pod force-deleted during a VK outage - // would leak a paid instance. This finalizer makes teardown level-triggered: - // the cluster-scoped claim outlives the namespaced Pod, so on delete the - // NodeClaim controller resolves the provider, finds the instance by claim name - // via List, and Terminates it before releasing the finalizer — independent of - // VK liveness (see docs/architecture.md §3). + // TerminateInstanceFinalizer is held by every NodeClaim to guarantee teardown. VK + // owns the happy path (DeletePod → provider.Terminate), but its teardown is + // edge-triggered and its tracking in-memory, so a Pod force-deleted during a VK + // outage would leak a paid instance. This finalizer makes teardown + // level-triggered: the cluster-scoped claim outlives the namespaced Pod, so on + // delete the NodeClaim controller resolves the provider, finds the instance by + // claim name via List, and Terminates before releasing — independent of VK + // liveness (see docs/architecture.md §3). TerminateInstanceFinalizer = "nebula.inftyai.com/terminate-instance" ) // Pod status reasons the virtual kubelet stamps on the Pods it reports, projecting -// the external instance's lifecycle onto standard Pod status (pkg/vnode/status.go -// is the only writer). +// the external instance's lifecycle onto standard Pod status (pkg/vnode/status.go is +// the only writer). // -// They live here, not privately in pkg/vnode, for two reasons. They are a CONTRACT -// between packages: the Pod phase is lossy — Provisioning and "booting" both -// surface as PodPending — so the reason is the only thing separating "no instance -// exists yet" from "an instance exists and is coming up", and the NodeClaim -// controller keys its teardown guard off exactly that distinction (see -// desiredPhase). A rename on the writing side that the reading side did not follow -// would still compile, still pass tests, and silently leak paid instances: every -// booting instance would read as Provisioning, so a Pod that vanished mid-boot -// would be left running behind the cache-lag grace window. And they are user-facing -// — operators match on status.reason in jsonpath and alerts — so every value is -// public API whether or not Nebula's own code currently reads it. That is why the -// whole set is here rather than the subset with in-tree readers: these are the -// values status.reason can take, and a reader should find them in one place. +// They are public rather than private to pkg/vnode because they are a CONTRACT +// between packages. The Pod phase is lossy — provisioning and booting both surface as +// PodPending — so the reason is the only thing separating "no instance yet" from "an +// instance exists and is coming up", and the NodeClaim controller keys its teardown +// guard off exactly that (see desiredPhase). A rename on the writing side that the +// reader missed would still compile, still pass tests, and silently leak paid +// instances. They are also user-facing (operators match status.reason in jsonpath and +// alerts), so the whole set lives here — not just the values with in-tree readers. const ( // PodReasonProvisioning: capacity has not been allocated yet. Stamped by CreatePod // before it calls Provision, and HELD if Provision returns an id without reserving @@ -181,17 +162,14 @@ const ( // as capacity is committed: at once for a provider that allocates synchronously // (AWS), otherwise when the first poll observes the instance. PodReasonProvisioning = "Provisioning" - // PodReasonInitializing: the instance EXISTS at the provider but is not yet - // reachable — it is booting (EC2 "pending"), running-but-not-yet-passing its - // reachability checks (running, <2/2, EC2's own "Initializing" status), or a Modal - // sandbox whose readiness probe has not passed. It mirrors that EC2 status-check - // term. Provisioning is done; the instance is coming up. Distinct from - // Provisioning so a Pod stuck here points at a slow boot / failing status checks, - // not a stuck allocation — and so the NodeClaim controller can tell that an - // instance exists. The virtual kubelet stamps it only on EVIDENCE of existence: - // either the provider observed the instance in its List, or Provision reported it - // reserved (capacity committed, not merely requested). That is what makes it - // trustworthy for the claim to key Bound off. + // PodReasonInitializing: the instance EXISTS but is not yet reachable — booting + // (EC2 "pending"), running with reachability checks outstanding (<2/2, EC2's own + // "Initializing" term, which this mirrors), or a Modal sandbox whose probe has not + // passed. Distinct from Provisioning so a Pod stuck here points at a slow boot + // rather than a stuck allocation, and so the NodeClaim controller can tell an + // instance exists. VK stamps it only on EVIDENCE of existence: the provider + // observed the instance in List, or Provision reported it reserved (capacity + // committed, not merely requested). That is what makes it safe to key Bound off. PodReasonInitializing = "Initializing" // PodReasonRunning: the provider reports the instance running. PodReasonRunning = "Running" diff --git a/api/v1alpha1/nodeclaim_types.go b/api/v1alpha1/nodeclaim_types.go index c7b1cde..51d493f 100644 --- a/api/v1alpha1/nodeclaim_types.go +++ b/api/v1alpha1/nodeclaim_types.go @@ -5,12 +5,11 @@ import ( ) // NodeClaimSpec is the durable identity of one external instance: who it serves, -// which provider it lives on, and which policy produced it. It is created and -// owned by the controller (not users), one per placed Pod. The workload shape -// (image, resources, GPU type/count, spot) is NOT duplicated here — that lives -// on the Pod, which the provider controller reads directly. NodeClaim is a -// ledger, not a spec: its reason to exist is to survive the Node so teardown can -// reclaim the external instance and never leak a paid GPU. +// which provider it lives on, and which policy produced it. Controller-created (not +// user-facing), one per placed Pod. The workload shape (image, resources, GPU +// type/count, spot) is NOT duplicated here — it lives on the Pod, which the provider +// controller reads directly. This is a ledger, not a spec: it exists to survive the +// Node so teardown can reclaim the instance and never leak a paid GPU. type NodeClaimSpec struct { // PodRef links this claim to the Pod it serves. UID pins the exact Pod so a // recreated Pod of the same name gets a fresh claim rather than adopting the @@ -22,43 +21,33 @@ type NodeClaimSpec struct { // knows which provider API to call even after status is lost. Provider string `json:"provider"` - // CapacityType is the purchase tier the placement optimizer selected - // (Spot/OnDemand). It is stored durably here because it is the one - // provisioning input that cannot be read off the Pod, and Provision needs it - // to re-issue the request after a controller restart. Immutable, like - // Provider. Empty means "let the provider use its default" (e.g. Modal is - // OnDemand-only and ignores it). + // CapacityType is the purchase tier placement selected (Spot/OnDemand). Stored + // durably because it is a provisioning input that cannot be read off the Pod, and + // Provision needs it to re-issue the request after a controller restart. Immutable, + // like Provider. Empty means "use the provider's default" (Modal is OnDemand-only + // and ignores it). // +optional CapacityType CapacityType `json:"capacityType,omitempty"` - // Region is the region candidate the placement optimizer selected, in the - // provider's own vocabulary (e.g. AWS "us-east-1"). Stored durably alongside - // Provider/CapacityType because it is a provisioning input that cannot be read - // off the Pod, and Provision needs it to re-issue the request in the same - // region after a controller restart. Immutable, like Provider. Empty means - // "the provider's configured default region" — a provider with no region - // constraint declared on the pool leaves it empty. + // Region is the region candidate placement selected, in the provider's own + // vocabulary (e.g. AWS "us-east-1"). Durable and immutable for the same reason as + // CapacityType: Provision must re-issue in the same region after a restart. Empty + // means the provider's default — a pool that declared no region constraint. // - // It is not always a single region NAME: a provider whose create cannot fail - // over collapses every region the pool declared into ONE candidate, and stores - // them joined by a provider-private separator (Modal uses "|", so a pool - // declaring us-east and us-west records "us-east|us-west"). Only that provider - // can split the value back, which it does at the API boundary. Treat the field - // as an opaque provider-scoped token rather than parsing it. + // Not always a single region NAME: a provider whose create cannot fail over + // collapses every declared region into ONE candidate, joined by a provider-private + // separator (Modal uses "|", so us-east + us-west records "us-east|us-west"). Only + // that provider can split it back. Treat the value as an opaque token. // +optional Region string `json:"region,omitempty"` - // Accelerator is the requested accelerator pool this claim serves, as - // "type:count" (e.g. "H100:8"), resolved at placement time from the Pod's - // accelerator type + count. It names the POOL, not the concrete SKU: a launch - // may span several interchangeable provider instance types (AWS's fleet tries - // alternates), so the exact instance type is only known post-launch from the - // observed instance — this field stays truthful regardless of which alternate - // lands. Unlike Provider/CapacityType/Region it is NOT a provisioning input (the - // provider re-derives it from the Pod) — it is recorded for reporting, like - // PoolRef, so `kubectl get nc` shows what each instance serves without - // cross-referencing the Pod. Empty for a CPU-only claim, which requests no - // accelerator. + // Accelerator is the accelerator pool this claim serves, as "type:count" (e.g. + // "H100:8"), resolved at placement time from the Pod. It names the POOL, not the + // SKU: a launch may span several interchangeable instance types (AWS's fleet tries + // alternates), so this stays truthful regardless of which alternate lands. Unlike + // Provider/CapacityType/Region it is NOT a provisioning input (the provider + // re-derives it from the Pod) — it is recorded so `kubectl get nc` shows what each + // instance serves. Empty for a CPU-only claim. // +optional Accelerator string `json:"accelerator,omitempty"` @@ -78,23 +67,18 @@ type PodReference struct { // NodeClaimPhase is the coarse, user-facing lifecycle state. // -// The NodeClaim is a passive teardown ledger, not a status mirror: it does NOT -// track finer workload runtime status (CPU/logs/restarts/readiness) — the Pod is -// the source of truth for that (see pkg/vnode/status.go). It tracks only the -// coarse states that matter to its own job as a ledger, keyed off the served -// Pod's phase/reason: Provisioning (instance does not exist yet), Bound (an -// instance EXISTS at the provider — the guard the teardown backstop trusts), and -// Terminated (instance gone). Finer states (e.g. Preempted) are deliberately -// absent: preemption cannot be detected — the provider contract's InstanceState -// has no Preempted value, and an absent instance only tells us it is gone, not -// why. Reintroduce a phase only when something actually sets it. +// The claim is a passive teardown ledger, not a status mirror: workload runtime +// status (CPU/logs/restarts/readiness) belongs to the Pod (see pkg/vnode/status.go). +// It tracks only what its own job needs, keyed off the served Pod: Provisioning +// (no instance yet), Bound (an instance EXISTS — the guard the teardown backstop +// trusts), Terminated (gone). Finer states like Preempted are absent because nothing +// can detect them: InstanceState has no Preempted value, and an absent instance only +// says it is gone, not why. Add a phase only when something actually sets it. // -// The ledger's question is EXISTENCE, not readiness: what the backstop must know -// is whether there is an instance out there to reclaim. A booting instance and a -// serving one are equally real — equally billable, equally in need of teardown — -// so both are Bound, and readiness is left entirely to the Pod. (This is why -// there is no Initializing phase: it would be a readiness distinction on an -// object that does not track readiness.) +// The ledger's question is EXISTENCE, not readiness — a booting instance is just as +// billable as a serving one, so both are Bound and readiness is left to the Pod. +// (Hence no Initializing phase: a readiness distinction on an object that does not +// track readiness.) type NodeClaimPhase string const ( @@ -104,39 +88,33 @@ const ( // provisioning is treated as possible cache lag (grace window), not a real // teardown, because we never confirmed an instance was actually created. NodeClaimProvisioning NodeClaimPhase = "Provisioning" - // NOTE: there is deliberately no "Initializing" phase. It used to mean "the - // instance exists but is not reachable yet" and did NOT earn the teardown guard, - // which stranded a real, billable instance behind the grace window whenever its - // Pod vanished mid-boot. Existence is what the ledger tracks, so that state is - // now Bound; readiness lives on the Pod alone. + // NOTE: there is deliberately no "Initializing" phase. It meant "exists but not + // reachable yet" and did NOT earn the teardown guard, which stranded a real, + // billable instance behind the grace window whenever its Pod vanished mid-boot. + // That state is now Bound; readiness lives on the Pod alone. // - // NodeClaimBound: an external instance EXISTS at the provider for this claim. - // This is the durable guard the backstop trusts — a Bound claim whose Pod later - // disappears is a real teardown, not cache lag, so it is reclaimed immediately - // rather than after the grace window. + // NodeClaimBound: an external instance EXISTS at the provider. This is the durable + // guard the backstop trusts — a Bound claim whose Pod later disappears is a real + // teardown, not cache lag, so it is reclaimed immediately instead of after the + // grace window. // - // Existence, NOT readiness: an instance that is booting (EC2 "pending", or - // running with its 2/2 status checks still pending; a Modal sandbox whose - // readiness probe has not passed) is Bound, because it is just as real and just - // as billable as one that is serving. Whether the workload is actually usable is - // the Pod's Ready condition, not this phase. + // Existence, NOT readiness: a booting instance (EC2 "pending", or running with its + // 2/2 checks outstanding; a Modal sandbox whose probe has not passed) is Bound, + // because it bills the same as one that is serving. Usability is the Pod's Ready + // condition. NodeClaimBound NodeClaimPhase = "Bound" - // NodeClaimTerminating: the served Pod is being deleted (its DeletionTimestamp - // is set) but the external instance may not be reclaimed yet — teardown is in - // flight. This is distinct from Terminated, which means the instance is already - // GONE: here the Pod object still exists (draining its grace period / VK's - // DeletePod running / a finalizer pending), so the phase reflects "going away" - // rather than stranding the claim on a stale Provisioning/Bound. It is a - // forward transition from ANY prior phase, since a deleting Pod is on its way - // out regardless of how far provisioning had progressed. The claim self-deletes - // (firing the terminate backstop) once the Pod object is fully gone. + // NodeClaimTerminating: the served Pod is being deleted but the instance may not be + // reclaimed yet — teardown is in flight. Distinct from Terminated (already GONE): + // here the Pod object still exists (grace period draining, VK's DeletePod running, + // a finalizer pending), so the claim reads "going away" rather than being stranded + // on a stale Provisioning/Bound. A forward transition from ANY prior phase, since a + // deleting Pod is on its way out regardless of provisioning progress. The claim + // self-deletes (firing the backstop) once the Pod object is fully gone. NodeClaimTerminating NodeClaimPhase = "Terminating" - // NodeClaimTerminated: the external instance is gone. Set when the served Pod - // has reached a terminal phase (Failed/Succeeded) — VK reports that when the - // provider's instance disappears (torn down, reclaimed, or exited). The claim - // stays around as a ledger of the vanished instance until its Pod is deleted; - // it does NOT self-delete on this transition (the instance is already gone, so - // there is nothing left to reclaim). + // NodeClaimTerminated: the instance is gone. Set when the served Pod reaches a + // terminal phase (Failed/Succeeded), which VK reports when the provider's instance + // disappears. The claim stays as the ledger of the vanished instance until its Pod + // is deleted, and does NOT self-delete here — there is nothing left to reclaim. NodeClaimTerminated NodeClaimPhase = "Terminated" ) diff --git a/api/v1alpha1/nodepool_types.go b/api/v1alpha1/nodepool_types.go index 85fe88d..aa6ff47 100644 --- a/api/v1alpha1/nodepool_types.go +++ b/api/v1alpha1/nodepool_types.go @@ -4,12 +4,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// NodePoolSpec is the placement policy for a set of workloads. It is the -// long-lived, user-facing object: editing it changes behaviour for every Pod -// that selects the pool, without touching any workload. +// NodePoolSpec is the placement policy for a set of workloads. Editing it changes +// behaviour for every Pod that selects the pool, without touching any workload. // -// Placement resolves along two orthogonal axes, and the ORDER between them is -// fixed: capacity type first, provider second. +// Placement walks two axes in a fixed order: capacity type first, provider second. // // FOR each capacityType in CapacityTypes (in listed order): // outer: hard tier // candidates = Providers x (each provider's Regions) x {this capacityType}, @@ -19,25 +17,18 @@ import ( // DONE // // else fall through to the next capacity tier // -// Region is a per-provider axis (see ProviderSpec.Regions), nested under each -// provider because a region name only means something to one provider. It -// widens the candidate key to {provider, region, accelerator, capacityType} -// without changing the tier-first ordering above. +// Region nests under each provider (see ProviderSpec.Regions) because a region name +// only means something to one provider. It widens the candidate key to +// {provider, region, accelerator, capacityType} but does not change the order above. // -// So CapacityTypes is a hard preference: every provider's Spot is tried before -// ANY provider's OnDemand. This is deliberate — "spot everywhere before any -// on-demand" is the least surprising behaviour, even if a different provider's -// on-demand were momentarily cheaper. Strategy only ranks providers *within* -// the active capacity tier; it never crosses tiers. +// So CapacityTypes is a HARD preference: every provider's Spot is tried before ANY +// provider's OnDemand, even if some provider's on-demand were momentarily cheaper. +// Strategy only ranks providers within the active tier; it never crosses tiers. // -// The Weighted strategy requires a weight on every provider ref. This is a -// static property of the spec, so it is enforced at admission by the CEL rule -// below rather than surfaced as a status condition after the fact. -// -// The rule is currently UNREACHABLE — Strategy's enum admits only Ordered, so no -// object can carry Weighted for it to check. It is retained rather than deleted so -// that widening the enum is a one-line change that cannot silently ship without its -// weight validation; the cost is one always-true CEL evaluation per admission. +// The CEL rule below enforces that Weighted has a weight on every provider — a static +// property of the spec, so admission is the right place for it. The rule is currently +// UNREACHABLE (the Strategy enum admits only Ordered), kept so widening the enum +// cannot ship without its weight validation. // +kubebuilder:validation:XValidation:rule="self.strategy != 'Weighted' || self.providers.all(p, has(p.weight))",message="strategy Weighted requires a weight on every provider" // (AWS once required at least one region here, because an omitted list meant "the // client's default region" and its client has none. Omitted now means "every region @@ -95,36 +86,27 @@ type ProviderSpec struct { // +optional Weight *int32 `json:"weight,omitempty"` - // Regions constrains where this provider may place, in the provider's OWN - // vocabulary. Region is provider-namespaced — there is no cross-provider region - // vocabulary — so it lives here per provider, not on the pool. It is a - // CONSTRAINT, not a list of regions to use, and it has three levels: - // - omitted/empty => unconstrained: every region the provider serves. For a - // region-simple provider (Modal) this means "send no region and let the - // provider place freely", which is also its widest and cheapest mode. - // - a geography GROUP token ("us", "eu", "ap", ...) => that geography's - // regions. This is the recommended way to ask for breadth with a data - // residency boundary. - // - a literal region name ("us-east-1" on AWS, "us-east" on Modal) => exactly - // that region. - // The provider resolves which level a value is, since only it knows its own - // geography (see provider.Provider's ExpandRegions). Group tokens are shared - // across providers but the regions behind them are not: "eu" is eu-west-1 and - // friends on AWS, while London is eu-west-2 there and there is no "uk" group. + // Regions CONSTRAINS where this provider may place, in the provider's own + // vocabulary. It lives here per provider because region names are + // provider-namespaced. Three levels: + // - omitted/empty => every region the provider serves. For a region-simple + // provider (Modal) this sends no region at all, its widest and cheapest mode. + // - a geography GROUP token ("us", "eu", "ap", ...) => that geography's regions. + // The recommended way to ask for breadth with a residency boundary. + // - a literal region name ("us-east-1" on AWS, "us-east" on Modal) => just that. + // Only the provider knows its own geography, so it resolves which level a value is + // (see provider.Provider's ExpandRegions). Group tokens are shared across + // providers; the regions behind them are not. // - // A value that is not a group token is passed to the provider UNVALIDATED. - // Region names change faster than Nebula ships, so an unrecognized one is - // forwarded rather than rejected: a genuinely bad name fails at provision time - // with the provider's own error, which is better than refusing a region that - // launched last week. It is also the escape hatch for AWS opt-in regions, which - // no group contains. + // A non-group value is passed through UNVALIDATED, because region names change + // faster than Nebula ships: a bad one fails at provision time with the provider's + // own error, which beats refusing a region that launched last week. It is also the + // escape hatch for AWS opt-in regions, which no group contains. // - // Unconstrained on a region-aware provider is the widest setting and costs - // something: every region becomes a placement candidate to walk on failover, and - // every region is swept by the observability poll loop. Prefer a group unless the - // workload genuinely needs global reach. There is no cap on the number of entries - // (a group already expands to many, so capping the declaration would be - // arbitrary); maxLength bounds each entry. + // Unconstrained is the widest and costliest setting: every region becomes a + // failover candidate and gets swept by the poll loop. Prefer a group unless the + // workload needs global reach. Entry count is uncapped (a group already expands to + // many); maxLength bounds each entry. // +optional // +kubebuilder:validation:items:MaxLength=32 Regions []string `json:"regions,omitempty"` diff --git a/api/v1alpha1/sandbox_types.go b/api/v1alpha1/sandbox_types.go index 2eb1c28..87bf20f 100644 --- a/api/v1alpha1/sandbox_types.go +++ b/api/v1alpha1/sandbox_types.go @@ -22,40 +22,24 @@ import ( ) // SandboxSpec is one long-lived, interactive remote box: an agent's workspace, a -// shell, a scratch GPU machine. It is the first workload class Nebula serves -// beyond a hand-written Pod. +// shell, a scratch GPU machine. // -// A Sandbox is SINGULAR — one object, one instance. There is deliberately no -// replicas field and no pod template, because a sandbox is not fungible: someone -// is attached to it, it accumulates state in its filesystem, and the object's own -// name IS its stable identity (`kubectl exec sandbox-alice` always reaches the -// same box). Set-shaped controllers exist because containers are interchangeable, -// which is exactly the property a sandbox lacks — a rolling update would evict a -// live session, and "scale in by one" would have to guess whose box to kill. A -// caller that wants N boxes creates N Sandboxes, each with its own image, -// lifetime and identity. +// A Sandbox is SINGULAR — one object, one instance, no replicas field. A sandbox is +// not fungible: someone is attached to it, it holds state in its filesystem, and its +// name is its stable identity. A rolling update would evict a live session, and +// "scale in by one" would have to guess whose box to kill. The count lives one level +// up in SandboxSet, which creates N Sandbox OBJECTS — that is what keeps per-box +// RBAC, image, TTL, and a visible failure working underneath a pool. // -// The count lives one level up, in SandboxSet, which maintains N Sandboxes and -// owns /scale so `kubectl scale` and HPA work. That split is what lets this type -// stay singular: because the set creates Sandbox OBJECTS rather than replicas -// inside one object, everything that depends on a box being its own object — -// per-box RBAC (grant a user their sandbox and not their neighbour's), a -// per-box image and TTL, and a failure that stays visible instead of being -// papered over by a replacement — keeps working underneath a pool. +// It reuses corev1 types (ResourceRequirements, EnvVar) because the controller +// synthesizes a Pod, so the spec must be PodSpec-shaped anyway. Re-declaring them +// would fork the source of truth for the accelerator COUNT, which placement and the +// scheduler both read from the container's nvidia.com/gpu limit. // -// The spec deliberately reuses corev1 types (ResourceRequirements, EnvVar) rather -// than inventing parallel fields. The controller synthesizes a Pod, so anything -// it accepts must ultimately BE PodSpec-shaped; re-declaring resources or env -// would fork the vocabulary and, worse, fork the source of truth for the -// accelerator COUNT — which placement and the scheduler's fit check both read -// from the container's nvidia.com/gpu limit (see util.AcceleratorRequest). -// -// The CEL rule below rejects a GPU count with no accelerator type. That pair is -// contradictory rather than merely incomplete — util.AcceleratorRequest returns an -// error for it — so without the rule the object is admitted and then fails at -// PLACEMENT, minutes later and one object removed from the mistake. Note the -// inverse is fine and deliberately allowed: a type with no count means one -// accelerator. +// The CEL rule below rejects a GPU count with no accelerator type: that pair is +// contradictory (util.AcceleratorRequest errors on it), so without the rule the +// object is admitted and then fails at PLACEMENT minutes later. The inverse is +// allowed — a type with no count means one accelerator. // +kubebuilder:validation:XValidation:rule="has(self.acceleratorType) || !has(self.resources) || ((!has(self.resources.limits) || !('nvidia.com/gpu' in self.resources.limits)) && (!has(self.resources.requests) || !('nvidia.com/gpu' in self.resources.requests)))",message="nvidia.com/gpu requires acceleratorType to be set" type SandboxSpec struct { // NodePoolRef names the NodePool whose policy places this sandbox: which @@ -65,26 +49,20 @@ type SandboxSpec struct { // +kubebuilder:validation:MinLength=1 NodePoolRef string `json:"nodePoolRef"` - // Image is the container image the sandbox runs. It defaults to a plain Ubuntu, - // because unlike the accelerator the image is not a decision a caller has to make - // to get a useful box: `kubectl exec` into a bare distro is exactly the "give me a - // remote shell" case, and anything else can be installed from inside it. Defaulting - // a paid GPU shape would be guessing at spend; defaulting a shell is not. + // Image is the container image the sandbox runs, defaulting to a plain Ubuntu: + // a bare distro to exec into IS the "give me a remote shell" case, and anything + // else can be installed from inside it. (Defaulting a paid GPU shape would be + // guessing at spend; defaulting a shell is not.) // - // Note it deliberately does NOT default to a CUDA image even when an accelerator is - // requested. A conditional default would make the image depend on another field, - // which structural-schema defaulting cannot express and which would surprise anyone - // reading the object back. Ask for a CUDA image explicitly when you want one. + // It does NOT switch to a CUDA image when an accelerator is requested: a default + // that depends on another field is not expressible in a structural schema. Ask for + // a CUDA image explicitly. // - // There is deliberately no command field, and one cannot be set: the CRD is a - // structural schema, so `command:` in a Sandbox spec is rejected as an unknown - // field by the apiserver itself — no webhook required. That is not a - // simplification, it is the process model: a sandbox has nothing to run at boot, - // so the controller supplies a placeholder command whose only job is to keep the - // container from exiting (today this is implemented with a long-running `sleep`, - // which must exist in the chosen image). Nebula does not currently support - // `kubectl exec`/`kubectl logs` against sandboxes; a user-supplied command would - // displace the placeholder and take the instance down with it. + // There is no command field and one cannot be set — the structural schema rejects + // `command:` as unknown, no webhook needed. A sandbox has nothing to run at boot, + // so the controller supplies a placeholder that only has to not exit (today a + // long-running `sleep`, which the image must have). A user command would displace + // it and take the instance down. // +kubebuilder:validation:MinLength=1 // +kubebuilder:default="ubuntu:24.04" // +optional diff --git a/api/v1alpha1/sandboxset_types.go b/api/v1alpha1/sandboxset_types.go index c7b9152..cc0452b 100644 --- a/api/v1alpha1/sandboxset_types.go +++ b/api/v1alpha1/sandboxset_types.go @@ -20,29 +20,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// SandboxSetSpec maintains N Sandboxes. That is the whole contract, and the name -// says exactly that much: it is a SET, not a pool. A pool would imply lease -// semantics — claim a box, hold it, return it, with the pool tracking who has what -// — and none of that is implemented here. Keeping N boxes alive is what ENABLES -// warm pooling (holding instances ready because provisioning takes minutes while -// an agent's exec call wants sub-second) and fan-out ("twenty boxes for this -// batch"), but those are uses of a set, not the set's job. "Pool" would also be a -// third meaning of that word in this API group, where NodePool already means -// placement policy. +// SandboxSetSpec maintains N Sandboxes. That is the whole contract — a SET, not a +// pool: there are no lease semantics here (claim a box, hold it, return it). Keeping +// N boxes alive is what ENABLES warm pooling and fan-out, but those are uses of a +// set, not its job. "Pool" is also already taken in this API group by NodePool. // -// It creates Sandbox OBJECTS, not replicas inside itself, and that is the point of -// having two types. The count is a genuinely different concern from the box: a set -// answers "how many", a Sandbox answers "which one, running what, for whom". -// Because each box stays its own object underneath, per-box RBAC, per-box status -// and a failure that stays visible all keep working — none of which survives being -// flattened into a replica index. +// It creates Sandbox OBJECTS, not replicas inside itself: a set answers "how many", +// a Sandbox answers "which one, running what, for whom". Because each box stays its +// own object, per-box RBAC, per-box status, and a visible failure keep working — +// none of which survives being flattened into a replica index. // -// Boxes get GENERATED names (myset-a4f2x), not ordinals. Ordinals would imply a -// slot that gets refilled, so a box that died would be replaced by an empty one -// wearing the same name — the same address with a different filesystem, which is -// the most confusing thing this API could do. A generated name means a replacement -// is visibly a NEW box, and callers that need a stable handle hold the Sandbox name -// they were given rather than an index into a set. +// Boxes get GENERATED names (myset-a4f2x), not ordinals. An ordinal implies a slot +// that gets refilled, so a dead box would be replaced by an empty one wearing the +// same name — same address, different filesystem. A generated name makes a +// replacement visibly a NEW box. type SandboxSetSpec struct { // Replicas is how many Sandboxes to maintain. Zero is legal and useful: it // releases every box while keeping the set's definition, which is how a set is diff --git a/cmd/main.go b/cmd/main.go index 1d52aa3..204bb4e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -83,6 +83,7 @@ func main() { var probeAddr string var secureMetrics bool var enableHTTP2 bool + var kubeletAddr, kubeletClientCA string var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") @@ -101,6 +102,14 @@ func main() { flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.StringVar(&kubeletAddr, "kubelet-bind-address", vnode.DefaultKubeletAddr, + "The address the virtual nodes' kubelet API (container logs, i.e. `kubectl logs`) binds to. "+ + "Set to \"\" to disable it, which makes logs unsupported.") + flag.StringVar(&kubeletClientCA, "kubelet-client-ca", "", + "PEM bundle of CAs whose client certificates may call the kubelet API. Empty (the default) "+ + "serves TLS without client verification, because which CA signs the API server's kubelet "+ + "client certificate is not portable across distributions; restrict the port with a "+ + "NetworkPolicy, or set this to your API server's kubelet client CA.") opts := zap.Options{ Development: true, } @@ -258,6 +267,11 @@ func main() { // both sides rather than persisted. blocklist := failover.New() + // The kubelet endpoint for `kubectl logs` — one listener shared by every provider's + // node, hence built here rather than in setupVirtualNodes. Nil is supported: the + // nodes then advertise no address, and logs report NotFound. + kubeletSrv := setupKubeletServer(mgr, kubeletAddr, kubeletClientCA) + // Controller and webhook registration is deferred until the cert exists, so it // runs in a goroutine: the cert cannot be minted until the manager is STARTED // (the rotator is a Runnable and needs a synced cache), so blocking here would @@ -274,7 +288,7 @@ func main() { <-certsReady setupLog.Info("webhook certificate ready") - if err := setupControllers(mgr, blocklist); err != nil { + if err := setupControllers(mgr, blocklist, kubeletSrv); err != nil { setupLog.Error(err, "unable to set up controllers") os.Exit(1) } @@ -340,7 +354,7 @@ func managerNamespace() string { // It runs only after the webhook serving cert is ready (see main), which is why it // is a function rather than inline: everything here depends on Pod admission // working, so none of it may be registered before the API server trusts the webhook. -func setupControllers(mgr ctrl.Manager, blocklist *failover.Blocklist) error { +func setupControllers(mgr ctrl.Manager, blocklist *failover.Blocklist, kubeletSrv *vnode.KubeletServer) error { if err := (&controller.NodePoolReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -383,7 +397,7 @@ func setupControllers(mgr ctrl.Manager, blocklist *failover.Blocklist) error { // provider.Terminate on DeletePod, so an ungated Pod bound to a provider's // virtual node materializes an external instance. Each Runner is a // manager.Runnable, so it shares the manager's lifecycle and leader election. - if err := setupVirtualNodes(mgr, blocklist); err != nil { + if err := setupVirtualNodes(mgr, blocklist, kubeletSrv); err != nil { return fmt.Errorf("unable to set up virtual nodes: %w", err) } if enableWebhooks() { @@ -395,11 +409,46 @@ func setupControllers(mgr ctrl.Manager, blocklist *failover.Blocklist) error { return nil } +// setupKubeletServer builds the shared endpoint that serves `kubectl logs` for Pods +// on the virtual nodes, and adds it to the manager. +// +// Returns nil — logs unsupported, nodes advertising no address — rather than failing +// the process, when addr is empty (turned off) or POD_IP is missing. That address is +// what the API server dials and nothing substitutes for it: a Service would balance to +// a non-leader replica, which holds no tracked Pods. Either way only logs degrade, so +// it is logged loudly and the manager carries on. +func setupKubeletServer(mgr ctrl.Manager, addr, clientCA string) *vnode.KubeletServer { + if addr == "" { + setupLog.Info("kubelet API disabled by configuration; `kubectl logs` will not work for Nebula pods") + return nil + } + podIP := vnode.PodIPFromEnv() + if podIP == "" { + setupLog.Info("POD_IP is not set; serving no kubelet API, so `kubectl logs` will not work " + + "for Nebula pods (project it with a fieldRef — see config/manager)") + return nil + } + srv, err := vnode.NewKubeletServer(podIP, addr, clientCA) + if err != nil { + // A real misconfiguration, but only of the log path: fail the feature, not the + // manager. + setupLog.Error(err, "unable to set up the kubelet API; `kubectl logs` will not work for Nebula pods") + return nil + } + if err := mgr.Add(srv); err != nil { + setupLog.Error(err, "unable to add the kubelet API to the manager") + return nil + } + setupLog.Info("kubelet API enabled", "addr", addr, "advertisedIP", podIP, "clientCertRequired", clientCA != "") + return srv +} + // setupVirtualNodes adds a vnode.Runner to the manager for every registered // provider. The Runner needs a typed clientset (the virtual kubelet's node/pod // controllers use client-go directly, not the controller-runtime client), built -// from the same rest.Config the manager uses. -func setupVirtualNodes(mgr ctrl.Manager, blocklist vnode.Blocklister) error { +// from the same rest.Config the manager uses. kubeletSrv is the shared kubelet API +// each node advertises for logs; nil disables it. +func setupVirtualNodes(mgr ctrl.Manager, blocklist vnode.Blocklister, kubeletSrv *vnode.KubeletServer) error { clientset, err := kubernetes.NewForConfig(mgr.GetConfig()) if err != nil { return err @@ -409,7 +458,7 @@ func setupVirtualNodes(mgr ctrl.Manager, blocklist vnode.Blocklister) error { if !ok { continue } - if err := mgr.Add(vnode.NewRunner(prov, clientset, blocklist)); err != nil { + if err := mgr.Add(vnode.NewRunner(prov, clientset, blocklist, kubeletSrv)); err != nil { return err } setupLog.Info("registered virtual node", "provider", name, "node", vnode.NodeName(name)) @@ -461,18 +510,15 @@ func registerProviders(ctx context.Context, c client.Client) { } // awsRegionSource returns the AWS adapter's RegionSource: the union of -// ProviderSpec.Regions across every NodePool that references the "aws" provider. It -// needs no env/flag — regions are the operator's per-pool declaration — and a pool -// added/edited at runtime widens the swept set on the next List tick, no restart -// required. +// ProviderSpec.Regions across every NodePool referencing the "aws" provider. No +// env/flag needed — regions are the operator's per-pool declaration — and editing a +// pool widens the swept set on the next List tick without a restart. // -// It is evaluated on each List/Offerings tick. The underlying List is served from -// the manager's informer cache (no API call), so scanning the pools per tick is -// cheap even though it is O(pools); sweepRegions dedupes the result. On a list error -// (cache not yet synced at startup, a transient failure) it returns nil and -// sweepRegions falls back to the regions already provisioned into. It uses a -// background context, not the registration ctx, since it runs long after -// registration returns. +// Evaluated per List/Offerings tick, served from the manager's informer cache (no API +// call), so the O(pools) scan is cheap; sweepRegions dedupes. On a list error (cache +// not synced yet, a transient failure) it returns nil and sweepRegions falls back to +// the regions already provisioned into. Uses a background context, since it runs long +// after registration returns. func awsRegionSource(c client.Client) awsprovider.RegionSource { return func() []string { var pools nebulav1alpha1.NodePoolList diff --git a/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml b/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml index de62ca2..dbc8b88 100644 --- a/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml +++ b/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml @@ -63,35 +63,29 @@ spec: spec: description: |- NodeClaimSpec is the durable identity of one external instance: who it serves, - which provider it lives on, and which policy produced it. It is created and - owned by the controller (not users), one per placed Pod. The workload shape - (image, resources, GPU type/count, spot) is NOT duplicated here — that lives - on the Pod, which the provider controller reads directly. NodeClaim is a - ledger, not a spec: its reason to exist is to survive the Node so teardown can - reclaim the external instance and never leak a paid GPU. + which provider it lives on, and which policy produced it. Controller-created (not + user-facing), one per placed Pod. The workload shape (image, resources, GPU + type/count, spot) is NOT duplicated here — it lives on the Pod, which the provider + controller reads directly. This is a ledger, not a spec: it exists to survive the + Node so teardown can reclaim the instance and never leak a paid GPU. properties: accelerator: description: |- - Accelerator is the requested accelerator pool this claim serves, as - "type:count" (e.g. "H100:8"), resolved at placement time from the Pod's - accelerator type + count. It names the POOL, not the concrete SKU: a launch - may span several interchangeable provider instance types (AWS's fleet tries - alternates), so the exact instance type is only known post-launch from the - observed instance — this field stays truthful regardless of which alternate - lands. Unlike Provider/CapacityType/Region it is NOT a provisioning input (the - provider re-derives it from the Pod) — it is recorded for reporting, like - PoolRef, so `kubectl get nc` shows what each instance serves without - cross-referencing the Pod. Empty for a CPU-only claim, which requests no - accelerator. + Accelerator is the accelerator pool this claim serves, as "type:count" (e.g. + "H100:8"), resolved at placement time from the Pod. It names the POOL, not the + SKU: a launch may span several interchangeable instance types (AWS's fleet tries + alternates), so this stays truthful regardless of which alternate lands. Unlike + Provider/CapacityType/Region it is NOT a provisioning input (the provider + re-derives it from the Pod) — it is recorded so `kubectl get nc` shows what each + instance serves. Empty for a CPU-only claim. type: string capacityType: description: |- - CapacityType is the purchase tier the placement optimizer selected - (Spot/OnDemand). It is stored durably here because it is the one - provisioning input that cannot be read off the Pod, and Provision needs it - to re-issue the request after a controller restart. Immutable, like - Provider. Empty means "let the provider use its default" (e.g. Modal is - OnDemand-only and ignores it). + CapacityType is the purchase tier placement selected (Spot/OnDemand). Stored + durably because it is a provisioning input that cannot be read off the Pod, and + Provision needs it to re-issue the request after a controller restart. Immutable, + like Provider. Empty means "use the provider's default" (Modal is OnDemand-only + and ignores it). enum: - Spot - OnDemand @@ -128,20 +122,15 @@ spec: type: string region: description: |- - Region is the region candidate the placement optimizer selected, in the - provider's own vocabulary (e.g. AWS "us-east-1"). Stored durably alongside - Provider/CapacityType because it is a provisioning input that cannot be read - off the Pod, and Provision needs it to re-issue the request in the same - region after a controller restart. Immutable, like Provider. Empty means - "the provider's configured default region" — a provider with no region - constraint declared on the pool leaves it empty. + Region is the region candidate placement selected, in the provider's own + vocabulary (e.g. AWS "us-east-1"). Durable and immutable for the same reason as + CapacityType: Provision must re-issue in the same region after a restart. Empty + means the provider's default — a pool that declared no region constraint. - It is not always a single region NAME: a provider whose create cannot fail - over collapses every region the pool declared into ONE candidate, and stores - them joined by a provider-private separator (Modal uses "|", so a pool - declaring us-east and us-west records "us-east|us-west"). Only that provider - can split the value back, which it does at the API boundary. Treat the field - as an opaque provider-scoped token rather than parsing it. + Not always a single region NAME: a provider whose create cannot fail over + collapses every declared region into ONE candidate, joined by a provider-private + separator (Modal uses "|", so us-east + us-west records "us-east|us-west"). Only + that provider can split it back. Treat the value as an opaque token. type: string required: - podRef diff --git a/config/crd/bases/nebula.inftyai.com_nodepools.yaml b/config/crd/bases/nebula.inftyai.com_nodepools.yaml index 9c5d824..197bcc9 100644 --- a/config/crd/bases/nebula.inftyai.com_nodepools.yaml +++ b/config/crd/bases/nebula.inftyai.com_nodepools.yaml @@ -50,36 +50,30 @@ spec: type: object spec: description: "NodePoolSpec is the placement policy for a set of workloads. - It is the\nlong-lived, user-facing object: editing it changes behaviour - for every Pod\nthat selects the pool, without touching any workload.\n\nPlacement - resolves along two orthogonal axes, and the ORDER between them is\nfixed: + Editing it changes\nbehaviour for every Pod that selects the pool, without + touching any workload.\n\nPlacement walks two axes in a fixed order: capacity type first, provider second.\n\n\tFOR each capacityType in CapacityTypes (in listed order): // outer: hard tier\n\t candidates = Providers x (each provider's Regions) x {this capacityType},\n\t available now, minus blocklist // region nests per provider\n\t \ IF candidates non-empty:\n\t pick one via Strategy (Ordered today; see Strategy) // inner: rank candidates\n\t DONE\n\t - \ // else fall through to the next capacity tier\n\nRegion is a per-provider - axis (see ProviderSpec.Regions), nested under each\nprovider because - a region name only means something to one provider. It\nwidens the candidate - key to {provider, region, accelerator, capacityType}\nwithout changing - the tier-first ordering above.\n\nSo CapacityTypes is a hard preference: - every provider's Spot is tried before\nANY provider's OnDemand. This - is deliberate — \"spot everywhere before any\non-demand\" is the least - surprising behaviour, even if a different provider's\non-demand were - momentarily cheaper. Strategy only ranks providers *within*\nthe active - capacity tier; it never crosses tiers.\n\nThe Weighted strategy requires - a weight on every provider ref. This is a\nstatic property of the spec, - so it is enforced at admission by the CEL rule\nbelow rather than surfaced - as a status condition after the fact.\n\nThe rule is currently UNREACHABLE - — Strategy's enum admits only Ordered, so no\nobject can carry Weighted - for it to check. It is retained rather than deleted so\nthat widening - the enum is a one-line change that cannot silently ship without its\nweight - validation; the cost is one always-true CEL evaluation per admission.\n(AWS - once required at least one region here, because an omitted list meant - \"the\nclient's default region\" and its client has none. Omitted now - means \"every region\nthe provider serves\", which is a valid — if broad - — AWS policy, so the rule is gone.\nSee ProviderSpec.Regions.)" + \ // else fall through to the next capacity tier\n\nRegion nests under + each provider (see ProviderSpec.Regions) because a region name\nonly + means something to one provider. It widens the candidate key to\n{provider, + region, accelerator, capacityType} but does not change the order above.\n\nSo + CapacityTypes is a HARD preference: every provider's Spot is tried before + ANY\nprovider's OnDemand, even if some provider's on-demand were momentarily + cheaper.\nStrategy only ranks providers within the active tier; it never + crosses tiers.\n\nThe CEL rule below enforces that Weighted has a weight + on every provider — a static\nproperty of the spec, so admission is + the right place for it. The rule is currently\nUNREACHABLE (the Strategy + enum admits only Ordered), kept so widening the enum\ncannot ship without + its weight validation.\n(AWS once required at least one region here, + because an omitted list meant \"the\nclient's default region\" and its + client has none. Omitted now means \"every region\nthe provider serves\", + which is a valid — if broad — AWS policy, so the rule is gone.\nSee + ProviderSpec.Regions.)" properties: capacityTypes: default: @@ -135,36 +129,27 @@ spec: type: string regions: description: |- - Regions constrains where this provider may place, in the provider's OWN - vocabulary. Region is provider-namespaced — there is no cross-provider region - vocabulary — so it lives here per provider, not on the pool. It is a - CONSTRAINT, not a list of regions to use, and it has three levels: - - omitted/empty => unconstrained: every region the provider serves. For a - region-simple provider (Modal) this means "send no region and let the - provider place freely", which is also its widest and cheapest mode. - - a geography GROUP token ("us", "eu", "ap", ...) => that geography's - regions. This is the recommended way to ask for breadth with a data - residency boundary. - - a literal region name ("us-east-1" on AWS, "us-east" on Modal) => exactly - that region. - The provider resolves which level a value is, since only it knows its own - geography (see provider.Provider's ExpandRegions). Group tokens are shared - across providers but the regions behind them are not: "eu" is eu-west-1 and - friends on AWS, while London is eu-west-2 there and there is no "uk" group. + Regions CONSTRAINS where this provider may place, in the provider's own + vocabulary. It lives here per provider because region names are + provider-namespaced. Three levels: + - omitted/empty => every region the provider serves. For a region-simple + provider (Modal) this sends no region at all, its widest and cheapest mode. + - a geography GROUP token ("us", "eu", "ap", ...) => that geography's regions. + The recommended way to ask for breadth with a residency boundary. + - a literal region name ("us-east-1" on AWS, "us-east" on Modal) => just that. + Only the provider knows its own geography, so it resolves which level a value is + (see provider.Provider's ExpandRegions). Group tokens are shared across + providers; the regions behind them are not. - A value that is not a group token is passed to the provider UNVALIDATED. - Region names change faster than Nebula ships, so an unrecognized one is - forwarded rather than rejected: a genuinely bad name fails at provision time - with the provider's own error, which is better than refusing a region that - launched last week. It is also the escape hatch for AWS opt-in regions, which - no group contains. + A non-group value is passed through UNVALIDATED, because region names change + faster than Nebula ships: a bad one fails at provision time with the provider's + own error, which beats refusing a region that launched last week. It is also the + escape hatch for AWS opt-in regions, which no group contains. - Unconstrained on a region-aware provider is the widest setting and costs - something: every region becomes a placement candidate to walk on failover, and - every region is swept by the observability poll loop. Prefer a group unless the - workload genuinely needs global reach. There is no cap on the number of entries - (a group already expands to many, so capping the declaration would be - arbitrary); maxLength bounds each entry. + Unconstrained is the widest and costliest setting: every region becomes a + failover candidate and gets swept by the poll loop. Prefer a group unless the + workload needs global reach. Entry count is uncapped (a group already expands to + many); maxLength bounds each entry. items: maxLength: 32 type: string diff --git a/config/crd/bases/nebula.inftyai.com_sandboxes.yaml b/config/crd/bases/nebula.inftyai.com_sandboxes.yaml index e98b198..8745a63 100644 --- a/config/crd/bases/nebula.inftyai.com_sandboxes.yaml +++ b/config/crd/bases/nebula.inftyai.com_sandboxes.yaml @@ -63,40 +63,24 @@ spec: spec: description: |- SandboxSpec is one long-lived, interactive remote box: an agent's workspace, a - shell, a scratch GPU machine. It is the first workload class Nebula serves - beyond a hand-written Pod. + shell, a scratch GPU machine. - A Sandbox is SINGULAR — one object, one instance. There is deliberately no - replicas field and no pod template, because a sandbox is not fungible: someone - is attached to it, it accumulates state in its filesystem, and the object's own - name IS its stable identity (`kubectl exec sandbox-alice` always reaches the - same box). Set-shaped controllers exist because containers are interchangeable, - which is exactly the property a sandbox lacks — a rolling update would evict a - live session, and "scale in by one" would have to guess whose box to kill. A - caller that wants N boxes creates N Sandboxes, each with its own image, - lifetime and identity. + A Sandbox is SINGULAR — one object, one instance, no replicas field. A sandbox is + not fungible: someone is attached to it, it holds state in its filesystem, and its + name is its stable identity. A rolling update would evict a live session, and + "scale in by one" would have to guess whose box to kill. The count lives one level + up in SandboxSet, which creates N Sandbox OBJECTS — that is what keeps per-box + RBAC, image, TTL, and a visible failure working underneath a pool. - The count lives one level up, in SandboxSet, which maintains N Sandboxes and - owns /scale so `kubectl scale` and HPA work. That split is what lets this type - stay singular: because the set creates Sandbox OBJECTS rather than replicas - inside one object, everything that depends on a box being its own object — - per-box RBAC (grant a user their sandbox and not their neighbour's), a - per-box image and TTL, and a failure that stays visible instead of being - papered over by a replacement — keeps working underneath a pool. + It reuses corev1 types (ResourceRequirements, EnvVar) because the controller + synthesizes a Pod, so the spec must be PodSpec-shaped anyway. Re-declaring them + would fork the source of truth for the accelerator COUNT, which placement and the + scheduler both read from the container's nvidia.com/gpu limit. - The spec deliberately reuses corev1 types (ResourceRequirements, EnvVar) rather - than inventing parallel fields. The controller synthesizes a Pod, so anything - it accepts must ultimately BE PodSpec-shaped; re-declaring resources or env - would fork the vocabulary and, worse, fork the source of truth for the - accelerator COUNT — which placement and the scheduler's fit check both read - from the container's nvidia.com/gpu limit (see util.AcceleratorRequest). - - The CEL rule below rejects a GPU count with no accelerator type. That pair is - contradictory rather than merely incomplete — util.AcceleratorRequest returns an - error for it — so without the rule the object is admitted and then fails at - PLACEMENT, minutes later and one object removed from the mistake. Note the - inverse is fine and deliberately allowed: a type with no count means one - accelerator. + The CEL rule below rejects a GPU count with no accelerator type: that pair is + contradictory (util.AcceleratorRequest errors on it), so without the rule the + object is admitted and then fails at PLACEMENT minutes later. The inverse is + allowed — a type with no count means one accelerator. properties: acceleratorType: description: |- @@ -235,26 +219,20 @@ spec: image: default: ubuntu:24.04 description: |- - Image is the container image the sandbox runs. It defaults to a plain Ubuntu, - because unlike the accelerator the image is not a decision a caller has to make - to get a useful box: `kubectl exec` into a bare distro is exactly the "give me a - remote shell" case, and anything else can be installed from inside it. Defaulting - a paid GPU shape would be guessing at spend; defaulting a shell is not. + Image is the container image the sandbox runs, defaulting to a plain Ubuntu: + a bare distro to exec into IS the "give me a remote shell" case, and anything + else can be installed from inside it. (Defaulting a paid GPU shape would be + guessing at spend; defaulting a shell is not.) - Note it deliberately does NOT default to a CUDA image even when an accelerator is - requested. A conditional default would make the image depend on another field, - which structural-schema defaulting cannot express and which would surprise anyone - reading the object back. Ask for a CUDA image explicitly when you want one. + It does NOT switch to a CUDA image when an accelerator is requested: a default + that depends on another field is not expressible in a structural schema. Ask for + a CUDA image explicitly. - There is deliberately no command field, and one cannot be set: the CRD is a - structural schema, so `command:` in a Sandbox spec is rejected as an unknown - field by the apiserver itself — no webhook required. That is not a - simplification, it is the process model: a sandbox has nothing to run at boot, - so the controller supplies a placeholder command whose only job is to keep the - container from exiting (today this is implemented with a long-running `sleep`, - which must exist in the chosen image). Nebula does not currently support - `kubectl exec`/`kubectl logs` against sandboxes; a user-supplied command would - displace the placeholder and take the instance down with it. + There is no command field and one cannot be set — the structural schema rejects + `command:` as unknown, no webhook needed. A sandbox has nothing to run at boot, + so the controller supplies a placeholder that only has to not exit (today a + long-running `sleep`, which the image must have). A user command would displace + it and take the instance down. minLength: 1 type: string nodePoolRef: diff --git a/config/crd/bases/nebula.inftyai.com_sandboxsets.yaml b/config/crd/bases/nebula.inftyai.com_sandboxsets.yaml index 9de1e6f..8c68256 100644 --- a/config/crd/bases/nebula.inftyai.com_sandboxsets.yaml +++ b/config/crd/bases/nebula.inftyai.com_sandboxsets.yaml @@ -55,29 +55,20 @@ spec: type: object spec: description: |- - SandboxSetSpec maintains N Sandboxes. That is the whole contract, and the name - says exactly that much: it is a SET, not a pool. A pool would imply lease - semantics — claim a box, hold it, return it, with the pool tracking who has what - — and none of that is implemented here. Keeping N boxes alive is what ENABLES - warm pooling (holding instances ready because provisioning takes minutes while - an agent's exec call wants sub-second) and fan-out ("twenty boxes for this - batch"), but those are uses of a set, not the set's job. "Pool" would also be a - third meaning of that word in this API group, where NodePool already means - placement policy. + SandboxSetSpec maintains N Sandboxes. That is the whole contract — a SET, not a + pool: there are no lease semantics here (claim a box, hold it, return it). Keeping + N boxes alive is what ENABLES warm pooling and fan-out, but those are uses of a + set, not its job. "Pool" is also already taken in this API group by NodePool. - It creates Sandbox OBJECTS, not replicas inside itself, and that is the point of - having two types. The count is a genuinely different concern from the box: a set - answers "how many", a Sandbox answers "which one, running what, for whom". - Because each box stays its own object underneath, per-box RBAC, per-box status - and a failure that stays visible all keep working — none of which survives being - flattened into a replica index. + It creates Sandbox OBJECTS, not replicas inside itself: a set answers "how many", + a Sandbox answers "which one, running what, for whom". Because each box stays its + own object, per-box RBAC, per-box status, and a visible failure keep working — + none of which survives being flattened into a replica index. - Boxes get GENERATED names (myset-a4f2x), not ordinals. Ordinals would imply a - slot that gets refilled, so a box that died would be replaced by an empty one - wearing the same name — the same address with a different filesystem, which is - the most confusing thing this API could do. A generated name means a replacement - is visibly a NEW box, and callers that need a stable handle hold the Sandbox name - they were given rather than an index into a set. + Boxes get GENERATED names (myset-a4f2x), not ordinals. An ordinal implies a slot + that gets refilled, so a dead box would be replaced by an empty one wearing the + same name — same address, different filesystem. A generated name makes a + replacement visibly a NEW box. properties: replicas: default: 1 @@ -259,26 +250,20 @@ spec: image: default: ubuntu:24.04 description: |- - Image is the container image the sandbox runs. It defaults to a plain Ubuntu, - because unlike the accelerator the image is not a decision a caller has to make - to get a useful box: `kubectl exec` into a bare distro is exactly the "give me a - remote shell" case, and anything else can be installed from inside it. Defaulting - a paid GPU shape would be guessing at spend; defaulting a shell is not. + Image is the container image the sandbox runs, defaulting to a plain Ubuntu: + a bare distro to exec into IS the "give me a remote shell" case, and anything + else can be installed from inside it. (Defaulting a paid GPU shape would be + guessing at spend; defaulting a shell is not.) - Note it deliberately does NOT default to a CUDA image even when an accelerator is - requested. A conditional default would make the image depend on another field, - which structural-schema defaulting cannot express and which would surprise anyone - reading the object back. Ask for a CUDA image explicitly when you want one. + It does NOT switch to a CUDA image when an accelerator is requested: a default + that depends on another field is not expressible in a structural schema. Ask for + a CUDA image explicitly. - There is deliberately no command field, and one cannot be set: the CRD is a - structural schema, so `command:` in a Sandbox spec is rejected as an unknown - field by the apiserver itself — no webhook required. That is not a - simplification, it is the process model: a sandbox has nothing to run at boot, - so the controller supplies a placeholder command whose only job is to keep the - container from exiting (today this is implemented with a long-running `sleep`, - which must exist in the chosen image). Nebula does not currently support - `kubectl exec`/`kubectl logs` against sandboxes; a user-supplied command would - displace the placeholder and take the instance down with it. + There is no command field and one cannot be set — the structural schema rejects + `command:` as unknown, no webhook needed. A sandbox has nothing to run at boot, + so the controller supplies a placeholder that only has to not exit (today a + long-running `sleep`, which the image must have). A user command would displace + it and take the instance down. minLength: 1 type: string nodePoolRef: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 10c9841..2745363 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -82,6 +82,17 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + # What the virtual nodes advertise as their kubelet endpoint, which is what + # makes `kubectl logs` work — the API server proxies logs there rather than + # reading them from etcd (see pkg/vnode/kubelet.go). + # + # Must be THIS Pod's IP, not a Service: tracked pods live in the leader's + # memory, so a Service could reach a replica that knows nothing about the pod. + # Unset (manager off-cluster) just disables the endpoint. + - name: POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP envFrom: # Provider credentials live in a per-provider Secret, one secretRef per # provider — NOT a single shared secret. This matches the "creds-absent → @@ -107,7 +118,19 @@ spec: # - secretRef: # name: nebula-runpod-credentials # optional: true - ports: [] + ports: + # The kubelet API the API server dials for `kubectl logs` (10250, like a real + # kubelet). Declaring it is documentation and NetworkPolicy surface; the + # listener binds either way. + # + # It serves TLS with a self-signed cert but does NOT verify client certs by + # default, because which CA signs the API server's kubelet client cert is not + # portable — requiring it would break logs on managed control planes. So + # anything able to reach this port can read any Nebula pod's logs: restrict it + # with a NetworkPolicy, or set --kubelet-client-ca to require mTLS. + - name: kubelet-api + containerPort: 10250 + protocol: TCP securityContext: readOnlyRootFilesystem: true allowPrivilegeEscalation: false diff --git a/config/samples/deployment.yaml b/config/samples/deployment.yaml index 8293eba..73b34d7 100644 --- a/config/samples/deployment.yaml +++ b/config/samples/deployment.yaml @@ -18,8 +18,9 @@ # fails or is torn down, so the workload — and its backing GPU instance — is # self-healing rather than a one-shot that lingers Failed. # -# Prereqs: the NodePool below must exist (kubectl apply -k config/samples applies -# both), and its providers must be registered (their credential Secrets present). +# Prereqs: the NodePool must exist (kubectl apply -f config/samples/nodepool.yaml +# -f config/samples/deployment.yaml applies both), and its providers must be +# registered (their credential Secrets present). apiVersion: apps/v1 kind: Deployment metadata: @@ -28,7 +29,7 @@ metadata: labels: app.kubernetes.io/managed-by: nebula spec: - replicas: 8 + replicas: 1 selector: matchLabels: app: gpu-workload-sample @@ -38,7 +39,7 @@ spec: app: gpu-workload-sample nebula.inftyai.com/enabled: "true" nebula.inftyai.com/nodepool: sample - nebula.inftyai.com/accelerator-type: a100-40gb + nebula.inftyai.com/accelerator-type: t4 spec: # Do NOT set nodeName or a provider nodeSelector yourself — the placement # controller fills the nodeSelector in when it ungates the Pod. Setting @@ -49,32 +50,75 @@ spec: # plain image (e.g. busybox) has no nvidia-smi. Any CUDA/GPU image works; # this one is small and only needs the driver from the host. image: nvidia/cuda:12.4.1-base-ubuntu22.04 - # Print the GPUs the sandbox sees at startup, then idle. + # Print the GPUs the sandbox sees, then run the daemon-server webserver in + # the foreground so the sandbox has a real, reachable service on it. # - # NOTE: `kubectl logs` and `kubectl exec` do NOT work against a Nebula - # virtual node — it does not serve the kubelet API (see pkg/vnode - # handler.go: GetContainerLogs/RunInContainer return NotFound). So to see - # this output, read it on the PROVIDER side (the Modal dashboard or - # `modal app logs`), not via kubectl. - command: ["sh", "-c", "nvidia-smi --query-gpu=index,name,memory.total --format=csv || echo 'no nvidia-smi'; sleep 3600"] - # The readiness bar for the EXTERNAL instance. On Modal this is the only - # thing that lets Nebula tell "still coming up" (queued, pulling the image, - # attaching the GPU) from "up and serving": the cheap poll signal answers - # only "has the process exited?", so WITHOUT a probe the Pod — and its - # Deployment's ready count — goes Running the moment the sandbox is created. - # With one, it stays Pending/Initializing until the probe passes. + # Split command/args purely for readability: the provider concatenates them + # (Command + Args) into the one argv Modal takes, so this is exactly + # ["sh", "-c", "