Hiera sensu client config #373
Conversation
* 'master' of github.com:pdaugavietis/sensu-puppet: update yum repo location ensure plugins installed before client service started add lsbdistid in tests facts include apt before apt::source call Fixing acceptance test for apt::source Fixing apt::source to apt module new version Update metadata.json allow modification of hasrestart attribute for services Filter attributes are a property, not a param
|
bah - not working after all - need to play more (works for me locally - but not against test cases :( |
|
What's the issue you're attacking with this PR? |
|
#366 - merging all layers of hiera, instead of lowest one - works fine locally, but the tests seem to fail :( and the one that passed looks OK - but doesn't appear to merge them. so i'm trying again :) apologies for the hacking... |
|
this one is looking good now... needed to combine the stdlib merge/union, with defaults and default hiera_array/hiera_hash values. working for me as needed |
|
I tend to override these settings from the default in my config, having them now be merged by default would be an issue for me personally. What's the issue you're working to solve? To go down this path I think it needs to be an option, not the default. |
|
i'm not sure i follow your comment - how to override - just update the module after installed to call hiera_array/hiera_hash? I'm having default subscriptions and/or client_custom data, based on certain facts, which are different layers of hierarchy (default, network, production/non, etc.) - and i want to merge all the layers of hiera into a single array/hash (as needed) to allow that type of config - i don't want to just override ALL the configs again on lowest level (hosts/fqdn in our case). To avoid duplication, that's why im using hiera_hash/array wrapped in a merge/union as well. |
|
I'll take pointers for how to make it more optional than default though... and i think i just realized what you meant - you override (change) the subscriptions/client_custom (more?) on lower levels of the hierarchy, instead of just mashing them together... fair point - this wouldn't allow for override, only merge... dammit ;P |
|
okay - as discussed on IRC - i'll rework this a bit to have override/merge triggers, instead of by default merge behaviour. |
|
need add one more merger -- for plugins as well... |
|
A couple of things on this when you're ready... please squish everything down to 1 commit and we need some tests around this as well. |
|
k - how to squish, and how to test :) |
|
Quickly googling on squishing I found this: https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request (look at squash) Same on testing I found this: http://ask.puppetlabs.com/question/60/what-is-the-recommended-method-for-testing-modules-that-use-hiera/?answer=17865#post-id-17865 |
|
Why is it necessary to introduce hiera calls from within the module. IMHO this is a very bad decision. Module's should not do hiera calls anymore. You can do the hiera call yourself outside the module and provide its result by using the respective parameter to sensu, i.e. in manifests/init.pp there's simply no need for using a hiera_call. The call to hiera_hash is client code and its result provded by $plugins. |
|
fair thought -- this is to provide merge options to override default hiera/puppet behaviour -- if this shouldn't sit in the module, you'd have to wrap a module/class around this one, to provide the override behaviour, which is the other way to go. i'm open to discussion and direction - i certainly have the need to override base behaviour, but others could still use it too? |
|
After @mrolli's comment I actually think that may be a better solution... This could be as simple as doing the hiera functions in your profile: class { 'sensu':
subscriptions => union ( hiera_array('sensu::subscriptions', []), $sensu::subscriptions )
....
} |
|
fair nuff On Tue, Jun 30, 2015 at 5:17 PM, Justin Lambert [email protected]
|
ghost
closed this
attempting to fix test cases for merging - refer to #366