puppet-letsencrypt
puppet-letsencrypt copied to clipboard
Avoid race condition when renewing certificates
Pull Request (PR) description
This PR makes sure that only single certificate can be processed at the same time. Make sure to obtain exclusive lock before executing renewal command.
flock command is used to obtain exclusive lock (with 30 seconds timeout).
This Pull Request (PR) fixes the following issues
Resolve errors like this, when randomized cron job time is not enough:
Another instance of Certbot is already running.
@ekohl Yes, there are locking mechanisms in cerbot, but the process will end with an error instead of waiting for releasing the lock.
I agree it's pretty complex. Part of the problem is that:
$cron_minute = fqdn_rand(60, $title)
$cron_hour = fqdn_rand(24, $title)
doesn't provide much flexibility. With ~20-30 certificates you can easily run into collisions. If you have only a few certificates you should be fine (or you could define those times manually).
@deric my whole suggestion was to use the command certbot renew instead of one cronjob per certificate. certbot can figure out what needs renewel on its own. It's a big refactor for the whole module, but it would greatly simplify things.
I agree with @ekohl . In the past I also had one systemd timer per certificate but that doesn't make much sense. certbot itself is really flexible with different hooks and can deal multiple certificates.
@ekohl You're right, reducing per-site cron jobs to a single cron job would be definitely a better option.
There's even an additional advantage: if multiple certificates are renewed at the same time you can reload/restart services which consume multiple certificates (Apache, nginx) only once instead of for each certificate.