fix(jobs): prevent non-due jobs firing on restart and enrich skip logs with resource links
- Refactor shouldRunNow() to only fire on first run (empty cache) if actually due by cron schedule, preventing spurious executions after cache loss or service restart - Add enrichSkipLogsWithLinks() method to fetch and populate resource names and links for tasks, backups, and docker cleanup jobs in skip logs - Update skip logs UI to display resource column with links to related resources, improving navigation and context - Add fallback display when linked resources are deleted - Expand tests to cover both restart scenarios: non-due jobs (should not fire) and due jobs (should fire)
This commit is contained in:
parent
63be5928ab
commit
31555f9e8a
5 changed files with 166 additions and 29 deletions
|
|
@ -341,8 +341,19 @@ private function shouldRunNow(string $frequency, string $timezone, ?string $dedu
|
||||||
|
|
||||||
$lastDispatched = Cache::get($dedupKey);
|
$lastDispatched = Cache::get($dedupKey);
|
||||||
|
|
||||||
// Run if: never dispatched before, OR there's been a due time since last dispatch
|
if ($lastDispatched === null) {
|
||||||
if ($lastDispatched === null || $previousDue->gt(Carbon::parse($lastDispatched))) {
|
// First run after restart or cache loss: only fire if actually due right now.
|
||||||
|
// Seed the cache so subsequent runs can use tolerance/catch-up logic.
|
||||||
|
$isDue = $cron->isDue($executionTime);
|
||||||
|
if ($isDue) {
|
||||||
|
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
||||||
|
}
|
||||||
|
|
||||||
|
return $isDue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Subsequent runs: fire if there's been a due time since last dispatch
|
||||||
|
if ($previousDue->gt(Carbon::parse($lastDispatched))) {
|
||||||
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
Cache::put($dedupKey, $executionTime->toIso8601String(), 86400);
|
||||||
|
|
||||||
return true;
|
return true;
|
||||||
|
|
|
||||||
|
|
@ -3,8 +3,11 @@
|
||||||
namespace App\Livewire\Settings;
|
namespace App\Livewire\Settings;
|
||||||
|
|
||||||
use App\Models\DockerCleanupExecution;
|
use App\Models\DockerCleanupExecution;
|
||||||
|
use App\Models\ScheduledDatabaseBackup;
|
||||||
use App\Models\ScheduledDatabaseBackupExecution;
|
use App\Models\ScheduledDatabaseBackupExecution;
|
||||||
|
use App\Models\ScheduledTask;
|
||||||
use App\Models\ScheduledTaskExecution;
|
use App\Models\ScheduledTaskExecution;
|
||||||
|
use App\Models\Server;
|
||||||
use App\Services\SchedulerLogParser;
|
use App\Services\SchedulerLogParser;
|
||||||
use Illuminate\Support\Carbon;
|
use Illuminate\Support\Carbon;
|
||||||
use Illuminate\Support\Collection;
|
use Illuminate\Support\Collection;
|
||||||
|
|
@ -102,13 +105,84 @@ private function loadData(?int $teamId = null): void
|
||||||
$parser = new SchedulerLogParser;
|
$parser = new SchedulerLogParser;
|
||||||
$allSkips = $parser->getRecentSkips(500, $teamId);
|
$allSkips = $parser->getRecentSkips(500, $teamId);
|
||||||
$this->skipTotalCount = $allSkips->count();
|
$this->skipTotalCount = $allSkips->count();
|
||||||
$this->skipLogs = $allSkips->slice($this->skipPage, $this->skipDefaultTake)->values();
|
$this->skipLogs = $this->enrichSkipLogsWithLinks(
|
||||||
|
$allSkips->slice($this->skipPage, $this->skipDefaultTake)->values()
|
||||||
|
);
|
||||||
$this->showSkipPrev = $this->skipPage > 0;
|
$this->showSkipPrev = $this->skipPage > 0;
|
||||||
$this->showSkipNext = ($this->skipPage + $this->skipDefaultTake) < $this->skipTotalCount;
|
$this->showSkipNext = ($this->skipPage + $this->skipDefaultTake) < $this->skipTotalCount;
|
||||||
$this->skipCurrentPage = intval($this->skipPage / $this->skipDefaultTake) + 1;
|
$this->skipCurrentPage = intval($this->skipPage / $this->skipDefaultTake) + 1;
|
||||||
$this->managerRuns = $parser->getRecentRuns(30, $teamId);
|
$this->managerRuns = $parser->getRecentRuns(30, $teamId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function enrichSkipLogsWithLinks(Collection $skipLogs): Collection
|
||||||
|
{
|
||||||
|
$taskIds = $skipLogs->where('type', 'task')->pluck('context.task_id')->filter()->unique()->values();
|
||||||
|
$backupIds = $skipLogs->where('type', 'backup')->pluck('context.backup_id')->filter()->unique()->values();
|
||||||
|
$serverIds = $skipLogs->where('type', 'docker_cleanup')->pluck('context.server_id')->filter()->unique()->values();
|
||||||
|
|
||||||
|
$tasks = $taskIds->isNotEmpty()
|
||||||
|
? ScheduledTask::with(['application.environment.project', 'service.environment.project'])->whereIn('id', $taskIds)->get()->keyBy('id')
|
||||||
|
: collect();
|
||||||
|
|
||||||
|
$backups = $backupIds->isNotEmpty()
|
||||||
|
? ScheduledDatabaseBackup::with(['database.environment.project'])->whereIn('id', $backupIds)->get()->keyBy('id')
|
||||||
|
: collect();
|
||||||
|
|
||||||
|
$servers = $serverIds->isNotEmpty()
|
||||||
|
? Server::whereIn('id', $serverIds)->get()->keyBy('id')
|
||||||
|
: collect();
|
||||||
|
|
||||||
|
return $skipLogs->map(function (array $skip) use ($tasks, $backups, $servers): array {
|
||||||
|
$skip['link'] = null;
|
||||||
|
$skip['resource_name'] = null;
|
||||||
|
|
||||||
|
if ($skip['type'] === 'task') {
|
||||||
|
$task = $tasks->get($skip['context']['task_id'] ?? null);
|
||||||
|
if ($task) {
|
||||||
|
$skip['resource_name'] = $skip['context']['task_name'] ?? $task->name;
|
||||||
|
$resource = $task->application ?? $task->service;
|
||||||
|
$environment = $resource?->environment;
|
||||||
|
$project = $environment?->project;
|
||||||
|
if ($project && $environment && $resource) {
|
||||||
|
$routeName = $task->application_id
|
||||||
|
? 'project.application.scheduled-tasks'
|
||||||
|
: 'project.service.scheduled-tasks';
|
||||||
|
$routeKey = $task->application_id ? 'application_uuid' : 'service_uuid';
|
||||||
|
$skip['link'] = route($routeName, [
|
||||||
|
'project_uuid' => $project->uuid,
|
||||||
|
'environment_uuid' => $environment->uuid,
|
||||||
|
$routeKey => $resource->uuid,
|
||||||
|
'task_uuid' => $task->uuid,
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} elseif ($skip['type'] === 'backup') {
|
||||||
|
$backup = $backups->get($skip['context']['backup_id'] ?? null);
|
||||||
|
if ($backup) {
|
||||||
|
$database = $backup->database;
|
||||||
|
$skip['resource_name'] = $database?->name ?? 'Database backup';
|
||||||
|
$environment = $database?->environment;
|
||||||
|
$project = $environment?->project;
|
||||||
|
if ($project && $environment && $database) {
|
||||||
|
$skip['link'] = route('project.database.backup.index', [
|
||||||
|
'project_uuid' => $project->uuid,
|
||||||
|
'environment_uuid' => $environment->uuid,
|
||||||
|
'database_uuid' => $database->uuid,
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} elseif ($skip['type'] === 'docker_cleanup') {
|
||||||
|
$server = $servers->get($skip['context']['server_id'] ?? null);
|
||||||
|
if ($server) {
|
||||||
|
$skip['resource_name'] = $server->name;
|
||||||
|
$skip['link'] = route('server.show', ['server_uuid' => $server->uuid]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $skip;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
private function getExecutions(?int $teamId = null): Collection
|
private function getExecutions(?int $teamId = null): Collection
|
||||||
{
|
{
|
||||||
$dateFrom = $this->getDateFrom();
|
$dateFrom = $this->getDateFrom();
|
||||||
|
|
|
||||||
|
|
@ -213,8 +213,8 @@ class="border-b border-gray-200 dark:border-coolgray-400">
|
||||||
<tr>
|
<tr>
|
||||||
<th class="px-4 py-3">Time</th>
|
<th class="px-4 py-3">Time</th>
|
||||||
<th class="px-4 py-3">Type</th>
|
<th class="px-4 py-3">Type</th>
|
||||||
|
<th class="px-4 py-3">Resource</th>
|
||||||
<th class="px-4 py-3">Reason</th>
|
<th class="px-4 py-3">Reason</th>
|
||||||
<th class="px-4 py-3">Details</th>
|
|
||||||
</tr>
|
</tr>
|
||||||
</thead>
|
</thead>
|
||||||
<tbody>
|
<tbody>
|
||||||
|
|
@ -235,6 +235,17 @@ class="border-b border-gray-200 dark:border-coolgray-400">
|
||||||
{{ ucfirst(str_replace('_', ' ', $skip['type'])) }}
|
{{ ucfirst(str_replace('_', ' ', $skip['type'])) }}
|
||||||
</span>
|
</span>
|
||||||
</td>
|
</td>
|
||||||
|
<td class="px-4 py-2">
|
||||||
|
@if($skip['link'] ?? null)
|
||||||
|
<a href="{{ $skip['link'] }}" class="text-white underline hover:no-underline">
|
||||||
|
{{ $skip['resource_name'] }}
|
||||||
|
</a>
|
||||||
|
@elseif($skip['resource_name'] ?? null)
|
||||||
|
{{ $skip['resource_name'] }}
|
||||||
|
@else
|
||||||
|
<span class="text-gray-500">{{ $skip['context']['task_name'] ?? $skip['context']['server_name'] ?? 'Deleted' }}</span>
|
||||||
|
@endif
|
||||||
|
</td>
|
||||||
<td class="px-4 py-2">
|
<td class="px-4 py-2">
|
||||||
@php
|
@php
|
||||||
$reasonLabel = match($skip['reason']) {
|
$reasonLabel = match($skip['reason']) {
|
||||||
|
|
@ -256,15 +267,6 @@ class="border-b border-gray-200 dark:border-coolgray-400">
|
||||||
@endphp
|
@endphp
|
||||||
<span class="{{ $reasonBg }}">{{ $reasonLabel }}</span>
|
<span class="{{ $reasonBg }}">{{ $reasonLabel }}</span>
|
||||||
</td>
|
</td>
|
||||||
<td class="px-4 py-2 text-xs text-gray-500">
|
|
||||||
@php
|
|
||||||
$details = collect($skip['context'])
|
|
||||||
->except(['type', 'skip_reason', 'execution_time'])
|
|
||||||
->map(fn($v, $k) => str_replace('_', ' ', $k) . ': ' . $v)
|
|
||||||
->implode(', ');
|
|
||||||
@endphp
|
|
||||||
{{ $details }}
|
|
||||||
</td>
|
|
||||||
</tr>
|
</tr>
|
||||||
@empty
|
@empty
|
||||||
<tr>
|
<tr>
|
||||||
|
|
|
||||||
|
|
@ -30,8 +30,11 @@
|
||||||
expect($result)->toBeTrue();
|
expect($result)->toBeTrue();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('dispatches backup when job is delayed past the cron minute', function () {
|
it('catches delayed job when cache has a baseline from previous run', function () {
|
||||||
// Freeze time at 02:07 — job was delayed 7 minutes past the 02:00 cron
|
// Simulate a previous dispatch yesterday at 02:00
|
||||||
|
Cache::put('test-backup:1', Carbon::create(2026, 2, 27, 2, 0, 0, 'UTC')->toIso8601String(), 86400);
|
||||||
|
|
||||||
|
// Freeze time at 02:07 — job was delayed 7 minutes past today's 02:00 cron
|
||||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 7, 0, 'UTC'));
|
Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 7, 0, 'UTC'));
|
||||||
|
|
||||||
$job = new ScheduledJobManager;
|
$job = new ScheduledJobManager;
|
||||||
|
|
@ -45,8 +48,8 @@
|
||||||
$method = $reflection->getMethod('shouldRunNow');
|
$method = $reflection->getMethod('shouldRunNow');
|
||||||
$method->setAccessible(true);
|
$method->setAccessible(true);
|
||||||
|
|
||||||
// isDue() would return false at 02:07, but getPreviousRunDate() = 02:00
|
// isDue() would return false at 02:07, but getPreviousRunDate() = 02:00 today
|
||||||
// No lastDispatched in cache → should run
|
// lastDispatched = 02:00 yesterday → 02:00 today > yesterday → fires
|
||||||
$result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1');
|
$result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1');
|
||||||
|
|
||||||
expect($result)->toBeTrue();
|
expect($result)->toBeTrue();
|
||||||
|
|
@ -106,8 +109,27 @@
|
||||||
expect($result3)->toBeTrue();
|
expect($result3)->toBeTrue();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('re-dispatches after cache loss instead of missing', function () {
|
it('does not fire non-due jobs on restart when cache is empty', function () {
|
||||||
// First run at 02:00 — dispatches
|
// Time is 10:00, cron is daily at 02:00 — NOT due right now
|
||||||
|
Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC'));
|
||||||
|
|
||||||
|
$job = new ScheduledJobManager;
|
||||||
|
$reflection = new ReflectionClass($job);
|
||||||
|
|
||||||
|
$executionTimeProp = $reflection->getProperty('executionTime');
|
||||||
|
$executionTimeProp->setAccessible(true);
|
||||||
|
$executionTimeProp->setValue($job, Carbon::now());
|
||||||
|
|
||||||
|
$method = $reflection->getMethod('shouldRunNow');
|
||||||
|
$method->setAccessible(true);
|
||||||
|
|
||||||
|
// Cache is empty (fresh restart) — should NOT fire daily backup at 10:00
|
||||||
|
$result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4');
|
||||||
|
expect($result)->toBeFalse();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fires due jobs on restart when cache is empty', function () {
|
||||||
|
// Time is exactly 02:00, cron is daily at 02:00 — IS due right now
|
||||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC'));
|
Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC'));
|
||||||
|
|
||||||
$job = new ScheduledJobManager;
|
$job = new ScheduledJobManager;
|
||||||
|
|
@ -120,16 +142,8 @@
|
||||||
$method = $reflection->getMethod('shouldRunNow');
|
$method = $reflection->getMethod('shouldRunNow');
|
||||||
$method->setAccessible(true);
|
$method->setAccessible(true);
|
||||||
|
|
||||||
$method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4');
|
// Cache is empty (fresh restart) — but cron IS due → should fire
|
||||||
|
$result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4b');
|
||||||
// Simulate Redis restart — cache lost
|
|
||||||
Cache::forget('test-backup:4');
|
|
||||||
|
|
||||||
// Run again at 02:01 — should re-dispatch (lastDispatched = null after cache loss)
|
|
||||||
Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC'));
|
|
||||||
$executionTimeProp->setValue($job, Carbon::now());
|
|
||||||
|
|
||||||
$result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4');
|
|
||||||
expect($result)->toBeTrue();
|
expect($result)->toBeTrue();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -234,3 +234,39 @@
|
||||||
// Cleanup
|
// Cleanup
|
||||||
@unlink($logPath);
|
@unlink($logPath);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('skipped jobs show fallback when resource is deleted', function () {
|
||||||
|
$this->actingAs($this->rootUser);
|
||||||
|
session(['currentTeam' => $this->rootTeam]);
|
||||||
|
|
||||||
|
$logPath = storage_path('logs/scheduled-'.now()->format('Y-m-d').'.log');
|
||||||
|
$logDir = dirname($logPath);
|
||||||
|
if (! is_dir($logDir)) {
|
||||||
|
mkdir($logDir, 0755, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Temporarily rename existing logs so they don't interfere
|
||||||
|
$existingLogs = glob(storage_path('logs/scheduled-*.log'));
|
||||||
|
$renamed = [];
|
||||||
|
foreach ($existingLogs as $log) {
|
||||||
|
$tmp = $log.'.bak';
|
||||||
|
rename($log, $tmp);
|
||||||
|
$renamed[$tmp] = $log;
|
||||||
|
}
|
||||||
|
|
||||||
|
$lines = [
|
||||||
|
'['.now()->format('Y-m-d H:i:s').'] production.INFO: Task skipped {"type":"task","skip_reason":"application_not_running","task_id":99999,"task_name":"my-cron-job","team_id":0}',
|
||||||
|
];
|
||||||
|
file_put_contents($logPath, implode("\n", $lines)."\n");
|
||||||
|
|
||||||
|
Livewire::test(ScheduledJobs::class)
|
||||||
|
->assertStatus(200)
|
||||||
|
->assertSee('my-cron-job')
|
||||||
|
->assertSee('Application not running');
|
||||||
|
|
||||||
|
// Cleanup
|
||||||
|
@unlink($logPath);
|
||||||
|
foreach ($renamed as $tmp => $original) {
|
||||||
|
rename($tmp, $original);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue