fix: error instead of panic on max_unavailable=0 in fly.toml (#4262)#4945
Draft
kylemclaren wants to merge 1 commit into
Draft
fix: error instead of panic on max_unavailable=0 in fly.toml (#4262)#4945kylemclaren wants to merge 1 commit into
kylemclaren wants to merge 1 commit into
Conversation
A [deploy] max_unavailable = 0 value read from fly.toml bypassed the
--max-unavailable flag's "> 0" validation, producing a pool size of 0
that reached acquireLeases and hit a hard panic ("pool size must be
> 0").
Validate the config-file value where it is read in NewMachineDeployment,
mirroring the flag check, so the user gets a clear error. As
defense-in-depth, clamp getPoolSize to a minimum of 1 so a 0/negative
value can never panic acquireLeases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4262.
fly deploypanicked withpool size must be > 0whenfly.tomlset[deploy] max_unavailable = 0. The config value was passed through unvalidated (only the--max-unavailableflag was checked), so the rolling planner computed a pool size of 0 and hitpanic("pool size must be > 0").This validates
max_unavailablecoming from both the flag andfly.toml, returning a clear error instead of crashing.Testing: added
TestGetPoolSize— it fails onmaster(the panic path) and passes with the fix.go test ./internal/command/deploy/...is green.✅ Live-verified against a real org (ephemeral app, cleaned up afterward):
[deploy] max_unavailable = 0, a secondfly deploypanics —panic: pool size must be > 0atinternal/command/deploy/plan.go:411(acquireLeases).Error: max_unavailable must be > 0 (got 0); set it to at least 1 or a fraction like 0.33.🤖 Generated with Claude Code