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

ITSE-3681 replace global variables with facter #1

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

wamui33
Copy link

@wamui33 wamui33 commented Sep 4, 2018

No description provided.

@wamui33 wamui33 requested a review from vchepkov September 4, 2018 16:33
$plugin_nginx = false,
$plugin_xcache = false,
$selinux = $::selinux,
$selinux = $facter['os']['selinux']['enabled'],
Copy link

Choose a reason for hiding this comment

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

Should be facts, right?

if $::nagios_pci_mptsas { class { '::nagios::check::mptsas': } }
if $::nagios_mysqld {
case $::operatingsystem {
if $facts['nagios_couchbase'] { class { '::nagios::check::couchbase': } }
Copy link

Choose a reason for hiding this comment

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

class doesn't need :: either
in fact, class notation here is unnecessary, should be include nagios::check::couchbase instead

}

# With selinux, some nrpe plugins require additional rules to work
if $selinux and $::selinux_enforced {
if $selinux and $facts['selinux_enforced'] {
Copy link

Choose a reason for hiding this comment

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

for uniformity, it's os.selinux.enforced

Copy link

@vchepkov vchepkov left a comment

Choose a reason for hiding this comment

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

see comments above

@wamui33 wamui33 force-pushed the ITSE-3681-replace-global-scope-variables-with-facts branch from 8bbf5cb to ed9bb62 Compare September 5, 2018 11:07
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.

2 participants