diff --git a/CHANGELOG.md b/CHANGELOG.md index d2f2403..9b14789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.1.2] - 2026-09-04 + +### Fixed + +- **Boot autotune no longer crash-loops small containers ([#133]).** When a + container's memory limit could not fit the selected PHP-FPM profile, the boot + calculator exited PID 1, so any image with a baked default profile (for example + `PHP_FPM_AUTOTUNE_PROFILE=medium`) crash-looped at boot below ~512MB. The + calculator now clamps to the largest worker count that fits (down to one worker), + warns naming the profile and the smallest limit that would run it, and boots; the + embedded runtime autotuner refines the number from there. Set + `global.autotune_strict: true` (or `PHP_FPM_AUTOTUNE_STRICT=1`) to keep the + fail-hard behavior. The pre-check minimum and the sizing now use one shared memory + model, so the reported minimum is a limit that actually boots. + +[#133]: https://github.com/cboxdk/init/issues/133 + ## [3.1.1] - 2026-09-04 ### Changed diff --git a/cmd/cbox-init/serve.go b/cmd/cbox-init/serve.go index e74b6dc..5163a0b 100644 --- a/cmd/cbox-init/serve.go +++ b/cmd/cbox-init/serve.go @@ -584,8 +584,13 @@ func runAutoTuning(profileName string, threshold float64, cfg *config.Config) er thresholdSource = "global config" } + // Fail-hard only when explicitly asked. By default the calculator clamps to + // what fits and boots, so a container too small for the profile does not + // crash-loop PID 1 — the runtime autotuner refines the number from there. + strict := cfg.Global.AutotuneStrict || truthyEnv("PHP_FPM_AUTOTUNE_STRICT") + // Create calculator - calc, err := autotune.NewCalculator(profile, finalThreshold, autotuneLog) + calc, err := autotune.NewCalculator(profile, finalThreshold, strict, autotuneLog) if err != nil { return fmt.Errorf("failed to create calculator: %w", err) } diff --git a/docs/configuration/php-fpm-autotune.md b/docs/configuration/php-fpm-autotune.md index fbbabed..feec285 100644 --- a/docs/configuration/php-fpm-autotune.md +++ b/docs/configuration/php-fpm-autotune.md @@ -250,14 +250,35 @@ Start Servers: 50% of max - **Prevents context switching**: Too many workers on limited CPUs degrade performance ### Validation Gates -- **Pre-calculation checks**: Minimum memory requirements per profile -- **Post-calculation validation**: PM relationships, memory limits -- **Warning system**: Logs adjustments (CPU limiting, profile minimums) +- **One shared memory model**: the reported minimum is the smallest limit that actually boots the profile, so the pre-check and the sizing never disagree +- **Post-calculation validation**: PM relationships (min_spare <= start_servers <= max_spare <= max_children) +- **Warning system**: Logs adjustments (CPU limiting, profile minimums, clamping) ### Profile Minimums -- Each profile enforces minimum worker count -- Prevents under-provisioning on small containers +- Each profile has a preferred minimum worker count - Dev: 2 workers, Light: 2, Medium: 4, Heavy: 8, Bursty: 4 +- The minimum is a preference, not a hard floor that can crash a small container (see below) + +### When the container is too small + +By default the calculator **never kills PID 1**. If the memory limit cannot fit the +profile's preferred worker count, it clamps to the largest count that fits (down to +one worker), logs a loud warning naming the profile and the smallest limit that +would run it, and boots. The container comes up degraded but working, and the +embedded runtime autotuner (`fpm_tune`) refines the number from there. + +This matters because images often bake a default profile (for example +`PHP_FPM_AUTOTUNE_PROFILE=medium`): a small sidecar should boot, not crash-loop. + +Set `global.autotune_strict: true` (or `PHP_FPM_AUTOTUNE_STRICT=1`) to keep the +fail-hard behavior instead: a profile that does not fit exits at startup with a +clear error naming the profile and the smallest limit that runs it. Use it when you +want a hard guarantee that the box is sized for the profile you chose. + +```yaml +global: + autotune_strict: false # default: clamp and boot. true: fail hard on a misfit. +``` ## Usage diff --git a/internal/autotune/calculator.go b/internal/autotune/calculator.go index 00e9ba5..a47e924 100644 --- a/internal/autotune/calculator.go +++ b/internal/autotune/calculator.go @@ -31,11 +31,17 @@ type Calculator struct { resources *ContainerResources profile ProfileConfig memoryThreshold float64 // Override for profile.MaxMemoryUsage (0.0 = use profile default) + strict bool // Fail hard when the profile does not fit, instead of clamping logger *slog.Logger } -// NewCalculator creates a new calculator with detected resources and profile -func NewCalculator(profile Profile, memoryThreshold float64, logger *slog.Logger) (*Calculator, error) { +// NewCalculator creates a new calculator with detected resources and profile. +// +// strict controls what happens when the container is too small for the profile: +// with strict false (the default) the calculator clamps to the largest worker +// count that fits and warns, so the container still boots and the runtime +// autotuner refines the number; with strict true a misfit is a hard error. +func NewCalculator(profile Profile, memoryThreshold float64, strict bool, logger *slog.Logger) (*Calculator, error) { profileConfig, err := profile.GetConfig() if err != nil { return nil, err @@ -50,6 +56,7 @@ func NewCalculator(profile Profile, memoryThreshold float64, logger *slog.Logger resources: resources, profile: profileConfig, memoryThreshold: memoryThreshold, + strict: strict, logger: logger, }, nil } @@ -80,13 +87,6 @@ func (c *Calculator) Calculate() (*PHPFPMConfig, error) { " Kubernetes: resources.limits.memory and cpu") } - // Validate minimum memory requirement - minRequired := c.profile.ReservedMemoryMB + c.profile.OPcacheMemoryMB + c.profile.AvgMemoryPerWorker - if c.resources.MemoryLimitMB < minRequired { - return nil, fmt.Errorf("insufficient memory: %dMB (minimum %dMB required: %dMB reserved + %dMB OPcache + %dMB per worker)", - c.resources.MemoryLimitMB, minRequired, c.profile.ReservedMemoryMB, c.profile.OPcacheMemoryMB, c.profile.AvgMemoryPerWorker) - } - // Determine memory threshold (allow override of profile default) threshold := c.profile.MaxMemoryUsage thresholdSource := "profile" @@ -109,58 +109,24 @@ func (c *Calculator) Calculate() (*PHPFPMConfig, error) { } } - // Calculate available memory for PHP-FPM workers - // Formula: (Total × Threshold%) - Reserved - OPcache (shared) = Worker Memory Pool - availableMemory := int(float64(c.resources.MemoryLimitMB) * threshold) - totalReserved := c.profile.ReservedMemoryMB + c.profile.OPcacheMemoryMB - workerMemory := availableMemory - totalReserved + // Size the pool through one shared memory model. It clamps to the largest + // worker count that fits (with a warning) rather than refusing, so a container + // too small for the profile still boots a degraded-but-working pool that the + // runtime autotuner then refines. In strict mode a misfit is a hard error. + sized, err := sizeWorkers(c.resources.MemoryLimitMB, c.resources.CPULimit, threshold, c.profile, c.strict) + if err != nil { + return nil, err + } + maxChildren := sized.maxChildren + cfg.Warnings = append(cfg.Warnings, sized.warnings...) c.logger.Debug("Memory calculation", "threshold", fmt.Sprintf("%.1f%%", threshold*100), "threshold_source", thresholdSource, "total_memory", c.resources.MemoryLimitMB, - "available_after_threshold", availableMemory, - "reserved", totalReserved, - "worker_pool", workerMemory, + "max_children", maxChildren, ) - if workerMemory < c.profile.AvgMemoryPerWorker { - return nil, fmt.Errorf("insufficient memory for workers: %dMB available after reserving %dMB (system: %dMB + OPcache: %dMB), need at least %dMB per worker", - workerMemory, totalReserved, c.profile.ReservedMemoryMB, c.profile.OPcacheMemoryMB, c.profile.AvgMemoryPerWorker) - } - - // Calculate max_children based on available memory - maxChildren := workerMemory / c.profile.AvgMemoryPerWorker - - // Apply CPU-based limit: max 4 workers per CPU core (industry standard) - cpuBasedMax := c.resources.CPULimit * 4 - if maxChildren > cpuBasedMax { - cfg.Warnings = append(cfg.Warnings, - fmt.Sprintf("Memory allows %d workers, but limiting to %d based on %d CPUs (max 4 per core)", - maxChildren, cpuBasedMax, c.resources.CPULimit)) - maxChildren = cpuBasedMax - } - - // Apply profile-specific max if set - if c.profile.MaxWorkers > 0 && maxChildren > c.profile.MaxWorkers { - cfg.Warnings = append(cfg.Warnings, - fmt.Sprintf("Calculated %d workers, but profile limits to %d", maxChildren, c.profile.MaxWorkers)) - maxChildren = c.profile.MaxWorkers - } - - // Enforce profile minimum - if maxChildren < c.profile.MinWorkers { - maxChildren = c.profile.MinWorkers - cfg.Warnings = append(cfg.Warnings, - fmt.Sprintf("Increasing to profile minimum: %d workers", maxChildren)) - } - - // Absolute minimum safety check - if maxChildren < 1 { - maxChildren = 1 - cfg.Warnings = append(cfg.Warnings, "Using absolute minimum: 1 worker") - } - cfg.MaxChildren = maxChildren cfg.ProcessManager = c.profile.ProcessManagerType cfg.MaxRequests = c.profile.MaxRequestsPerChild @@ -200,16 +166,11 @@ func (c *Calculator) Calculate() (*PHPFPMConfig, error) { return cfg, nil } -// validateConfig ensures the calculated configuration is safe and valid +// validateConfig ensures the calculated PM relationships are valid. The memory +// fit is owned by sizeWorkers, which clamps to what fits (or errors in strict +// mode), so this no longer refuses on memory: a deliberately over-committed +// one-worker pool on a tiny container has already warned and must still boot. func (c *Calculator) validateConfig(cfg *PHPFPMConfig) error { - // Validate memory won't exceed container limit - // Total = Workers + OPcache (shared) + Reserved (Nginx/system) - totalMemory := cfg.MemoryAllocated + cfg.MemoryOPcache + cfg.MemoryReserved - if totalMemory > cfg.MemoryTotal { - return fmt.Errorf("configuration would use %dMB (workers: %dMB + OPcache: %dMB + reserved: %dMB) but only %dMB available", - totalMemory, cfg.MemoryAllocated, cfg.MemoryOPcache, cfg.MemoryReserved, cfg.MemoryTotal) - } - // Validate PM settings if cfg.ProcessManager == "dynamic" { if cfg.MinSpare > cfg.MaxChildren { @@ -253,6 +214,90 @@ func (c *Calculator) logCalculation(cfg *PHPFPMConfig) { } } +// workerSizing is the result of the shared memory model. +type workerSizing struct { + maxChildren int + warnings []string +} + +// sizeWorkers is the single memory model the calculator uses. It returns the +// largest safe pm.max_children for the limit rather than refusing: the profile's +// minimum is a preference, not a hard floor that may OOM, so when the container +// cannot fit it the count is clamped to what fits and a warning explains it. Only +// strict mode turns a misfit into an error, naming the profile and the smallest +// limit that runs it. +func sizeWorkers(limitMB, cpus int, threshold float64, p ProfileConfig, strict bool) (workerSizing, error) { + overhead := p.ReservedMemoryMB + p.OPcacheMemoryMB + perWorker := p.AvgMemoryPerWorker + var warnings []string + + // Desired count from the soft (threshold) budget, capped by CPU and profile. + desired := 0 + if softPool := int(float64(limitMB)*threshold) - overhead; softPool > 0 { + desired = softPool / perWorker + } + if cpuCap := cpus * 4; cpuCap > 0 && desired > cpuCap { + warnings = append(warnings, + fmt.Sprintf("memory allows %d workers, limiting to %d for %d CPUs (max 4 per core)", desired, cpuCap, cpus)) + desired = cpuCap + } + if p.MaxWorkers > 0 && desired > p.MaxWorkers { + warnings = append(warnings, + fmt.Sprintf("calculated %d workers, but the profile limits to %d", desired, p.MaxWorkers)) + desired = p.MaxWorkers + } + if desired < p.MinWorkers { + warnings = append(warnings, fmt.Sprintf( + "raising to the profile minimum of %d workers (%q; memory and CPU alone suggested %d)", + p.MinWorkers, p.Name, desired)) + desired = p.MinWorkers + } + + // Hard ceiling: workers plus overhead must fit the real limit, not just the + // thresholded budget, or the pool would risk the OOM killer. + hardMax := 0 + if hardPool := limitMB - overhead; hardPool > 0 { + hardMax = hardPool / perWorker + } + + if desired <= hardMax { + return workerSizing{maxChildren: max(1, desired), warnings: warnings}, nil + } + + // The profile's floor does not fit this container. + minLimit := smallestBootableLimit(p) + if strict { + return workerSizing{}, fmt.Errorf( + "profile %q needs at least %dMB for its %d-worker minimum, but the limit is %dMB; "+ + "raise the limit, choose a smaller profile, or set global.autotune_strict: false to clamp and boot", + p.Name, minLimit, p.MinWorkers, limitMB) + } + if hardMax >= 1 { + warnings = append(warnings, fmt.Sprintf( + "profile %q wants %d workers (needs %dMB) but the %dMB limit fits %d; clamped to %d, the runtime autotuner will refine it", + p.Name, p.MinWorkers, minLimit, limitMB, hardMax, hardMax)) + return workerSizing{maxChildren: hardMax, warnings: warnings}, nil + } + // Even one worker plus overhead exceeds the limit: boot over-committed with a + // single worker rather than not at all, and say so loudly. + warnings = append(warnings, fmt.Sprintf( + "the %dMB limit is below what profile %q assumes (%dMB per worker plus %dMB overhead); "+ + "booting one worker over-committed, the runtime autotuner will refine it", + limitMB, p.Name, perWorker, overhead)) + return workerSizing{maxChildren: 1, warnings: warnings}, nil +} + +// smallestBootableLimit is the smallest memory limit at which the profile's +// intended minimum worker count fits. The clamp in sizeWorkers triggers on the +// hard limit (workers plus overhead must fit the real memory, not just the +// thresholded budget), so the honest minimum is that same hard floor: the +// threshold shapes the count above the floor, not whether the floor fits. This is +// the number a strict-mode error and a clamp warning report, computed from the +// same model as the sizing so the two can never disagree. +func smallestBootableLimit(p ProfileConfig) int { + return p.MinWorkers*p.AvgMemoryPerWorker + p.ReservedMemoryMB + p.OPcacheMemoryMB +} + // ToEnvVars converts the configuration to environment variables for PHP-FPM func (cfg *PHPFPMConfig) ToEnvVars() map[string]string { env := map[string]string{ diff --git a/internal/autotune/calculator_clamp_test.go b/internal/autotune/calculator_clamp_test.go new file mode 100644 index 0000000..340c9c6 --- /dev/null +++ b/internal/autotune/calculator_clamp_test.go @@ -0,0 +1,108 @@ +package autotune + +import ( + "fmt" + "strings" + "testing" +) + +// mockCalculatorStrict is mockCalculator with strict mode on. +func mockCalculatorStrict(profile Profile, memoryMB, cpus int) *Calculator { + c := mockCalculator(profile, memoryMB, cpus) + c.strict = true + + return c +} + +// TestClampBootsSmallContainers is the regression for issue #133: a container too +// small for its profile must BOOT (clamped, with a warning) rather than exit +// PID 1. The limits are from the issue's table, where cbox-init 3.0.0 and 3.1.1 +// crash-looped. +func TestClampBootsSmallContainers(t *testing.T) { + cases := []struct { + profile Profile + memoryMB int + cpus int + }{ + {ProfileDev, 192, 1}, + {ProfileDev, 256, 1}, + {ProfileLight, 256, 1}, + {ProfileLight, 320, 1}, + {ProfileMedium, 384, 1}, + {ProfileMedium, 448, 1}, + {ProfileHeavy, 1024, 2}, + } + + for _, tc := range cases { + t.Run(fmt.Sprintf("%s/%dMB", tc.profile, tc.memoryMB), func(t *testing.T) { + cfg, err := mockCalculator(tc.profile, tc.memoryMB, tc.cpus).Calculate() + if err != nil { + t.Fatalf("non-strict must clamp and boot, got error: %v", err) + } + if cfg.MaxChildren < 1 { + t.Errorf("expected at least 1 worker, got %d", cfg.MaxChildren) + } + if len(cfg.Warnings) == 0 { + t.Errorf("expected a clamp/over-commit warning, got none") + } + }) + } +} + +// TestStrictModeRefusesAMisfit: with strict on, a container too small for the +// profile is a hard error whose message names the profile and the smallest limit +// that boots it (issue #133, AC2 and AC4). +func TestStrictModeRefusesAMisfit(t *testing.T) { + _, err := mockCalculatorStrict(ProfileHeavy, 256, 1).Calculate() + if err == nil { + t.Fatal("strict mode should refuse a container too small for the profile") + } + + pc, _ := ProfileHeavy.GetConfig() + if !strings.Contains(err.Error(), pc.Name) { + t.Errorf("error should name the profile %q: %v", pc.Name, err) + } + minLimit := smallestBootableLimit(pc) + if !strings.Contains(err.Error(), fmt.Sprintf("%dMB", minLimit)) { + t.Errorf("error should name the smallest bootable limit %dMB: %v", minLimit, err) + } +} + +// TestStrictModeBootsWhenItFits: strict is not a blanket refusal — a container +// that fits the profile sizes normally. +func TestStrictModeBootsWhenItFits(t *testing.T) { + cfg, err := mockCalculatorStrict(ProfileMedium, 2048, 4).Calculate() + if err != nil { + t.Fatalf("strict mode should size a fitting container without error: %v", err) + } + if cfg.MaxChildren < 1 { + t.Errorf("expected at least 1 worker, got %d", cfg.MaxChildren) + } +} + +// TestSmallestBootableLimitIsConsistent: the reported minimum is exactly the +// boundary the sizer uses — at it the profile's minimum fits, one MB below it the +// sizer clamps. This is what keeps the pre-check number and the sizing from +// disagreeing (issue #133, point 2). +func TestSmallestBootableLimitIsConsistent(t *testing.T) { + for profile := range Profiles { + pc, _ := profile.GetConfig() + minLimit := smallestBootableLimit(pc) + + // A generous CPU budget so CPU never caps below the memory floor. + at, err := sizeWorkers(minLimit, 64, pc.MaxMemoryUsage, pc, false) + if err != nil { + t.Fatalf("%s: sizeWorkers at the reported minimum errored: %v", pc.Name, err) + } + if at.maxChildren < pc.MinWorkers { + t.Errorf("%s: minimum %dMB yields %d workers, below the profile minimum %d", + pc.Name, minLimit, at.maxChildren, pc.MinWorkers) + } + + below, _ := sizeWorkers(minLimit-1, 64, pc.MaxMemoryUsage, pc, false) + if below.maxChildren >= pc.MinWorkers { + t.Errorf("%s: %dMB (one below the reported minimum) already meets the floor %d; the minimum is overstated", + pc.Name, minLimit-1, pc.MinWorkers) + } + } +} diff --git a/internal/autotune/calculator_comprehensive_test.go b/internal/autotune/calculator_comprehensive_test.go index f70f78e..3e22983 100644 --- a/internal/autotune/calculator_comprehensive_test.go +++ b/internal/autotune/calculator_comprehensive_test.go @@ -128,7 +128,7 @@ func TestCalculator_AllWarningScenarios(t *testing.T) { logger: slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})), } }, - expectWarn: "minimum", + expectWarn: "clamped", }, } diff --git a/internal/autotune/calculator_edge_test.go b/internal/autotune/calculator_edge_test.go index 8ed77b8..948890b 100644 --- a/internal/autotune/calculator_edge_test.go +++ b/internal/autotune/calculator_edge_test.go @@ -23,7 +23,7 @@ func TestNewCalculator_ValidProfile(t *testing.T) { for _, profile := range profiles { t.Run(string(profile), func(t *testing.T) { - calc, err := NewCalculator(profile, 0.75, logger) + calc, err := NewCalculator(profile, 0.75, false, logger) // We expect success or only resource detection errors (which are valid) if err != nil { diff --git a/internal/autotune/calculator_error_test.go b/internal/autotune/calculator_error_test.go index 01e03ff..fdb3160 100644 --- a/internal/autotune/calculator_error_test.go +++ b/internal/autotune/calculator_error_test.go @@ -13,7 +13,7 @@ func TestNewCalculator_InvalidProfileValue(t *testing.T) { // Use invalid profile ("invalid" is not a valid Profile value) invalidProfile := Profile("invalid-profile-name") - calc, err := NewCalculator(invalidProfile, 0, logger) + calc, err := NewCalculator(invalidProfile, 0, false, logger) if err == nil { t.Error("Expected error for invalid profile, got nil") @@ -32,7 +32,7 @@ func TestNewCalculator_InvalidProfileValue(t *testing.T) { func TestNewCalculator_ZeroThreshold(t *testing.T) { logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) - calc, err := NewCalculator(ProfileMedium, 0, logger) + calc, err := NewCalculator(ProfileMedium, 0, false, logger) if err != nil { t.Fatalf("NewCalculator with zero threshold failed: %v", err) } @@ -52,7 +52,7 @@ func TestNewCalculator_CustomThreshold(t *testing.T) { logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) customThreshold := 0.75 - calc, err := NewCalculator(ProfileHeavy, customThreshold, logger) + calc, err := NewCalculator(ProfileHeavy, customThreshold, false, logger) if err != nil { t.Fatalf("NewCalculator with custom threshold failed: %v", err) } diff --git a/internal/autotune/calculator_extended_test.go b/internal/autotune/calculator_extended_test.go index b8e336a..69c1578 100644 --- a/internal/autotune/calculator_extended_test.go +++ b/internal/autotune/calculator_extended_test.go @@ -36,7 +36,7 @@ func TestNewCalculator_Success(t *testing.T) { logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) // Test with invalid profile - _, err := NewCalculator(Profile("invalid"), 0.0, logger) + _, err := NewCalculator(Profile("invalid"), 0.0, false, logger) if err == nil { t.Error("Expected error for invalid profile, got none") } @@ -60,7 +60,7 @@ func TestNewCalculator_InvalidProfile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := NewCalculator(tt.profile, 0.0, logger) + _, err := NewCalculator(tt.profile, 0.0, false, logger) if err == nil { t.Errorf("Expected error for profile %q, got none", tt.profile) } @@ -74,7 +74,7 @@ func TestNewCalculator_WithMemoryThreshold(t *testing.T) { // Note: This will attempt to detect real cgroup resources // In a containerized environment, this should succeed // In a non-containerized environment, it should fall back to host resources - calc, err := NewCalculator(ProfileMedium, 0.8, logger) + calc, err := NewCalculator(ProfileMedium, 0.8, false, logger) // We don't expect an error from resource detection failure // (it falls back to host resources) @@ -96,25 +96,24 @@ func TestNewCalculator_WithMemoryThreshold(t *testing.T) { } } -// TestCalculator_validateConfig tests the validateConfig function with edge cases -func TestCalculator_validateConfig_MemoryOverflow(t *testing.T) { +// TestCalculator_validateConfig_IgnoresMemory: validateConfig no longer polices +// memory. Sizing owns that — sizeWorkers clamps to what fits, and only strict +// mode turns a misfit into an error — so an over-committed config reaching this +// function is not its concern. Its job is the PM relationships. +func TestCalculator_validateConfig_IgnoresMemory(t *testing.T) { calc := mockCalculator(ProfileMedium, 1024, 2) cfg := &PHPFPMConfig{ ProcessManager: "dynamic", - MaxChildren: 100, // Way too many workers + MaxChildren: 100, // Would over-commit, but that is sizing's job, not this one's MemoryAllocated: 900, MemoryOPcache: 128, MemoryReserved: 100, MemoryTotal: 1024, } - err := calc.validateConfig(cfg) - if err == nil { - t.Error("Expected error for memory overflow, got none") - } - if !strings.Contains(err.Error(), "would use") && !strings.Contains(err.Error(), "available") { - t.Errorf("Expected memory overflow error, got: %v", err) + if err := calc.validateConfig(cfg); err != nil { + t.Errorf("validateConfig should not refuse on memory (sizing owns that), got: %v", err) } } diff --git a/internal/autotune/calculator_test.go b/internal/autotune/calculator_test.go index 37beb7a..6aa42a4 100644 --- a/internal/autotune/calculator_test.go +++ b/internal/autotune/calculator_test.go @@ -152,12 +152,21 @@ func TestCalculator_CPULimit(t *testing.T) { } func TestCalculator_InsufficientMemory(t *testing.T) { - // 64MB RAM - insufficient for any profile + // 64MB is far too small for any profile. The default (non-strict) calculator + // must not kill PID 1: it clamps to one worker and warns loudly, and the + // runtime autotuner refines it from there. Strict mode is where a misfit is a + // hard error (see TestCalculator_StrictModeRefusesAMisfit). calc := mockCalculator(ProfileMedium, 64, 2) - _, err := calc.Calculate() + cfg, err := calc.Calculate() - if err == nil { - t.Error("Expected error for insufficient memory, got none") + if err != nil { + t.Fatalf("non-strict Calculate() should clamp and boot, not error: %v", err) + } + if cfg.MaxChildren < 1 { + t.Errorf("expected at least 1 worker after clamping, got %d", cfg.MaxChildren) + } + if len(cfg.Warnings) == 0 { + t.Error("expected a loud warning that the container is too small for the profile") } } @@ -421,8 +430,10 @@ func TestCalculator_SafetyValidations(t *testing.T) { {"Sufficient resources", ProfileMedium, 2048, 4, false}, {"Minimal resources", ProfileLight, 768, 1, false}, // 768*0.7=537, 537-224=313, 313/64=4 workers {"Dev tiny", ProfileDev, 384, 1, false}, // 384*0.5=192, 192-128=64, 64/48=1 worker (min 2 enforced) - {"Insufficient memory", ProfileMedium, 100, 4, true}, - {"Too small", ProfileHeavy, 256, 1, true}, + // Containers too small for the profile are covered by the clamp/strict tests + // (calculator_clamp_test.go): non-strict clamps and boots rather than erroring, + // and the pathological over-commit case intentionally exceeds the limit, so it + // does not belong in this no-overprovisioning invariant table. } for _, tt := range tests { diff --git a/internal/config/types.go b/internal/config/types.go index eaec919..0b11726 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -27,6 +27,7 @@ type GlobalConfig struct { RestartBackoffMax time.Duration `yaml:"restart_backoff_max" json:"restart_backoff_max"` // max duration RestartStabilityWindow time.Duration `yaml:"restart_stability_window" json:"restart_stability_window"` // uptime after which the restart budget resets (default 60s; negative disables) AutotuneMemoryThreshold float64 `yaml:"autotune_memory_threshold" json:"autotune_memory_threshold"` // 0.0-2.0, overrides profile MaxMemoryUsage + AutotuneStrict bool `yaml:"autotune_strict" json:"autotune_strict"` // Fail boot (exit PID 1) when the profile does not fit; default false clamps and boots LogFormat string `yaml:"log_format" json:"log_format"` // json | text LogLevel string `yaml:"log_level" json:"log_level"` // debug | info | warn | error LogTimestamps bool `yaml:"log_timestamps" json:"log_timestamps"` //