search

elijah / chef-prometheus

Chef cookbook for Prometheus. The open source service monitoring system and time-series database.
Apache License 2.0
52 stars 93 forks source link

Implement Job LWRP #22

Closed ewr closed 9 years ago

ewr commented 9 years ago

As promised in #20, here's a first-pass at a job LWRP. It uses chef-accumulator to gather the job resources and write them into the config template.

Generation of the config template is moved into the default recipe, although currently there's still also a config template resource in the source recipe. It seemed like at least runit would fail if I took it out, since it tries to start up the service before we've gotten to the end of the run. This duplication seems to work, but I don't like it.

I moved prometheus.source.user, prometheus.source.group and prometheus.source.use_existing_user under just the prometheus namespace, since I think it's important to be able to access those in places other than just the source recipe.

I'm sure there are job attributes I haven't accounted for, and I'm glad to add more before this gets merged. I just wanted to get it up earlier to make sure it was headed in the right direction.

rayrod2030 commented 9 years ago

Looks like a great start. I don't like that our rubocop config enforces single space between method and first argument for cookbook/resources so feel free to add this block to .rubocop.yml: 

Style/SingleSpaceBeforeFirstArg:
  Enabled: false
rayrod2030 commented 9 years ago

Hey @ewr I'd be happy to commit this PR if you can add that rubocop stanza which should remove some of these warnings and we can clean up the line endings. Otherwise I think it looks good and would make the cookbook way more usable for job configuration.

ewr commented 9 years ago

Thanks for the reminder... I had totally missed that the ball was in my court.

I added the Rubocop stanza, fixed an issue in how I was creating the target for the default self-scrape target, and went ahead and merged in master while I was in there. I only tested against default-ubuntu-1404, but I'm sure travis will let us know if I missed anything.

rayrod2030 commented 9 years ago

Travis is probably failing based on some weird rubocop issues. I'll get it sorted after a merge. Thanks a bunch for this contribution @ewr!

rayrod2030 commented 9 years ago

Hmm actually this actually needs a rebase or merge from master due to some previous PR's being merged. Care to fix this PR up before we can merge @ewr? Thanks.

ewr commented 9 years ago

No problem. I merged instead of rebasing, but it should be clean now. It was just a non-conflict conflict in different dependencies getting added to metadata.rb.

I also snuck in de66789 to re-fix #21.

ewr commented 9 years ago

I think that travis rspec failure is related to the way chef-accumulator works, and rspec not seeing the template block write out its expected config. You can see in test-kitchen runs that the template does indeed get written, so I'm not sure whether you want to just pull that rspec test or whether it can be refactored.

rayrod2030 commented 9 years ago

I'll try to pull it first then see a better way to verify that the accumulator is doing the right thing. Thanks for catching that regression on https://github.com/rayrod2030/chef-prometheus/pull/21. I need to add a suite for chef 11.x to make sure it doesn't happen again. Merging!