From ac9eca3c051e15a52e1ab5f655d3011a21c9b112 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 24 Nov 2025 09:09:08 +0100 Subject: [PATCH] fix: don't show health status for exited containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exited containers don't run health checks, so showing "(unhealthy)" is misleading. This fix ensures exited status displays without health suffixes across all monitoring systems (SSH, Sentinel, services, etc.) and at the UI layer for backward compatibility with existing data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- app/Actions/Docker/GetContainersStatus.php | 4 ++- app/Actions/Shared/ComplexStatusCheck.php | 8 +++--- app/Jobs/PushServerUpdateJob.php | 5 +++- app/Models/Service.php | 7 ++++- app/Services/ContainerStatusAggregator.php | 10 +++---- app/Traits/CalculatesExcludedStatus.php | 2 +- .../views/components/status/stopped.blade.php | 16 +++++++++-- .../AllExcludedContainersConsistencyTest.php | 6 ++-- tests/Unit/ContainerStatusAggregatorTest.php | 28 +++++++++---------- tests/Unit/ExcludeFromHealthCheckTest.php | 11 +++----- tests/Unit/ServiceExcludedStatusTest.php | 16 +++++------ 11 files changed, 65 insertions(+), 48 deletions(-) diff --git a/app/Actions/Docker/GetContainersStatus.php b/app/Actions/Docker/GetContainersStatus.php index 98302f98e..61a3c4615 100644 --- a/app/Actions/Docker/GetContainersStatus.php +++ b/app/Actions/Docker/GetContainersStatus.php @@ -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([ diff --git a/app/Actions/Shared/ComplexStatusCheck.php b/app/Actions/Shared/ComplexStatusCheck.php index 588cca4ac..3649be986 100644 --- a/app/Actions/Shared/ComplexStatusCheck.php +++ b/app/Actions/Shared/ComplexStatusCheck.php @@ -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; } diff --git a/app/Jobs/PushServerUpdateJob.php b/app/Jobs/PushServerUpdateJob.php index 9f81155be..8786d8a18 100644 --- a/app/Jobs/PushServerUpdateJob.php +++ b/app/Jobs/PushServerUpdateJob.php @@ -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'); diff --git a/app/Models/Service.php b/app/Models/Service.php index af27070c7..2f8a64464 100644 --- a/app/Models/Service.php +++ b/app/Models/Service.php @@ -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}"; diff --git a/app/Services/ContainerStatusAggregator.php b/app/Services/ContainerStatusAggregator.php index 402a1f202..4a17ecdd6 100644 --- a/app/Services/ContainerStatusAggregator.php +++ b/app/Services/ContainerStatusAggregator.php @@ -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'; } } diff --git a/app/Traits/CalculatesExcludedStatus.php b/app/Traits/CalculatesExcludedStatus.php index 9cbc6a86b..5219878c0 100644 --- a/app/Traits/CalculatesExcludedStatus.php +++ b/app/Traits/CalculatesExcludedStatus.php @@ -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" diff --git a/resources/views/components/status/stopped.blade.php b/resources/views/components/status/stopped.blade.php index 7d97301f3..0af349d40 100644 --- a/resources/views/components/status/stopped.blade.php +++ b/resources/views/components/status/stopped.blade.php @@ -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(); diff --git a/tests/Unit/AllExcludedContainersConsistencyTest.php b/tests/Unit/AllExcludedContainersConsistencyTest.php index 73827702a..bdab6e145 100644 --- a/tests/Unit/AllExcludedContainersConsistencyTest.php +++ b/tests/Unit/AllExcludedContainersConsistencyTest.php @@ -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 }); diff --git a/tests/Unit/ContainerStatusAggregatorTest.php b/tests/Unit/ContainerStatusAggregatorTest.php index 39fd82b8e..353d6a948 100644 --- a/tests/Unit/ContainerStatusAggregatorTest.php +++ b/tests/Unit/ContainerStatusAggregatorTest.php @@ -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'); }); }); diff --git a/tests/Unit/ExcludeFromHealthCheckTest.php b/tests/Unit/ExcludeFromHealthCheckTest.php index 8046d77e3..6776d09b7 100644 --- a/tests/Unit/ExcludeFromHealthCheckTest.php +++ b/tests/Unit/ExcludeFromHealthCheckTest.php @@ -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 () { diff --git a/tests/Unit/ServiceExcludedStatusTest.php b/tests/Unit/ServiceExcludedStatusTest.php index 693131d34..80691190f 100644 --- a/tests/Unit/ServiceExcludedStatusTest.php +++ b/tests/Unit/ServiceExcludedStatusTest.php @@ -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 () {