Merge pull request #6967 from coollabsio/healthcheck-removal-fix
Fix healthcheck removal detection
This commit is contained in:
commit
b0026b3be8
3 changed files with 87 additions and 19 deletions
|
|
@ -1632,7 +1632,7 @@ private function health_check()
|
|||
return;
|
||||
}
|
||||
if ($this->application->custom_healthcheck_found) {
|
||||
$this->application_deployment_queue->addLogEntry('Custom healthcheck found, skipping default healthcheck.');
|
||||
$this->application_deployment_queue->addLogEntry('Custom healthcheck found in Dockerfile.');
|
||||
}
|
||||
if ($this->container_name) {
|
||||
$counter = 1;
|
||||
|
|
@ -2358,16 +2358,22 @@ private function generate_compose_file()
|
|||
];
|
||||
// Always use .env file
|
||||
$docker_compose['services'][$this->container_name]['env_file'] = ['.env'];
|
||||
$docker_compose['services'][$this->container_name]['healthcheck'] = [
|
||||
'test' => [
|
||||
'CMD-SHELL',
|
||||
$this->generate_healthcheck_commands(),
|
||||
],
|
||||
'interval' => $this->application->health_check_interval.'s',
|
||||
'timeout' => $this->application->health_check_timeout.'s',
|
||||
'retries' => $this->application->health_check_retries,
|
||||
'start_period' => $this->application->health_check_start_period.'s',
|
||||
];
|
||||
|
||||
// Only add Coolify healthcheck if no custom HEALTHCHECK found in Dockerfile
|
||||
// If custom_healthcheck_found is true, the Dockerfile's HEALTHCHECK will be used
|
||||
// If healthcheck is disabled, no healthcheck will be added
|
||||
if (! $this->application->custom_healthcheck_found && ! $this->application->isHealthcheckDisabled()) {
|
||||
$docker_compose['services'][$this->container_name]['healthcheck'] = [
|
||||
'test' => [
|
||||
'CMD-SHELL',
|
||||
$this->generate_healthcheck_commands(),
|
||||
],
|
||||
'interval' => $this->application->health_check_interval.'s',
|
||||
'timeout' => $this->application->health_check_timeout.'s',
|
||||
'retries' => $this->application->health_check_retries,
|
||||
'start_period' => $this->application->health_check_start_period.'s',
|
||||
];
|
||||
}
|
||||
|
||||
if (! is_null($this->application->limits_cpuset)) {
|
||||
data_set($docker_compose, 'services.'.$this->container_name.'.cpuset', $this->application->limits_cpuset);
|
||||
|
|
|
|||
|
|
@ -1806,6 +1806,19 @@ public function parseHealthcheckFromDockerfile($dockerfile, bool $isInit = false
|
|||
$dockerfile = str($dockerfile)->trim()->explode("\n");
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
|
||||
// Always check if healthcheck was removed, regardless of health_check_enabled setting
|
||||
if (! $hasHealthcheck && $this->custom_healthcheck_found) {
|
||||
// HEALTHCHECK was removed from Dockerfile, reset to defaults
|
||||
$this->custom_healthcheck_found = false;
|
||||
$this->health_check_interval = 5;
|
||||
$this->health_check_timeout = 5;
|
||||
$this->health_check_retries = 10;
|
||||
$this->health_check_start_period = 5;
|
||||
$this->save();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
if ($hasHealthcheck && ($this->isHealthcheckDisabled() || $isInit)) {
|
||||
$healthcheckCommand = null;
|
||||
$lines = $dockerfile->toArray();
|
||||
|
|
@ -1847,14 +1860,6 @@ public function parseHealthcheckFromDockerfile($dockerfile, bool $isInit = false
|
|||
$this->save();
|
||||
}
|
||||
}
|
||||
} elseif (! $hasHealthcheck && $this->custom_healthcheck_found) {
|
||||
// HEALTHCHECK was removed from Dockerfile, reset to defaults
|
||||
$this->custom_healthcheck_found = false;
|
||||
$this->health_check_interval = 5;
|
||||
$this->health_check_timeout = 5;
|
||||
$this->health_check_retries = 10;
|
||||
$this->health_check_start_period = 5;
|
||||
$this->save();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
57
tests/Unit/ApplicationHealthcheckRemovalTest.php
Normal file
57
tests/Unit/ApplicationHealthcheckRemovalTest.php
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
<?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.
|
||||
|
||||
$dockerfile = str("FROM nginx:latest\nCOPY . /app\nEXPOSE 80")->trim()->explode("\n");
|
||||
|
||||
// The key fix: hasHealthcheck check happens BEFORE the isHealthcheckDisabled check
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
|
||||
// Simulate an application with custom_healthcheck_found = true
|
||||
$customHealthcheckFound = true;
|
||||
|
||||
// The fixed logic: This condition should be true when HEALTHCHECK is removed
|
||||
$shouldReset = ! $hasHealthcheck && $customHealthcheckFound;
|
||||
|
||||
expect($shouldReset)->toBeTrue()
|
||||
->and($hasHealthcheck)->toBeFalse()
|
||||
->and($customHealthcheckFound)->toBeTrue();
|
||||
});
|
||||
|
||||
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");
|
||||
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
$customHealthcheckFound = true;
|
||||
|
||||
// When healthcheck exists, should not reset
|
||||
$shouldReset = ! $hasHealthcheck && $customHealthcheckFound;
|
||||
|
||||
expect($shouldReset)->toBeFalse()
|
||||
->and($hasHealthcheck)->toBeTrue();
|
||||
});
|
||||
|
||||
it('does not reset when custom_healthcheck_found is false', function () {
|
||||
$dockerfile = str("FROM nginx:latest\nCOPY . /app\nEXPOSE 80")->trim()->explode("\n");
|
||||
|
||||
$hasHealthcheck = str($dockerfile)->contains('HEALTHCHECK');
|
||||
$customHealthcheckFound = false;
|
||||
|
||||
// When custom_healthcheck_found is false, no need to reset
|
||||
$shouldReset = ! $hasHealthcheck && $customHealthcheckFound;
|
||||
|
||||
expect($shouldReset)->toBeFalse()
|
||||
->and($customHealthcheckFound)->toBeFalse();
|
||||
});
|
||||
Loading…
Reference in a new issue