Skip to content
This repository has been archived by the owner on Apr 23, 2019. It is now read-only.

Add cron recipe to allow per-app scheduled tasks. #160

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

Conversation

berkes
Copy link
Collaborator

@berkes berkes commented Jan 18, 2015

Note: WIP.

This adds a recipe that schedules tasks for apps. It simply adds entries to the root crontab
with commands that will be ran within the context of the app.

Mostly a simple wrapper around the cron resource; so we can define per-app scheduled tasks.

  • They will be ran by deploy user.
  • The command will be changed to become cd {path_to_app}/current && ( PATH=/opt/rbenv/shims:$PATH RAILS_ENV={your_env} bundle exec {your_command} )
  • All other attributes from http://docs.chef.io/resource_cron.html will be passed along to the cron resource.

This is a WIP, so please shoot:

  • Is this feature needed? (I need it, because I want to schedule some tasks and prefer not to use whenever gem for that).
  • Do we need a cleaner or more predictable outcome for the command?
  • Other comments?

* Defers attributes as set in node config.
* Adds crontab to deploy user's crontab unless provided in the node
* Prefixes command so it runs within app, using rbenv and bundle exec
cron "#{app}_#{id}" do
command(command)
user(user)
attributes.each {|attribute, value| send(attribute, value) }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space between { and | missing.

@jvanbaarsen
Copy link
Contributor

@berkes Maybe interesting to look at https://github.com/intercity/chef-repo/blob/master/vendor/cookbooks/backups/recipes/default.rb#L83 since I already implemented a cron cookbook for the backup feature.

@jvanbaarsen
Copy link
Contributor

For your concerns:

Is this feature needed?
I believe this would make a great addition!

Do we need a cleaner or more predictable outcome for the command?
Can you explain in a bit more detail what you mean with cleaner?

Other comments?
Awesome work :D 🍰

@berkes
Copy link
Collaborator Author

berkes commented Jan 26, 2015

since I already implemented a cron cookbook for the backup feature.

I was aware of that and my recipe uses the same underlying cron LWRP.

The two cron-tasks are a bit different, though.
These new custom tasks are "just any command" and ran by the deploy user (i.e. they are added to the crontab for the deploy user)
Your crontab runs as root and is a fixed command.

Edit: do you think we should make them use one crontab file?

Can you explain in a bit more detail what you mean with cleaner?

When you add "command": "rake app:cleanup_old_orders" under "intercity_example" app, this cron recipe will add an entry to the deploy user with the command cd /u/apps/intercity_example && ( PATH=/opt/rbenv/shims:$PATH RAILS_ENV=production bundle exec rake app:cleanup )

In other words: it adds a lot of extra commands and cruft to the command a user entered. This might cause weird behaviour when the user adds complex commands himself.

For now, I'd say a warning in documentation is in place, but we might want to add a user-settable flag to avoid adding our own commands around the users' command in future.

@berkes
Copy link
Collaborator Author

berkes commented Jan 30, 2015

@jvanbaarsen @michiels how do you think this should best be integrated?

How to include it:

  • as a recipe under the 'rails' cookbook?
  • as a separate cookbook (as proposed in this PR, but moving to the 'rails' cookbook is easy)

Where or when to run it:

  • as a role?
  • as an includable recipe?
  • default included in the rails::setup recipe?
  • default included in the base role? (ugh, ugly)
  • default included in both the rails and rails_passenger roles?

IMO this should simply always be ran, therefore I don't really like the options where people have to include these as roles or recipes themselves.

Edit: When the basics are agreed upon, I'll rebase this branch and create a new PR with a nicer history.

@ghost ghost added this to the 2.4.0 milestone Feb 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants