(#495) Notify Service[sensu-enterprise] from Sensu::Check resources #720
Conversation
| self[:notify] = [ | ||
| "Service[sensu-client]", | ||
| "Service[sensu-server]", | ||
| "Service[sensu-enterprise]", |
There was a problem hiding this comment.
if Service[sensu-enterprise] is not in the catalog, because we aren't using enterprise, does this cause a problem?
There was a problem hiding this comment.
I expect no problem becauseArray#select filters out the resource ref when absent from the catalog.
There was a problem hiding this comment.
Have you verified with vagrant up ?
There was a problem hiding this comment.
No, will do so now.
There was a problem hiding this comment.
@ghoneycutt I modified the provisioning manifest for sensu-server-enterprise to manage Service[sensu-enterprise] instead of Service[sensu-server]. Take a look at this change in 1d694e4, I assumed tests/sensu-server-enterprise.pp is intended to setup sensu-enterprise.
With vagrant up sensu-server, the notification doesn't cause a problem as we expect. With vagrant up sensu-server-enterprise, the notification does properly notify Service[sensu-enterprise] when a managed sensu::check resource changes state.
Without this patch the sensu-server-enterprise Vagrant VM is not managing the Service[sensu-enterprise] resource. This is a problem for testing the behavior changes made in sensu#720. This patch addresses the problem by changing the class parameters of the provisioning manifest for sensu-server-enterprise. The credentials for the Sensu Enterprise repository are expected to be set in the shell environment where `vagrant up` is executed. For example, the following direnv `.envrc` is sufficient for this project: ``` export FACTER_SE_USER='1234567890' export FACTER_SE_PASS='Password1' ```
|
@jeffmccune see PR #502 where they made similar changes, but to other files as well. |
|
@ghoneycutt Got it, I'll update this PR to add the notify to all of the files referenced in #502 |
Without this patch the sensu-server-enterprise Vagrant VM is not managing the Service[sensu-enterprise] resource. This is a problem for testing the behavior changes made in sensu#720. This patch addresses the problem by changing the class parameters of the provisioning manifest for sensu-server-enterprise. The credentials for the Sensu Enterprise repository are expected to be set in the shell environment where `vagrant up` is executed. For example, the following direnv `.envrc` is sufficient for this project: ``` export FACTER_SE_USER='1234567890' export FACTER_SE_PASS='Password1' ```
jeffmccune
force-pushed
the
495_check_should_notify_sensu-enterprise
branch
from
1d694e4 to
ae48859
Compare
|
@ghoneycutt Notifications and spec coverage added to all types listed in #502 This is ready for another review. |
| manage_services => true, | ||
| manage_user => true, | ||
| rabbitmq_password => 'correct-horse-battery-staple', | ||
| rabbitmq_vhost => '/sensu', | ||
| api => true, |
There was a problem hiding this comment.
Why was the API portion removed?
|
The api portion was removed because the API is mutually exclusive with setting sensu::enterprise => true. See init.pp L573:
```puppet
if ( $enterprise and $api ) or ( $enterprise and $server ) {
fail('Sensu Enterprise: sensu-enterprise replaces sensu-server and sensu-api')
}
```
|
|
Released in v2.19.1 |
Without this patch The sensu-enterprise service is not notified
automatically when
sensu::checkresources are declared. This patchaddresses the problem by checking the catalog for the service and
automatically adding the notify relationship.
resolves #502
resolves #495
closes #653