fix: don't show health status for exited containers (#7317)

This commit is contained in:
Andras Bacsai 2025-11-24 10:29:57 +01:00 committed by GitHub
commit bf428a0e1c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 65 additions and 48 deletions

View file

@ -107,6 +107,8 @@ public function handle(Server $server, ?Collection $containers = null, ?Collecti
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";
@ -332,7 +334,7 @@ public function handle(Server $server, ?Collection $containers = null, ?Collecti
if ($recentlyRestarted) {
// Keep it as degraded if it was recently in a crash loop
$application->update(['status' => 'degraded (unhealthy)']);
$application->update(['status' => 'degraded:unhealthy']);
} else {
// Reset restart count when application exits completely
$application->update([

View file

@ -20,11 +20,11 @@ public function handle(Application $application)
$is_main_server = $application->destination->server->id === $server->id;
if (! $server->isFunctional()) {
if ($is_main_server) {
$application->update(['status' => 'exited:unhealthy']);
$application->update(['status' => 'exited']);
continue;
} else {
$application->additional_servers()->updateExistingPivot($server->id, ['status' => 'exited:unhealthy']);
$application->additional_servers()->updateExistingPivot($server->id, ['status' => 'exited']);
continue;
}
@ -49,11 +49,11 @@ public function handle(Application $application)
}
} else {
if ($is_main_server) {
$application->update(['status' => 'exited:unhealthy']);
$application->update(['status' => 'exited']);
continue;
} else {
$application->additional_servers()->updateExistingPivot($server->id, ['status' => 'exited:unhealthy']);
$application->additional_servers()->updateExistingPivot($server->id, ['status' => 'exited']);
continue;
}

View file

@ -148,7 +148,10 @@ public function handle()
$containerStatus = data_get($container, 'state', 'exited');
$rawHealthStatus = data_get($container, 'health_status');
$containerHealth = $rawHealthStatus ?? 'unknown';
$containerStatus = "$containerStatus:$containerHealth";
// Only append health status if container is not exited
if ($containerStatus !== 'exited') {
$containerStatus = "$containerStatus:$containerHealth";
}
$labels = collect(data_get($container, 'labels'));
$coolify_managed = $labels->has('coolify.managed');

View file

@ -223,7 +223,12 @@ public function getStatusAttribute()
return 'unknown:unknown:excluded';
}
return 'exited:unhealthy:excluded';
return 'exited';
}
// If health is null/empty, return just the status without trailing colon
if ($complexHealth === null || $complexHealth === '') {
return $complexStatus;
}
return "{$complexStatus}:{$complexHealth}";

View file

@ -23,7 +23,7 @@
* 5. Dead/Removing degraded:unhealthy
* 6. Paused paused:unknown
* 7. Starting/Created starting:unknown
* 8. Exited exited:unhealthy
* 8. Exited exited
*/
class ContainerStatusAggregator
{
@ -52,7 +52,7 @@ public function aggregateFromStrings(Collection $containerStatuses, int $maxRest
}
if ($containerStatuses->isEmpty()) {
return 'exited:unhealthy';
return 'exited';
}
// Initialize state flags
@ -79,7 +79,6 @@ public function aggregateFromStrings(Collection $containerStatuses, int $maxRest
}
} elseif (str($status)->contains('exited')) {
$hasExited = true;
$hasUnhealthy = true;
} elseif (str($status)->contains('created') || str($status)->contains('starting')) {
$hasStarting = true;
} elseif (str($status)->contains('paused')) {
@ -128,7 +127,7 @@ public function aggregateFromContainers(Collection $containers, int $maxRestartC
}
if ($containers->isEmpty()) {
return 'exited:unhealthy';
return 'exited';
}
// Initialize state flags
@ -157,7 +156,6 @@ public function aggregateFromContainers(Collection $containers, int $maxRestartC
}
} elseif ($state === 'exited') {
$hasExited = true;
$hasUnhealthy = true;
} elseif ($state === 'created' || $state === 'starting') {
$hasStarting = true;
} elseif ($state === 'paused') {
@ -248,6 +246,6 @@ private function resolveStatus(
}
// Priority 8: All containers exited (no restart count = truly stopped)
return 'exited:unhealthy';
return 'exited';
}
}

View file

@ -86,7 +86,7 @@ private function appendExcludedSuffix(string $status): string
}
if (str($status)->startsWith('exited')) {
return 'exited:excluded';
return 'exited';
}
// For running states, keep the health status: "running:healthy:excluded"

View file

@ -4,18 +4,30 @@
])
@php
// Handle both colon format (backend) and parentheses format (from services.blade.php)
// exited:unhealthy → Exited (unhealthy)
// exited (unhealthy) → exited (unhealthy) (already formatted, display as-is)
// For exited containers, health status is hidden (health checks don't run on stopped containers)
// exited:unhealthy → Exited
// exited (unhealthy) → Exited
if (str($status)->contains('(')) {
// Already in parentheses format from services.blade.php - use as-is
$displayStatus = $status;
$healthStatus = str($status)->after('(')->before(')')->trim()->value();
// Don't show health status for exited containers (health checks don't run on stopped containers)
if (str($displayStatus)->lower()->contains('exited')) {
$displayStatus = str($status)->before('(')->trim()->headline();
$healthStatus = null;
}
} elseif (str($status)->contains(':')) {
// Colon format from backend - transform it
$parts = explode(':', $status);
$displayStatus = str($parts[0])->headline();
$healthStatus = $parts[1] ?? null;
// Don't show health status for exited containers (health checks don't run on stopped containers)
if (str($displayStatus)->lower()->contains('exited')) {
$healthStatus = null;
}
} else {
// Simple status without health
$displayStatus = str($status)->headline();

View file

@ -136,7 +136,7 @@
->toContain("return 'degraded:excluded';")
->toContain("return 'paused:excluded';")
->toContain("return 'starting:excluded';")
->toContain("return 'exited:excluded';")
->toContain("return 'exited';")
->toContain('return "$status:excluded";'); // For running:healthy:excluded, running:unhealthy:excluded, etc.
});
@ -179,7 +179,7 @@
->toContain("return 'degraded:excluded';")
->toContain("return 'paused:excluded';")
->toContain("return 'starting:excluded';")
->toContain("return 'exited:excluded';")
->toContain("return 'exited';")
->toContain('return "$status:excluded";'); // For running:healthy:excluded
});
@ -199,7 +199,7 @@
->toContain("return 'degraded:excluded';")
->toContain("return 'paused:excluded';")
->toContain("return 'starting:excluded';")
->toContain("return 'exited:excluded';")
->toContain("return 'exited';")
->toContain('return "$status:excluded";'); // For running:healthy:excluded
});

View file

@ -8,10 +8,10 @@
});
describe('aggregateFromStrings', function () {
test('returns exited:unhealthy for empty collection', function () {
test('returns exited for empty collection', function () {
$result = $this->aggregator->aggregateFromStrings(collect());
expect($result)->toBe('exited:unhealthy');
expect($result)->toBe('exited');
});
test('returns running:healthy for single healthy running container', function () {
@ -78,12 +78,12 @@
expect($result)->toBe('degraded:unhealthy');
});
test('returns exited:unhealthy for exited containers without restart count', function () {
test('returns exited for exited containers without restart count', function () {
$statuses = collect(['exited']);
$result = $this->aggregator->aggregateFromStrings($statuses, maxRestartCount: 0);
expect($result)->toBe('exited:unhealthy');
expect($result)->toBe('exited');
});
test('returns degraded:unhealthy for dead container', function () {
@ -200,10 +200,10 @@
});
describe('aggregateFromContainers', function () {
test('returns exited:unhealthy for empty collection', function () {
test('returns exited for empty collection', function () {
$result = $this->aggregator->aggregateFromContainers(collect());
expect($result)->toBe('exited:unhealthy');
expect($result)->toBe('exited');
});
test('returns running:healthy for single healthy running container', function () {
@ -299,7 +299,7 @@
expect($result)->toBe('degraded:unhealthy');
});
test('returns exited:unhealthy for exited containers without restart count', function () {
test('returns exited for exited containers without restart count', function () {
$containers = collect([
(object) [
'State' => (object) [
@ -310,7 +310,7 @@
$result = $this->aggregator->aggregateFromContainers($containers, maxRestartCount: 0);
expect($result)->toBe('exited:unhealthy');
expect($result)->toBe('exited');
});
test('returns degraded:unhealthy for dead container', function () {
@ -473,8 +473,8 @@
// With negative value, should be treated as 0 (no restarts)
$result = $this->aggregator->aggregateFromStrings($statuses, maxRestartCount: -5);
// Should return exited:unhealthy (not degraded) since corrected to 0
expect($result)->toBe('exited:unhealthy');
// Should return exited (not degraded) since corrected to 0
expect($result)->toBe('exited');
});
test('negative maxRestartCount is corrected to 0 in aggregateFromContainers', function () {
@ -493,8 +493,8 @@
// With negative value, should be treated as 0 (no restarts)
$result = $this->aggregator->aggregateFromContainers($containers, maxRestartCount: -10);
// Should return exited:unhealthy (not degraded) since corrected to 0
expect($result)->toBe('exited:unhealthy');
// Should return exited (not degraded) since corrected to 0
expect($result)->toBe('exited');
});
test('zero maxRestartCount works correctly', function () {
@ -503,7 +503,7 @@
$result = $this->aggregator->aggregateFromStrings($statuses, maxRestartCount: 0);
// Zero is valid default - no crash loop detection
expect($result)->toBe('exited:unhealthy');
expect($result)->toBe('exited');
});
test('positive maxRestartCount works correctly', function () {
@ -535,6 +535,6 @@
// Call without specifying maxRestartCount (should default to 0)
$result = $this->aggregator->aggregateFromStrings($statuses);
expect($result)->toBe('exited:unhealthy');
expect($result)->toBe('exited');
});
});

View file

@ -28,7 +28,7 @@
->toContain('$aggregator->aggregateFromContainers($excludedOnly)')
->toContain("return 'degraded:excluded';")
->toContain("return 'paused:excluded';")
->toContain("return 'exited:excluded';")
->toContain("return 'exited';")
->toContain('return "$status:excluded";'); // For running:healthy:excluded
});
@ -47,7 +47,7 @@
$serviceModelFile = file_get_contents(__DIR__.'/../../app/Models/Service.php');
// Check that when a service has no applications or databases at all,
// the Service model returns 'unknown:unknown:excluded' instead of 'exited:unhealthy:excluded'
// the Service model returns 'unknown:unknown:excluded' instead of 'exited'
// This prevents misleading status display when containers don't exist
expect($serviceModelFile)
->toContain('// If no status was calculated at all (no containers exist), return unknown')
@ -85,12 +85,9 @@
it('ensures UI displays excluded status correctly in status component', function () {
$servicesStatusFile = file_get_contents(__DIR__.'/../../resources/views/components/status/services.blade.php');
// Verify that the status component transforms :excluded suffix to (excluded) for better display
// Verify that the status component uses formatContainerStatus helper to display status
expect($servicesStatusFile)
->toContain('$isExcluded = str($complexStatus)->endsWith(\':excluded\');')
->toContain('$parts = explode(\':\', $complexStatus);')
->toContain('// Has health status: running:unhealthy:excluded → Running (unhealthy, excluded)')
->toContain('// No health status: exited:excluded → Exited (excluded)');
->toContain('formatContainerStatus($complexStatus)');
});
it('ensures UI handles excluded status in service heading buttons', function () {

View file

@ -73,7 +73,7 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
$service->shouldReceive('isStarting')->andReturn(false);
$app1 = makeResource('running:healthy', excludeFromStatus: false);
$app2 = makeResource('exited:unhealthy', excludeFromStatus: true);
$app2 = makeResource('exited', excludeFromStatus: true);
$service->shouldReceive('getAttribute')->with('applications')->andReturn(collect([$app1, $app2]));
$service->shouldReceive('getAttribute')->with('databases')->andReturn(collect());
@ -87,7 +87,7 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
$service->shouldReceive('isStarting')->andReturn(false);
$app1 = makeResource('running:healthy', excludeFromStatus: false);
$app2 = makeResource('exited:unhealthy', excludeFromStatus: false);
$app2 = makeResource('exited', excludeFromStatus: false);
$service->shouldReceive('getAttribute')->with('applications')->andReturn(collect([$app1, $app2]));
$service->shouldReceive('getAttribute')->with('databases')->andReturn(collect());
@ -224,7 +224,7 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
$service->shouldReceive('isStarting')->andReturn(false);
$app1 = makeResource('running:healthy', excludeFromStatus: false);
$app2 = makeResource('exited:unhealthy:excluded', excludeFromStatus: false);
$app2 = makeResource('exited:excluded', excludeFromStatus: false);
$service->shouldReceive('getAttribute')->with('applications')->andReturn(collect([$app1, $app2]));
$service->shouldReceive('getAttribute')->with('databases')->andReturn(collect());
@ -245,7 +245,7 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
expect($service->status)->toBe('running:healthy:excluded');
});
it('returns exited:unhealthy:excluded when excluded containers have no valid status', function () {
it('returns exited when excluded containers have no valid status', function () {
$service = Mockery::mock(Service::class)->makePartial();
$service->shouldReceive('isStarting')->andReturn(false);
@ -254,7 +254,7 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
$service->shouldReceive('getAttribute')->with('applications')->andReturn(collect([$app1]));
$service->shouldReceive('getAttribute')->with('databases')->andReturn(collect());
expect($service->status)->toBe('exited:unhealthy:excluded');
expect($service->status)->toBe('exited');
});
it('handles all excluded containers with degraded state', function () {
@ -262,7 +262,7 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
$service->shouldReceive('isStarting')->andReturn(false);
$app1 = makeResource('running:healthy', excludeFromStatus: true);
$app2 = makeResource('exited:unhealthy', excludeFromStatus: true);
$app2 = makeResource('exited', excludeFromStatus: true);
$service->shouldReceive('getAttribute')->with('applications')->andReturn(collect([$app1, $app2]));
$service->shouldReceive('getAttribute')->with('databases')->andReturn(collect());
@ -286,12 +286,12 @@ function makeResource(string $status, bool $excludeFromStatus = false): object
$service = Mockery::mock(Service::class)->makePartial();
$service->shouldReceive('isStarting')->andReturn(false);
$app1 = makeResource('exited:unhealthy', excludeFromStatus: false);
$app1 = makeResource('exited', excludeFromStatus: false);
$service->shouldReceive('getAttribute')->with('applications')->andReturn(collect([$app1]));
$service->shouldReceive('getAttribute')->with('databases')->andReturn(collect());
expect($service->status)->toBe('exited:unhealthy');
expect($service->status)->toBe('exited');
});
it('prefers running over starting status', function () {