Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement persistent/reliable cron service using supervisor. #1041

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jasonhildebrand
Copy link

@jasonhildebrand jasonhildebrand commented Jan 11, 2024

This PR addresses #760.

There were several non-obvious challenges which I want to document here.

  • if you want to start multiple processes in a single docker container (which IMO is a valid thing to do if they are tightly coupled), then you need a way to start/restart these process. I'm using supervisor for this purpose.
  • cron needs to run as root, which means that the container entrypoint also needs to run as root (we want to avoid needing sudo to launch cron). the implication of this change is that docker-compose exec phpfpm runs as root by default (not app), which sucks. thankfully, we can update the bin/cli and bin/clinotty wrappers to use the app user and maintain existing behaviour (and I have done so).
  • /dev/stdout is inaccessible by an unprivileged user in a container started as root (/dev/stdout is inaccessible by an unprivileged user in a container started as root moby/moby#31243), which makes it difficult to have php-fpm (running as app) to log to the docker console. The workaround is to set tty: true in compose.yaml and add app to the tty group.

Other notes

  • bin/cron has been updated and can still be used to start/stop cron or check status
  • you can set START_CRON=true (or =false) in env/phpfpm.env to control whether you want cron to be started when the container is started.

Alternative approaches:

  • My aim was to get cron working in the same container as php-fpm. An alternative approach would be to run cron in a separate container. This would require ensuring that the cron container has access to the identical codebase as the php-fpm container (e.g. the same volumes and/or bind mounts), because cron needs to be able to execute php code. My sense was that keeping this in sync could be tricky, which is one of the reasons I didn't pursue this approach. However, it would avoid the need for supervisor and would side-step the /dev/stdout permissions issue.
  • Use https://github.com/aptible/supercronic instead of Vixie cron. supercronic can run as non-root, so that would simplify a number of things. But I have no direct experience with it, so I don't know about compatibility issues. A quick glance at the issue queue suggests it might not yet be quite mature enough (as of Jan 2024), but looks promising.

At the moment, cron support is only implemented in the PHP 8.2 image. If this approach is acceptable, I would be happy to extend it to support the PHP 8.1 image as well.

Copy link

what-the-diff bot commented Jan 11, 2024

PR Summary

  • Modification of cli script
    The cli script under compose/bin has been updated to better run the phpfpm command, a popular software that web servers use to process PHP requests, as if it was being executed by a user named app.

  • Adjustment in clinotty script
    The script for clinotty in compose/bin was changed to allow the phpfpm command to run with the user 'app' and without using a pseudo terminal. This can help improve the interaction with the command line in certain situations.

  • Update of cron script
    The changes to the cron script in compose/bin enable the running of the supervisorctl command to manage the cron service. This allows better control and manipulation of tasks on a schedule.

  • Inclusion of new option in compose.yaml
    An inclusion of a tty: true option has been made in the compose.yaml file for the phpfpm service. This option allows the service to control a terminal line, which can help in interacting directly with the user running the service.

  • New environment variable in phpfpm.env
    The phpfpm.env file has been updated to include START_CRON=true which will auto start the cron service — a time-based job scheduler in Unix-like operating systems that can automate certain tasks.

  • Update of Dockerfile
    Changes in the Dockerfile ensure the inclusion of tini and supervisor packages, both tools used to manage and control processes within the docker container. Additionally, a newsupervisord.conf file will be copied, and adjustments made to ENTRYPOINT and CMD instructions for better container behavior.

  • Revisions on php-fpm.conf
    The php-fpm.conf file underwent modification to change the paths of the error_log and access.log. These modifications provide more accurate log tracking for developers.

  • Addition of supervisord.conf file
    A new supervisord.conf file, a configuration file for supervisor (a process control system), has been added in images/php/8.2/conf. This enables better process management within the PHP image used.

@markshust
Copy link
Owner

Thanks for this PR! Since this is one of the larger updates that came in, I need to test it a little more extensively. I really appreciate the detailed comments, explanations, and reasonings behind this PR.

Since I'd like to get a tagged version of docker-magento out imminently, I'm going to skip over this PR and come back to it after the other PRs are resolved, as this one is already having a conflict or two from the other merged PRs. Once those are all resolved, I'll take a deeper look into this one 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants