fix: correct status for services with all containers excluded from health checks
When all containers are excluded from health checks, display their actual status
with :excluded suffix instead of misleading hardcoded statuses. This prevents
broken UI state with incorrect action buttons and provides clarity that monitoring
is disabled.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
f81640e316
commit
498b189286
7 changed files with 317 additions and 31 deletions
|
|
@ -98,11 +98,13 @@ public function handle(Server $server, ?Collection $containers = null, ?Collecti
|
|||
$labels = data_get($container, 'Config.Labels');
|
||||
}
|
||||
$containerStatus = data_get($container, 'State.Status');
|
||||
$containerHealth = data_get($container, 'State.Health.Status', 'unhealthy');
|
||||
$containerHealth = data_get($container, 'State.Health.Status');
|
||||
if ($containerStatus === 'restarting') {
|
||||
$containerStatus = "restarting ($containerHealth)";
|
||||
$healthSuffix = $containerHealth ?? 'unknown';
|
||||
$containerStatus = "restarting ($healthSuffix)";
|
||||
} else {
|
||||
$containerStatus = "$containerStatus ($containerHealth)";
|
||||
$healthSuffix = $containerHealth ?? 'unknown';
|
||||
$containerStatus = "$containerStatus ($healthSuffix)";
|
||||
}
|
||||
$labels = Arr::undot(format_docker_labels_to_json($labels));
|
||||
$applicationId = data_get($labels, 'coolify.applicationId');
|
||||
|
|
|
|||
|
|
@ -97,14 +97,14 @@ private function aggregateContainerStatuses($application, $containers)
|
|||
|
||||
$relevantContainerCount++;
|
||||
$containerStatus = data_get($container, 'State.Status');
|
||||
$containerHealth = data_get($container, 'State.Health.Status', 'unhealthy');
|
||||
$containerHealth = data_get($container, 'State.Health.Status');
|
||||
|
||||
if ($containerStatus === 'restarting') {
|
||||
$hasRestarting = true;
|
||||
$hasUnhealthy = true;
|
||||
} elseif ($containerStatus === 'running') {
|
||||
$hasRunning = true;
|
||||
if ($containerHealth === 'unhealthy') {
|
||||
if ($containerHealth && $containerHealth === 'unhealthy') {
|
||||
$hasUnhealthy = true;
|
||||
}
|
||||
} elseif ($containerStatus === 'exited') {
|
||||
|
|
@ -113,8 +113,53 @@ private function aggregateContainerStatuses($application, $containers)
|
|||
}
|
||||
}
|
||||
|
||||
// If all containers are excluded, calculate status from excluded containers
|
||||
// but mark it with :excluded to indicate monitoring is disabled
|
||||
if ($relevantContainerCount === 0) {
|
||||
return 'exited:healthy';
|
||||
$excludedHasRunning = false;
|
||||
$excludedHasRestarting = false;
|
||||
$excludedHasUnhealthy = false;
|
||||
$excludedHasExited = false;
|
||||
|
||||
foreach ($containers as $container) {
|
||||
$labels = data_get($container, 'Config.Labels', []);
|
||||
$serviceName = data_get($labels, 'com.docker.compose.service');
|
||||
|
||||
// Only process excluded containers
|
||||
if (! $serviceName || ! $excludedContainers->contains($serviceName)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$containerStatus = data_get($container, 'State.Status');
|
||||
$containerHealth = data_get($container, 'State.Health.Status');
|
||||
|
||||
if ($containerStatus === 'restarting') {
|
||||
$excludedHasRestarting = true;
|
||||
$excludedHasUnhealthy = true;
|
||||
} elseif ($containerStatus === 'running') {
|
||||
$excludedHasRunning = true;
|
||||
if ($containerHealth && $containerHealth === 'unhealthy') {
|
||||
$excludedHasUnhealthy = true;
|
||||
}
|
||||
} elseif ($containerStatus === 'exited') {
|
||||
$excludedHasExited = true;
|
||||
$excludedHasUnhealthy = true;
|
||||
}
|
||||
}
|
||||
|
||||
if ($excludedHasRestarting) {
|
||||
return 'degraded:excluded';
|
||||
}
|
||||
|
||||
if ($excludedHasRunning && $excludedHasExited) {
|
||||
return 'degraded:excluded';
|
||||
}
|
||||
|
||||
if ($excludedHasRunning) {
|
||||
return 'running:excluded';
|
||||
}
|
||||
|
||||
return 'exited:excluded';
|
||||
}
|
||||
|
||||
if ($hasRestarting) {
|
||||
|
|
|
|||
|
|
@ -184,11 +184,13 @@ public function getStatusAttribute()
|
|||
|
||||
$complexStatus = null;
|
||||
$complexHealth = null;
|
||||
$hasNonExcluded = false;
|
||||
|
||||
foreach ($applications as $application) {
|
||||
if ($application->exclude_from_status) {
|
||||
continue;
|
||||
}
|
||||
$hasNonExcluded = true;
|
||||
$status = str($application->status)->before('(')->trim();
|
||||
$health = str($application->status)->between('(', ')')->trim();
|
||||
if ($complexStatus === 'degraded') {
|
||||
|
|
@ -218,6 +220,7 @@ public function getStatusAttribute()
|
|||
if ($database->exclude_from_status) {
|
||||
continue;
|
||||
}
|
||||
$hasNonExcluded = true;
|
||||
$status = str($database->status)->before('(')->trim();
|
||||
$health = str($database->status)->between('(', ')')->trim();
|
||||
if ($complexStatus === 'degraded') {
|
||||
|
|
@ -244,9 +247,78 @@ public function getStatusAttribute()
|
|||
}
|
||||
}
|
||||
|
||||
// If all services are excluded from status checks, return a default exited status
|
||||
if ($complexStatus === null && $complexHealth === null) {
|
||||
return 'exited:healthy';
|
||||
// If all services are excluded from status checks, calculate status from excluded containers
|
||||
// but mark it with :excluded to indicate monitoring is disabled
|
||||
if (! $hasNonExcluded && ($complexStatus === null && $complexHealth === null)) {
|
||||
$excludedStatus = null;
|
||||
$excludedHealth = null;
|
||||
|
||||
// Calculate status from excluded containers
|
||||
foreach ($applications as $application) {
|
||||
$status = str($application->status)->before('(')->trim();
|
||||
$health = str($application->status)->between('(', ')')->trim();
|
||||
if ($excludedStatus === 'degraded') {
|
||||
continue;
|
||||
}
|
||||
if ($status->startsWith('running')) {
|
||||
if ($excludedStatus === 'exited') {
|
||||
$excludedStatus = 'degraded';
|
||||
} else {
|
||||
$excludedStatus = 'running';
|
||||
}
|
||||
} elseif ($status->startsWith('restarting')) {
|
||||
$excludedStatus = 'degraded';
|
||||
} elseif ($status->startsWith('exited')) {
|
||||
$excludedStatus = 'exited';
|
||||
}
|
||||
if ($health->value() === 'healthy') {
|
||||
if ($excludedHealth === 'unhealthy') {
|
||||
continue;
|
||||
}
|
||||
$excludedHealth = 'healthy';
|
||||
} else {
|
||||
$excludedHealth = 'unhealthy';
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($databases as $database) {
|
||||
$status = str($database->status)->before('(')->trim();
|
||||
$health = str($database->status)->between('(', ')')->trim();
|
||||
if ($excludedStatus === 'degraded') {
|
||||
continue;
|
||||
}
|
||||
if ($status->startsWith('running')) {
|
||||
if ($excludedStatus === 'exited') {
|
||||
$excludedStatus = 'degraded';
|
||||
} else {
|
||||
$excludedStatus = 'running';
|
||||
}
|
||||
} elseif ($status->startsWith('restarting')) {
|
||||
$excludedStatus = 'degraded';
|
||||
} elseif ($status->startsWith('exited')) {
|
||||
$excludedStatus = 'exited';
|
||||
}
|
||||
if ($health->value() === 'healthy') {
|
||||
if ($excludedHealth === 'unhealthy') {
|
||||
continue;
|
||||
}
|
||||
$excludedHealth = 'healthy';
|
||||
} else {
|
||||
$excludedHealth = 'unhealthy';
|
||||
}
|
||||
}
|
||||
|
||||
// Return status with :excluded suffix to indicate monitoring is disabled
|
||||
if ($excludedStatus && $excludedHealth) {
|
||||
return "{$excludedStatus}:excluded";
|
||||
}
|
||||
|
||||
// If no status was calculated at all (no containers exist), return unknown
|
||||
if ($excludedStatus === null && $excludedHealth === null) {
|
||||
return 'unknown:excluded';
|
||||
}
|
||||
|
||||
return 'exited:excluded';
|
||||
}
|
||||
|
||||
return "{$complexStatus}:{$complexHealth}";
|
||||
|
|
|
|||
|
|
@ -1,13 +1,20 @@
|
|||
@if (str($complexStatus)->contains('running'))
|
||||
<x-status.running :status="$complexStatus" />
|
||||
@elseif(str($complexStatus)->contains('starting'))
|
||||
<x-status.restarting :status="$complexStatus" />
|
||||
@elseif(str($complexStatus)->contains('restarting'))
|
||||
<x-status.restarting :status="$complexStatus" />
|
||||
@elseif(str($complexStatus)->contains('degraded'))
|
||||
<x-status.degraded :status="$complexStatus" />
|
||||
@php
|
||||
$isExcluded = str($complexStatus)->endsWith(':excluded');
|
||||
$displayStatus = $isExcluded ? str($complexStatus)->beforeLast(':excluded') : $complexStatus;
|
||||
@endphp
|
||||
@if (str($displayStatus)->contains('running'))
|
||||
<x-status.running :status="$displayStatus" />
|
||||
@elseif(str($displayStatus)->contains('starting'))
|
||||
<x-status.restarting :status="$displayStatus" />
|
||||
@elseif(str($displayStatus)->contains('restarting'))
|
||||
<x-status.restarting :status="$displayStatus" />
|
||||
@elseif(str($displayStatus)->contains('degraded'))
|
||||
<x-status.degraded :status="$displayStatus" />
|
||||
@else
|
||||
<x-status.stopped :status="$complexStatus" />
|
||||
<x-status.stopped :status="$displayStatus" />
|
||||
@endif
|
||||
@if ($isExcluded)
|
||||
<span class="text-xs text-neutral-500 dark:text-neutral-400" title="All containers excluded from health monitoring">(Monitoring Disabled)</span>
|
||||
@endif
|
||||
@if (!str($complexStatus)->contains('exited') && $showRefreshButton)
|
||||
<button wire:loading.remove.delay.shortest wire:target="manualCheckStatus" title="Refresh Status" wire:click='manualCheckStatus'
|
||||
|
|
|
|||
|
|
@ -29,12 +29,12 @@
|
|||
@if ($service->isDeployable)
|
||||
<div class="flex flex-wrap order-first gap-2 items-center sm:order-last">
|
||||
<x-services.advanced :service="$service" />
|
||||
@if (str($service->status)->contains('running'))
|
||||
@if (str($service->status)->contains('running') || (str($service->status)->startsWith('running:') && !str($service->status)->contains('exited')))
|
||||
<x-forms.button title="Restart" @click="$wire.dispatch('restartEvent')">
|
||||
<svg class="w-5 h-5 dark:text-warning" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
|
||||
<g fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round"
|
||||
stroke-width="2">
|
||||
<path d="M19.933 13.041a8 8 0 1 1-9.925-8.788c3.899-1 7.935 1.007 9.425 4.747" />
|
||||
<path d="M19.933 13.041 a8 8 0 1 1-9.925-8.788c3.899-1 7.935 1.007 9.425 4.747" />
|
||||
<path d="M20 4v5h-5" />
|
||||
</g>
|
||||
</svg>
|
||||
|
|
|
|||
116
tests/Unit/ContainerHealthStatusTest.php
Normal file
116
tests/Unit/ContainerHealthStatusTest.php
Normal file
|
|
@ -0,0 +1,116 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use Mockery;
|
||||
|
||||
/**
|
||||
* Unit tests to verify that containers without health checks are not
|
||||
* incorrectly marked as unhealthy.
|
||||
*
|
||||
* This tests the fix for the issue where defaulting missing health status
|
||||
* to 'unhealthy' would treat containers without healthchecks as unhealthy.
|
||||
*
|
||||
* The fix removes the 'unhealthy' default and only checks health status
|
||||
* when it explicitly exists and equals 'unhealthy'.
|
||||
*/
|
||||
it('does not mark containers as unhealthy when health status is missing', function () {
|
||||
// Mock an application with a server
|
||||
$application = Mockery::mock(Application::class)->makePartial();
|
||||
$server = Mockery::mock('App\Models\Server')->makePartial();
|
||||
$destination = Mockery::mock('App\Models\StandaloneDocker')->makePartial();
|
||||
|
||||
$destination->shouldReceive('getAttribute')
|
||||
->with('server')
|
||||
->andReturn($server);
|
||||
|
||||
$application->shouldReceive('getAttribute')
|
||||
->with('destination')
|
||||
->andReturn($destination);
|
||||
|
||||
$application->shouldReceive('getAttribute')
|
||||
->with('additional_servers')
|
||||
->andReturn(collect());
|
||||
|
||||
$server->shouldReceive('getAttribute')
|
||||
->with('id')
|
||||
->andReturn(1);
|
||||
|
||||
$server->shouldReceive('isFunctional')
|
||||
->andReturn(true);
|
||||
|
||||
// Create a container without health check (State.Health.Status is null)
|
||||
$containerWithoutHealthCheck = [
|
||||
'Config' => [
|
||||
'Labels' => [
|
||||
'com.docker.compose.service' => 'web',
|
||||
],
|
||||
],
|
||||
'State' => [
|
||||
'Status' => 'running',
|
||||
// Note: State.Health.Status is intentionally missing
|
||||
],
|
||||
];
|
||||
|
||||
// Mock the remote process to return our container
|
||||
$application->shouldReceive('getAttribute')
|
||||
->with('id')
|
||||
->andReturn(123);
|
||||
|
||||
// We can't easily test the private aggregateContainerStatuses method directly,
|
||||
// but we can verify that the code doesn't default to 'unhealthy'
|
||||
$complexStatusCheckFile = file_get_contents(__DIR__.'/../../app/Actions/Shared/ComplexStatusCheck.php');
|
||||
|
||||
// Verify the fix: health status should not default to 'unhealthy'
|
||||
expect($complexStatusCheckFile)
|
||||
->not->toContain("data_get(\$container, 'State.Health.Status', 'unhealthy')")
|
||||
->toContain("data_get(\$container, 'State.Health.Status')");
|
||||
|
||||
// Verify the null check exists for non-excluded containers
|
||||
expect($complexStatusCheckFile)
|
||||
->toContain('if ($containerHealth && $containerHealth === \'unhealthy\') {');
|
||||
});
|
||||
|
||||
it('only marks containers as unhealthy when health status explicitly equals unhealthy', function () {
|
||||
$complexStatusCheckFile = file_get_contents(__DIR__.'/../../app/Actions/Shared/ComplexStatusCheck.php');
|
||||
|
||||
// For non-excluded containers (line ~107)
|
||||
expect($complexStatusCheckFile)
|
||||
->toContain('if ($containerHealth && $containerHealth === \'unhealthy\') {')
|
||||
->toContain('$hasUnhealthy = true;');
|
||||
|
||||
// For excluded containers (line ~141)
|
||||
expect($complexStatusCheckFile)
|
||||
->toContain('if ($containerHealth && $containerHealth === \'unhealthy\') {')
|
||||
->toContain('$excludedHasUnhealthy = true;');
|
||||
});
|
||||
|
||||
it('handles missing health status correctly in GetContainersStatus', function () {
|
||||
$getContainersStatusFile = file_get_contents(__DIR__.'/../../app/Actions/Docker/GetContainersStatus.php');
|
||||
|
||||
// Verify health status doesn't default to 'unhealthy'
|
||||
expect($getContainersStatusFile)
|
||||
->not->toContain("data_get(\$container, 'State.Health.Status', 'unhealthy')")
|
||||
->toContain("data_get(\$container, 'State.Health.Status')");
|
||||
|
||||
// Verify it uses 'unknown' when health status is missing
|
||||
expect($getContainersStatusFile)
|
||||
->toContain('$healthSuffix = $containerHealth ?? \'unknown\';');
|
||||
});
|
||||
|
||||
it('treats containers with running status and no healthcheck as not unhealthy', function () {
|
||||
$complexStatusCheckFile = file_get_contents(__DIR__.'/../../app/Actions/Shared/ComplexStatusCheck.php');
|
||||
|
||||
// The logic should be:
|
||||
// 1. Get health status (may be null)
|
||||
// 2. Only mark as unhealthy if health status EXISTS and equals 'unhealthy'
|
||||
// 3. Don't mark as unhealthy if health status is null/missing
|
||||
|
||||
// Verify the condition requires both health to exist AND be unhealthy
|
||||
expect($complexStatusCheckFile)
|
||||
->toContain('if ($containerHealth && $containerHealth === \'unhealthy\')');
|
||||
|
||||
// Verify this check is done for running containers
|
||||
expect($complexStatusCheckFile)
|
||||
->toContain('} elseif ($containerStatus === \'running\') {')
|
||||
->toContain('$hasRunning = true;');
|
||||
});
|
||||
|
|
@ -5,27 +5,49 @@
|
|||
* excluded from health checks (exclude_from_hc: true) show correct status.
|
||||
*
|
||||
* These tests verify the fix for the issue where services with all containers
|
||||
* excluded would show incorrect "running:healthy" or ":" status, causing
|
||||
* broken UI state with active start/stop buttons.
|
||||
* excluded would show incorrect status, causing broken UI state.
|
||||
*
|
||||
* The fix now returns status with :excluded suffix to show real container state
|
||||
* while indicating monitoring is disabled (e.g., "running:excluded").
|
||||
*/
|
||||
it('ensures ComplexStatusCheck returns exited status when all containers excluded', function () {
|
||||
it('ensures ComplexStatusCheck returns excluded status when all containers excluded', function () {
|
||||
$complexStatusCheckFile = file_get_contents(__DIR__.'/../../app/Actions/Shared/ComplexStatusCheck.php');
|
||||
|
||||
// Check that when all containers are excluded (relevantContainerCount === 0),
|
||||
// the status is set to 'exited:healthy' instead of 'running:healthy'
|
||||
// Check that when all containers are excluded, the status calculation
|
||||
// processes excluded containers and returns status with :excluded suffix
|
||||
expect($complexStatusCheckFile)
|
||||
->toContain("if (\$relevantContainerCount === 0) {\n return 'exited:healthy';\n }")
|
||||
->not->toContain("if (\$relevantContainerCount === 0) {\n return 'running:healthy';\n }");
|
||||
->toContain('// If all containers are excluded, calculate status from excluded containers')
|
||||
->toContain('// but mark it with :excluded to indicate monitoring is disabled')
|
||||
->toContain('if ($relevantContainerCount === 0) {')
|
||||
->toContain("return 'running:excluded';")
|
||||
->toContain("return 'degraded:excluded';")
|
||||
->toContain("return 'exited:excluded';");
|
||||
});
|
||||
|
||||
it('ensures Service model returns exited status when all services excluded', function () {
|
||||
it('ensures Service model returns excluded status when all services excluded', function () {
|
||||
$serviceModelFile = file_get_contents(__DIR__.'/../../app/Models/Service.php');
|
||||
|
||||
// Check that when all services are excluded from status checks,
|
||||
// the Service model returns 'exited:healthy' instead of ':' (null:null)
|
||||
// the Service model calculates real status and returns it with :excluded suffix
|
||||
expect($serviceModelFile)
|
||||
->toContain('// If all services are excluded from status checks, return a default exited status')
|
||||
->toContain("if (\$complexStatus === null && \$complexHealth === null) {\n return 'exited:healthy';\n }");
|
||||
->toContain('// If all services are excluded from status checks, calculate status from excluded containers')
|
||||
->toContain('// but mark it with :excluded to indicate monitoring is disabled')
|
||||
->toContain('if (! $hasNonExcluded && ($complexStatus === null && $complexHealth === null)) {')
|
||||
->toContain('// Calculate status from excluded containers')
|
||||
->toContain('return "{$excludedStatus}:excluded";')
|
||||
->toContain("return 'exited:excluded';");
|
||||
});
|
||||
|
||||
it('ensures Service model returns unknown:excluded when no containers exist', function () {
|
||||
$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:excluded' instead of 'exited:excluded'
|
||||
// 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')
|
||||
->toContain('if ($excludedStatus === null && $excludedHealth === null) {')
|
||||
->toContain("return 'unknown:excluded';");
|
||||
});
|
||||
|
||||
it('ensures GetContainersStatus returns null when all containers excluded', function () {
|
||||
|
|
@ -57,3 +79,25 @@
|
|||
->toContain('if ($excludeFromHc || $restartPolicy === \'no\') {')
|
||||
->toContain('$excludedContainers->push($serviceName);');
|
||||
});
|
||||
|
||||
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 detects :excluded suffix and shows monitoring disabled message
|
||||
expect($servicesStatusFile)
|
||||
->toContain('$isExcluded = str($complexStatus)->endsWith(\':excluded\');')
|
||||
->toContain('$displayStatus = $isExcluded ? str($complexStatus)->beforeLast(\':excluded\') : $complexStatus;')
|
||||
->toContain('(Monitoring Disabled)');
|
||||
});
|
||||
|
||||
it('ensures UI handles excluded status in service heading buttons', function () {
|
||||
$headingFile = file_get_contents(__DIR__.'/../../resources/views/livewire/project/service/heading.blade.php');
|
||||
|
||||
// Verify that the heading properly handles running/degraded/exited status with :excluded suffix
|
||||
// The logic should use contains() to match the base status (running, degraded, exited)
|
||||
// which will work for both regular statuses and :excluded suffixed ones
|
||||
expect($headingFile)
|
||||
->toContain('str($service->status)->contains(\'running\')')
|
||||
->toContain('str($service->status)->contains(\'degraded\')')
|
||||
->toContain('str($service->status)->contains(\'exited\')');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue