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

Include cron in the docker image #1869

Closed
kordianbruck opened this issue Apr 15, 2018 · 10 comments

Comments

@kordianbruck
Copy link

commented Apr 15, 2018

I would love to suggest to add the cronjob directly to the docker image, instead of relying on the host to run the job.

It could be as easy as adding the following to the Dockerfile:

COPY root /var/spool/cron/crontabs/root
CMD crond -l 2 -f

File root would simply contain our usual job:

*/5 * * * * php /usr/share/FreshRSS/app/actualize_script.php > /tmp/FreshRSS.log 2>&1

I'll happily work on a PR if this resonates well with you guys.

@Frenzie

This comment has been minimized.

Copy link
Member

commented Apr 15, 2018

I don't know if automatically running cronjobs is necessarily desirable. Admittedly, for me the cronjob is probably one of the biggest reasons for running something like FreshRSS in the first place. It also grabs feeds if you don't open your feedreader for a few days. But it might not be benign, or whatever the right word to use is.

Btw, every 5 minutes might be a bit often.

@Alkarex

This comment has been minimized.

Copy link
Member

commented Apr 15, 2018

Hello @kordianbruck and thanks for the proposition.
I am a bit ambivalent, because it is best to have only a single process running per Docker instance.
On the other hand, having a built-in cron would indeed be more convenient.

There is a number of possible variations though. We could for instance have a single image with the cron job ready but not running, making it possible to run the image twice: once for Apache, once for cron thanks to an additional command parameter. I think I would like this approach better.

@Alkarex Alkarex added this to the 1.11.0 milestone Apr 15, 2018
@Alkarex

This comment has been minimized.

Copy link
Member

commented Apr 15, 2018

@kordianbruck

This comment has been minimized.

Copy link
Author

commented Apr 15, 2018

@Frenzie we are only talking about the docker image here. If you have any arguments against having it in there from the start, please add them. I can only see arguments for bundling the cronjob with alpine.

@Alkarex Yes, absolutely - separation of concerns is certainly a good thing to have. I think in this case it is more a shortcoming of php that does not allow for reoccurring tasks.

A second image would be less desiderable(as in double the space required + need to update both images), but I would love to instead have a ENV variable one can easily pass in from the docker command/kubernetes deployment to enable the cron daemon.

Alternatively we could have two versions of the image: one with cron enable and one without.

Adding a cronjob externally on my kubernetes cluster is unfortunately not an option, as the cluster might reassign the pod/deployment to another machine inside the cluster.

@Alkarex

This comment has been minimized.

Copy link
Member

commented Apr 15, 2018

@kordianbruck My proposition was a single image, but run two times, once for Apache, once for cron. Would that work for you?

@kordianbruck

This comment has been minimized.

Copy link
Author

commented Apr 15, 2018

@Alkarex I would go for a single image, that will enable cron with a env variable - two images make less sense, as you would need the same code deployed twice with the same persistency settings. So yes, I would love that

@Frenzie

This comment has been minimized.

Copy link
Member

commented Apr 15, 2018

@Alkarex

I am a bit ambivalent, because it is best to have only a single process running per Docker instance.

A single logical service, not a single process. (And that's not a hard rule either, anyway; really depends on what makes sense :-) ) I think a cronjob counts as part of the FreshRSS logical service.

@kordianbruck

we are only talking about the docker image here. If you have any arguments against having it in there from the start, please add them. I can only see arguments for bundling the cronjob with alpine.

I'm not really sure what you expect me to add to what I already wrote. An image with a built-in cronjob automatically uses resources all by itself without being told to. I'm inclined to think of the base Docker image more as a minimal working box than as a full-featured all the bells and whistles configuration.

@Alkarex

This comment has been minimized.

Copy link
Member

commented Apr 16, 2018

I will try to prepare a PR, which should satisfy the different aspects of what we discussed.

  • Parameter to control the cron frequency
  • Parameter to start the cron process or not
  • Possibility of running only the cron process

@Frenzie I meant a single process (not only logical). While not mandatory of course, it is more robust and makes it easier to work with e.g. auto-restart (in case of crash), logs, scaling, etc.

Alkarex added a commit to Alkarex/FreshRSS that referenced this issue Apr 23, 2018
Alkarex added a commit to Alkarex/FreshRSS that referenced this issue Apr 23, 2018
@Alkarex

This comment has been minimized.

Copy link
Member

commented Apr 23, 2018

@kordianbruck Please test #1871 if you can. Feedback appreciated

Alkarex added a commit to Alkarex/FreshRSS that referenced this issue Apr 23, 2018
Alkarex added a commit that referenced this issue Apr 26, 2018
* cron in Docker image

#1869

* Fix cron CMD

* Minor readme

* Docker run d instead of dit

There should not be a need for STDIN or TTY

* Minor sed param
@Alkarex

This comment has been minimized.

Copy link
Member

commented Apr 26, 2018

Merged in /dev. Tests welcome

@Alkarex Alkarex closed this Apr 26, 2018
Alkarex added a commit that referenced this issue Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.