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.

Sign up for GitHub

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

Jump to bottom

nagios: various improvements #76551

Merged
merged 2 commits into from Dec 30, 2019
Merged

nagios: various improvements #76551

merged 2 commits into from Dec 30, 2019

Conversation

symphorien
Copy link
Member

  • structured config for main config file allows to launch nagios in
    debug mode without having to write the whole config file by hand
  • build time syntax check
  • all options have types (All options should have types #76184) , one more example
  • I find it misleading that the main nagios config file is linked in
    /etc but that if you change the link in /etc/ and restart nagios, it
    has no effect. Have nagios use /etc/nagios.cfg
  • fix paths in example nagios config files, which allows to reuse it:
services.nagios.objectDefs =
  (map (x: "${pkgs.nagios}/etc/objects/${x}.cfg")
  [ "templates" "timeperiods" "commands" ]) ++ [ ./main.cfg ]
  • for the above reason, add mailutils to default plugins
Things done

I tested by running nagios for a few days on 19.09.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @thoughtpolice @relrod

@aanderse
Copy link
Member

@symphorien do you (or anyone) actually use nagios on NixOS?

@ofborg ofborg bot requested a review from thoughtpolice December 26, 2019 20:24
@symphorien
Copy link
Member Author

since a few days I have been setting one nagios instance on nixos, yes.

@aanderse
Copy link
Member

Nice! I was evaluating it for a while but ended up going with zabbix.

The nagios module is pretty much abandoned at this point... would you be interested in becoming a maintainer for this module?

@symphorien
Copy link
Member Author

I added myself as maintainer of the module.

aanderse
aanderse requested changes Dec 27, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Thanks for listing yourself as a maintainer! I was recently thinking we should remove the module because it is so unmaintained!

This change looks good. Just one minor nitpick.

Do you have any motivation either in this PR or a future PR to write a NixOS test? This has always been a sticky point for me when making changes to the nagios module or packages in NixOS, and I always thought a NixOS module would be really cool for nagios as it could demonstrate a fair bit.

Gosh now that I think about it... a zabbix test might be pretty cool too, right @mmahut 😉 (just razzing 😄)

nixos/modules/services/monitoring/nagios.nix Outdated Show resolved Hide resolved
@symphorien
Copy link
Member Author

A test that checks that alerts work might be easy to do, but not testing the web interface. I might try if I find the time and motivation, we'll see.

@dasJ dasJ mentioned this pull request Dec 29, 2019
Closed
16 tasks
@aanderse
Copy link
Member

@symphorien this looks good. Can you possibly fixup the whitespace in the code generating nagios-checked.cfg? I'm interested in running this... then merging it. Do you have a minimal test example I could reproduce? Thanks!

@symphorien
Copy link
Member Author

symphorien commented Dec 30, 2019 •
edited

@aanderse I fixed the spacing and added a simplistic nixos test.

aanderse
aanderse approved these changes Dec 30, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

@symphorien you're a rock star! 🚀 Please squash into either 1 or 2 commits (if 2 then a single commit for the module and a single commit for the test, your call) and we'll merge this.

Thank you!

symphorien+git@xlumurb.eu and others added 2 commits December 30, 2019 16:40
@aanderse @symphorien
nagios: various improvements
be25e45 
* structured config for main config file allows to launch nagios in
debug mode without having to write the whole config file by hand
* build time syntax check
* all options have types, one more example
* I find it misleading that the main nagios config file is linked in
/etc but that if you change the link in /etc/ and restart nagios, it
has no effect. Have nagios use /etc/nagios.cfg
* fix paths in example nagios config files, which allows to reuse it:
  services.nagios.objectDefs =
   (map (x: "${pkgs.nagios}/etc/objects/${x}.cfg")
   [ "templates" "timeperiods" "commands" ]) ++ [ ./main.cfg ]
* for the above reason, add mailutils to default plugins

Co-Authored-By: Aaron Andersen <aaron@fosslib.net>
@symphorien
nagios: add nixos test
cb38bf3
@symphorien
Copy link
Member Author

done

@aanderse
Copy link
Member

@GrahamcOfBorg test nagios

@aanderse
Copy link
Member

@GrahamcOfBorg eval

@aanderse aanderse merged commit 66bf754 into NixOS:master Dec 30, 2019
@symphorien symphorien deleted the nagios2 branch March 21, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dasJ dasJ dasJ left review comments

@aanderse aanderse aanderse approved these changes

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@symphorien @aanderse @dasJ