From c6b8dfe367b568aad98bc6f7d603f6adcd2e870d Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:16:44 +0200 Subject: [PATCH] fix(status): store the status of a database or service right after it starts After a start, the UI showed "exited" until the regular status check ran. That check runs on the shared high queue, which can be busy (60 s on Cloud). The start job (database) and the start task (service, on the deployment queue) now inspect only the started containers and store their status before they broadcast. While a health check reports "starting", they inspect again for up to about 15 s. ContainerStatusAggregator now builds the per-container status and the Compose aggregation for both writers, so GetContainersStatus and the targeted check store the same strings. --- app/Actions/Docker/GetContainersStatus.php | 63 +----- .../Service/DeployServiceApplication.php | 2 +- app/Actions/Service/StartService.php | 2 +- app/Events/ServiceStartFinished.php | 16 ++ app/Jobs/DatabaseStartJob.php | 9 + .../RefreshServiceStatusAfterStart.php | 35 ++++ app/Services/ContainerStatusAggregator.php | 39 ++++ app/Services/ResourceStatusRefresher.php | 108 ++++++++++ tests/Feature/ResourceStatusRefresherTest.php | 184 ++++++++++++++++++ 9 files changed, 396 insertions(+), 62 deletions(-) create mode 100644 app/Events/ServiceStartFinished.php create mode 100644 app/Listeners/RefreshServiceStatusAfterStart.php create mode 100644 app/Services/ResourceStatusRefresher.php create mode 100644 tests/Feature/ResourceStatusRefresherTest.php diff --git a/app/Actions/Docker/GetContainersStatus.php b/app/Actions/Docker/GetContainersStatus.php index fbe26dd586..aabecdbd2e 100644 --- a/app/Actions/Docker/GetContainersStatus.php +++ b/app/Actions/Docker/GetContainersStatus.php @@ -16,7 +16,6 @@ use App\Models\ServiceDatabase; use App\Notifications\Application\RestartLimitReached as ApplicationRestartLimitReached; use App\Services\ContainerStatusAggregator; use App\Services\RestartCountTracker; -use App\Traits\CalculatesExcludedStatus; use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; @@ -25,7 +24,6 @@ use Lorisleiva\Actions\Concerns\AsAction; class GetContainersStatus { use AsAction; - use CalculatesExcludedStatus; public string $jobQueue = 'high'; @@ -121,17 +119,7 @@ class GetContainersStatus } else { $labels = data_get($container, 'Config.Labels'); } - $containerStatus = data_get($container, 'State.Status'); - $containerHealth = data_get($container, 'State.Health.Status'); - if ($containerStatus === 'restarting') { - $healthSuffix = $containerHealth ?? 'unknown'; - $containerStatus = "restarting:$healthSuffix"; - } elseif ($containerStatus === 'exited') { - // Keep as-is, no health suffix for exited containers - } else { - $healthSuffix = $containerHealth ?? 'unknown'; - $containerStatus = "$containerStatus:$healthSuffix"; - } + $containerStatus = ContainerStatusAggregator::containerStatus($container); $flatLabels = format_docker_labels_to_json($labels); $labels = Arr::undot($flatLabels); if (filter_var(data_get($labels, 'com.docker.compose.oneoff'), FILTER_VALIDATE_BOOLEAN)) { @@ -538,25 +526,7 @@ class GetContainersStatus private function aggregateApplicationStatus($application, Collection $containerStatuses, int $maxRestartCount = 0): ?string { - // Parse docker compose to check for excluded containers - $dockerComposeRaw = data_get($application, 'docker_compose_raw'); - $excludedContainers = $this->getExcludedContainersFromDockerCompose($dockerComposeRaw); - - // Filter out excluded containers - $relevantStatuses = $containerStatuses->filter(function ($status, $containerName) use ($excludedContainers) { - return ! $excludedContainers->contains($containerName); - }); - - // If all containers are excluded, calculate status from excluded containers - if ($relevantStatuses->isEmpty()) { - return $this->calculateExcludedStatusFromStrings($containerStatuses); - } - - // Use ContainerStatusAggregator service for state machine logic - // Use preserveRestarting: true so applications show "Restarting" instead of "Degraded" - $aggregator = new ContainerStatusAggregator; - - return $aggregator->aggregateFromStrings($relevantStatuses, $maxRestartCount, preserveRestarting: true); + return (new ContainerStatusAggregator)->aggregateForCompose($containerStatuses, data_get($application, 'docker_compose_raw'), $maxRestartCount); } private function aggregateServiceContainerStatuses($services) @@ -596,34 +566,7 @@ class GetContainersStatus continue; } - // Parse docker compose from service to check for excluded containers - $dockerComposeRaw = data_get($service, 'docker_compose_raw'); - $excludedContainers = $this->getExcludedContainersFromDockerCompose($dockerComposeRaw); - - // Filter out excluded containers - $relevantStatuses = $containerStatuses->filter(function ($status, $containerName) use ($excludedContainers) { - return ! $excludedContainers->contains($containerName); - }); - - // If all containers are excluded, calculate status from excluded containers - if ($relevantStatuses->isEmpty()) { - $aggregatedStatus = $this->calculateExcludedStatusFromStrings($containerStatuses); - if ($aggregatedStatus) { - $statusFromDb = $subResource->status; - if ($statusFromDb !== $aggregatedStatus) { - $subResource->update(['status' => $aggregatedStatus]); - } else { - $subResource->update(['last_online_at' => now()]); - } - } - - continue; - } - - // Use ContainerStatusAggregator service for state machine logic - // Use preserveRestarting: true so individual sub-resources show "Restarting" instead of "Degraded" - $aggregator = new ContainerStatusAggregator; - $aggregatedStatus = $aggregator->aggregateFromStrings($relevantStatuses, preserveRestarting: true); + $aggregatedStatus = (new ContainerStatusAggregator)->aggregateForCompose($containerStatuses, data_get($service, 'docker_compose_raw')); // Update service sub-resource status with aggregated result if ($aggregatedStatus) { diff --git a/app/Actions/Service/DeployServiceApplication.php b/app/Actions/Service/DeployServiceApplication.php index c89c8e1e85..b8d1d832fd 100644 --- a/app/Actions/Service/DeployServiceApplication.php +++ b/app/Actions/Service/DeployServiceApplication.php @@ -64,6 +64,6 @@ class DeployServiceApplication $commands->push("docker network connect --alias {$networkAlias} {$network} {$containerName} >/dev/null 2>&1 || true"); } - return remote_process($commands->toArray(), $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStatusChanged', queue: deployment_queue()); + return remote_process($commands->toArray(), $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStartFinished', callEventData: $service->id, queue: deployment_queue()); } } diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index 1580045d09..ca26ccbd23 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -64,7 +64,7 @@ class StartService } $commands = array_merge($commands, $this->logDrainNetworkConnectCommands($service)); - return remote_process($commands, $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStatusChanged', queue: deployment_queue()); + return remote_process($commands, $service->server, type_uuid: $service->uuid, callEventOnFinish: 'ServiceStartFinished', callEventData: $service->id, queue: deployment_queue()); } /** diff --git a/app/Events/ServiceStartFinished.php b/app/Events/ServiceStartFinished.php new file mode 100644 index 0000000000..3496a490ae --- /dev/null +++ b/app/Events/ServiceStartFinished.php @@ -0,0 +1,16 @@ +refreshDatabase($database); + } catch (Throwable $e) { + Log::warning('Could not refresh the status of a started database.', ['database' => $database->uuid, 'error' => $e->getMessage()]); + } + event(new DatabaseStatusChanged($this->userId)); } diff --git a/app/Listeners/RefreshServiceStatusAfterStart.php b/app/Listeners/RefreshServiceStatusAfterStart.php new file mode 100644 index 0000000000..094d9b3425 --- /dev/null +++ b/app/Listeners/RefreshServiceStatusAfterStart.php @@ -0,0 +1,35 @@ +find($event->serviceId); + if (! $service) { + return; + } + + try { + $this->refresher->refreshService($service); + } catch (Throwable $e) { + Log::warning('Could not refresh the status of a started service.', ['service' => $service->uuid, 'error' => $e->getMessage()]); + } + + ServiceStatusChanged::dispatch($service->server?->team_id); + } +} diff --git a/app/Services/ContainerStatusAggregator.php b/app/Services/ContainerStatusAggregator.php index 3a59ad58fa..f963c0a6a1 100644 --- a/app/Services/ContainerStatusAggregator.php +++ b/app/Services/ContainerStatusAggregator.php @@ -2,6 +2,7 @@ namespace App\Services; +use App\Traits\CalculatesExcludedStatus; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; @@ -35,6 +36,44 @@ use Illuminate\Support\Facades\Log; */ class ContainerStatusAggregator { + use CalculatesExcludedStatus; + + /** + * Status string of one container from `docker container inspect` output + * (e.g. "running:healthy", "restarting:unknown", "exited"). + * + * Every status writer (the full server check and the targeted check after a start) + * uses this, so they store the same strings. + */ + public static function containerStatus(array|object $container): string + { + $state = data_get($container, 'State.Status'); + if ($state === 'exited') { + return $state; + } + + return $state.':'.(data_get($container, 'State.Health.Status') ?? 'unknown'); + } + + /** + * Aggregate the container statuses of one Compose-based resource (application, service + * application or service database). Containers excluded from health checks in the Compose + * file are ignored; when every container is excluded, the result has the :excluded suffix. + * + * @param Collection $containerStatuses Status strings keyed by Compose service name + */ + public function aggregateForCompose(Collection $containerStatuses, ?string $dockerComposeRaw, int $maxRestartCount = 0): string + { + $excludedContainers = $this->getExcludedContainersFromDockerCompose($dockerComposeRaw); + $relevantStatuses = $containerStatuses->reject(fn ($status, $containerName) => $excludedContainers->contains($containerName)); + + if ($relevantStatuses->isEmpty()) { + return $this->calculateExcludedStatusFromStrings($containerStatuses); + } + + return $this->aggregateFromStrings($relevantStatuses, $maxRestartCount, preserveRestarting: true); + } + /** * Aggregate container statuses from status strings into a single status. * diff --git a/app/Services/ResourceStatusRefresher.php b/app/Services/ResourceStatusRefresher.php new file mode 100644 index 0000000000..23dd1743fd --- /dev/null +++ b/app/Services/ResourceStatusRefresher.php @@ -0,0 +1,108 @@ +destination->server; + $command = 'docker container inspect '.escapeshellarg($database->uuid).' --format \'{{json .}}\''; + + $this->refresh($server, $command, function (Collection $containers) use ($database): void { + $container = $containers->first(fn ($container) => data_get($container, 'Name') === "/{$database->uuid}"); + if ($container) { + $this->store($database, ContainerStatusAggregator::containerStatus($container)); + } + }); + } + + public function refreshService(Service $service): void + { + $script = containerIdsByOwnerScript('service', $service->uuid) + .'; [ -z "$container_ids" ] || docker container inspect $container_ids --format \'{{json .}}\''; + + $this->refresh($service->server, 'sh -c '.escapeshellarg($script), function (Collection $containers) use ($service): void { + $services = collect([$service]); + $parts = []; + foreach ($containers as $container) { + $labels = format_docker_labels_to_json(data_get($container, 'Config.Labels') ?? []); + $containerName = $labels->get('com.docker.compose.service'); + if (! $containerName || filter_var($labels->get('com.docker.compose.oneoff'), FILTER_VALIDATE_BOOLEAN)) { + continue; + } + [, $subType, $part] = resolveServiceContainerOwner($services, $labels); + if (! $part) { + continue; + } + $key = $subType.':'.$part->id; + $parts[$key] ??= ['part' => $part, 'statuses' => collect()]; + $parts[$key]['statuses']->put($containerName, ContainerStatusAggregator::containerStatus($container)); + } + + $aggregator = new ContainerStatusAggregator; + foreach ($parts as ['part' => $part, 'statuses' => $statuses]) { + $this->store($part, $aggregator->aggregateForCompose($statuses, $service->docker_compose_raw)); + } + }); + } + + /** + * Inspect the containers and store their status. While a health check reports "starting", + * store that status (so the UI shows it) and inspect again a few times. + * + * @param callable(Collection): void $storeStatuses + */ + private function refresh(Server $server, string $command, callable $storeStatuses): void + { + if (! $server->isFunctional() || $server->isSwarm()) { + return; + } + + for ($attempt = 1; ; $attempt++) { + $containers = format_docker_command_output_to_json(instant_remote_process([$command], $server, false) ?? '')->filter(); + if ($containers->isEmpty()) { + return; + } + + $storeStatuses($containers); + + $healthStarting = $containers->contains(fn ($container) => data_get($container, 'State.Health.Status') === 'starting'); + if (! $healthStarting || $attempt >= self::HEALTH_WAIT_ATTEMPTS) { + return; + } + + Sleep::for(self::HEALTH_WAIT_INTERVAL_SECONDS)->seconds(); + } + } + + private function store(Model $resource, string $status): void + { + if ($resource->status !== $status) { + $resource->update(['status' => $status]); + } else { + $resource->update(['last_online_at' => now()]); + } + } +} diff --git a/tests/Feature/ResourceStatusRefresherTest.php b/tests/Feature/ResourceStatusRefresherTest.php new file mode 100644 index 0000000000..35b589cad6 --- /dev/null +++ b/tests/Feature/ResourceStatusRefresherTest.php @@ -0,0 +1,184 @@ + 0]); + config()->set('cache.default', 'array'); + config()->set('constants.ssh.mux_enabled', false); + Sleep::fake(); + + $this->team = Team::factory()->create(); + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $this->team->id])->id, + ]); + $this->server->settings()->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); + $this->destination = StandaloneDocker::query()->where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->database = StandalonePostgresql::create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'db', + 'postgres_user' => 'postgres', + 'postgres_password' => 'password', + 'postgres_db' => 'db', + 'image' => 'postgres:17', + 'status' => 'exited', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); +}); + +/** + * One line of `docker container inspect --format '{{json .}}'` output. + */ +function inspectedContainer(string $name, string $state, ?string $health = null, array $labels = []): string +{ + return json_encode([ + 'Name' => "/{$name}", + 'State' => array_filter(['Status' => $state, 'Health' => $health ? ['Status' => $health] : null]), + 'Config' => ['Labels' => $labels], + ]); +} + +/** + * Answer each SSH command with the next output in the list (the last one repeats). + * + * @param list $outputs + */ +function fakeInspectOutputs(array $outputs): void +{ + $sequence = Process::sequence()->dontFailWhenEmpty(); + foreach ($outputs as $output) { + $sequence->push(Process::result(output: $output)); + } + Process::fake(['*' => $sequence->whenEmpty(Process::result(output: end($outputs)))]); +} + +test('a container status has the same format for every status writer', function (string $state, ?string $health, string $expected) { + expect(ContainerStatusAggregator::containerStatus(['State' => ['Status' => $state, 'Health' => $health ? ['Status' => $health] : null]])) + ->toBe($expected); +})->with([ + 'healthy' => ['running', 'healthy', 'running:healthy'], + 'no health check' => ['running', null, 'running:unknown'], + 'restarting' => ['restarting', null, 'restarting:unknown'], + 'exited' => ['exited', 'unhealthy', 'exited'], +]); + +test('it stores the status of a started database', function () { + fakeInspectOutputs([inspectedContainer($this->database->uuid, 'running', 'healthy')]); + + app(ResourceStatusRefresher::class)->refreshDatabase($this->database); + + expect($this->database->fresh()->status)->toBe('running:healthy'); + Sleep::assertNeverSlept(); +}); + +test('it inspects the database again while its health check is starting', function () { + fakeInspectOutputs([ + inspectedContainer($this->database->uuid, 'running', 'starting'), + inspectedContainer($this->database->uuid, 'running', 'healthy'), + ]); + + app(ResourceStatusRefresher::class)->refreshDatabase($this->database); + + expect($this->database->fresh()->status)->toBe('running:healthy'); + Sleep::assertSleptTimes(1); +}); + +test('it stops waiting for the health check after the limit', function () { + fakeInspectOutputs([inspectedContainer($this->database->uuid, 'running', 'starting')]); + + app(ResourceStatusRefresher::class)->refreshDatabase($this->database); + + expect($this->database->fresh()->status)->toBe('running:starting'); + Sleep::assertSleptTimes(ResourceStatusRefresher::HEALTH_WAIT_ATTEMPTS - 1); +}); + +test('it keeps the stored status when the database container is not found', function () { + fakeInspectOutputs(['']); + $storedStatus = $this->database->fresh()->status; + + app(ResourceStatusRefresher::class)->refreshDatabase($this->database); + + expect($this->database->fresh()->status)->toBe($storedStatus); +}); + +test('the database start job stores the running status before it broadcasts', function () { + Event::fake([DatabaseStatusChanged::class]); + fakeInspectOutputs([inspectedContainer($this->database->uuid, 'running', 'healthy')]); + $activity = activity()->withProperties([ + 'status' => ProcessStatus::QUEUED->value, + 'type_uuid' => $this->database->uuid, + 'operation' => ResourceStartActivity::DATABASE_START_OPERATION, + ])->log('[]'); + StartPostgresql::shouldRun()->andReturnUsing(function ($database, $activity) { + $activity->properties = $activity->properties->merge(['status' => ProcessStatus::FINISHED->value]); + $activity->save(); + + return $activity; + }); + + (new DatabaseStartJob($this->database->getMorphClass(), $this->database->id, $this->team->id, $activity->id, null))->handle(); + + expect($this->database->fresh()->status)->toBe('running:healthy'); + Event::assertDispatched(DatabaseStatusChanged::class); +}); + +test('a finished service start stores the status of each service part and broadcasts it', function () { + Event::fake([ServiceStatusChanged::class]); + $service = Service::factory()->create([ + 'environment_id' => $this->environment->id, + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'docker_compose_raw' => "services:\n app:\n image: nginx\n db:\n image: postgres\n", + ]); + $app = ServiceApplication::create(['name' => 'app', 'service_id' => $service->id, 'status' => 'exited']); + $db = ServiceDatabase::create(['name' => 'db', 'service_id' => $service->id, 'status' => 'exited']); + $labels = fn (string $subType, string $subUuid, string $name) => [ + 'coolify.serviceUuid' => $service->uuid, + 'coolify.service.subType' => $subType, + 'coolify.service.subUuid' => $subUuid, + 'com.docker.compose.service' => $name, + ]; + fakeInspectOutputs([implode("\n", [ + inspectedContainer("app-{$service->uuid}", 'running', null, $labels('application', $app->uuid, 'app')), + inspectedContainer("db-{$service->uuid}", 'running', 'healthy', $labels('database', $db->uuid, 'db')), + ])]); + + event(new ServiceStartFinished($service->id)); + + expect($app->fresh()->status)->toBe('running:unknown') + ->and($db->fresh()->status)->toBe('running:healthy'); + Event::assertDispatched(ServiceStatusChanged::class, fn (ServiceStatusChanged $event) => $event->teamId === $this->team->id); +});