fix(docker): prevent false container exits on failed docker queries (#8860)
This commit is contained in:
commit
633b1803e1
4 changed files with 187 additions and 27 deletions
|
|
@ -327,6 +327,12 @@ public function handle(Server $server, ?Collection $containers = null, ?Collecti
|
|||
if (str($exitedService->status)->startsWith('exited')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only protection: If no containers at all, Docker query might have failed
|
||||
if ($this->containers->isEmpty()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$name = data_get($exitedService, 'name');
|
||||
$fqdn = data_get($exitedService, 'fqdn');
|
||||
if ($name) {
|
||||
|
|
@ -406,6 +412,12 @@ public function handle(Server $server, ?Collection $containers = null, ?Collecti
|
|||
if (str($database->status)->startsWith('exited')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only protection: If no containers at all, Docker query might have failed
|
||||
if ($this->containers->isEmpty()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Reset restart tracking when database exits completely
|
||||
$database->update([
|
||||
'status' => 'exited',
|
||||
|
|
|
|||
|
|
@ -307,6 +307,8 @@ private function aggregateMultiContainerStatuses()
|
|||
if ($aggregatedStatus && $application->status !== $aggregatedStatus) {
|
||||
$application->status = $aggregatedStatus;
|
||||
$application->save();
|
||||
} elseif ($aggregatedStatus) {
|
||||
$application->update(['last_online_at' => now()]);
|
||||
}
|
||||
|
||||
continue;
|
||||
|
|
@ -321,6 +323,8 @@ private function aggregateMultiContainerStatuses()
|
|||
if ($aggregatedStatus && $application->status !== $aggregatedStatus) {
|
||||
$application->status = $aggregatedStatus;
|
||||
$application->save();
|
||||
} elseif ($aggregatedStatus) {
|
||||
$application->update(['last_online_at' => now()]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -371,6 +375,8 @@ private function aggregateServiceContainerStatuses()
|
|||
if ($aggregatedStatus && $subResource->status !== $aggregatedStatus) {
|
||||
$subResource->status = $aggregatedStatus;
|
||||
$subResource->save();
|
||||
} elseif ($aggregatedStatus) {
|
||||
$subResource->update(['last_online_at' => now()]);
|
||||
}
|
||||
|
||||
continue;
|
||||
|
|
@ -386,6 +392,8 @@ private function aggregateServiceContainerStatuses()
|
|||
if ($aggregatedStatus && $subResource->status !== $aggregatedStatus) {
|
||||
$subResource->status = $aggregatedStatus;
|
||||
$subResource->save();
|
||||
} elseif ($aggregatedStatus) {
|
||||
$subResource->update(['last_online_at' => now()]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -399,6 +407,8 @@ private function updateApplicationStatus(string $applicationId, string $containe
|
|||
if ($application->status !== $containerStatus) {
|
||||
$application->status = $containerStatus;
|
||||
$application->save();
|
||||
} else {
|
||||
$application->update(['last_online_at' => now()]);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -413,6 +423,8 @@ private function updateApplicationPreviewStatus(string $applicationId, string $p
|
|||
if ($application->status !== $containerStatus) {
|
||||
$application->status = $containerStatus;
|
||||
$application->save();
|
||||
} else {
|
||||
$application->update(['last_online_at' => now()]);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -508,6 +520,8 @@ private function updateDatabaseStatus(string $databaseUuid, string $containerSta
|
|||
if ($database->status !== $containerStatus) {
|
||||
$database->status = $containerStatus;
|
||||
$database->save();
|
||||
} else {
|
||||
$database->update(['last_online_at' => now()]);
|
||||
}
|
||||
if ($this->isRunning($containerStatus) && $tcpProxy) {
|
||||
$tcpProxyContainerFound = $this->containers->filter(function ($value, $key) use ($databaseUuid) {
|
||||
|
|
@ -545,8 +559,12 @@ private function updateNotFoundDatabaseStatus()
|
|||
$database = $this->databases->where('uuid', $databaseUuid)->first();
|
||||
if ($database) {
|
||||
if (! str($database->status)->startsWith('exited')) {
|
||||
$database->status = 'exited';
|
||||
$database->save();
|
||||
$database->update([
|
||||
'status' => 'exited',
|
||||
'restart_count' => 0,
|
||||
'last_restart_at' => null,
|
||||
'last_restart_type' => null,
|
||||
]);
|
||||
}
|
||||
if ($database->is_public) {
|
||||
StopDatabaseProxy::dispatch($database);
|
||||
|
|
@ -555,31 +573,6 @@ private function updateNotFoundDatabaseStatus()
|
|||
});
|
||||
}
|
||||
|
||||
private function updateServiceSubStatus(string $serviceId, string $subType, string $subId, string $containerStatus)
|
||||
{
|
||||
$service = $this->services->where('id', $serviceId)->first();
|
||||
if (! $service) {
|
||||
return;
|
||||
}
|
||||
if ($subType === 'application') {
|
||||
$application = $service->applications->where('id', $subId)->first();
|
||||
if ($application) {
|
||||
if ($application->status !== $containerStatus) {
|
||||
$application->status = $containerStatus;
|
||||
$application->save();
|
||||
}
|
||||
}
|
||||
} elseif ($subType === 'database') {
|
||||
$database = $service->databases->where('id', $subId)->first();
|
||||
if ($database) {
|
||||
if ($database->status !== $containerStatus) {
|
||||
$database->status = $containerStatus;
|
||||
$database->save();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private function updateNotFoundServiceStatus()
|
||||
{
|
||||
$notFoundServiceApplicationIds = $this->allServiceApplicationIds->diff($this->foundServiceApplicationIds);
|
||||
|
|
|
|||
101
tests/Feature/PushServerUpdateJobLastOnlineTest.php
Normal file
101
tests/Feature/PushServerUpdateJobLastOnlineTest.php
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
<?php
|
||||
|
||||
use App\Jobs\PushServerUpdateJob;
|
||||
use App\Models\Server;
|
||||
use App\Models\StandalonePostgresql;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
test('database last_online_at is updated when status unchanged', function () {
|
||||
$team = Team::factory()->create();
|
||||
$database = StandalonePostgresql::factory()->create([
|
||||
'team_id' => $team->id,
|
||||
'status' => 'running:healthy',
|
||||
'last_online_at' => now()->subMinutes(5),
|
||||
]);
|
||||
|
||||
$server = $database->destination->server;
|
||||
|
||||
$data = [
|
||||
'containers' => [
|
||||
[
|
||||
'name' => $database->uuid,
|
||||
'state' => 'running',
|
||||
'health_status' => 'healthy',
|
||||
'labels' => [
|
||||
'coolify.managed' => 'true',
|
||||
'coolify.type' => 'database',
|
||||
'com.docker.compose.service' => $database->uuid,
|
||||
],
|
||||
],
|
||||
],
|
||||
];
|
||||
|
||||
$oldLastOnline = $database->last_online_at;
|
||||
|
||||
$job = new PushServerUpdateJob($server, $data);
|
||||
$job->handle();
|
||||
|
||||
$database->refresh();
|
||||
|
||||
// last_online_at should be updated even though status didn't change
|
||||
expect($database->last_online_at->greaterThan($oldLastOnline))->toBeTrue();
|
||||
expect($database->status)->toBe('running:healthy');
|
||||
});
|
||||
|
||||
test('database status is updated when container status changes', function () {
|
||||
$team = Team::factory()->create();
|
||||
$database = StandalonePostgresql::factory()->create([
|
||||
'team_id' => $team->id,
|
||||
'status' => 'exited',
|
||||
]);
|
||||
|
||||
$server = $database->destination->server;
|
||||
|
||||
$data = [
|
||||
'containers' => [
|
||||
[
|
||||
'name' => $database->uuid,
|
||||
'state' => 'running',
|
||||
'health_status' => 'healthy',
|
||||
'labels' => [
|
||||
'coolify.managed' => 'true',
|
||||
'coolify.type' => 'database',
|
||||
'com.docker.compose.service' => $database->uuid,
|
||||
],
|
||||
],
|
||||
],
|
||||
];
|
||||
|
||||
$job = new PushServerUpdateJob($server, $data);
|
||||
$job->handle();
|
||||
|
||||
$database->refresh();
|
||||
|
||||
expect($database->status)->toBe('running:healthy');
|
||||
});
|
||||
|
||||
test('database is not marked exited when containers list is empty', function () {
|
||||
$team = Team::factory()->create();
|
||||
$database = StandalonePostgresql::factory()->create([
|
||||
'team_id' => $team->id,
|
||||
'status' => 'running:healthy',
|
||||
]);
|
||||
|
||||
$server = $database->destination->server;
|
||||
|
||||
// Empty containers = Sentinel might have failed, should NOT mark as exited
|
||||
$data = [
|
||||
'containers' => [],
|
||||
];
|
||||
|
||||
$job = new PushServerUpdateJob($server, $data);
|
||||
$job->handle();
|
||||
|
||||
$database->refresh();
|
||||
|
||||
// Status should remain running, NOT be set to exited
|
||||
expect($database->status)->toBe('running:healthy');
|
||||
});
|
||||
|
|
@ -0,0 +1,54 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Unit tests verifying that GetContainersStatus has empty container
|
||||
* safeguards for ALL resource types (applications, previews, databases, services).
|
||||
*
|
||||
* When Docker queries fail and return empty container lists, resources should NOT
|
||||
* be falsely marked as "exited". This was originally added for applications and
|
||||
* previews (commit 684bd823c) but was missing for databases and services.
|
||||
*
|
||||
* @see https://github.com/coollabsio/coolify/issues/8826
|
||||
*/
|
||||
it('has empty container safeguard for applications', function () {
|
||||
$actionFile = file_get_contents(__DIR__.'/../../app/Actions/Docker/GetContainersStatus.php');
|
||||
|
||||
// The safeguard should appear before marking applications as exited
|
||||
expect($actionFile)
|
||||
->toContain('$notRunningApplications = $this->applications->pluck(\'id\')->diff($foundApplications);');
|
||||
|
||||
// Count occurrences of the safeguard pattern in the not-found sections
|
||||
$safeguardPattern = '// Only protection: If no containers at all, Docker query might have failed';
|
||||
$safeguardCount = substr_count($actionFile, $safeguardPattern);
|
||||
|
||||
// Should appear at least 4 times: applications, previews, databases, services
|
||||
expect($safeguardCount)->toBeGreaterThanOrEqual(4);
|
||||
});
|
||||
|
||||
it('has empty container safeguard for databases', function () {
|
||||
$actionFile = file_get_contents(__DIR__.'/../../app/Actions/Docker/GetContainersStatus.php');
|
||||
|
||||
// Extract the database not-found section
|
||||
$databaseSectionStart = strpos($actionFile, '$notRunningDatabases = $databases->pluck(\'id\')->diff($foundDatabases);');
|
||||
expect($databaseSectionStart)->not->toBeFalse('Database not-found section should exist');
|
||||
|
||||
// Get the code between database section start and the next major section
|
||||
$databaseSection = substr($actionFile, $databaseSectionStart, 500);
|
||||
|
||||
// The empty container safeguard must exist in the database section
|
||||
expect($databaseSection)->toContain('$this->containers->isEmpty()');
|
||||
});
|
||||
|
||||
it('has empty container safeguard for services', function () {
|
||||
$actionFile = file_get_contents(__DIR__.'/../../app/Actions/Docker/GetContainersStatus.php');
|
||||
|
||||
// Extract the service exited section
|
||||
$serviceSectionStart = strpos($actionFile, '$exitedServices = $exitedServices->unique(\'uuid\');');
|
||||
expect($serviceSectionStart)->not->toBeFalse('Service exited section should exist');
|
||||
|
||||
// Get the code in the service exited loop
|
||||
$serviceSection = substr($actionFile, $serviceSectionStart, 500);
|
||||
|
||||
// The empty container safeguard must exist in the service section
|
||||
expect($serviceSection)->toContain('$this->containers->isEmpty()');
|
||||
});
|
||||
Loading…
Reference in a new issue