Checks not working as expected

[ghoneycutt]

From @alvagante

@ghoneycutt I think the error is due to how checks files are created:
if a single check is added in a conf.d/checks/ file then the json file should begin with:

{
    "check": {

and not

{
    "checks": {

manually changing the files in conf.d/checks/ on the Vagrant sensu-server vm allows the api service to start.

[ghoneycutt]

@agoddard This seems like a sensu bug if you cannot specify checks with only a single check. The docs lead you to believe that the plural form is correct.

[agoddard]

@ghoneycutt it should be checks and not check, and a single key within checks should work - what was the error you saw?

Here's my config for my test:

    "checks": {
        "test": {
            "command": "true",
            "subscribers": [
                "test"
            ],
            "interval": 10,
            "contact": "ops"
        }
    },

and here's the log showing the config in use:

{"timestamp":"2017-08-24T14:30:26.640000-0600","level":"info","message":"publishing check request","payload":{"command":"true","contact":"ops","name":"test","issued":1503606626},"subscribers":["test"]}

[alvagante]

@agoddard so, I'm finding this behaviour on the sensu-server vagrant vm of the puppet module .
3 files are created under /etc/sensu/conf.d/checks.

They are like this:

{
    "checks": {
        "check_ntp": {
            "command": "PATH=$PATH:/usr/lib64/nagios/plugins check_ntp_time -H pool.ntp.org -w 30 -c 60",
            "handlers": "default",
            "interval": 60,
            "standalone": true,
            "subscribers": "sensu-test"
        }
    }
}

and I get these errors:

{"timestamp":"2017-08-24T20:34:56.349771+0000","level":"fatal","message":"check handlers must be an array","object":{"command":"PATH=$PATH:/usr/lib64/nagios/plugins check_ntp_time -H pool.ntp.org -w 30 -c 60","handlers":"default","interval":60,"standalone":true,"subscribers":"sensu-test","name":"check_ntp"}}
{"timestamp":"2017-08-24T20:34:56.349796+0000","level":"fatal","message":"SENSU NOT RUNNING!"}

If I change the above file to :

{
    "check": {
        "check_ntp": {
            "command": "PATH=$PATH:/usr/lib64/nagios/plugins check_ntp_time -H pool.ntp.org -w 30 -c 60",
            "handlers": "default",
            "interval": 60,
            "standalone": true,
            "subscribers": "sensu-test"
        }
    }
}

the api service starts.
Package installed is sensu-1.0.2-1.el7.x86_64

[agoddard]

@alvagante ah! ok cool, that makes sense - check isn't a config option, so it's being ignored in the second example, which means the config options under it are not being validated (so with no validation issues, the API service starts but the check won't run).
The root issue is that the value for "handlers" is not an array in the broken example. It either needs to always use handlers and always make the value an array, even if it has a single element e.g.:

"handlers": ["default"]

or it needs to use "handler", but only when there is a single handler, e.g.:

"handler": "default"

I would recommend the first approach (always using "handlers" and always making the value an array) for simplicity.

[alvagante]

@agoddard great, now everything is clearer (sorry but my sensu experience is still basic (but I should have looked better at the log message)).
@ghoneycutt @jeffmccune I think we have to enforce an array somewhere here: https://github.com/sensu/sensu-puppet/blob/master/manifests/check.pp#L176
given that we accept also normal strings for the handler parameter (https://github.com/sensu/sensu-puppet/blob/master/manifests/check.pp#L97 which reproduces the previous behaviour wherre both strings and arrays were accepted).
I suspect that this conversion of strings to single element arrays might be needed somewhere else in check.pp (subscribers, aggregates...)

[ghoneycutt]

Can you correct this by casting the string as an array? Also if a string is detected, we should not that it's deprecated and to switch to using an array.

[alvagante]

Yes, if I pass an array it works, same applies to subscribers (and probbaly aggreagates, but haven't tested directly).
We can check for the type, send deprecation notices and pass always an array. Can do this tomorrow, night time here..

[alvagante]

FYI: in the pr I convert string to array (with deprecation notice) to the following check params :se configs for the following parameters: handlers, subscribers, aggregates, contacts, dependencies.

This somehow reproduces the params of the old sensu_check type.

[ghoneycutt]

Released in v2.33.1