Implement Job LWRP #22
Conversation
* Adds a `job` LWRP and an accumulator to write jobs into the prometheus config file. * Move `prometheus.source.user` and `prometheus.source.group` to `prometheus.user` and `prometheus.group` so that we can use them in other recipes * FIXME: `source` and `default` both define `template`s for the config file * FIXME: Are there other attributes that the `job` resource needs to accept?
| @@ -0,0 +1,7 @@ | |||
| action :create do | |||
|
|
|||
There was a problem hiding this comment.
Extra empty line detected at block body beginning.
|
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 |
|
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. |
Conflicts: recipes/default.rb recipes/source.rb
|
|
||
| accumulator node['prometheus']['flags']['config.file'] do | ||
| filter { |res| res.is_a? Chef::Resource::PrometheusJob } | ||
| target :template => node['prometheus']['flags']['config.file'] |
There was a problem hiding this comment.
Use the new Ruby 1.9 hash syntax.
|
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 |
|
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! |
|
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. |
Conflicts: metadata.rb
|
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. |
|
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 #21. I need to add a suite for chef 11.x to make sure it doesn't happen again. Merging! |
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
defaultrecipe, although currently there's still also a config template resource in thesourcerecipe. 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.groupandprometheus.source.use_existing_userunder just theprometheusnamespace, 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.