Skip to content

Commit 8a7ebff

Browse files
committed
feat(featureflags): read int flag fallbacks from the environment
A deployment with no LAUNCH_DARKLY_API_KEY serves every flag's compiled-in fallback, so an int flag resolves to a value only a rebuild can change. That is fine for flags whose fallback is a safe default, but not for the per-node limits: max-sandboxes-per-node falls back to 200, which is sized for cloud node types rather than for the host it runs on. A self-hosted node with 384 CPUs and 1.5 TB of memory refuses the 201st sandbox with ResourceExhausted, and the only way past it is editing a literal in this file. Add envIntOr, the int twin of envBoolOr, and use it for the two per-node limits: max-sandboxes-per-node (MAX_SANDBOXES_PER_NODE) and max-starting-instances-per-node (MAX_STARTING_INSTANCES_PER_NODE). The override applies to the fallback rather than to the evaluated value, so both modes stay consistent: without a LaunchDarkly key it is the value the offline store serves, and with a key set it is the default for flags that the LaunchDarkly environment does not define. A defined LaunchDarkly flag still wins, so live configuration is not bypassed. Unset, empty, unparseable and non-positive values all keep the existing fallback. Rejecting non-positive values means a typo cannot silently stop a node from accepting sandboxes, and it matches the "Must be > 0" requirement already documented on MaxStartingInstancesPerNode. Only these two flags opt in; other int flags are unchanged. Other flag types can follow the same shape if there is demand. Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
1 parent 9219790 commit 8a7ebff

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

‎packages/shared/pkg/featureflags/flags.go‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,25 @@ func (f IntFlag) Fallback() int {
506506
return f.fallback
507507
}
508508

509+
// envIntOr reads key as an int, falling back when it is unset, unparseable, or
510+
// not positive. It exists for the same reason as envBoolOr: on a cluster with no
511+
// LaunchDarkly an int flag resolves to a value only a rebuild can change, and
512+
// max-sandboxes-per-node's fallback is sized for cloud node types rather than
513+
// for the host it runs on. A non-positive value keeps the fallback so a typo
514+
// cannot silently stop a node from accepting sandboxes.
515+
func envIntOr(key string, fallback int) int {
516+
raw := env.GetEnv(key, "")
517+
if raw == "" {
518+
return fallback
519+
}
520+
parsed, err := strconv.Atoi(raw)
521+
if err != nil || parsed <= 0 {
522+
return fallback
523+
}
524+
525+
return parsed
526+
}
527+
509528
func NewIntFlag(name string, fallback int) IntFlag {
510529
flag := IntFlag{name: name, fallback: fallback}
511530
builder := launchDarklyOfflineStore.Flag(flag.name).ValueForAll(ldvalue.Int(fallback))
@@ -521,7 +540,7 @@ var (
521540
// future retirements.
522541
EgressRetirementTimeoutMsFlag = NewIntFlag("egress-retirement-timeout-ms", 60000)
523542

524-
MaxSandboxesPerNode = NewIntFlag("max-sandboxes-per-node", 200)
543+
MaxSandboxesPerNode = NewIntFlag("max-sandboxes-per-node", envIntOr("MAX_SANDBOXES_PER_NODE", 200))
525544
// The LD keys keep the legacy "gcloud-" prefix, but the limits apply to uploads on all storage providers.
526545
StorageConcurrentUploadLimit = NewIntFlag("gcloud-concurrent-upload-limit", 8)
527546
StorageMaxUploadTasks = NewIntFlag("gcloud-max-tasks", 16)
@@ -681,7 +700,7 @@ var (
681700

682701
// MaxStartingInstancesPerNode limits concurrent sandbox start/resume operations on a single orchestrator node.
683702
// Must be > 0.
684-
MaxStartingInstancesPerNode = NewIntFlag("max-starting-instances-per-node", 3)
703+
MaxStartingInstancesPerNode = NewIntFlag("max-starting-instances-per-node", envIntOr("MAX_STARTING_INSTANCES_PER_NODE", 3))
685704

686705
// MaxConcurrentEvictions caps the number of sandbox evictions that can run
687706
// in parallel per API instance. Excess items remain expired in the store
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package featureflags
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
//nolint:paralleltest,tparallel // t.Setenv
10+
func TestEnvIntOr(t *testing.T) {
11+
const key = "MAX_SANDBOXES_PER_NODE_TEST_ONLY"
12+
13+
for _, tc := range []struct {
14+
name string
15+
set bool
16+
value string
17+
fallback int
18+
want int
19+
}{
20+
{name: "unset keeps the fallback", fallback: 200, want: 200},
21+
{name: "empty keeps the fallback", set: true, value: "", fallback: 200, want: 200},
22+
{name: "a value overrides the fallback", set: true, value: "1200", fallback: 200, want: 1200},
23+
{name: "garbage keeps the fallback", set: true, value: "many", fallback: 200, want: 200},
24+
{name: "zero keeps the fallback", set: true, value: "0", fallback: 200, want: 200},
25+
{name: "negative keeps the fallback", set: true, value: "-1", fallback: 200, want: 200},
26+
} {
27+
t.Run(tc.name, func(t *testing.T) {
28+
if tc.set {
29+
t.Setenv(key, tc.value)
30+
}
31+
32+
require.Equal(t, tc.want, envIntOr(key, tc.fallback))
33+
})
34+
}
35+
}

0 commit comments

Comments
 (0)