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.

Sign up for GitHub

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

Jump to bottom

SUP-2192 Migration from Cron, to SystemD timers for Database Maintenance #99

Merged
merged 1 commit into from
Jun 17, 2021
Merged

SUP-2192 Migration from Cron, to SystemD timers for Database Maintenance #99

merged 1 commit into from
Jun 17, 2021

Conversation

m0dular
Copy link
Contributor

@m0dular m0dular commented May 26, 2021

No description provided.

@m0dular m0dular requested a review from a team as a code owner May 26, 2021 16:04
@m0dular m0dular force-pushed the SUP-2192-systemd-timers branch 2 times, most recently from e139bb8 to 6a2a76b Compare June 7, 2021 22:36
@MartyEwings
Copy link
Contributor

We need to make sure we update the readme for the crons vs systems timers also

@m0dular m0dular force-pushed the SUP-2192-systemd-timers branch from 958584a to e268acc Compare June 9, 2021 15:39
@m0dular m0dular changed the title DO NOT MERGE WIP Sup 2192 systemd timers Sup 2192 systemd timers Jun 9, 2021
@m0dular m0dular force-pushed the SUP-2192-systemd-timers branch from e268acc to f973054 Compare June 9, 2021 16:00
@m0dular m0dular requested review from jarretlavallee and Sharpie June 9, 2021 16:01
jarretlavallee
jarretlavallee suggested changes Jun 9, 2021
View reviewed changes
Copy link
Contributor

@jarretlavallee jarretlavallee left a comment

Choose a reason for hiding this comment

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

Looks good over all. Some comments in there and one larger concern is the behavior change that puppet_metrics_collector::system may now depend on puppet_metrics_collector.

.sync.yml Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
manifests/init.pp Show resolved Hide resolved
manifests/sar_metric.pp Outdated Show resolved Hide resolved
manifests/system/postgres.pp Outdated Show resolved Hide resolved
metadata.json Outdated Show resolved Hide resolved
spec/classes/puppet_metrics_collector_system_spec.rb Outdated Show resolved Hide resolved
@MartyEwings MartyEwings changed the title Sup 2192 systemd timers SUP-2192 Migration from Cron, to SystemD timers for Database Maintenance Jun 10, 2021
@MartyEwings
Copy link
Contributor

updated the PR name for better changelog entry

MartyEwings
MartyEwings reviewed Jun 10, 2021
View reviewed changes
.sync.yml Outdated Show resolved Hide resolved
@MartyEwings MartyEwings reopened this Jun 10, 2021
@m0dular m0dular force-pushed the SUP-2192-systemd-timers branch 7 times, most recently from 7b943e6 to 59c8719 Compare June 15, 2021 16:03
jarretlavallee
jarretlavallee suggested changes Jun 17, 2021
View reviewed changes
Copy link
Contributor

@jarretlavallee jarretlavallee left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few small changes to naming and labeling. This module is used by OSP users so we want to ensure the naming is consistent.

.sync.yml Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
metadata.json Outdated Show resolved Hide resolved
manifests/system/vmware.pp Outdated Show resolved Hide resolved
templates/service.epp Outdated Show resolved Hide resolved
jarretlavallee
jarretlavallee previously approved these changes Jun 17, 2021
View reviewed changes
@m0dular
(SUP-2192) Migrate from cron to systemd timers
ec0b499 
This commit replaces cron entries with systemd services and timers.  It
also fixes the existing tests and adds a few to the pe_metric spec tests:

* When systemd is not the init provider
* Collecting and not collecting Puppet server metrics
* Customizing the collection interval
@m0dular m0dular force-pushed the SUP-2192-systemd-timers branch from 0883993 to ec0b499 Compare June 17, 2021 17:10
@m0dular m0dular requested a review from jarretlavallee June 17, 2021 17:14
@m0dular m0dular merged commit 299900e into puppetlabs:main Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartyEwings MartyEwings MartyEwings left review comments

@jarretlavallee jarretlavallee jarretlavallee approved these changes

@Sharpie Sharpie Awaiting requested review from Sharpie

Assignees
No one assigned
Labels
backwards-incompatible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@m0dular @MartyEwings @jarretlavallee