From 036dc41c26bf05ccd0934c81d060ee32a5804144 Mon Sep 17 00:00:00 2001
From: kerthcet <kerthcet@gmail.com>
Date: Thu, 24 Sep 2026 13:16:44 +0100
Subject: [PATCH 1/2] rename ExpandRegions

Signed-off-by: kerthcet <kerthcet@gmail.com>
---
 api/v1alpha1/nodepool_types.go                |  2 +-
 cmd/main.go                                   | 26 ++++-----
 .../bases/nebula.inftyai.com_nodepools.yaml   |  2 +-
 docs/add-a-provider.md                        |  2 +-
 docs/architecture.md                          |  4 +-
 .../controller/nodeclaim_controller_test.go   |  6 +--
 .../pod_placement_controller_test.go          | 16 ------
 internal/controller/pod_placement_helpers.go  |  7 ++-
 pkg/provider/aws/aws.go                       | 46 +++++++++-------
 pkg/provider/aws/aws_test.go                  | 53 +++++++++++--------
 pkg/provider/catalog/base.go                  |  2 +-
 pkg/provider/fake/fake.go                     |  4 +-
 pkg/provider/fake/fake_test.go                |  8 +--
 pkg/provider/modal/modal.go                   | 13 ++---
 pkg/provider/modal/modal_test.go              | 30 +++++------
 pkg/provider/provider.go                      |  8 +--
 pkg/vnode/handler_test.go                     |  2 +-
 17 files changed, 114 insertions(+), 117 deletions(-)

diff --git a/api/v1alpha1/nodepool_types.go b/api/v1alpha1/nodepool_types.go
index eedfd82..7cf369f 100644
--- a/api/v1alpha1/nodepool_types.go
+++ b/api/v1alpha1/nodepool_types.go
@@ -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"`
diff --git a/cmd/main.go b/cmd/main.go
index a52e24e..ed42e5f 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -651,10 +651,11 @@ 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
@@ -662,28 +663,23 @@ func registerProviders(ctx context.Context, c client.Client) {
 // 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
 	}
 }
diff --git a/config/crd/bases/nebula.inftyai.com_nodepools.yaml b/config/crd/bases/nebula.inftyai.com_nodepools.yaml
index 6a82b8e..144fb6d 100644
--- a/config/crd/bases/nebula.inftyai.com_nodepools.yaml
+++ b/config/crd/bases/nebula.inftyai.com_nodepools.yaml
@@ -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
diff --git a/docs/add-a-provider.md b/docs/add-a-provider.md
index 851db4b..0ebd3ef 100644
--- a/docs/add-a-provider.md
+++ b/docs/add-a-provider.md
@@ -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
diff --git a/docs/architecture.md b/docs/architecture.md
index cddbe3a..9e78bd8 100644
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -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)
@@ -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
diff --git a/internal/controller/nodeclaim_controller_test.go b/internal/controller/nodeclaim_controller_test.go
index ec933ca..2410f28 100644
--- a/internal/controller/nodeclaim_controller_test.go
+++ b/internal/controller/nodeclaim_controller_test.go
@@ -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
 }
 
@@ -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)
 	}
diff --git a/internal/controller/pod_placement_controller_test.go b/internal/controller/pod_placement_controller_test.go
index c54ea6f..98b5696 100644
--- a/internal/controller/pod_placement_controller_test.go
+++ b/internal/controller/pod_placement_controller_test.go
@@ -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
diff --git a/internal/controller/pod_placement_helpers.go b/internal/controller/pod_placement_helpers.go
index 595a6eb..7bc5b43 100644
--- a/internal/controller/pod_placement_helpers.go
+++ b/internal/controller/pod_placement_helpers.go
@@ -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",
@@ -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])
diff --git a/pkg/provider/aws/aws.go b/pkg/provider/aws/aws.go
index dd6c72f..b7bfe2f 100644
--- a/pkg/provider/aws/aws.go
+++ b/pkg/provider/aws/aws.go
@@ -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
@@ -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
 
@@ -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.
@@ -256,7 +259,7 @@ 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.
@@ -264,7 +267,7 @@ func newSingleRegion(client Client, cat catalog.Lookup, region string) *Provider
 	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
@@ -272,8 +275,8 @@ func newSingleRegion(client Client, cat catalog.Lookup, region string) *Provider
 //	["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
@@ -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) {
@@ -337,8 +340,9 @@ 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
+// 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).
@@ -353,8 +357,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()
diff --git a/pkg/provider/aws/aws_test.go b/pkg/provider/aws/aws_test.go
index 94d6d43..1c180ef 100644
--- a/pkg/provider/aws/aws_test.go
+++ b/pkg/provider/aws/aws_test.go
@@ -339,7 +339,7 @@ func TestProvision_EmptyRegionIsError(t *testing.T) {
 
 	// There is NO default region: a request that omits one cannot build a client, so
 	// Provision errors rather than silently guessing. In production every request
-	// carries a region (ExpandRegions never yields an empty one; placement stamps it),
+	// carries a region (ResolveRegions never yields an empty one; placement stamps it),
 	// so this only guards a malformed request.
 	if _, err := p.Provision(context.Background(), gpuPod("H100", 8), provider.ProvisionRequest{
 		ClaimName: "claim-def",
@@ -511,7 +511,7 @@ func TestTerminate_RegionOutsideTheSweep(t *testing.T) {
 	p := New(
 		func(context.Context, string) (Client, error) { return f, nil },
 		fakeCatalog{},
-		func() []string { return nil }, // no pool declares a region
+		func() [][]string { return nil }, // no aws pool
 	)
 
 	if regions := p.sweepRegions(); len(regions) != 0 {
@@ -525,6 +525,24 @@ func TestTerminate_RegionOutsideTheSweep(t *testing.T) {
 	}
 }
 
+// TestSweepRegions_CoversEveryPoolsPlacement pins that the sweep covers every region
+// placement can provision into, per pool. The unconstrained pool is the case that breaks if
+// declarations are flattened before resolving: appended to ["us"] it would vanish, leaving
+// eu-west-1 unswept and a live instance there reported as Terminated.
+func TestSweepRegions_CoversEveryPoolsPlacement(t *testing.T) {
+	pools := [][]string{nil, {"us"}, {"me-central-1"}}
+	p := New(nil, fakeCatalog{}, func() [][]string { return pools })
+
+	swept := p.sweepRegions()
+	for _, declared := range pools {
+		for _, r := range p.ResolveRegions(declared, nil) {
+			if !slices.Contains(swept, r) {
+				t.Errorf("pool %v places into %q, which the sweep %v misses", declared, r, swept)
+			}
+		}
+	}
+}
+
 func TestClassifyProvisionError(t *testing.T) {
 	p := newTestProvider(&fakeClient{})
 	const accel = "H100"
@@ -605,7 +623,7 @@ func TestCapabilities(t *testing.T) {
 	}
 }
 
-func TestExpandRegions(t *testing.T) {
+func TestExpandDeclared(t *testing.T) {
 	cases := []struct {
 		name     string
 		declared []string
@@ -679,22 +697,15 @@ func TestExpandRegions(t *testing.T) {
 	}}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			got := ExpandRegions(tc.declared)
+			got := expandDeclared(tc.declared)
 			if !slices.Equal(got, tc.want) {
-				t.Fatalf("ExpandRegions(%v)\n got %v\nwant %v", tc.declared, got, tc.want)
-			}
-			// The method must agree with the package function: cmd/main.go's region
-			// source calls the function while placement calls the method, and the two
-			// diverging is exactly the bug that reports a live fleet as Terminated.
-			p := newTestProvider(&fakeClient{})
-			if m := p.ExpandRegions(tc.declared, nil); !slices.Equal(m, got) {
-				t.Fatalf("method %v != function %v", m, got)
+				t.Fatalf("expandDeclared(%v)\n got %v\nwant %v", tc.declared, got, tc.want)
 			}
 		})
 	}
 }
 
-func TestExpandRegions_NarrowTo(t *testing.T) {
+func TestResolveRegions_NarrowTo(t *testing.T) {
 	cases := []struct {
 		name     string
 		declared []string
@@ -767,9 +778,9 @@ func TestExpandRegions_NarrowTo(t *testing.T) {
 	p := newTestProvider(&fakeClient{})
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			got := p.ExpandRegions(tc.declared, tc.narrowTo)
+			got := p.ResolveRegions(tc.declared, tc.narrowTo)
 			if !slices.Equal(got, tc.want) {
-				t.Fatalf("ExpandRegions(%v, %v)\n got %v\nwant %v",
+				t.Fatalf("ResolveRegions(%v, %v)\n got %v\nwant %v",
 					tc.declared, tc.narrowTo, got, tc.want)
 			}
 			// The invariant that makes narrowing safe without any membership check:
@@ -778,24 +789,24 @@ func TestExpandRegions_NarrowTo(t *testing.T) {
 			// List never polls, and absence from List reports a live instance as
 			// Terminated.
 			for _, r := range got {
-				if !slices.Contains(ExpandRegions(tc.declared), r) {
+				if !slices.Contains(expandDeclared(tc.declared), r) {
 					t.Fatalf("narrowed region %q is outside the swept expansion %v",
-						r, ExpandRegions(tc.declared))
+						r, expandDeclared(tc.declared))
 				}
 			}
 		})
 	}
 }
 
-// TestExpandRegions_NarrowToTakesVocabularyOnly covers the IsGeography gate itself, which no
+// TestResolveRegions_NarrowToTakesVocabularyOnly covers the IsGeography gate itself, which no
 // other case can reach: a narrowing resolves only because the token is VOCABULARY, not merely
 // because this adapter happens to have a table entry for it. Without the gate, adding a key
 // here would silently give one provider a narrowing token its siblings drop.
-func TestExpandRegions_NarrowToTakesVocabularyOnly(t *testing.T) {
+func TestResolveRegions_NarrowToTakesVocabularyOnly(t *testing.T) {
 	regionsByGeography["jp"] = []string{"ap-northeast-1"}
 	defer delete(regionsByGeography, "jp")
 
-	if got := narrowRegions(ExpandRegions(nil), []string{"jp"}); len(got) != 0 {
+	if got := narrowRegions(expandDeclared(nil), []string{"jp"}); len(got) != 0 {
 		t.Errorf("narrowTo [jp] resolved to %v; only provider.Geographies tokens may narrow", got)
 	}
 }
@@ -812,7 +823,7 @@ func TestRegionGroups_ExcludeOptInRegions(t *testing.T) {
 		"ca-west-1", "eu-central-2", "eu-south-1", "eu-south-2",
 		"il-central-1", "me-central-1", "me-south-1", "mx-central-1",
 	}
-	all := ExpandRegions(nil)
+	all := expandDeclared(nil)
 	for _, r := range optIn {
 		if slices.Contains(all, r) {
 			t.Errorf("opt-in region %q must not expand (it is disabled by default)", r)
diff --git a/pkg/provider/catalog/base.go b/pkg/provider/catalog/base.go
index 427dd2f..e12591e 100644
--- a/pkg/provider/catalog/base.go
+++ b/pkg/provider/catalog/base.go
@@ -53,7 +53,7 @@ type Lookup interface {
 // only while a provider's catalog price is all-in; one that meters CPU/memory separately
 // overrides it and adds those components.
 //
-// Lifecycle, Capabilities, ClassifyProvisionError and ExpandRegions are genuinely
+// Lifecycle, Capabilities, ClassifyProvisionError and ResolveRegions are genuinely
 // provider-specific and are not provided here.
 type Base struct {
 	// ProviderName is this provider's stable identifier (e.g. "modal"), used both
diff --git a/pkg/provider/fake/fake.go b/pkg/provider/fake/fake.go
index e5efd76..32b662b 100644
--- a/pkg/provider/fake/fake.go
+++ b/pkg/provider/fake/fake.go
@@ -92,14 +92,14 @@ func (p *Provider) Capabilities() provider.Capabilities {
 	}
 }
 
-// ExpandRegions expands geography tokens through regionsByGeography and narrows the
+// ResolveRegions expands geography tokens through regionsByGeography and narrows the
 // result to the requested geographies, mirroring the AWS adapter. Only that shape gives
 // placement a NAMED region per candidate, which is what lets the e2e suite assert which
 // region a Pod landed in rather than just that it landed.
 //
 // The fake names regions without partitioning anything behind them: Provision, Get and
 // List ignore the region entirely, so no instance behaves differently per region.
-func (p *Provider) ExpandRegions(declared, narrowTo []string) []string {
+func (p *Provider) ResolveRegions(declared, narrowTo []string) []string {
 	expanded := expandRegions(declared)
 	if len(narrowTo) == 0 {
 		return expanded
diff --git a/pkg/provider/fake/fake_test.go b/pkg/provider/fake/fake_test.go
index 710a71a..a457a9a 100644
--- a/pkg/provider/fake/fake_test.go
+++ b/pkg/provider/fake/fake_test.go
@@ -136,7 +136,7 @@ func TestTerminateIsIdempotent(t *testing.T) {
 // geography expands, a literal passes through, and a narrowing request subsets the
 // expansion. If any of this drifted toward a pass-through, e2e would sign off on a
 // narrowing the real adapters never perform.
-func TestExpandRegions(t *testing.T) {
+func TestResolveRegions(t *testing.T) {
 	tests := []struct {
 		name     string
 		declared []string
@@ -183,13 +183,13 @@ func TestExpandRegions(t *testing.T) {
 	p := New()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := p.ExpandRegions(tt.declared, tt.narrowTo)
+			got := p.ResolveRegions(tt.declared, tt.narrowTo)
 			if len(got) != len(tt.want) {
-				t.Fatalf("ExpandRegions(%v, %v) = %v, want %v", tt.declared, tt.narrowTo, got, tt.want)
+				t.Fatalf("ResolveRegions(%v, %v) = %v, want %v", tt.declared, tt.narrowTo, got, tt.want)
 			}
 			for i := range got {
 				if got[i] != tt.want[i] {
-					t.Fatalf("ExpandRegions(%v, %v) = %v, want %v", tt.declared, tt.narrowTo, got, tt.want)
+					t.Fatalf("ResolveRegions(%v, %v) = %v, want %v", tt.declared, tt.narrowTo, got, tt.want)
 				}
 			}
 		})
diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go
index fda78da..e945fb9 100644
--- a/pkg/provider/modal/modal.go
+++ b/pkg/provider/modal/modal.go
@@ -30,7 +30,7 @@ limitations under the License.
 //   - A pool's regions go out in ONE call rather than being walked. Create never fails
 //     on capacity (it queues), so nothing would re-drive placement to a second region
 //     and walking would strand the workload in whichever came first. Modal's scheduler
-//     has the live capacity view, so ExpandRegions collapses the declaration into one
+//     has the live capacity view, so ResolveRegions collapses the declaration into one
 //     opaque candidate and regionsOf splits it back at the API boundary. The cost is
 //     blocklist precision, which is free here since no Modal failure is
 //     region-attributable.
@@ -189,7 +189,7 @@ type SandboxSpec struct {
 	//
 	// It carries EVERY region the pool declared, not one per attempt, because a Modal
 	// create cannot fail over — it returns an accepted id with no capacity error, so
-	// nothing here could try a second region afterwards. See ExpandRegions and regionsOf.
+	// nothing here could try a second region afterwards. See ResolveRegions and regionsOf.
 	Regions []string
 	// Timeout is the sandbox's maximum lifetime. It MUST be non-zero: Modal treats
 	// a zero timeout as its 5-minute default, which would terminate a real
@@ -318,6 +318,7 @@ func New(client Client, cat catalog.Lookup) *Provider {
 const regionSeparator = "|"
 
 // regionsByGeography maps a geography token to the Modal regions it encompasses.
+// See: https://modal.com/docs/guide/region-selection#container-region-options.
 var regionsByGeography = map[string][]string{
 	"us": {"us", "us-east", "us-central", "us-south", "us-west"},
 	"eu": {"eu", "eu-west", "eu-north", "eu-south"},
@@ -380,9 +381,9 @@ func dedupeRegions(regions []string) []string {
 	return out
 }
 
-// ExpandRegions implements provider.Provider. It resolves the pool's whole declaration to at
+// ResolveRegions implements provider.Provider. It resolves the pool's whole declaration to at
 // most ONE candidate, carrying every declared region in it, rather than one per region.
-func (p *Provider) ExpandRegions(declared, narrowTo []string) []string {
+func (p *Provider) ResolveRegions(declared, narrowTo []string) []string {
 	regions := dedupeRegions(declared)
 	if len(narrowTo) > 0 {
 		regions = dedupeRegions(narrowRegions(regions, narrowTo))
@@ -562,7 +563,7 @@ func (p *Provider) List(ctx context.Context) ([]provider.Instance, error) {
 // sentinel; ClassifyError honours those first, then falls back to string heuristics.
 //
 // It confines the block to the failing CANDIDATE, which is not necessarily one region:
-// ExpandRegions sends every declared region at once, so the token may name the whole set
+// ResolveRegions sends every declared region at once, so the token may name the whole set
 // and the block then covers all of it. That is honest — a multi-region create never says
 // which region was short — and a pool wanting per-region blocking declares per-region
 // pools. An empty region leaves Region nil, which per BlockScope matches only candidates
@@ -684,7 +685,7 @@ func (p *Provider) sandboxSpecFromPod(pod *corev1.Pod, req provider.ProvisionReq
 }
 
 // regionsOf turns placement's single region candidate back into the slice Modal's
-// API takes. It is the exact inverse of ExpandRegions' join: that collapses the
+// API takes. It is the exact inverse of ResolveRegions' join: that collapses the
 // pool's whole declaration into ONE candidate (see there for why Modal cannot fail
 // over region by region), and this expands it again at the call boundary, so the
 // set the operator declared is what Modal's scheduler gets to choose among.
diff --git a/pkg/provider/modal/modal_test.go b/pkg/provider/modal/modal_test.go
index 0db9e9c..f211871 100644
--- a/pkg/provider/modal/modal_test.go
+++ b/pkg/provider/modal/modal_test.go
@@ -903,13 +903,13 @@ func TestProvision_CarriesRegion(t *testing.T) {
 	}
 }
 
-// TestExpandRegions_CollapsesToOneCandidate pins the axis decision that matters most
+// TestResolveRegions_CollapsesToOneCandidate pins the axis decision that matters most
 // for Modal: a pool's whole region declaration becomes exactly ONE placement
 // candidate. Modal's create accepts a sandbox and queues it without a capacity error,
 // so nothing re-drives placement afterwards — one candidate per region would mean the
 // first region walked is the only one ever tried, silently discarding the rest of the
 // operator's declaration. Collapsing hands the full set to Modal's own scheduler.
-func TestExpandRegions_CollapsesToOneCandidate(t *testing.T) {
+func TestResolveRegions_CollapsesToOneCandidate(t *testing.T) {
 	p := newTestProvider(&fakeClient{})
 
 	for _, tc := range []struct {
@@ -939,19 +939,19 @@ func TestExpandRegions_CollapsesToOneCandidate(t *testing.T) {
 		want:     []string{"eu-west" + regionSeparator + "us-east"},
 	}} {
 		t.Run(tc.name, func(t *testing.T) {
-			got := p.ExpandRegions(tc.declared, nil)
+			got := p.ResolveRegions(tc.declared, nil)
 			if !slices.Equal(got, tc.want) {
-				t.Fatalf("ExpandRegions(%v) = %v, want %v", tc.declared, got, tc.want)
+				t.Fatalf("ResolveRegions(%v) = %v, want %v", tc.declared, got, tc.want)
 			}
 			if len(got) > 1 {
-				t.Fatalf("ExpandRegions(%v) produced %d candidates; Modal cannot fail "+
+				t.Fatalf("ResolveRegions(%v) produced %d candidates; Modal cannot fail "+
 					"over, so every extra candidate is a region silently never tried", tc.declared, len(got))
 			}
 		})
 	}
 }
 
-func TestExpandRegions_NarrowTo(t *testing.T) {
+func TestResolveRegions_NarrowTo(t *testing.T) {
 	p := newTestProvider(&fakeClient{})
 	for _, tc := range []struct {
 		name     string
@@ -1024,9 +1024,9 @@ func TestExpandRegions_NarrowTo(t *testing.T) {
 		want:     nil,
 	}} {
 		t.Run(tc.name, func(t *testing.T) {
-			got := p.ExpandRegions(tc.declared, tc.narrowTo)
+			got := p.ResolveRegions(tc.declared, tc.narrowTo)
 			if !slices.Equal(got, tc.want) {
-				t.Fatalf("ExpandRegions(%v, %v) = %v, want %v",
+				t.Fatalf("ResolveRegions(%v, %v) = %v, want %v",
 					tc.declared, tc.narrowTo, got, tc.want)
 			}
 			// An empty result with a narrowTo means NO CANDIDATE. Reading it as
@@ -1039,17 +1039,17 @@ func TestExpandRegions_NarrowTo(t *testing.T) {
 	}
 }
 
-// TestExpandRegions_NarrowToTakesVocabularyOnly covers the IsGeography gate itself, which no
+// TestResolveRegions_NarrowToTakesVocabularyOnly covers the IsGeography gate itself, which no
 // other case can reach: a narrowing resolves only because the token is VOCABULARY, not merely
 // because this adapter has a table entry for it. Modal makes the stake concrete — "jp" IS a
 // region it serves, so without the gate an unconstrained pool would hand it straight to the
 // API as a candidate, honouring a token every sibling provider drops.
-func TestExpandRegions_NarrowToTakesVocabularyOnly(t *testing.T) {
+func TestResolveRegions_NarrowToTakesVocabularyOnly(t *testing.T) {
 	regionsByGeography["jp"] = []string{"jp"}
 	defer delete(regionsByGeography, "jp")
 
 	p := newTestProvider(&fakeClient{})
-	if got := p.ExpandRegions(nil, []string{"jp"}); len(got) != 0 {
+	if got := p.ResolveRegions(nil, []string{"jp"}); len(got) != 0 {
 		t.Errorf("narrowTo [jp] resolved to %v; only provider.Geographies tokens may narrow", got)
 	}
 }
@@ -1078,13 +1078,13 @@ func TestRegionsByGeography_IsResolvable(t *testing.T) {
 	}
 }
 
-// TestExpandRegions_RoundTripsThroughProvision is the invariant that makes the
-// collapse safe: whatever ExpandRegions joins, regionsOf must split back to the exact
+// TestResolveRegions_RoundTripsThroughProvision is the invariant that makes the
+// collapse safe: whatever ResolveRegions joins, regionsOf must split back to the exact
 // declared set by the time it reaches Modal's API. The two are inverses, and this
 // asserts it end to end through Provision rather than on the helpers alone — a
 // mismatch here would send Modal a region name it has never heard of (the joined
 // token), which is precisely the failure a unit test on either half would miss.
-func TestExpandRegions_RoundTripsThroughProvision(t *testing.T) {
+func TestResolveRegions_RoundTripsThroughProvision(t *testing.T) {
 	for _, declared := range [][]string{
 		nil,
 		{"us"},
@@ -1094,7 +1094,7 @@ func TestExpandRegions_RoundTripsThroughProvision(t *testing.T) {
 		f := &fakeClient{createID: "sb-1"}
 		p := newTestProvider(f)
 
-		candidates := p.ExpandRegions(declared, nil)
+		candidates := p.ResolveRegions(declared, nil)
 		// Placement's own fallback when expansion is empty: one unconstrained candidate.
 		if len(candidates) == 0 {
 			candidates = []string{""}
diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go
index aad2b6e..6320787 100644
--- a/pkg/provider/provider.go
+++ b/pkg/provider/provider.go
@@ -7,7 +7,7 @@
 // (the cheapest one on Modal). Zone is not modeled — AWS's CreateFleet already spreads
 // across a region's AZs and no NeoCloud exposes zones. Region vocabularies differ per
 // provider, and the pool speaks group tokens ("us") on top, so translation lives behind
-// ExpandRegions rather than in the control plane.
+// ResolveRegions rather than in the control plane.
 //
 // Design rules:
 //   - The Pod is the source of truth for the workload shape. Provision reads
@@ -114,7 +114,7 @@ type Provider interface {
 	// blocklist — an alternate running dry does not disable the primary.
 	MapAccelerator(canonical string, count int32) (providerAcceleratorIDs []string, ok bool)
 
-	// ExpandRegions resolves a pool's declared region constraint (ProviderSpec.Regions)
+	// ResolveRegions resolves a pool's declared region constraint (ProviderSpec.Regions)
 	// into the concrete regions placement may walk, in this provider's own vocabulary —
 	// only the provider knows its geography:
 	//
@@ -126,7 +126,7 @@ type Provider interface {
 	//
 	// An empty result means no candidate: placement skips this provider. A provider that
 	// can place without a region returns [""] for it — one candidate, unpinned.
-	ExpandRegions(declared, narrowTo []string) []string
+	ResolveRegions(declared, narrowTo []string) []string
 
 	// ClassifyProvisionError maps a Provision error to the granularity at which
 	// the failing placement should be blocklisted. This keeps failover precise:
@@ -213,7 +213,7 @@ type ProvisionRequest struct {
 	// nowhere to live on the Pod.
 	CapacityType nebulav1alpha1.CapacityType
 	// Region is the ONE candidate placement chose, exactly as this provider's own
-	// ExpandRegions minted it — already resolved (never a group token) and opaque to the
+	// ResolveRegions minted it — already resolved (never a group token) and opaque to the
 	// control plane. Usually one concrete region (AWS "us-east-1"), which is what lets a
 	// capacity failure blocklist just that region; a provider that cannot fail over
 	// (Modal) may encode several for its own scheduler, and only that adapter parses it.
diff --git a/pkg/vnode/handler_test.go b/pkg/vnode/handler_test.go
index cb7218a..87b3fe5 100644
--- a/pkg/vnode/handler_test.go
+++ b/pkg/vnode/handler_test.go
@@ -127,7 +127,7 @@ func (f *fakeProvider) List(context.Context) ([]provider.Instance, error) {
 }
 func (f *fakeProvider) Offerings(context.Context) ([]provider.Offering, error) { return nil, nil }
 func (f *fakeProvider) MapAccelerator(c string, _ int32) ([]string, bool)      { return []string{c}, true }
-func (f *fakeProvider) ExpandRegions(declared, _ []string) []string            { return declared }
+func (f *fakeProvider) ResolveRegions(declared, _ []string) []string           { return declared }
 func (f *fakeProvider) ClassifyProvisionError(_ error, accel, region string) provider.BlockScope {
 	f.classifyAccel = accel
 	f.classifyRegion = region

From 1570574c239eab19e75e2d59b5ff4edfa4b1c7c8 Mon Sep 17 00:00:00 2001
From: kerthcet <kerthcet@gmail.com>
Date: Thu, 24 Sep 2026 14:04:18 +0100
Subject: [PATCH 2/2] fix lint

Signed-off-by: kerthcet <kerthcet@gmail.com>
---
 pkg/provider/aws/aws.go | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/pkg/provider/aws/aws.go b/pkg/provider/aws/aws.go
index b7bfe2f..7838536 100644
--- a/pkg/provider/aws/aws.go
+++ b/pkg/provider/aws/aws.go
@@ -342,10 +342,11 @@ func expandDeclared(declared []string) []string {
 // sweepRegions returns the regions List and Offerings fan out across: the union of
 // 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).
+// 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