diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e6f57318..835b7bfc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 0.41.3 - Unreleased +### Fixed + +- Stopped market-independent AWS Spot launch request errors from being retried as On-Demand while preserving fallback for Spot-recoverable capacity, quota, and unsupported-market failures. Thanks @vincentkoc. + ## 0.41.2 - 2026-08-10 ### Fixed diff --git a/docs/features/capacity-fallback.md b/docs/features/capacity-fallback.md index 2dcf70ea8..c5575f82e 100644 --- a/docs/features/capacity-fallback.md +++ b/docs/features/capacity-fallback.md @@ -103,11 +103,14 @@ is `on-demand-after-120s`) enables the On-Demand retry pass; set it to `none` (or leave it empty) to never fall back. The On-Demand pass runs after every Spot candidate in the class chain has been -tried and rejected — it reruns the same chain on On-Demand. AWS fallback fires -on provider rejection. Azure also treats a slow Spot VM provisioning operation -as a capacity miss after the configured `on-demand-after-*` duration, or after -the default 120 seconds when on-demand fallback is disabled with `spot-only` or -`none`. +tried and rejected. AWS retries only the candidates whose individual failures +are Spot-recoverable, including Spot capacity or quota errors and request errors +that explicitly identify Spot as unsupported. Market-independent request or +image errors, such as an invalid block-device mapping, are not retried on +On-Demand. Azure +also treats a slow Spot VM provisioning operation as a capacity miss after the +configured `on-demand-after-*` duration, or after the default 120 seconds when +on-demand fallback is disabled with `spot-only` or `none`. Slow Azure On-Demand creates are bounded too, so the coordinator can keep trying the class chain before the CLI lease wait expires. diff --git a/internal/cli/aws.go b/internal/cli/aws.go index e0604ee4b..b66f227a5 100644 --- a/internal/cli/aws.go +++ b/internal/cli/aws.go @@ -560,6 +560,7 @@ func (c *AWSClient) createServerWithFallbackInRegion(ctx context.Context, cfg Co } candidates := awsLaunchCandidates(cfg) useSpot := cfg.Capacity.Market != "on-demand" + var marketFallbackCandidates []string var errs []error for i, instanceType := range candidates { next := cfg @@ -573,6 +574,9 @@ func (c *AWSClient) createServerWithFallbackInRegion(ctx context.Context, cfg Co if !isRetryableAWSProvisioningError(err) { return Server{}, next, joinErrors(errs) } + if useSpot { + marketFallbackCandidates = appendAWSMarketFallbackCandidate(marketFallbackCandidates, instanceType, err) + } continue } server, err := c.createServer(ctx, next, publicKey, leaseID, slug, keep, imageID, securityGroupID, useSpot, control) @@ -583,9 +587,12 @@ func (c *AWSClient) createServerWithFallbackInRegion(ctx context.Context, cfg Co if !isRetryableAWSProvisioningError(err) { return Server{}, next, joinErrors(errs) } + if useSpot { + marketFallbackCandidates = appendAWSMarketFallbackCandidate(marketFallbackCandidates, instanceType, err) + } } - if useSpot && strings.HasPrefix(cfg.Capacity.Fallback, "on-demand") { - for _, instanceType := range candidates { + if len(marketFallbackCandidates) > 0 && strings.HasPrefix(cfg.Capacity.Fallback, "on-demand") { + for _, instanceType := range marketFallbackCandidates { next := cfg next.ServerType = instanceType if logf != nil { @@ -1429,6 +1436,7 @@ func awsTagsWithName(labels map[string]string, name string) []types.Tag { func isRetryableAWSProvisioningError(err error) bool { s := err.Error() return strings.Contains(s, "InsufficientInstanceCapacity") || + strings.Contains(s, "UnfulfillableCapacity") || strings.Contains(s, "MaxSpotInstanceCountExceeded") || strings.Contains(s, "VcpuLimitExceeded") || strings.Contains(s, "InvalidHostID.NotFound") || @@ -1443,6 +1451,31 @@ func isRetryableAWSProvisioningError(err error) bool { strings.Contains(s, "instance type"))) } +func isAWSMarketFallbackError(err error) bool { + s := err.Error() + if strings.Contains(s, "InsufficientInstanceCapacity") || + strings.Contains(s, "UnfulfillableCapacity") || + strings.Contains(s, "MaxSpotInstanceCountExceeded") || + strings.Contains(s, "VcpuLimitExceeded") { + return true + } + spotSpecific := strings.Contains(strings.ToLower(s), "spot") + return spotSpecific && (strings.Contains(s, "Unsupported") || + strings.Contains(s, "InvalidParameterValue") || + (strings.Contains(s, "InvalidParameterCombination") && + (strings.Contains(s, "Free Tier") || + strings.Contains(s, "eligible") || + strings.Contains(s, "InstanceType") || + strings.Contains(s, "instance type")))) +} + +func appendAWSMarketFallbackCandidate(candidates []string, instanceType string, err error) []string { + if !isAWSMarketFallbackError(err) { + return candidates + } + return append(candidates, instanceType) +} + func isRetryableAWSRegionProvisioningError(err error) bool { s := err.Error() return isRetryableAWSProvisioningError(err) || diff --git a/internal/cli/aws_test.go b/internal/cli/aws_test.go index 3cc718e02..6d6322754 100644 --- a/internal/cli/aws_test.go +++ b/internal/cli/aws_test.go @@ -104,6 +104,37 @@ func TestAWSFixedAttemptAttestationIsNonCircularAndSecretFree(t *testing.T) { } } +func TestAWSMarketFallbackError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {name: "spot capacity", err: errors.New("UnfulfillableCapacity: no Spot capacity"), want: true}, + {name: "spot quota", err: errors.New("MaxSpotInstanceCountExceeded: quota"), want: true}, + {name: "spot unsupported", err: errors.New("UnsupportedOperation: Spot is not supported"), want: true}, + {name: "parameter independent", err: errors.New("InvalidParameterValue: invalid subnet"), want: false}, + {name: "unsupported independent", err: errors.New("UnsupportedOperation: architecture is not supported"), want: false}, + {name: "image independent", err: errors.New("no AWS AMI found in eu-west-1"), want: false}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := isAWSMarketFallbackError(test.err); got != test.want { + t.Fatalf("isAWSMarketFallbackError(%q) = %v, want %v", test.err, got, test.want) + } + }) + } + if !isRetryableAWSProvisioningError(errors.New("UnfulfillableCapacity: no Spot capacity")) { + t.Fatal("UnfulfillableCapacity must remain eligible for type and market fallback") + } + var candidates []string + candidates = appendAWSMarketFallbackCandidate(candidates, "t3.small", errors.New("InvalidParameterValue: invalid subnet")) + candidates = appendAWSMarketFallbackCandidate(candidates, "t3.medium", errors.New("UnfulfillableCapacity: no Spot capacity")) + if len(candidates) != 1 || candidates[0] != "t3.medium" { + t.Fatalf("mixed market fallback candidates = %v, want [t3.medium]", candidates) + } +} + func TestAWSFixedPinnedAttemptBlocksBroadRegionFallback(t *testing.T) { control := &AWSFixedCreateControl{PinnedAttempt: &AWSLaunchAttempt{ClientToken: "pinned"}} err := errors.New("transport closed while waiting for capacity response") diff --git a/worker/src/aws.ts b/worker/src/aws.ts index 6f8e7fd40..715321825 100644 --- a/worker/src/aws.ts +++ b/worker/src/aws.ts @@ -947,6 +947,7 @@ export class EC2SpotClient { const attempts: ProvisioningAttempt[] = []; const quotaCache = new Map(); const imageCache = new Map(); + const marketFallbackCandidates: string[] = []; const pinnedMacOSImageID = config.target === "macos" ? config.awsAMI || this.env.CRABBOX_AWS_AMI || "" : ""; const resolveCandidateImageID = async (candidateConfig: LeaseConfig): Promise => { @@ -993,6 +994,9 @@ export class EC2SpotClient { if (preflight) { attempts.push(preflight); failures.push(`${serverType}: ${preflight.message}`); + if (config.capacityMarket === "spot") { + marketFallbackCandidates.push(serverType); + } continue; } try { @@ -1029,12 +1033,17 @@ export class EC2SpotClient { }); failures.push(`${serverType}: ${message}`); if (!isRetryableAWSProvisioningError(message)) { + marketFallbackCandidates.length = 0; break; } + if (config.capacityMarket === "spot" && isAWSMarketFallbackError(message)) { + marketFallbackCandidates.push(serverType); + } } } - if (config.capacityMarket === "spot" && config.capacityFallback.startsWith("on-demand")) { - for (const serverType of candidates) { + // Retry only candidates whose Spot failure can be recovered by On-Demand. + if (marketFallbackCandidates.length > 0 && config.capacityFallback.startsWith("on-demand")) { + for (const serverType of marketFallbackCandidates) { // oxlint-disable-next-line eslint/no-await-in-loop -- on-demand fallback must stay sequential. const preflight = await this.quotaPreflightAttempt(serverType, "on-demand", quotaCache); if (preflight) { @@ -3455,6 +3464,7 @@ export function awsProvisioningErrorCategory(message: string): string { } if ( message.includes("InsufficientInstanceCapacity") || + message.includes("UnfulfillableCapacity") || isAWSInsufficientCapacityOnHostError(message) ) { return "capacity"; @@ -3480,6 +3490,19 @@ export function awsProvisioningErrorCategory(message: string): string { return ""; } +export function isAWSMarketFallbackError(message: string): boolean { + if ( + message.includes("InsufficientInstanceCapacity") || + message.includes("UnfulfillableCapacity") || + message.includes("MaxSpotInstanceCountExceeded") || + message.includes("VcpuLimitExceeded") + ) { + return true; + } + const category = awsProvisioningErrorCategory(message); + return /\bspot\b/i.test(message) && (category === "unsupported" || category === "policy"); +} + function isOpaqueAWSHTTP400XMLOnlyError(message: string): boolean { const entries = message .split(";") diff --git a/worker/test/aws.test.ts b/worker/test/aws.test.ts index c992d493f..2d9955bb0 100644 --- a/worker/test/aws.test.ts +++ b/worker/test/aws.test.ts @@ -23,6 +23,7 @@ import { isAWSInstanceCleanedAfterReadinessFailure, isAWSInvalidHostIDError, isAWSInstanceNotFoundError, + isAWSMarketFallbackError, isRetryableAWSProvisioningError, staleCrabboxSSHIngressRules, } from "../src/aws"; @@ -1187,7 +1188,124 @@ describe("aws provider", () => { ), ).toBe(""); expect(awsProvisioningErrorCategory("InsufficientInstanceCapacity: nope")).toBe("capacity"); + expect(awsProvisioningErrorCategory("UnfulfillableCapacity: nope")).toBe("capacity"); expect(awsProvisioningErrorCategory("VcpuLimitExceeded: nope")).toBe("quota"); + expect(awsProvisioningErrorCategory("InvalidBlockDeviceMapping: nope")).toBe(""); + expect(isRetryableAWSProvisioningError("InvalidBlockDeviceMapping: nope")).toBe(false); + expect(isAWSMarketFallbackError("UnfulfillableCapacity: nope")).toBe(true); + expect(isAWSMarketFallbackError("InvalidParameterValue: nope")).toBe(false); + expect(isAWSMarketFallbackError("UnsupportedOperation: Spot is not supported")).toBe(true); + expect(isAWSMarketFallbackError("UnsupportedOperation: architecture is not supported")).toBe( + false, + ); + }); + + it("does not retry fatal spot launch requests as on-demand", async () => { + const { client, config, markets } = awsMarketFallbackHarness("InvalidBlockDeviceMapping"); + + await expect( + client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ), + ).rejects.toThrow("InvalidBlockDeviceMapping"); + expect(markets).toEqual(["spot"]); + }); + + it("does not retry opaque spot launch failures as on-demand", async () => { + const { client, config, markets } = awsMarketFallbackHarness("__opaque__"); + + await expect( + client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ), + ).rejects.toThrow("http 400"); + expect(markets).toEqual(["spot"]); + }); + + it("does not retry market-independent parameter errors as on-demand", async () => { + const { client, config, markets } = awsMarketFallbackHarness("InvalidParameterValue"); + + await expect( + client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ), + ).rejects.toThrow("InvalidParameterValue"); + expect(markets).toEqual(["spot"]); + }); + + it("falls back only the market-recoverable candidate from a mixed spot chain", async () => { + const { client, config, attempted } = awsMarketFallbackHarness( + ["InvalidParameterValue", "UnfulfillableCapacity"], + "spot", + ["t3.small", "t3.medium"], + ); + + const result = await client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ); + + expect(attempted).toEqual(["spot:t3.small", "spot:t3.medium", "on-demand:t3.medium"]); + expect(result.serverType).toBe("t3.medium"); + expect(result.market).toBe("on-demand"); + }); + + it("falls back from unfulfillable spot capacity to on-demand", async () => { + const { client, config, markets } = awsMarketFallbackHarness("UnfulfillableCapacity"); + + const result = await client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ); + + expect(markets).toEqual(["spot", "on-demand"]); + expect(result.market).toBe("on-demand"); + expect(result.attempts?.[0]).toMatchObject({ market: "spot", category: "capacity" }); + }); + + it("does not enter a second market pass for an on-demand request", async () => { + const { client, config, markets } = awsMarketFallbackHarness( + "UnfulfillableCapacity", + "on-demand", + ); + + await expect( + client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ), + ).rejects.toThrow("UnfulfillableCapacity"); + expect(markets).toEqual(["on-demand"]); + }); + + it("falls back from spot capacity failure to on-demand", async () => { + const { client, config, markets } = awsMarketFallbackHarness("InsufficientInstanceCapacity"); + + const result = await client.createServerWithFallback( + config, + "cbx_abcdef123456", + "violet-prawn", + "alice@example.com", + ); + + expect(markets).toEqual(["spot", "on-demand"]); + expect(result.market).toBe("on-demand"); + expect(result.attempts?.[0]).toMatchObject({ market: "spot", category: "capacity" }); }); it("classifies stale AWS instance ID errors", () => { @@ -2899,6 +3017,82 @@ function ec2XMLResponse(body: string, status = 200): Response { return new Response(body, { status, headers: { "content-type": "application/xml" } }); } +function awsMarketFallbackHarness( + failureCode: string | string[], + capacityMarket: "spot" | "on-demand" = "spot", + instanceTypes: string[] = ["t3.small"], +) { + const markets: string[] = []; + const attempted: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + if (new URL(request.url).hostname.startsWith("servicequotas.")) { + return new Response(JSON.stringify({ Quota: { Value: 999 } }), { + headers: { "content-type": "application/json" }, + }); + } + const params = new URLSearchParams(await request.clone().text()); + const action = params.get("Action") ?? ""; + const securityGroupResponse = ec2ConfiguredSecurityGroupResponse(action, params); + if (securityGroupResponse) return securityGroupResponse; + if (action === "DescribeKeyPairs") { + return ec2XMLResponse( + "test-keyssh-ed25519 test", + ); + } + if (action === "RunInstances") { + const market = params.has("InstanceMarketOptions.MarketType") ? "spot" : "on-demand"; + const instanceType = params.get("InstanceType") ?? ""; + markets.push(market); + attempted.push(`${market}:${instanceType}`); + const failureCodes = Array.isArray(failureCode) ? failureCode : [failureCode]; + const currentFailure = failureCodes[markets.length - 1]; + if (currentFailure) { + if (currentFailure === "__opaque__") { + return ec2XMLResponse('', 400); + } + return ec2XMLResponse( + `${currentFailure}launch failed`, + 400, + ); + } + return ec2XMLResponse( + "i-fallbackt3.small203.0.113.44pending", + ); + } + return ec2XMLResponse( + `Unexpected${action}`, + 500, + ); + }), + ); + return { + client: new EC2SpotClient( + { + AWS_ACCESS_KEY_ID: "test", + AWS_SECRET_ACCESS_KEY: "secret", + CRABBOX_AWS_SECURITY_GROUP_ID: "sg-123", + CRABBOX_AWS_SSH_CIDRS: "203.0.113.7/32", + CRABBOX_AWS_AMI: "ami-test", + } as never, + "eu-west-1", + ), + config: leaseConfig({ + provider: "aws", + serverType: instanceTypes[0] ?? "t3.small", + serverTypeExplicit: instanceTypes.length === 1, + awsInstanceTypes: instanceTypes, + capacity: { market: capacityMarket, fallback: "on-demand-after-120s" }, + providerKey: "test-key", + sshPublicKey: "ssh-ed25519 test", + }), + markets, + attempted, + }; +} + async function gunzipBase64(value: string): Promise { const bytes = Uint8Array.from(atob(value), (char) => char.charCodeAt(0)); const stream = new Blob([bytes]).stream().pipeThrough(new DecompressionStream("gzip"));