feature: Adds support for enabling / disabling modules per OS #4963

Merged
merged 16 commits into from Nov 13, 2016

Projects

None yet

4 participants

@vitalisator
Contributor
vitalisator commented Nov 9, 2016 edited

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

vitalisator added some commits Nov 9, 2016
@vitalisator vitalisator fixes #4946
first try, should be checked first
only discovery first
055f601
@vitalisator vitalisator add OS modules status indication
110893f
@vitalisator
Owner

#4946

need a solution to reset device module states

vitalisator added some commits Nov 9, 2016
@vitalisator vitalisator add debug output
add os poller module support
57c7a83
@vitalisator vitalisator fix some whitespaces
156a8cd
@vitalisator vitalisator fix missing semicolon
c80b090
@vitalisator vitalisator fix some whitespaces
e1d5b78
@murrant murrant Modify the modules page
It now shows unset for unset device settings.
Also, `$os_module_status = $config['os'][$device['os']]['poller_modules'][$module];` doesn't work like you would expect. Fix that behavior.
706a60e
@murrant

I made a change to the modules.inc.php file, but left a little bit for the poller/discovery.

Also, you'll need to add documentation. If you wanted extra points, you could make a unit test, but that is not required.

includes/polling/functions.inc.php
@@ -247,7 +247,14 @@ function poll_device($device, $options)
}
}
foreach ($config['poller_modules'] as $module => $module_status) {
- if ($force_module === true || $attribs['poll_'.$module] || ( $module_status && !isset($attribs['poll_'.$module]))) {
+ $os_module_status = $config['os'][$device['os']]['poller_modules'][$module];
+ d_echo("Global module enabled: ".$module_status. "\n");
@murrant
murrant Nov 11, 2016 edited Contributor

This is too verbose, if you can condense to one line that would be great.
"Enabled G+ O- D+" or something like that. or maybe c_echo("%gEnabled %gGlobal,%rOS,%gDevice%n", $debug);

includes/polling/functions.inc.php
+ if ($force_module === true ||
+ $attribs['poll_'.$module] ||
+ ($os_module_status && !isset($attribs['poll_'.$module])) ||
+ ($module_status && !isset($os_module_status) && !isset($attribs['poll_' . $module]))) {
@murrant
murrant Nov 11, 2016 Contributor

This isset won't work as expected.

includes/polling/functions.inc.php
@@ -275,6 +282,8 @@ function poll_device($device, $options)
}
} elseif (isset($attribs['poll_'.$module]) && $attribs['poll_'.$module] == '0') {
echo "Module [ $module ] disabled on host.\n";
+ } elseif (isset($os_module_status) && $os_module_status == '0') {
@murrant
murrant Nov 11, 2016 Contributor

and here

includes/discovery/functions.inc.php
+ if ($force_module === true ||
+ $attribs['discover_' . $module] ||
+ ($os_module_status && !isset($attribs['discover_' . $module])) ||
+ ($module_status && !isset($os_module_status) && !isset($attribs['discover_' . $module]))) {
@murrant
murrant Nov 11, 2016 Contributor

isset($os_module_status) won't work as expected. You'll need to do isset($config['os'][$device['os']]['discovery_modules'][$module])

@vitalisator
vitalisator Nov 12, 2016 edited Contributor

hmm, are you sure? I have tested and it works fine. If I understand it right a assignment of an unset array value results an unassigned variable too.
But I'm not a PHP expert :-)

In C I would prefer something like that (by reference):
$os_module_status = &$config['os'][$device['os']]['poller_modules'][$module];

My intention was just make the code some readable

@@ -145,6 +152,8 @@ function discover_device($device, $options = null)
echo "#### Unload disco module $module ####\n\n";
} elseif (isset($attribs['discover_' . $module]) && $attribs['discover_' . $module] == '0') {
echo "Module [ $module ] disabled on host.\n\n";
+ } elseif (isset($os_module_status) && $os_module_status == '0') {
@murrant
murrant Nov 11, 2016 Contributor

And here. You could probably find a way to only have one isset($config['os'][$device['os']]['discovery_modules'][$module])

@murrant murrant Fix mangled tabs
fd5a303
@vitalisator vitalisator optimize debug output
add docs
d3f0bf9
@vitalisator vitalisator text highliting
2c623af
@laf laf changed the title from Issue 4946 to Adds support for enabling / disabling modules per OS Nov 13, 2016
@murrant murrant Streamline docs #1
There is no unix-agent discovery module :0
bbeff80
murrant added some commits Nov 13, 2016
@murrant murrant Update docs
528fead
@murrant murrant Update Poller Support.md
5599e4b
@murrant murrant Update Discovery Support.md
9285c8c
@murrant murrant Spell out Global, OS, Device.
38e4ad4
@murrant murrant Spell out Global, OS, Device.
90f3f33
@murrant murrant changed the title from Adds support for enabling / disabling modules per OS to feature: Adds support for enabling / disabling modules per OS Nov 13, 2016
@murrant murrant merged commit 8841eec into librenms:master Nov 13, 2016

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@scrutinizer-notifier

The inspection completed: 3 new issues

@vitalisator vitalisator deleted the vitalisator:issue-4946 branch Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment