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

Run Laravel schedule in docker image using supervisord #6606

Merged

Conversation

JonathonReinhart
Copy link
Contributor

@JonathonReinhart JonathonReinhart commented Jan 19, 2019

This PR updates the docker image so that artisan schedule:run is executed every minute by a supervisord command.

Since it seems that Laravel Task Scheduling is preferred over creating various crontab entries, this should cover all of snipe-it's needs. If everyone can agree to this, then this effectively fixes issue #6344. If not, then I'll have to modify this to use cron (see below).

Cron

History:

Using cron to run the Laravel schedule is problematic because cron doesn't preserve the outside environment variables. This is especially unfortunate for Docker containers where the Docker-provided environment is the key source of configuration.

While there is a method to access the docker-set environment variables from the cron job, the whole thing seemed overkill.

Supervisor

Rather than using tini, Supervisor is added to the docker image and is used to start apache2 and run the Laravel schedule in a simple sleep loop.

I also considered forego, which was preferred over supervisor by nginx-proxy/nginx-proxy#165. Unfortunately, it didn't work well; sending SIGINT to forego failed to properly shut down apache. Also, this wouldn't have provided as clean of a solution for the Laravel schedule.

Testing

I temporarily made the following change in my running Docker container.

diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php
index 371362442..f84a00505 100644
--- a/app/Console/Kernel.php
+++ b/app/Console/Kernel.php
@@ -45,7 +45,7 @@ class Kernel extends ConsoleKernel
         $schedule->command('snipeit:inventory-alerts')->daily();
         $schedule->command('snipeit:expiring-alerts')->daily();
         $schedule->command('snipeit:expected-checkin')->daily();
-        $schedule->command('snipeit:backup')->weekly();
+        $schedule->command('snipeit:backup')->everyMinute()->appendOutputTo("/tmp/backup.log");
         $schedule->command('backup:clean')->daily();
     }

After a few minutes:

  • I navigated to https://localhost:8888/admin/backups and saw that several backups had been created.
  • I saw that the following had been written to /tmp/backup.log:
    Starting backup...
    Dumping database snipeit...
    Determining files to backup...
    Zipped 19 files...
    Copying 2019-01-19-063352.zip (size: 32.82 KB) to disk named local...
    Successfully copied .zip file to disk named local.
    Backup completed!
    
  • I verified that the backups created in /var/lib/snipeit/dumps/ were owned by docker:staff
  • I verified that I could still create a backup by clicking Generate Backup

This script is not configured as the docker image ENTRYPOINT, thus it is
misleading to name it so.
Note that this uses `apache2ctl -DFOREGROUND` rather than manually
sourcing /etc/apache2/envvars and running apache2, as recommended at
https://advancedweb.hu/2018/07/03/supervisor_docker/.
By terminating PID 1, this will also terminate the Docker container.
This also switches to executing /var/www/html/artisan directly.
@JonathonReinhart JonathonReinhart changed the title Fix cron in docker image via supervisord WIP Fix cron in docker image via supervisord Jan 19, 2019
This has the following benefits over using cron:
- Cron doesn't need to be installed
- Docker-provided environment variables are preserved
- It's easy and explicit to run as the docker user
@JonathonReinhart JonathonReinhart changed the title WIP Fix cron in docker image via supervisord Run Laravel schedule in docker image using supervisord Jan 19, 2019
@JonathonReinhart
Copy link
Contributor Author

Interesting to note: there's also a Golang port of supervisord, which could help minimize the docker image size.

@snipe
Copy link
Owner

snipe commented Jan 22, 2019

@uberbrady can you take a look at this when you get a moment?

@snipe
Copy link
Owner

snipe commented Jan 24, 2019

I just realized this PR is against master. Can you make it against develop please, per the contributing documentation? Thanks!

@adoering-te
Copy link

@JonathonReinhart were you able to rerun the code against develop so that this could be merged? Would make fixing our gaps easier. Appreciate the PR by the way! :)

@snipe snipe merged commit e59ec8b into snipe:master Mar 7, 2019
@snipe
Copy link
Owner

snipe commented Mar 7, 2019

Thanks for this contribution. In the future, please try to follow the contributing documentation so we can merge these more quickly.

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.

3 participants