From 5a08c35e7d94fa1c85d2ddd2fc73a9e442a2ab3d Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:40:48 +0100 Subject: [PATCH] fix(applications): stop again while a container restarts past its limit - The restart limit claim sent only one stop. If that stop failed or came too late, the container kept restarting past the max restart count. - Sentinel pushes and status checks now dispatch the stop again while a container still runs; the notification is still sent once. Closes #10761 --- .ai/lessons.md | 3 +- app/Actions/Docker/GetContainersStatus.php | 6 +- app/Jobs/PushServerUpdateJob.php | 6 +- ...ApplicationRestartLimitEnforcementTest.php | 103 ++++++++++++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/ApplicationRestartLimitEnforcementTest.php diff --git a/.ai/lessons.md b/.ai/lessons.md index 11ca067b78..dfe393876b 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -28,7 +28,8 @@ - To assert that the unsaved-changes bar shows, check `el.classList.contains('is-dirty')`. Playwright treats the hidden bar (`opacity-0`) as visible, and its class attribute always contains the text `is-dirty` from Tailwind variants. - For load tests against a dev VM, use your own instance (`COOLIFY_DEV_INSTANCE= COOLIFY_DEV_SIDE_BY_SIDE=1 ./scripts/dev start`); other sessions can reseed the localhost VM of a shared instance. The 1 GiB VM drops packets when its conntrack table is full (`dmesg`: `nf_conntrack: table full`), so raise `net.netfilter.nf_conntrack_max` on it and run an idle load first to prove a clean baseline. - Do not call `sleep()` in a browser test. The app server runs in the test process, so `sleep()` stops it; use `$page->wait()`. -- `Queue::fake()` also captures `dispatchSync()` of a `ShouldQueue` job, so the job never runs. Fake only the jobs to block (`Queue::fake([SomeJob::class])`). +- `Queue::fake()` also captures `dispatchSync()` of a `ShouldQueue` job, so the job never runs. Fake only the jobs to block (`Queue::fake([SomeJob::class])`). Exception: actions (`AsAction`) are queued as `JobDecorator`, so `Queue::fake([SomeAction::class])` does not stop them; use `Queue::fake()` and `SomeAction::assertPushed()`. +- `Notification::fake()` records nothing for a team that has no enabled channel. Turn one on (for example `emailNotificationSettings()->update(['use_instance_email_settings' => true, '_email_notifications' => true])`) before `assertSentTo`. - In tests, turning on `is_metrics_enabled` (and other Sentinel settings) with `save()`/`update()` runs the settings hook, which starts Sentinel over a real SSH connection on the sync queue. Use `saveQuietly()` or `Queue::fake()`. - Call `visit()` directly in each `tests/v4/Browser` test body; Pest does not mark a test that only uses helper-wrapped `visit()` as a browser test, so it fails with `sendText() on null`. - The root team (id 0) has `personal_team = true`, so `personal_team` does not identify a user's own team. To clean up test users, detach them from shared teams and then delete the user; never delete a team selected by a filter. A Team delete revokes the team's API tokens and deletes its private keys before any guard can stop it. diff --git a/app/Actions/Docker/GetContainersStatus.php b/app/Actions/Docker/GetContainersStatus.php index 5ce2a90f6e..7faeb37b78 100644 --- a/app/Actions/Docker/GetContainersStatus.php +++ b/app/Actions/Docker/GetContainersStatus.php @@ -529,7 +529,9 @@ class GetContainersStatus implements ShouldBeUnique ->where('restart_limit_reached', false) ->update(['restart_limit_reached' => true]) === 1; - if ($restartLimitClaimed) { + // Stop again while a container still runs: an earlier stop can fail or come too late. + $stillRunning = $containerStatuses->contains(fn (string $status): bool => ! str($status)->contains('exited')); + if ($restartLimitClaimed || $stillRunning) { $application->refresh(); StopApplication::dispatch( application: $application, @@ -538,6 +540,8 @@ class GetContainersStatus implements ShouldBeUnique resetRestartCount: false, removeContainers: false, ); + } + if ($restartLimitClaimed) { $application->environment->project->team?->notify(new ApplicationRestartLimitReached($application)); } } diff --git a/app/Jobs/PushServerUpdateJob.php b/app/Jobs/PushServerUpdateJob.php index 9a9fd8a56c..92df6bd8c6 100644 --- a/app/Jobs/PushServerUpdateJob.php +++ b/app/Jobs/PushServerUpdateJob.php @@ -614,7 +614,9 @@ class PushServerUpdateJob implements ShouldBeEncrypted, ShouldQueue, Silenced ->where('restart_limit_reached', false) ->update(['restart_limit_reached' => true]) === 1; - if ($restartLimitClaimed) { + // Stop again while a container still runs: an earlier stop can fail or come too late. + $stillRunning = $containerStatuses->contains(fn (string $status): bool => ! str($status)->contains('exited')); + if ($restartLimitClaimed || $stillRunning) { $application->refresh(); StopApplication::dispatch( application: $application, @@ -623,6 +625,8 @@ class PushServerUpdateJob implements ShouldBeEncrypted, ShouldQueue, Silenced resetRestartCount: false, removeContainers: false, ); + } + if ($restartLimitClaimed) { $application->environment->project->team?->notify(new ApplicationRestartLimitReached($application)); } } diff --git a/tests/Feature/ApplicationRestartLimitEnforcementTest.php b/tests/Feature/ApplicationRestartLimitEnforcementTest.php new file mode 100644 index 0000000000..412938fa0d --- /dev/null +++ b/tests/Feature/ApplicationRestartLimitEnforcementTest.php @@ -0,0 +1,103 @@ + 0, 'fqdn' => 'https://coolify.test']); + + $this->team = Team::factory()->create(); + $this->team->emailNotificationSettings()->update([ + 'use_instance_email_settings' => true, + 'restart_limit_reached_email_notifications' => true, + ]); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->server->settings()->update(['is_reachable' => true, 'is_usable' => true]); + $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = $project->environments()->first() + ?? Environment::factory()->create(['project_id' => $project->id]); + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'status' => 'restarting', + ]); + $this->application->forceFill(['max_restart_count' => 3, 'container_present' => true])->save(); +}); + +/** + * Reports one container of the application with the given state and Docker restart count. + */ +function reportApplicationContainer(string $path, Application $application, Server $server, string $state, int $restartCount): void +{ + $labels = [ + 'coolify.managed' => 'true', + 'coolify.applicationUuid' => $application->uuid, + 'coolify.pullRequestId' => '0', + 'com.docker.compose.project' => $application->uuid, + 'com.docker.compose.service' => $application->uuid, + ]; + + if ($path === 'sentinel') { + (new PushServerUpdateJob($server, ['containers' => [[ + 'name' => $application->uuid, + 'state' => $state, + 'health_status' => 'unhealthy', + 'restart_count' => $restartCount, + 'labels' => $labels, + ]]]))->handle(); + + return; + } + + GetContainersStatus::run($server->fresh(), collect([[ + 'Name' => '/'.$application->uuid, + 'State' => ['Status' => $state], + 'RestartCount' => $restartCount, + 'Config' => ['Labels' => $labels], + ]]), collect()); +} + +it('stops the application and notifies once when the restart limit is reached', function (string $path) { + reportApplicationContainer($path, $this->application, $this->server, 'restarting', 3); + + expect($this->application->fresh()->restart_limit_reached)->toBeTrue(); + StopApplication::assertPushed(1); + Notification::assertSentToTimes($this->team, RestartLimitReached::class, 1); +})->with(['sentinel', 'status check']); + +it('stops the application again when it still restarts after the restart limit was reached', function (string $path) { + $this->application->forceFill(['restart_count' => 3, 'restart_limit_reached' => true])->save(); + + reportApplicationContainer($path, $this->application, $this->server, 'restarting', 5); + + StopApplication::assertPushed(1); + Notification::assertNothingSent(); +})->with(['sentinel', 'status check']); + +it('does not stop the application again after its containers exited at the restart limit', function (string $path) { + $this->application->forceFill(['status' => 'exited', 'restart_count' => 3, 'restart_limit_reached' => true])->save(); + + reportApplicationContainer($path, $this->application, $this->server, 'exited', 3); + + StopApplication::assertNotPushed(); + Notification::assertNothingSent(); + expect($this->application->fresh()->stoppedAfterRestartLimit())->toBeTrue(); +})->with(['sentinel', 'status check']);