mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-04 10:42:23 +00:00
fix(healthcheck): read the Dockerfile HEALTHCHECK only from the built stage
Closes #9475 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6497cc73a6
commit
fcf92b751f
3 files changed
+170
-78
No files matched your search
+20
-39
@@ -2850,8 +2850,8 @@ class Application extends BaseModel
|
||||
|
||||
public function parseHealthcheckFromDockerfile($dockerfile, bool $isInit = false)
|
||||
{
|
||||
$dockerfile = str($dockerfile)->trim()->explode("\n");
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
$healthcheckCommand = dockerfileHealthcheckInstruction($dockerfile, $this->dockerfile_target_build);
|
||||
$hasHealthcheck = $healthcheckCommand !== null;
|
||||
|
||||
// Always check if healthcheck was removed, regardless of health_check_enabled setting
|
||||
if (! $hasHealthcheck && $this->custom_healthcheck_found) {
|
||||
@@ -2867,45 +2867,26 @@ class Application extends BaseModel
|
||||
}
|
||||
|
||||
if ($hasHealthcheck && ($this->isHealthcheckDisabled() || $isInit)) {
|
||||
$healthcheckCommand = null;
|
||||
$lines = $dockerfile->toArray();
|
||||
foreach ($lines as $line) {
|
||||
$trimmedLine = trim($line);
|
||||
if (str_starts_with($trimmedLine, 'HEALTHCHECK')) {
|
||||
$healthcheckCommand .= trim($trimmedLine, '\\ ');
|
||||
$interval = str($healthcheckCommand)->match('/--interval=([0-9]+[a-zµ]*)/');
|
||||
$timeout = str($healthcheckCommand)->match('/--timeout=([0-9]+[a-zµ]*)/');
|
||||
$start_period = str($healthcheckCommand)->match('/--start-period=([0-9]+[a-zµ]*)/');
|
||||
$retries = str($healthcheckCommand)->match('/--retries=(\d+)/');
|
||||
|
||||
continue;
|
||||
}
|
||||
if (isset($healthcheckCommand) && str_contains($trimmedLine, '\\')) {
|
||||
$healthcheckCommand .= ' '.trim($trimmedLine, '\\ ');
|
||||
}
|
||||
if (isset($healthcheckCommand) && ! str_contains($trimmedLine, '\\') && ! empty($healthcheckCommand)) {
|
||||
$healthcheckCommand .= ' '.$trimmedLine;
|
||||
break;
|
||||
}
|
||||
if ($interval->isNotEmpty()) {
|
||||
$this->health_check_interval = parseDockerfileInterval($interval);
|
||||
}
|
||||
if (str($healthcheckCommand)->isNotEmpty()) {
|
||||
$interval = str($healthcheckCommand)->match('/--interval=([0-9]+[a-zµ]*)/');
|
||||
$timeout = str($healthcheckCommand)->match('/--timeout=([0-9]+[a-zµ]*)/');
|
||||
$start_period = str($healthcheckCommand)->match('/--start-period=([0-9]+[a-zµ]*)/');
|
||||
$retries = str($healthcheckCommand)->match('/--retries=(\d+)/');
|
||||
|
||||
if ($interval->isNotEmpty()) {
|
||||
$this->health_check_interval = parseDockerfileInterval($interval);
|
||||
}
|
||||
if ($timeout->isNotEmpty()) {
|
||||
$this->health_check_timeout = parseDockerfileInterval($timeout);
|
||||
}
|
||||
if ($start_period->isNotEmpty()) {
|
||||
$this->health_check_start_period = parseDockerfileInterval($start_period);
|
||||
}
|
||||
if ($retries->isNotEmpty()) {
|
||||
$this->health_check_retries = $retries->toInteger();
|
||||
}
|
||||
if ($interval || $timeout || $start_period || $retries) {
|
||||
$this->custom_healthcheck_found = true;
|
||||
$this->save();
|
||||
}
|
||||
if ($timeout->isNotEmpty()) {
|
||||
$this->health_check_timeout = parseDockerfileInterval($timeout);
|
||||
}
|
||||
if ($start_period->isNotEmpty()) {
|
||||
$this->health_check_start_period = parseDockerfileInterval($start_period);
|
||||
}
|
||||
if ($retries->isNotEmpty()) {
|
||||
$this->health_check_retries = $retries->toInteger();
|
||||
}
|
||||
if ($interval || $timeout || $start_period || $retries) {
|
||||
$this->custom_healthcheck_found = true;
|
||||
$this->save();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5126,6 +5126,75 @@ function getJobStatus(?string $jobId = null)
|
||||
return $jobFound->first()->status;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the HEALTHCHECK instruction that applies to the image built from the Dockerfile.
|
||||
*
|
||||
* Docker builds the target stage, or the last stage when no target is set. A stage keeps the
|
||||
* HEALTHCHECK of an earlier stage that it starts FROM, and the last HEALTHCHECK of a stage wins.
|
||||
* Returns null when no HEALTHCHECK applies or when it is HEALTHCHECK NONE.
|
||||
*/
|
||||
function dockerfileHealthcheckInstruction(?string $dockerfile, ?string $target = null): ?string
|
||||
{
|
||||
$instructions = [];
|
||||
$current = '';
|
||||
foreach (preg_split('/\R/', (string) $dockerfile) as $line) {
|
||||
$line = trim($line);
|
||||
if ($line === '' || str_starts_with($line, '#')) {
|
||||
continue;
|
||||
}
|
||||
$continues = str_ends_with($line, '\\');
|
||||
$current = trim($current.' '.rtrim($line, '\\ '));
|
||||
if (! $continues) {
|
||||
$instructions[] = $current;
|
||||
$current = '';
|
||||
}
|
||||
}
|
||||
if ($current !== '') {
|
||||
$instructions[] = $current;
|
||||
}
|
||||
|
||||
/** @var array<int, array{name: ?string, parent: string, healthcheck: ?string}> $stages */
|
||||
$stages = [];
|
||||
foreach ($instructions as $instruction) {
|
||||
if (preg_match('/^FROM\s+(?:--\S+\s+)*(\S+)(?:\s+AS\s+(\S+))?/i', $instruction, $matches)) {
|
||||
$stages[] = [
|
||||
'name' => isset($matches[2]) ? strtolower($matches[2]) : null,
|
||||
'parent' => strtolower($matches[1]),
|
||||
'healthcheck' => null,
|
||||
];
|
||||
} elseif ($stages !== [] && preg_match('/^HEALTHCHECK(\s|$)/i', $instruction)) {
|
||||
$stages[array_key_last($stages)]['healthcheck'] = $instruction;
|
||||
}
|
||||
}
|
||||
if ($stages === []) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$target = filled($target) ? strtolower(trim($target)) : null;
|
||||
$index = $target === null ? array_key_last($stages) : null;
|
||||
foreach ($stages as $i => $stage) {
|
||||
if ($target !== null && $stage['name'] === $target) {
|
||||
$index = $i;
|
||||
}
|
||||
}
|
||||
|
||||
while ($index !== null) {
|
||||
$stage = $stages[$index];
|
||||
if ($stage['healthcheck'] !== null) {
|
||||
return preg_match('/^HEALTHCHECK\s+NONE$/i', $stage['healthcheck']) ? null : $stage['healthcheck'];
|
||||
}
|
||||
$parentIndex = null;
|
||||
foreach ($stages as $i => $candidate) {
|
||||
if ($i < $index && $candidate['name'] === $stage['parent']) {
|
||||
$parentIndex = $i;
|
||||
}
|
||||
}
|
||||
$index = $parentIndex;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
function parseDockerfileInterval(string $something)
|
||||
{
|
||||
$value = preg_replace('/[^0-9]/', '', $something);
|
||||
|
||||
@@ -1,57 +1,99 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* Tests for parseHealthcheckFromDockerfile method
|
||||
*
|
||||
* NOTE: These tests verify the logic for detecting when a HEALTHCHECK directive
|
||||
* is removed from a Dockerfile. The fix ensures that healthcheck removal is detected
|
||||
* regardless of the health_check_enabled setting.
|
||||
*/
|
||||
|
||||
use App\Models\Application;
|
||||
|
||||
it('detects when HEALTHCHECK is removed from dockerfile', function () {
|
||||
// This test verifies the fix for the bug where Coolify doesn't detect
|
||||
// when a HEALTHCHECK is removed from a Dockerfile, causing deployments to fail.
|
||||
function healthcheckApplication(array $attributes = []): Application
|
||||
{
|
||||
$application = Mockery::mock(Application::class)->makePartial();
|
||||
$application->shouldReceive('save')->andReturnTrue();
|
||||
$application->forceFill([
|
||||
'health_check_enabled' => false,
|
||||
'custom_healthcheck_found' => false,
|
||||
'health_check_interval' => 5,
|
||||
'health_check_timeout' => 5,
|
||||
'health_check_retries' => 10,
|
||||
'health_check_start_period' => 5,
|
||||
'dockerfile_target_build' => null,
|
||||
...$attributes,
|
||||
]);
|
||||
|
||||
$dockerfile = str("FROM nginx:latest\nCOPY . /app\nEXPOSE 80")->trim()->explode("\n");
|
||||
return $application;
|
||||
}
|
||||
|
||||
// The key fix: hasHealthcheck check happens BEFORE the isHealthcheckDisabled check
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
const MULTI_STAGE_DOCKERFILE = <<<'DOCKERFILE'
|
||||
FROM node:20-alpine AS builder
|
||||
RUN npm ci
|
||||
|
||||
// Simulate an application with custom_healthcheck_found = true
|
||||
$customHealthcheckFound = true;
|
||||
FROM node:20-alpine AS api
|
||||
COPY --from=builder /app/dist ./dist
|
||||
HEALTHCHECK --interval=30s --timeout=5s --start-period=15s --retries=3 \
|
||||
CMD node -e "require('http').get('http://localhost:3000/health')"
|
||||
CMD ["node", "dist/main.js"]
|
||||
|
||||
// The fixed logic: This condition should be true when HEALTHCHECK is removed
|
||||
$shouldReset = ! $hasHealthcheck && $customHealthcheckFound;
|
||||
# === Stage: worker ===
|
||||
FROM node:20-alpine AS worker
|
||||
COPY --from=builder /app/dist ./dist
|
||||
# No HEALTHCHECK - this stage has no HTTP server
|
||||
CMD ["node", "dist/worker.js"]
|
||||
DOCKERFILE;
|
||||
|
||||
expect($shouldReset)->toBeTrue()
|
||||
->and($hasHealthcheck)->toBeFalse()
|
||||
->and($customHealthcheckFound)->toBeTrue();
|
||||
it('resets the custom healthcheck when HEALTHCHECK is removed from the Dockerfile', function () {
|
||||
$application = healthcheckApplication(['custom_healthcheck_found' => true, 'health_check_interval' => 30]);
|
||||
|
||||
$application->parseHealthcheckFromDockerfile("FROM nginx:latest\nCOPY . /app\nEXPOSE 80");
|
||||
|
||||
expect($application->custom_healthcheck_found)->toBeFalse()
|
||||
->and($application->health_check_interval)->toBe(5);
|
||||
});
|
||||
|
||||
it('does not reset when HEALTHCHECK exists in dockerfile', function () {
|
||||
$dockerfile = str("FROM nginx:latest\nHEALTHCHECK --interval=30s CMD curl\nEXPOSE 80")->trim()->explode("\n");
|
||||
it('reads the HEALTHCHECK options of a single-stage Dockerfile', function () {
|
||||
$application = healthcheckApplication();
|
||||
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
$customHealthcheckFound = true;
|
||||
$application->parseHealthcheckFromDockerfile("FROM nginx:latest\nHEALTHCHECK --interval=30s --retries=4 \\\n CMD curl -f http://localhost/\nEXPOSE 80");
|
||||
|
||||
// When healthcheck exists, should not reset
|
||||
$shouldReset = ! $hasHealthcheck && $customHealthcheckFound;
|
||||
|
||||
expect($shouldReset)->toBeFalse()
|
||||
->and($hasHealthcheck)->toBeTrue();
|
||||
expect($application->custom_healthcheck_found)->toBeTrue()
|
||||
->and($application->health_check_interval)->toBe(30)
|
||||
->and($application->health_check_retries)->toBe(4);
|
||||
});
|
||||
|
||||
it('does not reset when custom_healthcheck_found is false', function () {
|
||||
$dockerfile = str("FROM nginx:latest\nCOPY . /app\nEXPOSE 80")->trim()->explode("\n");
|
||||
it('ignores a HEALTHCHECK of another stage than the build target', function () {
|
||||
$application = healthcheckApplication(['dockerfile_target_build' => 'worker']);
|
||||
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
$customHealthcheckFound = false;
|
||||
$application->parseHealthcheckFromDockerfile(MULTI_STAGE_DOCKERFILE);
|
||||
|
||||
// When custom_healthcheck_found is false, no need to reset
|
||||
$shouldReset = ! $hasHealthcheck && $customHealthcheckFound;
|
||||
|
||||
expect($shouldReset)->toBeFalse()
|
||||
->and($customHealthcheckFound)->toBeFalse();
|
||||
expect($application->custom_healthcheck_found)->toBeFalse()
|
||||
->and($application->health_check_interval)->toBe(5);
|
||||
});
|
||||
|
||||
it('resets a custom healthcheck found earlier when the build target has no HEALTHCHECK', function () {
|
||||
$application = healthcheckApplication(['dockerfile_target_build' => 'worker', 'custom_healthcheck_found' => true]);
|
||||
|
||||
$application->parseHealthcheckFromDockerfile(MULTI_STAGE_DOCKERFILE);
|
||||
|
||||
expect($application->custom_healthcheck_found)->toBeFalse();
|
||||
});
|
||||
|
||||
it('reads the HEALTHCHECK of the build target stage', function () {
|
||||
$application = healthcheckApplication(['dockerfile_target_build' => 'API']);
|
||||
|
||||
$application->parseHealthcheckFromDockerfile(MULTI_STAGE_DOCKERFILE);
|
||||
|
||||
expect($application->custom_healthcheck_found)->toBeTrue()
|
||||
->and($application->health_check_interval)->toBe(30)
|
||||
->and($application->health_check_start_period)->toBe(15)
|
||||
->and($application->health_check_retries)->toBe(3);
|
||||
});
|
||||
|
||||
it('finds the HEALTHCHECK that applies to the built image', function (string $dockerfile, ?string $target, ?string $expected) {
|
||||
expect(dockerfileHealthcheckInstruction($dockerfile, $target))->toBe($expected);
|
||||
})->with([
|
||||
'last stage when no target is set' => [MULTI_STAGE_DOCKERFILE, null, null],
|
||||
'HEALTHCHECK only in a builder stage' => ["FROM node AS build\nHEALTHCHECK CMD true\nFROM nginx\nCOPY --from=build /a /b", null, null],
|
||||
'inherited from a parent stage' => ["FROM node AS base\nHEALTHCHECK --interval=10s CMD true\nFROM base AS app\nCMD run", 'app', 'HEALTHCHECK --interval=10s CMD true'],
|
||||
'HEALTHCHECK NONE overrides the parent stage' => ["FROM node AS base\nHEALTHCHECK CMD true\nFROM base AS worker\nHEALTHCHECK NONE", 'worker', null],
|
||||
'last HEALTHCHECK of a stage wins' => ["FROM node\nHEALTHCHECK --interval=1s CMD a\nHEALTHCHECK --interval=2s CMD b", null, 'HEALTHCHECK --interval=2s CMD b'],
|
||||
'FROM with platform flag' => ["FROM --platform=linux/amd64 node AS app\nhealthcheck --retries=2 CMD true", 'app', 'healthcheck --retries=2 CMD true'],
|
||||
'target that does not exist' => ["FROM node AS app\nHEALTHCHECK CMD true", 'missing', null],
|
||||
'comment mentions HEALTHCHECK' => ["FROM node\n# HEALTHCHECK is not needed\nCMD run", null, null],
|
||||
'empty Dockerfile' => ['', null, null],
|
||||
]);
|
||||
Reference in new issue
Block a user