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

Backup cron configuration will trigger gitlab restart #204

Closed
baurmatt opened this issue Mar 13, 2018 · 3 comments
Closed

Backup cron configuration will trigger gitlab restart #204

baurmatt opened this issue Mar 13, 2018 · 3 comments

Comments

@baurmatt
Copy link
Contributor

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: Every
  • Ruby: Every
  • Distribution: Every
  • Module version: Current Master

How to reproduce (e.g Puppet code you use)

Change any backup_cron_* paramerters configuration

What are you seeing

Changes on backup cron will restart Gitlab

What behaviour did you expect instead

Changes on backup cron shouldn't restart Gitlab - only changes which are directly connected to the gitlab service should restart it

Output log

Any additional information you'd like to impart

@baurmatt
Copy link
Contributor Author

I see to options:

  • Removing the ~> in init.pp and replacing it with a notify => Class['gitlab::service'] for each resource in gitlab::config which needs it.
  • Move backup into a separate class

I happily implement it, just tell me which you prefer.

@LongLiveCHIEF
Copy link
Contributor

For now, let's move the the backup cron into it's own manifest.

In the long run, I think any configuration which affects the service should ideally wind up in the service.pp manifest, and any other configs get their own manifests, and the config.pp manifest at that point simply becomes an anchoring and ordering manifests using resource collectors if needed to control timing of specific configs.

This would work similarly to how the puppet/docker module config and service manifests are designed.

m also looking working on an option to manage backups as a systemd service using timers instead of CRON, and allowing implementors to choose which is preferred. Moving the backup into it's own manifest will make that PR easier as well.

Thanks @baurmatt I'll keep an eye out for this PR. Do you think it will be soon? I'm in the process of dropping a new release, and this is something I'd wait 24 hours to make sure it gets included.

@baurmatt
Copy link
Contributor Author

on it :)

baurmatt added a commit to syseleven/puppet-gitlab that referenced this issue Mar 14, 2018
This commit moves the backup cronjob to its own class. This ensures that
backup configuration changes do not trigger a Gitlab service restart.

Fixes voxpupuli#204.
baurmatt added a commit to syseleven/puppet-gitlab that referenced this issue Mar 16, 2018
This commit moves the backup cronjob to its own class. This ensures that
backup configuration changes do not trigger a Gitlab service restart.

Fixes voxpupuli#204.
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

No branches or pull requests

2 participants