From 63c2d31ca0628a9b0552906981a4f49d2efb3079 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 11 May 2026 23:43:53 +0200 Subject: [PATCH] feat(applications): add configurable stop grace period Add centralized stop grace period resolution for application settings and use it across manual stops, preview stops, and deployments. Validate the Livewire advanced setting against shared min/max constants and cover persistence, fillable creation, and fallback behavior with tests. --- app/Actions/Application/StopApplication.php | 4 +- .../Application/StopApplicationOneServer.php | 4 +- app/Jobs/ApplicationDeploymentJob.php | 8 +- app/Livewire/Project/Application/Advanced.php | 27 +++-- app/Livewire/Project/Application/Previews.php | 4 +- app/Models/ApplicationSetting.php | 22 +++++ bootstrap/helpers/constants.php | 2 + .../project/application/advanced.blade.php | 6 +- .../AdvancedStopGracePeriodTest.php | 98 +++++++++++++++++++ tests/Feature/ModelFillableCreationTest.php | 2 + .../Unit/ApplicationSettingStaticCastTest.php | 42 +++++--- 11 files changed, 173 insertions(+), 46 deletions(-) create mode 100644 tests/Feature/Livewire/Project/Application/AdvancedStopGracePeriodTest.php diff --git a/app/Actions/Application/StopApplication.php b/app/Actions/Application/StopApplication.php index 2badcbb67..b79709c5a 100644 --- a/app/Actions/Application/StopApplication.php +++ b/app/Actions/Application/StopApplication.php @@ -36,9 +36,7 @@ public function handle(Application $application, bool $previewDeployments = fals : getCurrentApplicationContainerStatus($server, $application->id, 0); $containersToStop = $containers->pluck('Names')->toArray(); - $timeout = ($application->settings->stop_grace_period > 0) - ? $application->settings->stop_grace_period - : DEFAULT_STOP_GRACE_PERIOD_SECONDS; + $timeout = $application->settings->stopGracePeriodSeconds(); foreach ($containersToStop as $containerName) { instant_remote_process(command: [ diff --git a/app/Actions/Application/StopApplicationOneServer.php b/app/Actions/Application/StopApplicationOneServer.php index cb51bc865..09de9b628 100644 --- a/app/Actions/Application/StopApplicationOneServer.php +++ b/app/Actions/Application/StopApplicationOneServer.php @@ -20,9 +20,7 @@ public function handle(Application $application, Server $server) } try { $containers = getCurrentApplicationContainerStatus($server, $application->id, 0); - $timeout = ($application->settings->stop_grace_period > 0) - ? $application->settings->stop_grace_period - : DEFAULT_STOP_GRACE_PERIOD_SECONDS; + $timeout = $application->settings->stopGracePeriodSeconds(); if ($containers->count() > 0) { foreach ($containers as $container) { diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 6d8cf059f..c2be064c4 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -3790,13 +3790,7 @@ private function build_image() private function graceful_shutdown_container(string $containerName, bool $skipRemove = false) { try { - if (isDev()) { - $timeout = 1; - } else { - $timeout = ($this->application->settings->stop_grace_period > 0) - ? $this->application->settings->stop_grace_period - : DEFAULT_STOP_GRACE_PERIOD_SECONDS; - } + $timeout = $this->application->settings->deploymentStopGracePeriodSeconds(); if ($skipRemove) { $this->execute_remote_command( diff --git a/app/Livewire/Project/Application/Advanced.php b/app/Livewire/Project/Application/Advanced.php index 862181ecf..618958140 100644 --- a/app/Livewire/Project/Application/Advanced.php +++ b/app/Livewire/Project/Application/Advanced.php @@ -4,6 +4,8 @@ use App\Models\Application; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; +use Illuminate\Support\Facades\Validator; +use Illuminate\Validation\ValidationException; use Livewire\Attributes\Validate; use Livewire\Component; @@ -264,24 +266,21 @@ public function saveStopGracePeriod() try { $this->authorize('update', $this->application); - // Convert empty string to null, otherwise cast to integer - $value = ($this->stopGracePeriod === '' || $this->stopGracePeriod === null) + $validated = Validator::make( + ['stopGracePeriod' => $this->stopGracePeriod === '' ? null : $this->stopGracePeriod], + ['stopGracePeriod' => ['nullable', 'integer', 'min:'.MIN_STOP_GRACE_PERIOD_SECONDS, 'max:'.MAX_STOP_GRACE_PERIOD_SECONDS]], + [], + ['stopGracePeriod' => 'stop grace period'] + )->validate(); + + $this->application->settings->stop_grace_period = $validated['stopGracePeriod'] === null ? null - : (int) $this->stopGracePeriod; - - // Validate the integer value - if ($value !== null && ($value < 1 || $value > 3600)) { - $this->dispatch('error', 'Stop grace period must be between 1 and 3600 seconds.'); - - return; - } - - // Save to model - $this->application->settings->stop_grace_period = $value; + : (int) $validated['stopGracePeriod']; $this->application->settings->save(); - // User feedback $this->dispatch('success', 'Stop grace period updated.'); + } catch (ValidationException $e) { + throw $e; } catch (\Throwable $e) { return handleError($e, $this); } diff --git a/app/Livewire/Project/Application/Previews.php b/app/Livewire/Project/Application/Previews.php index 9dd494f5c..59b52f557 100644 --- a/app/Livewire/Project/Application/Previews.php +++ b/app/Livewire/Project/Application/Previews.php @@ -338,9 +338,7 @@ public function addDockerImagePreview() private function stopContainers(array $containers, $server) { $containersToStop = collect($containers)->pluck('Names')->toArray(); - $timeout = ($this->application->settings->stop_grace_period > 0) - ? $this->application->settings->stop_grace_period - : DEFAULT_STOP_GRACE_PERIOD_SECONDS; + $timeout = $this->application->settings->stopGracePeriodSeconds(); foreach ($containersToStop as $containerName) { instant_remote_process(command: [ diff --git a/app/Models/ApplicationSetting.php b/app/Models/ApplicationSetting.php index c365bd187..ef09c0c48 100644 --- a/app/Models/ApplicationSetting.php +++ b/app/Models/ApplicationSetting.php @@ -65,8 +65,30 @@ class ApplicationSetting extends Model 'inject_build_args_to_dockerfile', 'include_source_commit_in_build', 'docker_images_to_keep', + 'stop_grace_period', ]; + public function stopGracePeriodSeconds(): int + { + if ( + $this->stop_grace_period >= MIN_STOP_GRACE_PERIOD_SECONDS && + $this->stop_grace_period <= MAX_STOP_GRACE_PERIOD_SECONDS + ) { + return $this->stop_grace_period; + } + + return DEFAULT_STOP_GRACE_PERIOD_SECONDS; + } + + public function deploymentStopGracePeriodSeconds(): int + { + if (isDev() && $this->stop_grace_period === null) { + return MIN_STOP_GRACE_PERIOD_SECONDS; + } + + return $this->stopGracePeriodSeconds(); + } + public function isStatic(): Attribute { return Attribute::make( diff --git a/bootstrap/helpers/constants.php b/bootstrap/helpers/constants.php index e395f3faf..79049e8c7 100644 --- a/bootstrap/helpers/constants.php +++ b/bootstrap/helpers/constants.php @@ -36,6 +36,8 @@ ]; const RESTART_MODE = 'unless-stopped'; const DEFAULT_STOP_GRACE_PERIOD_SECONDS = 30; +const MIN_STOP_GRACE_PERIOD_SECONDS = 1; +const MAX_STOP_GRACE_PERIOD_SECONDS = 3600; const DATABASE_DOCKER_IMAGES = [ 'bitnami/mariadb', diff --git a/resources/views/livewire/project/application/advanced.blade.php b/resources/views/livewire/project/application/advanced.blade.php index 362539a3c..82ee31933 100644 --- a/resources/views/livewire/project/application/advanced.blade.php +++ b/resources/views/livewire/project/application/advanced.blade.php @@ -93,9 +93,9 @@ id="stopGracePeriod" label="Stop Grace Period (seconds)" placeholder="{{ DEFAULT_STOP_GRACE_PERIOD_SECONDS }}" - helper="How long to wait for graceful shutdown during rolling updates, manual stops, and restarts. Applies to all containers for this application. Default: {{ DEFAULT_STOP_GRACE_PERIOD_SECONDS }} seconds. Range: 1-3600 seconds (1 hour)." - min="1" - max="3600" + helper="How long to wait for graceful shutdown during rolling updates, manual stops, and restarts. Applies to all containers for this application. Default: {{ DEFAULT_STOP_GRACE_PERIOD_SECONDS }} seconds. Range: {{ MIN_STOP_GRACE_PERIOD_SECONDS }}-{{ MAX_STOP_GRACE_PERIOD_SECONDS }} seconds (1 hour)." + min="{{ MIN_STOP_GRACE_PERIOD_SECONDS }}" + max="{{ MAX_STOP_GRACE_PERIOD_SECONDS }}" canGate="update" :canResource="$application" /> diff --git a/tests/Feature/Livewire/Project/Application/AdvancedStopGracePeriodTest.php b/tests/Feature/Livewire/Project/Application/AdvancedStopGracePeriodTest.php new file mode 100644 index 000000000..8d8de1d47 --- /dev/null +++ b/tests/Feature/Livewire/Project/Application/AdvancedStopGracePeriodTest.php @@ -0,0 +1,98 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + return Application::create([ + 'name' => 'stop-grace-period-test-app', + 'git_repository' => 'https://github.com/coollabsio/coolify', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'ports_exposes' => '3000', + 'environment_id' => $environment->id, + 'destination_id' => $server->standaloneDockers()->firstOrFail()->id, + 'destination_type' => $server->standaloneDockers()->firstOrFail()->getMorphClass(), + ]); +} + +beforeEach(function () { + $this->actingAs(User::factory()->create()); +}); + +it('saves a valid stop grace period', function () { + $application = createApplicationForAdvancedStopGracePeriodTest(); + + Livewire::test(Advanced::class, ['application' => $application]) + ->set('stopGracePeriod', '300') + ->call('saveStopGracePeriod') + ->assertHasNoErrors() + ->assertDispatched('success'); + + expect($application->settings()->first()->stop_grace_period)->toBe(300); +}); + +it('clears the stop grace period when submitted empty', function () { + $application = createApplicationForAdvancedStopGracePeriodTest(); + $application->settings->update(['stop_grace_period' => 300]); + + Livewire::test(Advanced::class, ['application' => $application->fresh()]) + ->set('stopGracePeriod', '') + ->call('saveStopGracePeriod') + ->assertHasNoErrors() + ->assertDispatched('success'); + + expect($application->settings()->first()->stop_grace_period)->toBeNull(); +}); + +it('rejects invalid stop grace periods', function (string $value, string $rule) { + $application = createApplicationForAdvancedStopGracePeriodTest(); + + Livewire::test(Advanced::class, ['application' => $application]) + ->set('stopGracePeriod', $value) + ->call('saveStopGracePeriod') + ->assertHasErrors(['stopGracePeriod' => [$rule]]); + + expect($application->settings()->first()->stop_grace_period)->toBeNull(); +})->with([ + 'below minimum' => ['0', 'min'], + 'above maximum' => [(string) (MAX_STOP_GRACE_PERIOD_SECONDS + 1), 'max'], + 'malformed integer' => ['10abc', 'integer'], + 'decimal' => ['1.9', 'integer'], +]); + +it('uses one second deployment timeout in local only when stop grace period is unset', function () { + config(['app.env' => 'local']); + + $setting = new ApplicationSetting; + + expect($setting->deploymentStopGracePeriodSeconds())->toBe(MIN_STOP_GRACE_PERIOD_SECONDS); + + $setting->stop_grace_period = 10; + + expect($setting->deploymentStopGracePeriodSeconds())->toBe(10); +}); + +it('uses default deployment timeout outside local when stop grace period is unset', function () { + config(['app.env' => 'production']); + + $setting = new ApplicationSetting; + + expect($setting->deploymentStopGracePeriodSeconds())->toBe(DEFAULT_STOP_GRACE_PERIOD_SECONDS); +}); diff --git a/tests/Feature/ModelFillableCreationTest.php b/tests/Feature/ModelFillableCreationTest.php index b72e7381e..e6d340a4f 100644 --- a/tests/Feature/ModelFillableCreationTest.php +++ b/tests/Feature/ModelFillableCreationTest.php @@ -299,6 +299,7 @@ 'inject_build_args_to_dockerfile' => true, 'include_source_commit_in_build' => true, 'docker_images_to_keep' => 5, + 'stop_grace_period' => 300, ]); expect($setting->exists)->toBeTrue(); @@ -309,6 +310,7 @@ expect($setting->custom_internal_name)->toBe('my-custom-app'); expect($setting->is_spa)->toBeTrue(); expect($setting->docker_images_to_keep)->toBe(5); + expect($setting->stop_grace_period)->toBe(300); }); it('creates ServerSetting with all fillable attributes', function () { diff --git a/tests/Unit/ApplicationSettingStaticCastTest.php b/tests/Unit/ApplicationSettingStaticCastTest.php index d06ee920d..fe0eaec22 100644 --- a/tests/Unit/ApplicationSettingStaticCastTest.php +++ b/tests/Unit/ApplicationSettingStaticCastTest.php @@ -11,7 +11,7 @@ it('casts is_static to boolean when true', function () { $setting = new ApplicationSetting; - $setting->is_static = true; + $setting->setRawAttributes(['is_static' => true]); // Verify it's cast to boolean expect($setting->is_static)->toBeTrue() @@ -20,7 +20,7 @@ it('casts is_static to boolean when false', function () { $setting = new ApplicationSetting; - $setting->is_static = false; + $setting->setRawAttributes(['is_static' => false]); // Verify it's cast to boolean expect($setting->is_static)->toBeFalse() @@ -29,7 +29,7 @@ it('casts is_static from string "1" to boolean true', function () { $setting = new ApplicationSetting; - $setting->is_static = '1'; + $setting->setRawAttributes(['is_static' => '1']); // Should cast string to boolean expect($setting->is_static)->toBeTrue() @@ -38,7 +38,7 @@ it('casts is_static from string "0" to boolean false', function () { $setting = new ApplicationSetting; - $setting->is_static = '0'; + $setting->setRawAttributes(['is_static' => '0']); // Should cast string to boolean expect($setting->is_static)->toBeFalse() @@ -47,7 +47,7 @@ it('casts is_static from integer 1 to boolean true', function () { $setting = new ApplicationSetting; - $setting->is_static = 1; + $setting->setRawAttributes(['is_static' => 1]); // Should cast integer to boolean expect($setting->is_static)->toBeTrue() @@ -56,7 +56,7 @@ it('casts is_static from integer 0 to boolean false', function () { $setting = new ApplicationSetting; - $setting->is_static = 0; + $setting->setRawAttributes(['is_static' => 0]); // Should cast integer to boolean expect($setting->is_static)->toBeFalse() @@ -128,9 +128,6 @@ }); it('casts stop_grace_period zero to integer (documents fallback trigger)', function () { - // Value of 0 is not a valid grace period — consumers guard with - // `($value > 0) ? $value : DEFAULT_STOP_GRACE_PERIOD_SECONDS`, so - // the cast itself must still round-trip cleanly without throwing. $setting = new ApplicationSetting; $setting->stop_grace_period = 0; @@ -139,13 +136,32 @@ }); it('casts stop_grace_period negative value to integer (documents fallback trigger)', function () { - // Negative values should never be persisted (UI validates `min=1`), - // but if one slips through (direct DB write, older data), the cast - // must not throw and consumers will treat it as the fallback via the - // `> 0` guard. $setting = new ApplicationSetting; $setting->stop_grace_period = -10; expect($setting->stop_grace_period)->toBe(-10) ->and($setting->stop_grace_period)->toBeInt(); }); + +it('resolves valid stop grace periods', function (?int $storedValue, int $expectedValue) { + $setting = new ApplicationSetting; + $setting->stop_grace_period = $storedValue; + + expect($setting->stopGracePeriodSeconds())->toBe($expectedValue); +})->with([ + 'minimum' => [MIN_STOP_GRACE_PERIOD_SECONDS, MIN_STOP_GRACE_PERIOD_SECONDS], + 'custom' => [300, 300], + 'maximum' => [MAX_STOP_GRACE_PERIOD_SECONDS, MAX_STOP_GRACE_PERIOD_SECONDS], +]); + +it('falls back to default stop grace period for invalid stored values', function (?int $storedValue) { + $setting = new ApplicationSetting; + $setting->stop_grace_period = $storedValue; + + expect($setting->stopGracePeriodSeconds())->toBe(DEFAULT_STOP_GRACE_PERIOD_SECONDS); +})->with([ + 'null' => [null], + 'zero' => [0], + 'negative' => [-10], + 'above maximum' => [MAX_STOP_GRACE_PERIOD_SECONDS + 1], +]);