Skip to content

Commit 7e0238c

Browse files
authored
fix: batch fixes for #60, #38, #59, #67, #68 (#70)
* fix: batch fixes for issues #60, #38, #59, #67, #68 - #68: Suppress /up health check from nginx access logs - #60: Use schedule:work instead of schedule:run while loop - #38: Fix ssh-keygen quoting with escapeshellarg() - #59: Clear git_commit_sha after deploying specific commit - #67: Add errorlog channel + PHP-FPM catch_workers_output for container logging Closes #60, closes #38, closes #59, closes #67, closes #68 * fix: address PR review feedback - Wrap deploy() in try/finally so git_commit_sha is cleared even on failure - Use configured health_check_path for nginx log suppression instead of hardcoded /up - Add tests: php-fpm.conf generation, schedule:work supervisor config, deploy SHA cleanup, write-to-disk includes php-fpm.conf
1 parent f02af45 commit 7e0238c

File tree

6 files changed

+86
-8
lines changed

6 files changed

+86
-8
lines changed

src/Console/InstallCommand.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ public function handle(): int
7171
info('Laravel Coolify installed successfully!');
7272
$this->newLine();
7373

74+
// Logging tip for containerised deployments
75+
$this->components->info('Recommended .env for Coolify containers:');
76+
$this->components->bulletList([
77+
'<comment>LOG_STACK=daily,errorlog</comment> — file logs + visible in Coolify log viewer',
78+
]);
79+
$this->newLine();
80+
7481
// Check if .env is configured
7582
if ($this->isConfigured()) {
7683
$this->testConnection();

src/Console/ProvisionCommand.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,12 @@ protected function generateSshKeyPair(string $keyName): ?array
641641
$keyPath = "{$tempDir}/{$keyName}_".time();
642642

643643
// Generate ED25519 key (more secure, shorter than RSA)
644-
$result = Process::run("ssh-keygen -t ed25519 -f {$keyPath} -N '' -C '{$keyName}' 2>&1");
644+
$result = Process::run(sprintf(
645+
'ssh-keygen -t ed25519 -f %s -N %s -C %s 2>&1',
646+
escapeshellarg($keyPath),
647+
escapeshellarg(''),
648+
escapeshellarg($keyName)
649+
));
645650

646651
if (! $result->successful()) {
647652
$this->components->error('Failed to generate SSH key: '.$result->output());
@@ -1123,7 +1128,9 @@ protected function setApplicationEnvVars(
11231128
$envVars[] = ['key' => 'ASSET_URL', 'value' => "https://{$domain}"];
11241129

11251130
// Log management best practices (prevents runaway log files)
1126-
$envVars[] = ['key' => 'LOG_STACK', 'value' => 'daily'];
1131+
// Uses errorlog channel (not stderr) because PHP-FPM redirects worker stderr.
1132+
// error_log() → FPM catch_workers_output → supervisord → container stderr → Coolify
1133+
$envVars[] = ['key' => 'LOG_STACK', 'value' => 'daily,errorlog'];
11271134
$envVars[] = ['key' => 'LOG_DAILY_MAX_FILES', 'value' => '7'];
11281135

11291136
// Set Coolify connection (so deployed app can use this package's dashboard/API)

src/Docker/DockerGenerator.php

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public function write(?string $basePath = null): array
8787
'docker/supervisord.conf' => $this->generateSupervisordConf(),
8888
'docker/nginx.conf' => $this->generateNginxConf(),
8989
'docker/php.ini' => $this->generatePhpIni(),
90+
'docker/php-fpm.conf' => $this->generatePhpFpmConf(),
9091
'docker/entrypoint.sh' => $this->generateEntrypoint(),
9192
];
9293

@@ -172,6 +173,9 @@ protected function generateDockerfileWithBaseImage(): string
172173
# Custom PHP config
173174
COPY docker/php.ini "\$PHP_INI_DIR/conf.d/99-custom.ini"
174175
176+
# PHP-FPM config (enables container log forwarding)
177+
COPY docker/php-fpm.conf /usr/local/etc/php-fpm.d/zz-laravel.conf
178+
175179
# Nginx config
176180
COPY docker/nginx.conf /etc/nginx/nginx.conf
177181
@@ -296,6 +300,9 @@ protected function generateDockerfileFromScratch(): string
296300
RUN mv "\$PHP_INI_DIR/php.ini-production" "\$PHP_INI_DIR/php.ini"
297301
COPY docker/php.ini "\$PHP_INI_DIR/conf.d/99-custom.ini"
298302
303+
# PHP-FPM config (enables container log forwarding)
304+
COPY docker/php-fpm.conf /usr/local/etc/php-fpm.d/zz-laravel.conf
305+
299306
# Nginx config
300307
COPY docker/nginx.conf /etc/nginx/nginx.conf
301308
@@ -436,6 +443,7 @@ public function generateNginxConf(): string
436443
$clientMaxBodySize = config('coolify.docker.nginx.client_max_body_size') ?? '35M';
437444
$uploadMaxFilesize = config('coolify.docker.nginx.upload_max_filesize') ?? '30M';
438445
$postMaxSize = config('coolify.docker.nginx.post_max_size') ?? '35M';
446+
$healthCheckPath = config('coolify.docker.health_check_path') ?? '/up';
439447

440448
// Collect nginx location blocks from detectors
441449
$extraLocations = [];
@@ -501,6 +509,7 @@ public function generateNginxConf(): string
501509
502510
location = /favicon.ico { access_log off; log_not_found off; }
503511
location = /robots.txt { access_log off; log_not_found off; }
512+
location = {$healthCheckPath} { access_log off; log_not_found off; try_files \$uri \$uri/ /index.php?\$query_string; }
504513
505514
{$extraLocationStr}
506515
@@ -584,6 +593,19 @@ public function generatePhpIni(): string
584593
INI;
585594
}
586595

596+
/**
597+
* Generate PHP-FPM pool config for container logging.
598+
* Enables catch_workers_output so error_log() output reaches container stderr via supervisord.
599+
*/
600+
public function generatePhpFpmConf(): string
601+
{
602+
return <<<'CONF'
603+
[www]
604+
catch_workers_output = yes
605+
decorate_workers_output = no
606+
CONF;
607+
}
608+
587609
/**
588610
* Generate entrypoint.sh content.
589611
* Runs migrations and optimizations before starting supervisor.
@@ -894,7 +916,7 @@ protected function getDockerSupervisorProgram(PackageDetector $detector): ?strin
894916
; Laravel Scheduler
895917
; ===================
896918
[program:scheduler]
897-
command=/bin/sh -c "while true; do /usr/local/bin/php /var/www/html/artisan schedule:run --verbose --no-interaction; sleep 60; done"
919+
command=/usr/local/bin/php /var/www/html/artisan schedule:work
898920
user=www-data
899921
autostart=true
900922
autorestart=true

src/Repositories/CoolifyApplicationRepository.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public function delete(string $uuid): bool
101101
*/
102102
public function deploy(string $uuid, bool $force = false, ?string $commit = null): array
103103
{
104-
// If deploying a specific commit, update the app's git_commit_sha first
104+
// If deploying a specific commit, temporarily set git_commit_sha
105105
if ($commit !== null) {
106106
$this->update($uuid, ['git_commit_sha' => $commit]);
107107
}
@@ -112,7 +112,14 @@ public function deploy(string $uuid, bool $force = false, ?string $commit = null
112112
$params['force'] = 'true';
113113
}
114114

115-
$response = $this->client->get('deploy', $params);
115+
try {
116+
$response = $this->client->get('deploy', $params);
117+
} finally {
118+
// Clear pinned commit so future deploys use HEAD
119+
if ($commit !== null) {
120+
$this->update($uuid, ['git_commit_sha' => '']);
121+
}
122+
}
116123

117124
// API returns {deployments: [{message, resource_uuid, deployment_uuid}]}
118125
$deployment = $response['deployments'][0] ?? $response;

tests/Unit/Docker/DockerGeneratorTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,22 @@
189189
expect($content)->toContain('[program:nginx]');
190190
});
191191

192+
it('generates scheduler supervisor using schedule:work', function () {
193+
// Create routes/console.php with a scheduled task to trigger detection
194+
File::ensureDirectoryExists(base_path('routes'));
195+
File::put(base_path('routes/console.php'), '<?php Schedule::command("test")->daily();');
196+
197+
$generator = new DockerGenerator;
198+
$generator->detect();
199+
$content = $generator->generateSupervisordConf();
200+
201+
expect($content)->toContain('[program:scheduler]')
202+
->and($content)->toContain('schedule:work')
203+
->and($content)->not->toContain('schedule:run');
204+
205+
File::delete(base_path('routes/console.php'));
206+
});
207+
192208
it('generates nginx.conf with correct settings', function () {
193209
config(['coolify.docker.nginx.client_max_body_size' => '50M']);
194210

@@ -214,6 +230,15 @@
214230
expect($content)->toContain('opcache.enable = 1');
215231
});
216232

233+
it('generates php-fpm.conf with catch_workers_output enabled', function () {
234+
$generator = new DockerGenerator;
235+
$content = $generator->generatePhpFpmConf();
236+
237+
expect($content)->toContain('[www]')
238+
->and($content)->toContain('catch_workers_output = yes')
239+
->and($content)->toContain('decorate_workers_output = no');
240+
});
241+
217242
it('writes all Docker files to disk', function () {
218243
$generator = new DockerGenerator;
219244
$generator->detect();
@@ -223,12 +248,14 @@
223248
expect(File::exists(base_path('docker/supervisord.conf')))->toBeTrue();
224249
expect(File::exists(base_path('docker/nginx.conf')))->toBeTrue();
225250
expect(File::exists(base_path('docker/php.ini')))->toBeTrue();
251+
expect(File::exists(base_path('docker/php-fpm.conf')))->toBeTrue();
226252
expect(File::exists(base_path('docker/entrypoint.sh')))->toBeTrue();
227253

228254
expect($files)->toHaveKey('Dockerfile');
229255
expect($files)->toHaveKey('docker/supervisord.conf');
230256
expect($files)->toHaveKey('docker/nginx.conf');
231257
expect($files)->toHaveKey('docker/php.ini');
258+
expect($files)->toHaveKey('docker/php-fpm.conf');
232259
expect($files)->toHaveKey('docker/entrypoint.sh');
233260
});
234261

tests/Unit/Repositories/ApplicationRepositoryTest.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,19 @@
9999
expect($result['deployment_uuid'])->toBe('deploy-commit')
100100
->and($result['commit'])->toBe('abc123');
101101

102-
// Verify it first updates the app with the commit SHA
103-
Http::assertSent(function ($request) {
102+
// Verify it sets then clears the commit SHA
103+
$patchRequests = Http::recorded(function ($request) {
104104
return str_contains($request->url(), 'applications/app-123')
105105
&& $request->method() === 'PATCH';
106-
});
106+
})->values();
107+
108+
expect($patchRequests)->toHaveCount(2);
109+
110+
// First PATCH sets the commit SHA
111+
expect($patchRequests[0][0]->data()['git_commit_sha'])->toBe('abc123');
112+
113+
// Second PATCH clears it so future deploys use HEAD
114+
expect($patchRequests[1][0]->data()['git_commit_sha'])->toBe('');
107115
});
108116

109117
it('restarts an application', function () {

0 commit comments

Comments
 (0)