Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[542] Add dependencies for sensu_gem plugins #817

Merged
merged 3 commits into from
Jan 15, 2018

Conversation

glarizza
Copy link

Previously, on upgrades to Sensu that installed a new embedded Ruby
installation, there was a chance that plugins installed with the sensu_gem
package provider would be evaluated BEFORE the Sensu package was upgraded
(which would mean that a second run of Puppet was necessary to get the plugins
installed into the new embedded Ruby installation). This commit adds a resource
chain ensuring that a change to the Sensu package will come before any package
resources using the sensu_gem provider (which would in turn notify the Sensu
services of their change).

Previously, on upgrades to Sensu that installed a new embedded Ruby
installation, there was a chance that plugins installed with the `sensu_gem`
package provider would be evaluated BEFORE the Sensu package was upgraded
(which would mean that a second run of Puppet was necessary to get the plugins
installed into the new embedded Ruby installation). This commit adds a resource
chain ensuring that a change to the Sensu package will come before any package
resources using the `sensu_gem` provider (which would in turn notify the Sensu
services of their change).
@glarizza glarizza changed the title Add dependencies for sensu_gem plugins [542] Add dependencies for sensu_gem plugins Sep 21, 2017
@glarizza
Copy link
Author

Tied to #542

# necessary to get the plugins installed into the new embedded Ruby folder.
Package['sensu']
~> Package<| provider == 'sensu_gem' |>
~> Service['sensu-client', 'sensu-server', 'sensu-api']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test with the enterprise version as it uses different service names, we would need to take that into account.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I'll test and most likely will ultimately add sensu-enterprise to the service list. I think we're fine with only needing Package['sensu'] because I think the Sensu core package is the one that installs the embedded Ruby (not sure what Package['sensu-enterprise'] adds, but as long as it's something other than the Ruby implementation I don't care about it).

This commit adds a script that will allow you to replicate and check for the
problem where plugins may get installed before a sensu package change. Note
that I have hardcoded version '0.23.0' as an 'older' version of Sensu because
I know it uses a different Ruby MRI than Latest. This may need adjusted later.
@glarizza
Copy link
Author

glarizza commented Sep 21, 2017

Tests are failing on some rspec-puppet tests because of the last line:

Package['sensu'] ~>
Package<| provider == 'sensu_gem' |> ~>
Service['sensu-client', 'sensu-server', 'sensu-api']

On cases where the api or client hasn't been installed there won't be any resources for the sensu-api or sensu-client service. In its place I can do something like this:

~> Service<| title == 'sensu-client' or title == 'sensu-server' or title == 'sensu-api'|>

That collector will discover the resources if they're there and setup the dependencies, or exit silently if they're not in the catalog. It's not the prettiest, but it will do.

Package['sensu']
~> Package<| provider == 'sensu_gem' |>
~> Service<| title == 'sensu-client' or title == 'sensu-server' or title == 'sensu-api'|>
#~> Service['sensu-client', 'sensu-server', 'sensu-api']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still working through the enterprise side?

These will need the corresponding spec tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to pull out that comment for sure. As for the enterprise side, I'm waiting for @agoddard to reply to what services (if any) on the enterprise we will need to bump (see: #542 (comment)). I don't think we need to bounce that service, but I'll add it when I get a reply

Tests were failing because of this stanza (and specifically the last line):

```puppet
Package['sensu'] ~>
Package<| provider == 'sensu_gem' |> ~>
Service['sensu-client', 'sensu-server', 'sensu-api']
```

On cases where the api or client hasn't been installed there won't be any
resources for the sensu-api or sensu-client service. In its place is this line:

```puppet
~> Service<| title == 'sensu-client' or title == 'sensu-server' or title == 'sensu-api'|>
```

That collector will discover the resources if they're there and setup the
dependencies, or exit silently if they're not in the catalog. It's not the
prettiest, but it will do.
@glarizza glarizza force-pushed the 542_sensu_gem_dependencies branch from ba63dbd to b1be918 Compare September 22, 2017 20:51
@ghoneycutt
Copy link
Collaborator

ping @agoddard

@cwjohnston
Copy link
Contributor

@ghoneycutt I think I'm missing the outstanding question here. Are we trying to determine how to handle the case where no Sensu service resources are defined?

@ghoneycutt ghoneycutt self-assigned this Jan 15, 2018
@ghoneycutt ghoneycutt merged commit 337bba4 into sensu:master Jan 15, 2018
@ghoneycutt
Copy link
Collaborator

Fixes #542

@ghoneycutt
Copy link
Collaborator

Released in v2.46.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants