Crontab rewrite with sections causes migration errors

[SimJoSt]

• Deployer version: 7.3.3
• Deployment OS: Ubuntu 22.04

import:
  - contrib/crontab.php

config:
  crontab:identifier: 'application4'
  crontab:jobs:
      - '* * * * * cd {{current_path}} && {{bin/php}} wp-cron.php >> /dev/null 2>&1'

hosts:
  ***redacted***:
    deploy_path: '~/application4'

We've been using the crontab recipe in our config for some time now, and I welcome the rewrite with sections, as it prevents duplicate entries when the crontab config is changed and resynced.
Without initially knowing about the change, it broke some of our applications and no crontab was running for some time. Only the terminal output messages, pointed me towards the new behavior. I get that this is only a change in a smaller recipe, but it seems to be a breaking change.

If there is a migration in place, it shouldn't be an issue. However, I noted some issues while using it and migrating existing applications to it.
Running the crontab:sync task showed it recognized a previous job in the crontab config on the server, said it would move it to the section, said it couldn't find the section, and said it would create the section.
The section was created, and the previous job was removed. However, the pre-existing job was not created anew in the new section. It seems like it cannot perform the migration and creation of a new section at the same time, breaking existing configurations.
The fix is to run the crontab:sync task again, which fixes the problem.

Before:

➜  dep ssh
➜  crontab -l
* * * * * cd ~/application1/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application2/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application3/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application4/current && /usr/bin/php8.2 wp-cron.php >> /dev/null 2>&1

Migration:

➜  dep crontab:sync
task crontab:sync
[***redacted***] Crontab: Found existing job in crontab, moving it to the section
[***redacted***] Crontab: Found no section, created the section with configured jobs
➜  dep ssh
➜  crontab -l
* * * * * cd ~/application1/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application2/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application3/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
###< application4
###> application4

Re-run:

➜  dep crontab:sync
➜  dep ssh
➜  crontab -l
* * * * * cd ~/application1/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application2/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
* * * * * cd ~/application3/current && /usr/bin/php8.1 artisan schedule:run >> /dev/null 2>&1
###< application4
* * * * * cd ~/application4/current && /usr/bin/php8.2 wp-cron.php >> /dev/null 2>&1
###> application4

Upvote & Fund

• We're using Polar.sh so you can upvote and help fund this issue.
• We receive the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[ardentsword]

Hello @SimJoSt I'm sorry to hear your experience, I of course agree 100% that it should not be a breaking change in a not-major release.
I'm would love to fix this issue but looking at my code I don't understand how it does not include the migrated cronjob when first creating the section, I remember testing it as well but will try to recreate the issue myself later.
In the meantime you can perhaps look at the code responsible for the migration: https://github.com/deployphp/deployer/blob/master/contrib/crontab.php#L62-L74 I probably stared at it too much to see what could be wrong ;)

[ardentsword]

@SimJoSt I noticed your other issue (#3769) as well, I have the feeling there is something strange going on with the yaml config translation, I must admit that I have not used or seen that style of configuration before. Especially since you mention the default setting for the identifier. There is a default value set as can be seen here: https://github.com/deployphp/deployer/blob/master/contrib/crontab.php#L33-L35.

For reference, I use the following code:

$receivers = implode(" ", [
    'scheduler_main_schedule',
]);
add('crontab:jobs', [
    "30 1 * * * {{bin/php}} {{current_path}}/bin/console messenger:consume $receivers --time-limit=86400  >> /dev/null 2>&1",
]);

I've never even set the identifier and it works without issues. I might have time to investigate this more but I'm not sure when.

[github-actions]

This issue has been automatically closed. Please, open a discussion for bug reports and feature requests.

Read more: [https://github.com//discussions/3888]

[SimJoSt]

@ardentsword thank you for the feedback.
As there are some other issues with the YAML config, I believe you might be right.

For now, we will switch to a PHP first config for more complex projects.
Typically, with one deploy.php in the root directory and all other files, PHP or YAML, in a .deployer directory.
I still like YAML for its simplicity and readability, especially for config, hosts and event hooks.
For tasks, I will use PHP, as it has a lot more options.

By now, I am setting crontab identifiers per host, as we might run 3 environments: dev, stage and production.
The 2 former we would run on the same server and need different crontab sections.

I checked the code as well and cannot see any glaring mistakes.
$cronJobsLocal must not be empty, for the unset() function to work at line

deployer/contrib/crontab.php

Line 64 in 28a229b

if (in_array($cronJob, $cronJobsLocal)) {

So, I have no idea why it would apparently turn up empty at line

deployer/contrib/crontab.php

Line 72 in 28a229b

$cronJobs = [...$cronJobs, ...$cronJobsLocal];

To really debug the issue, it would be necessary to recreate the scenario and check the content of the variables at every step.

Is there a reason array_push wasn't chosen to merge the content of $cronJobsLocal into $cronJobs?

deployer/contrib/crontab.php

Line 72 in 28a229b

$cronJobs = [...$cronJobs, ...$cronJobsLocal];

array_push($cronJobs, ...$cronJobsLocal); should work as well. #notASeniorPHPDev

[SimJoSt]

Migrated to discussion #3895