Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1alpha1/nodepool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ type ProviderSpec struct {
// level a workload can also request per-Pod.
// - a literal region name ("us-east-1" on AWS, "us-east" on Modal) => just that.
// Geographies are shared across providers; the regions behind them are not, so only
// the provider resolves which level a value is (provider.Provider's ExpandRegions).
// the provider resolves which level a value is (provider.Provider's ResolveRegions).
// +optional
// +kubebuilder:validation:items:MaxLength=32
Regions []string `json:"regions,omitempty"`
Expand Down
26 changes: 11 additions & 15 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,39 +651,35 @@ func registerProviders(ctx context.Context, c client.Client) {
}
}

// awsRegionSource returns the AWS adapter's RegionSource: the union of
// 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.
// awsRegionSource returns the AWS adapter's RegionSource: ProviderSpec.Regions of every
// NodePool referencing the "aws" provider, one entry per pool and unexpanded — the
// adapter resolves them, as it does for placement. 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.
//
// 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 {
return func() [][]string {
var pools nebulav1alpha1.NodePoolList
if err := c.List(context.Background(), &pools); err != nil {
setupLog.V(1).Info("aws region source: list NodePools failed; sweeping provisioned regions only",
"reason", err.Error())
return nil
}
var regions []string
var declared [][]string
for i := range pools.Items {
for _, ps := range pools.Items[i].Spec.Providers {
if ps.Name == provider.ProviderAWS {
// Expand PER POOL, before unioning. ProviderSpec.Regions is a
// constraint, not a list: an omitted one means "every region", and
// unioning the raw lists first would collapse that to "nothing" —
// the swept set would miss regions placement provisions into, and
// List's absence is reported as Terminated on live instances.
// This is the same expansion selectPlacement applies through the
// adapter's ExpandRegions; both must agree, so both call this one function.
regions = append(regions, awsprovider.ExpandRegions(ps.Regions)...)
// Never flatten: an omitted ProviderSpec.Regions means "every region",
// and appended to another pool's list it would vanish.
declared = append(declared, ps.Regions)
}
}
}
return regions
return declared
}
}
2 changes: 1 addition & 1 deletion config/crd/bases/nebula.inftyai.com_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ spec:
level a workload can also request per-Pod.
- a literal region name ("us-east-1" on AWS, "us-east" on Modal) => just that.
Geographies are shared across providers; the regions behind them are not, so only
the provider resolves which level a value is (provider.Provider's ExpandRegions).
the provider resolves which level a value is (provider.Provider's ResolveRegions).
items:
maxLength: 32
type: string
Expand Down
2 changes: 1 addition & 1 deletion docs/add-a-provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Create `pkg/provider/<name>/` and implement `provider.Provider`
| `Offerings(ctx)` | Price/availability rows for the optimizer (see the catalog below). |
| `MapAccelerator(canonical, count)` | Translate a canonical accelerator (type + count) to the provider's own id; `ok=false` if unsupported. |
| `ClassifyProvisionError(err, accel, region)` | Map a Provision failure to the `BlockScope` failover should blocklist. Only an **auth** error widens to the whole provider (`DenyAll`); capacity, quota, and unrecognized errors are all scoped to that {accel, tier, region} so failover can route around one candidate instead of fencing off the provider. Delegate to `provider.ClassifyError` for the shared part and decorate only what is provider-specific (e.g. the region axis). |
| `ExpandRegions(declared, narrowTo)` | Turn a pool's `regions` into the region candidates placement will walk, optionally narrowed to the geographies one workload asked for. |
| `ResolveRegions(declared, narrowTo)` | Turn a pool's `regions` into the region candidates placement will walk, optionally narrowed to the geographies one workload asked for. |

The Pod is the single source of truth for the workload shape; `ProvisionRequest`
carries only what the Pod cannot express (the optimizer's capacity tier and the
Expand Down
4 changes: 2 additions & 2 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ Follow one GPU Pod from creation to teardown:
```text
for each capacityType in pool.spec.capacityTypes: # outer axis
for each provider in pool.spec.providers: # listed order today
for each region in ExpandRegions(provider.regions, podGeographies):
for each region in ResolveRegions(provider.regions, podGeographies):
skip unregistered providers
skip providers that do not offer the accelerator type/count
skip providers that cannot serve the tier (Modal has no Spot)
Expand All @@ -173,7 +173,7 @@ Follow one GPU Pod from creation to teardown:
choose the first remaining candidate
```

The inner axis is whatever the provider's `ExpandRegions` returns, which is not
The inner axis is whatever the provider's `ResolveRegions` returns, which is not
one iteration per declared region: AWS expands a group token into many candidates,
while Modal collapses every declared region into a single candidate carrying them
all (so its inner loop always runs exactly once, and the chosen `region` may be a
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/nodeclaim_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type fakeProvider struct {
gpus []string // accelerators MapAccelerator offers; nil = offer any
spot bool // Capabilities().SupportsSpot (placement skips Spot without it)
egress bool // Capabilities().SupportsEgressPolicy (placement skips restricted pools without it)
// expandRegions overrides ExpandRegions; nil = pass the declaration through.
// expandRegions overrides ResolveRegions; nil = pass the declaration through.
expandRegions func([]string) []string
}

Expand Down Expand Up @@ -86,10 +86,10 @@ func (f *fakeProvider) MapAccelerator(c string, _ int32) ([]string, bool) {
return nil, false
}

// ExpandRegions is region-simple like Modal: declared tokens are their own geographies, an
// ResolveRegions is region-simple like Modal: declared tokens are their own geographies, an
// unconstrained pool takes the narrowing as its constraint, and no declaration at all is
// one unpinned candidate. Tests that need group expansion set expandRegions.
func (f *fakeProvider) ExpandRegions(declared, narrowTo []string) []string {
func (f *fakeProvider) ResolveRegions(declared, narrowTo []string) []string {
if f.expandRegions != nil {
declared = f.expandRegions(declared)
}
Expand Down
16 changes: 0 additions & 16 deletions internal/controller/pod_placement_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,22 +641,6 @@ func TestPlacement_UnresolvableRegionAnnotationLeavesPodGated(t *testing.T) {
}
}

func TestPlacementExpansion_AgreesWithAWSSweep(t *testing.T) {
// The two readers of ProviderSpec.Regions — selectPlacement and the AWS
// RegionSource in cmd/main.go — MUST expand a declaration identically. If the
// sweep covers less than placement provisions into, the missing region's instances
// are absent from List, and applyState maps absence to Terminated: a live, billing
// fleet reported as gone. Both go through ExpandRegions; this pins that they do.
for _, declared := range [][]string{nil, {"us"}, {"eu"}, {"us-east-1"}, {"us", "me-central-1"}} {
placementSide := awsprovider.New(nil, nil, nil).ExpandRegions(declared, nil)
sweepSide := awsprovider.ExpandRegions(declared)
if !slices.Equal(placementSide, sweepSide) {
t.Errorf("declared %v: placement walks %v but the sweep covers %v",
declared, placementSide, sweepSide)
}
}
}

func TestPlacement_SkipsSpotWhenProviderHasNoSpotTier(t *testing.T) {
// Modal has no user-facing preemptible capacity (SupportsSpot=false). The pool
// asks for Spot first, but that candidate is unservable, so the walk falls
Expand Down
7 changes: 3 additions & 4 deletions internal/controller/pod_placement_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,8 @@ func (r *PodPlacementReconciler) selectPlacement(ctx context.Context, pod *corev
}
}
// Empty means the pool's declaration, or the Pod's narrowing of it, reaches
// no region this provider can place in. The expansion must match
// awsRegionSource's (cmd/main.go); see its comment.
regions := prov.ExpandRegions(ref.Regions, narrowTo)
// no region this provider can place in.
regions := prov.ResolveRegions(ref.Regions, narrowTo)
if len(regions) == 0 {
metrics.RecordCandidateSkip(ref.Name, tier, "", metrics.SkipNoAvailableRegions)
log.V(1).Info("skipping candidate: no available region serves the requested geographies",
Expand Down Expand Up @@ -263,7 +262,7 @@ func servesEgress(prov provider.Provider, policy *nebulav1alpha1.EgressPolicy) b
return prov.Capabilities().SupportsEgressPolicy
}

// requestedGeographies reads the Pod's RegionsAnnotation into the narrowing ExpandRegions
// requestedGeographies reads the Pod's RegionsAnnotation into the narrowing ResolveRegions
// takes. Absent means no narrowing, the common case.
func requestedGeographies(pod *corev1.Pod) (narrowTo []string, ok bool) {
raw := strings.TrimSpace(pod.Annotations[nebulav1alpha1.RegionsAnnotation])
Expand Down
53 changes: 30 additions & 23 deletions pkg/provider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,19 @@ type EC2Instance struct {
// set serves every region and only the region endpoint differs.
type ClientFactory func(ctx context.Context, region string) (Client, error)

// RegionSource reports the regions the adapter should sweep in List and Offerings. The
// NodePool is the source of truth: cmd/main.go backs this with a lister over
// ProviderSpec.Regions across every "aws" pool, so the set is DYNAMIC (a pool added at
// runtime widens the sweep) and needs no env var. Provisioning never needed it — the target
// region rides on the request — only the fan-out does.
// RegionSource reports the declarations the adapter should sweep in List and Offerings: one
// entry per "aws" NodePool, its ProviderSpec.Regions verbatim. The NodePool is the source of
// truth: cmd/main.go backs this with a lister, so the set is DYNAMIC (a pool added at runtime
// widens the sweep) and needs no env var. Provisioning never needed it — the target region
// rides on the request — only the fan-out does.
//
// Per pool, not flattened: a nil entry is a pool with no constraint (every region), which a
// flat list would lose. See sweepRegions.
//
// It may return empty (no aws pool yet, or an unsynced cache); sweepRegions then falls back
// to the lazy client cache's keys, so a fleet placed by a prior generation is still swept.
// Must be safe to call concurrently.
type RegionSource func() []string
type RegionSource func() [][]string

// Provider is the EC2 implementation of provider.Provider. It embeds catalog.Base
// for the generic catalog methods (Name, Offerings, and MapAccelerator — which
Expand All @@ -220,7 +223,7 @@ type Provider struct {
// newClient lazily builds the Client for a region (AMI/subnet resolution). The
// factory seam keeps the adapter SDK-free and unit-testable.
newClient ClientFactory
// regionSource reports the NodePool-declared regions to sweep in List/Offerings.
// regionSource reports the NodePool declarations to sweep in List/Offerings.
// May be nil in tests, in which case sweepRegions uses only the cache keys.
regionSource RegionSource

Expand All @@ -229,12 +232,12 @@ type Provider struct {
}

// New returns an EC2 Provider backed by a client factory and price catalog.
// regionSource supplies the NodePool-declared region set the List/Offerings fan-out
// regionSource supplies the NodePool declarations the List/Offerings fan-out
// sweeps (nil is tolerated — the sweep then uses only the regions already
// provisioned into). cat is the catalog.Lookup seam so tests can inject a fake.
//
// There is deliberately NO default region: every request carries its own region
// (ExpandRegions turns even an omitted pool declaration into concrete regions, and
// (ResolveRegions turns even an omitted pool declaration into concrete regions, and
// placement stamps one onto the ProvisionRequest), and observed instances report
// their region from the region-pinned client — so nothing needs a fallback, and no
// AWS_REGION env is read.
Expand All @@ -256,24 +259,24 @@ func newSingleRegion(client Client, cat catalog.Lookup, region string) *Provider
p := New(
func(context.Context, string) (Client, error) { return client, nil },
cat,
func() []string { return []string{region} },
func() [][]string { return [][]string{{region}} },
)
// Pre-seed the cache so even a stray region lookup returns the fake rather than
// invoking the (constant) factory.
p.clients[region] = client
return p
}

// ExpandRegions implements provider.Provider. EC2 region names are not geographies, so AWS
// ResolveRegions implements provider.Provider. EC2 region names are not geographies, so AWS
// resolves them through regionsByGeography. Three levels, in the order they are checked:
//
// nil/[] => every default-enabled region
// ["us"] => the default-enabled regions in that geography
// ["us-east-1"] => itself, verbatim and unvalidated
//
// narrowTo filters the expansion to only include regions within the requested geographies.
func (p *Provider) ExpandRegions(declared, narrowTo []string) []string {
return narrowRegions(ExpandRegions(declared), narrowTo)
func (p *Provider) ResolveRegions(declared, narrowTo []string) []string {
return narrowRegions(expandDeclared(declared), narrowTo)
}

// narrowRegions keeps the regions that fall inside at least one requested geography. An
Expand Down Expand Up @@ -302,9 +305,9 @@ func narrowRegions(expanded, narrowTo []string) []string {
return out
}

// ExpandRegions is the package-level function that mirrors Provider.ExpandRegions, expanding
// declared regions into the full set of EC2 regions, with geographies resolved.
func ExpandRegions(declared []string) []string {
// expandDeclared turns a pool's declaration into concrete EC2 regions, with no narrowing.
// Placement narrows it and sweepRegions reads it as is, so the two agree.
func expandDeclared(declared []string) []string {
seen := make(map[string]bool)
var out []string
add := func(r string) {
Expand Down Expand Up @@ -337,11 +340,13 @@ func ExpandRegions(declared []string) []string {
}

// sweepRegions returns the regions List and Offerings fan out across: the union of
// the NodePool-declared set (regionSource) and every region already in the lazy
// client cache. The cache half is what makes teardown survive a NodePool edit — an
// instance still running in a region just dropped from every pool is still swept and
// so still observed/reclaimed, rather than being stranded because the region left
// the declared set. Order is not significant (callers concatenate results).
// each NodePool declaration (regionSource), resolved per pool as placement resolves it,
// and every region already in the lazy client cache. A region placed into but not swept
// is absent from List, which reports a live instance as Terminated. The cache half is
// what makes teardown survive a NodePool edit — an instance still running in a region
// just dropped from every pool is still swept and so still observed/reclaimed, rather than
// being stranded because the region left the declared set. Order is not significant
// (callers concatenate results).
func (p *Provider) sweepRegions() []string {
seen := make(map[string]bool)
var out []string
Expand All @@ -353,8 +358,10 @@ func (p *Provider) sweepRegions() []string {
out = append(out, r)
}
if p.regionSource != nil {
for _, r := range p.regionSource() {
add(strings.TrimSpace(r))
for _, declared := range p.regionSource() {
for _, r := range expandDeclared(declared) {
add(r)
}
}
}
p.mu.Lock()
Expand Down
Loading
Loading