Skip to content

Commit 09bb28a

Browse files
committed
refactor(plan): migrate Acquirer.Acquire to lagotto/pkg/snipe.Snipe (lagotto#106)
Replace the hand-rolled retry/backoff/AZ-sweep/classify loop in Acquirer.Acquire with a thin wrapper over snipe.Snipe, now that lagotto v0.52.1 ships it with region-pinned client resolution (lagotto#111). Deletes ~180 lines of duplicated logic; SpawnLauncher becomes a pure LaunchConfig builder (Client/Timeout/Provision removed). Local retry-behavior tests are dropped in favor of lagotto/pkg/snipe's own 16-test suite. The Substrate-backed offline test tier for Acquirer is also lost, since Snipe builds its own client internally with no custom-endpoint injection point — filed lagotto#113 to restore it. Verified live against real AWS with Placements populated (calque's actual usage pattern); also found and filed lagotto#114 (AZ field bug), which doesn't affect calque since all call sites pin Placements.
1 parent 75acece commit 09bb28a

13 files changed

Lines changed: 218 additions & 653 deletions

File tree

‎cmd/calque/fleetrun.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,12 @@ func runShard(ctx context.Context, s3c *s3.Client, spawnClient *spawnaws.Client,
178178
ManifestKey: shardLayout.ManifestKey, WorkerDir: hostWorkerDir, Region: o.region,
179179
LogKey: shardLayout.LogKey, HostMode: false, ModelEnv: o.model,
180180
}
181-
launcher := &plan.SpawnLauncher{
182-
Client: spawnClient, RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
183-
Username: "ubuntu", Timeout: 5 * time.Minute, AMI: o.ami, PricePerHour: pricePerHr,
181+
launchCfg := plan.SpawnLauncher{
182+
RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
183+
Username: "ubuntu", AMI: o.ami, PricePerHour: pricePerHr,
184184
IMDSv2HopLimit: 2, RootVolumeGiB: 200,
185-
}
186-
acq := &plan.Acquirer{Launcher: launcher, Report: rep.rep, Deadline: o.deadline, Placements: places}
185+
}.Build()
186+
acq := &plan.Acquirer{LaunchConfig: launchCfg, Report: rep.rep, Deadline: o.deadline, Placements: places}
187187
tgt := &target.Target{Card: target.DefaultCard, Instance: o.instance}
188188
acquired, err := acq.Acquire(ctx, tgt, o.region)
189189
if err != nil {

‎cmd/calque/realrun.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,14 @@ func realRun(o realOpts) (err error) {
135135
if err != nil {
136136
return fmt.Errorf("spawn client: %w", err)
137137
}
138-
launcher := &plan.SpawnLauncher{
139-
Client: spawnClient, RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
140-
Username: "ubuntu", Timeout: 5 * time.Minute, AMI: o.ami, PricePerHour: pricePerHr,
138+
launchCfg := plan.SpawnLauncher{
139+
RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
140+
Username: "ubuntu", AMI: o.ami, PricePerHour: pricePerHr,
141141
IMDSv2HopLimit: 2, // warmd runs INSIDE docker; needs IMDS creds one hop away
142142
RootVolumeGiB: 200, // vLLM image + weights blow past spawn's 20 GiB default
143-
}
143+
}.Build()
144144
acq := &plan.Acquirer{
145-
Launcher: launcher, Report: rep, Deadline: o.deadline, Placements: places,
145+
LaunchConfig: launchCfg, Report: rep, Deadline: o.deadline, Placements: places,
146146
OnProgress: func(attempt int, code, detail string, waited time.Duration) {
147147
fmt.Printf(" ...swept %d attempt(s), no capacity (%s, %s)\n", attempt, code, waited.Round(time.Second))
148148
},

‎cmd/calque/session.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ func runSession(o sessionOpts) (err error) {
107107
}
108108
}
109109

110-
launcher := &plan.SpawnLauncher{
111-
Client: spawnClient, RunCmd: prep.PrepCommand(artifactPfx), TTL: o.ttl,
110+
launchCfg := plan.SpawnLauncher{
111+
RunCmd: prep.PrepCommand(artifactPfx), TTL: o.ttl,
112112
OnComplete: "", // do NOT terminate on command completion — we hold the box
113-
Username: "ubuntu", Timeout: 5 * time.Minute, AMI: o.ami, PricePerHour: pricePerHr,
113+
Username: "ubuntu", AMI: o.ami, PricePerHour: pricePerHr,
114114
IMDSv2HopLimit: 2, RootVolumeGiB: 200,
115115
Spot: o.spot, SpotMaxPrice: o.spotMaxPrice,
116-
}
116+
}.Build()
117117
if o.spot {
118118
// Honesty (§9/§10): a spot ramp measures K against a SPOT R_a, and the box
119119
// can be reclaimed mid-ramp. Say so loudly and leak it, so the resulting K
@@ -130,7 +130,7 @@ func runSession(o sessionOpts) (err error) {
130130
round := 0
131131
lastDetail := ""
132132
acq := &plan.Acquirer{
133-
Launcher: launcher, Report: rep, Deadline: o.acquireDeadline, Placements: places,
133+
LaunchConfig: launchCfg, Report: rep, Deadline: o.acquireDeadline, Placements: places,
134134
OnProgress: func(attempt int, code, detail string, waited time.Duration) {
135135
// Print the full AWS message on the first round, whenever it CHANGES, and
136136
// every 10th round — so a capacity opening (message names an AZ) or a

‎cmd/calque/smoke.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,12 @@ func smoke(o smokeOpts) (err error) {
121121
if err != nil {
122122
return fmt.Errorf("spawn client: %w", err)
123123
}
124-
launcher := &plan.SpawnLauncher{
125-
Client: spawnClient, RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
126-
Username: "ubuntu", Timeout: 5 * time.Minute, AMI: o.ami, PricePerHour: pricePerHr,
127-
}
124+
launchCfg := plan.SpawnLauncher{
125+
RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
126+
Username: "ubuntu", AMI: o.ami, PricePerHour: pricePerHr,
127+
}.Build()
128128
acq := &plan.Acquirer{
129-
Launcher: launcher, Report: rep, Deadline: o.deadline, Placements: places,
129+
LaunchConfig: launchCfg, Report: rep, Deadline: o.deadline, Placements: places,
130130
OnProgress: func(attempt int, code, detail string, waited time.Duration) {
131131
fmt.Printf(" ...swept %d attempt(s), still no capacity (%s, %s)\n", attempt, code, waited.Round(time.Second))
132132
},

‎cmd/calque/spawnrun.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,11 @@ func runSpawnShard(ctx context.Context, s3c *s3.Client, spawnClient *spawnaws.Cl
175175
Bucket: o.bucket, ArtifactPrefix: shardLayout.ArtifactPfx, ManifestKey: shardLayout.ManifestKey,
176176
WorkerDir: hostWorkerDir, Region: o.region, LogKey: shardLayout.LogKey, HostMode: true,
177177
}
178-
launcher := &plan.SpawnLauncher{
179-
Client: spawnClient, RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
180-
Username: "ubuntu", Timeout: 5 * time.Minute, AMI: o.ami,
181-
}
182-
acq := &plan.Acquirer{Launcher: launcher, Report: rep.rep, Deadline: o.deadline, Placements: places}
178+
launchCfg := plan.SpawnLauncher{
179+
RunCmd: boot.Command(), TTL: o.ttl, OnComplete: "terminate",
180+
Username: "ubuntu", AMI: o.ami,
181+
}.Build()
182+
acq := &plan.Acquirer{LaunchConfig: launchCfg, Report: rep.rep, Deadline: o.deadline, Placements: places}
183183
tgt := &target.Target{Card: target.DefaultCard, Instance: o.instance}
184184
acquired, err := acq.Acquire(ctx, tgt, o.region)
185185
if err != nil {

‎cmd/gpuprobe/main.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,14 @@ func probe(instanceType, region, ami, ttl string, deadline time.Duration, spot b
111111
}
112112
}
113113

114-
launcher := &plan.SpawnLauncher{
115-
Client: spawnClient, TTL: ttl, OnComplete: "terminate",
116-
Username: "ubuntu", Timeout: 5 * time.Minute, AMI: ami, Spot: spot,
114+
launchCfg := plan.SpawnLauncher{
115+
TTL: ttl, OnComplete: "terminate",
116+
Username: "ubuntu", AMI: ami, Spot: spot,
117117
// No RunCmd: boot plain, no docker/GPU job — this probe runs over SSM
118118
// after the instance is up, not via user-data.
119-
}
119+
}.Build()
120120
acq := &plan.Acquirer{
121-
Launcher: launcher, Report: rep, Deadline: deadline, Placements: places,
121+
LaunchConfig: launchCfg, Report: rep, Deadline: deadline, Placements: places,
122122
OnProgress: func(attempt int, code, detail string, waited time.Duration) {
123123
fmt.Printf(" ...swept %d attempt(s), no capacity (%s, %s)\n", attempt, code, waited.Round(time.Second))
124124
},

‎docs/substrate-offline-test-tier.md‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,45 @@
11
# Design note: Substrate as calque's offline AWS test tier (calque#114)
22

3-
**Status:** shipped, with one documented scope reduction from the original
4-
issue text. `github.com/scttfrdmn/substrate/emulator` (Scott's event-sourced
5-
AWS emulator) is now calque's middle test tier: hand-written fakes (today's
6-
`plan_test.go` `fakeResolver`/`scriptedLauncher` pattern, unchanged) at one
7-
end, real-`AWS_PROFILE=aws` spend at the other, and Substrate in between —
8-
real request/response wire round-trips, no billing.
3+
**Status:** shipped, then PARTIALLY REGRESSED by calque#106's later
4+
`Acquirer` → `lagotto/pkg/snipe.Snipe` migration — see "Regression" below.
5+
`github.com/scttfrdmn/substrate/emulator` (Scott's event-sourced AWS
6+
emulator) was calque's middle test tier: hand-written fakes at one end,
7+
real-`AWS_PROFILE=aws` spend at the other, Substrate in between — real
8+
request/response wire round-trips, no billing. That middle tier for
9+
`Acquirer` specifically no longer exists as of the `Snipe` migration; see
10+
below for why and what's tracked to restore it.
911

10-
## What shipped
12+
## Regression: the Acquirer↔Substrate tests were deleted (calque#106 migration)
1113

12-
`internal/plan/substrate_test.go`:
14+
`internal/plan/substrate_test.go` (originally `TestAcquireAgainstSubstrate`/
15+
`TestAcquireAgainstSubstrateInjectedFailure`, both described below in their
16+
original, now-historical form) was DELETED when `Acquirer.Acquire` was
17+
migrated to delegate to `lagotto/pkg/snipe.Snipe` (lagotto#106/#111,
18+
calque commit history — the same migration that deleted calque's own
19+
hand-rolled retry/backoff/AZ-sweep/classify loop and its 5 local tests,
20+
per `internal/plan/plan_test.go`'s own updated comments).
21+
22+
The reason: `Snipe` builds its own `*spawnaws.Client` internally via
23+
`spawnaws.NewClientWithRegion` (region-pinned as of lagotto#111), which
24+
loads AWS config through the default credential chain with NO way to
25+
point at a custom endpoint. The only constructor that supports a custom
26+
endpoint, `spawnaws.NewClientFromConfig`, is unreachable from `Snipe` —
27+
its internal `clientFor` is unexported and `Options`/`Target` have no
28+
field for injecting a client or a custom `aws.Config`. Filed upstream:
29+
[lagotto#113](https://github.com/spore-host/lagotto/issues/113).
30+
31+
Until that lands, `Acquirer`'s real request-building/response-parsing
32+
code has NO offline test tier at all — only real `AWS_PROFILE=aws` runs
33+
exercise it (as they already did for the retry/backoff logic before
34+
`Snipe`, and as `fleetrun.go`'s own historical untested-without-spend
35+
precedent already established for this exact code path). This is a real,
36+
acknowledged loss of coverage, accepted as the cost of deleting ~180 lines
37+
of hand-rolled retry logic in favor of a well-tested (16 tests) upstream
38+
leaf — re-evaluate once lagotto#113 lands.
39+
40+
## What shipped (historical — describes the NOW-DELETED tests, kept for context)
41+
42+
`internal/plan/substrate_test.go` (deleted, see above):
1343

1444
- **`TestAcquireAgainstSubstrate`** — points the REAL `plan.SpawnLauncher`
1545
(wrapping spawn's real `*spawnaws.Client`/`launcher.Provision`) at a
@@ -26,7 +56,7 @@ real request/response wire round-trips, no billing.
2656
is handled by the bounded-retry-then-fail-fast path — never an infinite
2757
loop on an unclassifiable error.
2858

29-
Both tests run with `PricePerHour` pinned on the launcher (skips spawn's own
59+
Both tests ran with `PricePerHour` pinned on the launcher (skips spawn's own
3060
live AWS Pricing API call — a real network dependency separate from EC2 that
3161
would otherwise sneak into an "offline" test) and a pinned `AMI` (skips AMI
3262
auto-detection's SSM round-trip, which Substrate would need separately

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ require (
1414
github.com/aws/smithy-go v1.27.3
1515
github.com/scttfrdmn/substrate v0.94.0
1616
github.com/spore-host/cohort v0.2.0
17-
github.com/spore-host/lagotto v0.52.0
17+
github.com/spore-host/lagotto v0.52.1
1818
github.com/spore-host/spawn v0.98.0
1919
github.com/spore-host/truffle v0.48.1
2020
)

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,8 @@ github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU=
405405
github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY=
406406
github.com/spore-host/cohort v0.2.0 h1:PGKkYawNzymE6jPebRqsZHK9X/m2UVYupujMnIdK8f4=
407407
github.com/spore-host/cohort v0.2.0/go.mod h1:sNMWDccvNp3Qso8ZVMcOvJTJ8GC/QY9LDUYQK1zYWLc=
408-
github.com/spore-host/lagotto v0.52.0 h1:UMl0ZnbFauPPhZDXRo903HrwUAnpUa96XpHOtx82Rzg=
409-
github.com/spore-host/lagotto v0.52.0/go.mod h1:ljK6zEq0cd+sxsIksJo4Y6E8eHji756exsIquNmNrao=
408+
github.com/spore-host/lagotto v0.52.1 h1:rqKN2VvNmGk+gxYfS9FeaqL4ROZ70vMWQkxiOiZnc2w=
409+
github.com/spore-host/lagotto v0.52.1/go.mod h1:ljK6zEq0cd+sxsIksJo4Y6E8eHji756exsIquNmNrao=
410410
github.com/spore-host/libs v0.43.3 h1:Pa/DC49S8uxkmhodyi7x2nnu19qZ2uqmwZEnb6ATi1k=
411411
github.com/spore-host/libs v0.43.3/go.mod h1:q9UOt1DiO8Zmz4t5EWbSQV+r9FQsUOYaRkCK6TE60G4=
412412
github.com/spore-host/spawn v0.98.0 h1:Yp05t9UtjJBLNdwWdcuB+6sVfb9VsYWATb4oPdTKuiQ=

0 commit comments

Comments
 (0)