Skip to content
Draft
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
7 changes: 7 additions & 0 deletions internal/command/deploy/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,13 @@ func NewMachineDeployment(ctx context.Context, args MachineDeploymentArgs) (_ Ma
if appConfig.Deploy != nil && appConfig.Deploy.MaxUnavailable != nil {
maxUnavailable = *appConfig.Deploy.MaxUnavailable
}
// The --max-unavailable flag is validated in deploy.go, but a value read
// straight from fly.toml's [deploy] section bypasses that check. Guard it
// here so a max_unavailable of 0 yields a friendly error instead of an
// eventual "pool size must be > 0" panic. See #4262.
if maxUnavailable <= 0 {
return nil, fmt.Errorf("max_unavailable must be > 0 (got %v); set it to at least 1 or a fraction like 0.33", maxUnavailable)
}

maxConcurrent := max(args.MaxConcurrent, 1)

Expand Down
4 changes: 3 additions & 1 deletion internal/command/deploy/machines_deploymachinesapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,9 @@ func (md *machineDeployment) getPoolSize(totalMachines int) int {
case mu >= 1:
return int(mu)
default:
return int(math.Ceil(float64(totalMachines) * mu))
// Never return a pool size below 1: a 0 (or negative) value would
// panic in acquireLeases ("pool size must be > 0"). See #4262.
return max(1, int(math.Ceil(float64(totalMachines)*mu)))
}
}

Expand Down
24 changes: 24 additions & 0 deletions internal/command/deploy/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,30 @@ func TestCompareConfig(t *testing.T) {
assert.False(t, compareConfigs(ctx, config1, config2))
}

func TestGetPoolSize(t *testing.T) {
t.Parallel()

// Regression test for #4262: a max_unavailable of 0 (e.g. from fly.toml
// [deploy] max_unavailable = 0) must never produce a pool size of 0, which
// previously flowed into acquireLeases and panicked with
// "pool size must be > 0".
cases := []struct {
maxUnavailable float64
totalMachines int
want int
}{
{maxUnavailable: 0, totalMachines: 4, want: 1},
{maxUnavailable: 0.33, totalMachines: 4, want: 2},
{maxUnavailable: 1, totalMachines: 4, want: 1},
{maxUnavailable: 3, totalMachines: 4, want: 3},
}
for _, tc := range cases {
md := &machineDeployment{maxUnavailable: tc.maxUnavailable}
assert.GreaterOrEqual(t, md.getPoolSize(tc.totalMachines), 1)
assert.Equal(t, tc.want, md.getPoolSize(tc.totalMachines))
}
}

func TestAppState(t *testing.T) {
t.Parallel()

Expand Down
Loading