From 93a68fa5bb99f38cafc4b03bd9f024032849044f Mon Sep 17 00:00:00 2001 From: Sylvester Damgaard Date: Fri, 4 Sep 2026 21:52:14 +0200 Subject: [PATCH] fix(autotune): clamp instead of killing PID 1 when a profile does not fit (#133) The boot PHP-FPM calculator exited PID 1 when the container's memory limit could not fit the selected profile, so any image with a baked default profile (for example PHP_FPM_AUTOTUNE_PROFILE=medium) crash-looped at boot below ~512MB. Since 3.1 the boot value is only the seed for the embedded runtime autotuner, so refusing to start is the wrong failure mode. The calculator now sizes through one shared memory model that clamps to the largest worker count that fits (down to one worker, over-committed with a loud warning on a pathologically small box) and boots, rather than erroring. A new global.autotune_strict (env PHP_FPM_AUTOTUNE_STRICT) keeps the fail-hard behaviour for operators who want a hard guarantee; its error names the profile and the smallest limit that runs it. The pre-check minimum and the sizing now share smallestBootableLimit, so the reported minimum is a limit that actually boots (the two used to disagree, which made the refusal message confusing). Verified end to end: a --memory=256m medium container boots (clamped) instead of exiting 1, and fails clearly under strict mode. Closes #133 --- CHANGELOG.md | 17 ++ cmd/cbox-init/serve.go | 7 +- docs/configuration/php-fpm-autotune.md | 31 +++- internal/autotune/calculator.go | 171 +++++++++++------- internal/autotune/calculator_clamp_test.go | 108 +++++++++++ .../autotune/calculator_comprehensive_test.go | 2 +- internal/autotune/calculator_edge_test.go | 2 +- internal/autotune/calculator_error_test.go | 6 +- internal/autotune/calculator_extended_test.go | 23 ++- internal/autotune/calculator_test.go | 23 ++- internal/config/types.go | 1 + 11 files changed, 299 insertions(+), 92 deletions(-) create mode 100644 internal/autotune/calculator_clamp_test.go 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"` //