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

Dynamic commands, services, and NRPE associations #16

Merged
merged 3 commits into from Jun 19, 2015
Merged

Dynamic commands, services, and NRPE associations #16

merged 3 commits into from Jun 19, 2015

Conversation

thenewwazoo
Copy link
Contributor

This is the first of two forthcoming PRs relating to dynamic Nagios configuration using Salt.

This PR adds the capability to define a Nagios service check in one place in the pillar data, and for both the NRPE-daemon-running minion and the Nagios-running minion to emit appropriate configurations for each. The data added to pillar.example illustrates how to use this data. As the author, it's hard to judge how straightforward this is; please let me know if actual documentation is necessary.

There is one breaking change: I changed the name of the nagios.nrpe.checks dict (used to embed command[...]=... lines directly into nrpe.cfg) to be nagios.nrpe.nrpe_commands. This is to reduce confusion about where checks (in the broader sense) are defined vs simpler command specifications.

This PR also includes fixes in pillar.example relating to use of the nagios.lookup dict (and the key names defined therein).

Still TODO is porting the dynamic NRPE configuration emitter states to non-Debian distributions (read: distros which don't do /etc/nagios/nrpe.d), and validation that /usr/lib/nagios/plugins/ is a sane default for nrpe.plugin_dir (see map.jinja).

puneetk added a commit that referenced this pull request Jun 19, 2015
Dynamic commands, services, and NRPE associations
@puneetk puneetk merged commit 261ff62 into saltstack-formulas:master Jun 19, 2015
@puneetk
Copy link
Contributor

puneetk commented Jun 19, 2015

In the next pull, can you please update the readme to doc the breaking change and possibly use salt'grains.get' and salt'pillar.get'

@wwentland
Copy link
Contributor

Oh man .. This breaks backwards compatibility as checks is being replaced with nrpe_commands in the pillar and was even mentioned in the discussion. Changes to the pillar datastructure shouldn't be done if there isn't a very good reason as many people might have pushed changes that caused them to remove all nrpe commands from their respective config.

@wwentland
Copy link
Contributor

I am in favour of reverting this PR as it causes a lot of issues and forces users to adapt their pillars. What do you guys think?

@gravyboat
Copy link
Contributor

@BABILEN I'm fine with reverting it since I don't think the issues that were originally brought up were fixed. Just keep in mind it's been around for almost a year now. Is it going to be less work to fix it?

@thenewwazoo
Copy link
Contributor Author

Sorry, I thought it was pretty clear when I wrote "There is one breaking change..." The reason for renaming checks to nrpe_commands is because a nrpe command isn't a check, and was colliding with the (properly named) checks object in the nagios config.

I haven't been following issues, but I've gone through anything linked. What issues remain unresolved? I'm happy to chime in if there's any question.

@wwentland
Copy link
Contributor

Sure, fixing the issues would be much preferred. Let's see if fixing #28 sorts out the last issue.

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

Successfully merging this pull request may close these issues.

None yet

4 participants