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

The sensu purge_config option now removes rpm deployed plugins #304

Closed
jaxxstorm opened this issue Jan 28, 2015 · 6 comments
Closed

The sensu purge_config option now removes rpm deployed plugins #304

jaxxstorm opened this issue Jan 28, 2015 · 6 comments

Comments

@jaxxstorm
Copy link
Contributor

The change in #302 has broken a use case in which we deploy the plugins separately via an RPM. The original change in that PR called for a separate config value which I personally believe is the right way to go - config and plugins can often be managed separately and this change means this is now not possible.

@jaxxstorm jaxxstorm changed the title The sensu purge_config plugin now removes rpm deployed plugins The sensu purge_config option now removes rpm deployed plugins Jan 28, 2015
@jlambert121
Copy link
Contributor

I didn't notice purge was added to plugins as well, I agree that shouldn't have been there. I was afraid getting lots of various purge_* settings which would cause more confusion than benefit. What's your thought of just pulling the purge off of the plugins dir? Or is reverting to multiple purge settings desired? With multiple, should we go all the way and add a purge config setting for each managed directory (or maybe an array of directories to purge?)

@jaxxstorm
Copy link
Contributor Author

The original PR states that the change will remove plugins, so I'm not surprised this happened. I think the original requester, @nhinds, even stated that purge_config seems to him to be more for JSON configuration. We use the puppet module to manage the JSON, but we manage the code (plugins, handlers, extensions) separately using RPM's we deploy. I think adding separate options (purge_plugin) and setting them to false by default should be sufficient.

@nhinds
Copy link
Contributor

nhinds commented Jan 29, 2015

Separate options for each (purge_plugins, purge_handlers, etc.) would make package.pp messier (the combined "ensure all these directories exist" resource would need to be split out), but it's doable.

Alternatively we could migrate to a single enum-like setting, e.g. purge => false, purge => config, purge => all? Possibly keeping the purge_config => true flag as an alias for purge => config if you want to maintain backwards compatibility, or remove it entirely to avoid confusion.

@jaxxstorm
Copy link
Contributor Author

I think what @nhinds recommends is the best way forward

@jamtur01
Copy link
Contributor

I am unfussed.

@jlambert121
Copy link
Contributor

This was reverted (and re-added as a separate parameter)

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

No branches or pull requests

4 participants