(#783) Add sensu::check content parameter, use sensu::write_json

[jeffmccune]

sensu::write_json

Without this patch arbitrary to-level configuration scope items cannot be
managed in a sensu check. This patch addresses the problem by adding a
content parameter to the sensu::check defined type.

This patch changes the defined type to pass a JSON object to sensu::write_json
instead of passing each named property to the sensu_check custom type.
sensu::write_json is used to ensure all attributes in a configuration file are
manageable and arbitrary attributes need only be placed in the content hash
map, or have a new parameter defined in the sensu::check defined type.

When adding a new parameter, as long as the parameter and value are passed as a
key/value pare to the sensu_check_content() function, the attribute and value
will be placed in the check configuration scope under checks.mycheck. The same
behavior may be achieved by passing the key and value via the custom
parameter.

The ultimate goal of this work is to eliminate the custom parameter and
eventually refactor all defined types in the Puppet module to use
sensu::write_json and this approach.

Remaining work

• Fix failing tests which are now marked pending.
• Add new expectations and fixture data for the expected output of
  sensu::check via the content property of sensu::write_json.
• Vagrant testing of example checks, ensure backwards compatibility.

Resolves #783

[jeffmccune]

@ghoneycutt I'd love to get your early feedback on this approach of using a parser function to create the output hash for sensu::write_json using the existing parameters of the defined type as input.

[ghoneycutt]

This seems to be the same thing as custom, so I don't think we need it on this iteration.

Next iteration would be to remove all parameters and only have content. At that point we can use the Struct data type for validation.

[jeffmccune]

They're slightly different. Custom is a hash which gets merged into the scope of the check configuration at checks.mycheck. Content is a hash merged onto the top level JSON object. Without content, there isn't a way to specify configuration for the sensu-mailer-plugin example.

I kept this in the define, please take a look at the Puppet DSL behavior replacing the parser function in the latest patch.

[ghoneycutt]

Why can't sensu-mailer-plugin be specified using custom?

[jeffmccune]

They're different configuration scopes. Mailer is up at top-level (the same as "checks"). Custom adds key/value pairs two levels deep within the checks.mycheck scope.

[ghoneycutt]

It was my understanding that custom adds key/values at top scope. So if you want data from custom to be in the checks scope you need the hash in custom specified as such.

Would that work? Does Puppet's $hash + $another_hash work for that?

[jeffmccune]

It was my understanding that custom adds key/values at top scope. So if you want data from custom to be in the checks scope you need the hash in custom specified as such.

We could change the behavior to operate like that, but today the value of custom is assumed to be a hash merged with the value of checks.mycheck. Check it out at https://github.com/sensu/sensu-puppet/blob/v2.30.0/lib/puppet/provider/sensu_check/json.rb#L92

Would that work? Does Puppet's $hash + $another_hash work for that?

We'd probably want to use the deep_merge() function in stdlib. The + operator isn't a deep merge, so duplicated keys result in the rightmost value clobbering the other values.

An example of the gymnastics I'm doing with the Puppet DSL to merge nested hash values is at cddda1a#diff-7bfaf46cbbb09c017062dc6fbdfdd383R262

[ghoneycutt]

I think this would create a key named custom with the data in it and so it should be removed. We can join the hashes like

$content_hash = $resource_hash + $custom

[jeffmccune]

That's effectively what sensu_check_content() does, with content and custom. It could definitely be a little cleaner. I used a function because my first pass at manipulating immutable Hashes the Puppet DSL turned out awkward. Is there a better way to handle the behavior of content and custom to get the final JSON?

Something like:

# as before, with name, custom and content stripped out.
$resource_hash = { ... }
# Final structure of the output.
$content_hash = $content + {'checks' => { "$check_name" => $resource_hash + $custom } }

The only other thing is to deal with all of the undefined and "absent" values, so this should work:

$resource_hash_real = $resource_hash.reduce({}) |$memo, $kv| {
  if ($kv[1] == "absent" or $kv[1] == undef) {
    $memo
  } else {
    $memo + Hash.new($kv)
  } 
}

I'll replace the function with this Puppet DSL and some comments describing the structure.

[jeffmccune]

Behavior has been moved to the Puppet DSL in the latest patch. I'll proceed with it, let me know if you'd still like content removed after taking a look at the structure being built up.

[jeffmccune]

@ghoneycutt This is ready for another review. All of the tests have been updated and are passing and sensu::check behavior is implemented entirely in the Puppet DSL now, no custom Ruby code. This should make it much easier to adapt for the other defined types.

It may make the most sense to merge this together with changes to other defined types so we minimize the number of major version releases.

[jeffmccune]

This should also address #782 but need to test it out.

[jeffmccune]

I think it is:

diff --git a/tests/sensu-server.pp b/tests/sensu-server.pp
index c0a6616..1106403 100644
--- a/tests/sensu-server.pp
+++ b/tests/sensu-server.pp
@@ -64,6 +64,7 @@ node 'sensu-server' {

   # Example check using the cron schedule.
   sensu::check { 'remote_check_ntp':
+    ensure         => 'absent',
     command        => 'PATH=$PATH:/usr/lib64/nagios/plugins check_ntp_time -H :::address::: -w 30 -c 60',
     standalone     => false,
     handlers       => 'default',

$ sudo puppet apply /vagrant/tests/sensu-server.pp
Notice: Compiled catalog for sensu-server.example.com in environment production in 0.78 seconds
Notice: /Stage[main]/Main/Node[sensu-server]/Sensu::Check[remote_check_ntp]/Sensu::Write_json[/etc/sensu/conf.d/checks/remote_check_ntp.json]/File[/etc/sensu/conf.d/checks/remote_check_ntp.json]/ensure: removed
Notice: /Stage[main]/Sensu::Client::Service/Service[sensu-client]: Triggered 'refresh' from 1 events
Notice: /Stage[main]/Sensu::Api::Service/Service[sensu-api]: Triggered 'refresh' from 1 events
Notice: /Stage[main]/Sensu::Server::Service/Service[sensu-server]: Triggered 'refresh' from 1 events
Notice: Applied catalog in 4.24 seconds

[jeffmccune]

This has been rebased.

[ghoneycutt]

Thank you!!

Released in v2.32.0