Munin: Fix broken plugins #31916
Munin: Fix broken plugins #31916
Conversation
|
This reminds me that I tried to fix this myself, in https://github.com/bjornfor/nixpkgs/commits/munin-master. But while testing I found a regression that I couldn't solve/pinpoint, so I dropped it. Looking at this PR, I see the same issue I was having. The list of enabled plugins change! Look in /etc/munin/plugins with/without this PR and see the difference. $ diff -u before-pr after-pr
--- before-pr 2017-11-22 21:05:02.266009841 +0100
+++ after-pr 2017-11-22 21:05:02.267009844 +0100
@@ -7,10 +7,21 @@
df
df_abs
df_inode
+diskstat_iops_sda
+diskstat_iops_sdb
+diskstat_iops_sdc
+diskstat_iops_sdd
+diskstat_latency_sda
+diskstat_latency_sdb
+diskstat_latency_sdc
+diskstat_latency_sdd
+diskstat_throughput_sda
+diskstat_throughput_sdb
+diskstat_throughput_sdc
+diskstat_throughput_sdd
entropy
forks
fw_packets
-hddtemp_smartctl
if_docker0
if_enp3s0
if_enp4s0
@@ -28,7 +39,6 @@
interrupts
irqstats
load
-meminfo
memory
munin_stats
munin_update
@@ -42,11 +52,11 @@
nfsd4
open_files
open_inodes
-port_36113
-port_38357
-port_40641
-port_42059
+port_33867
+port_34535
+port_34809
port_51413
+port_59903
port_apcupsd
port_domain
port_gitBTW, I don't think we should remove "apc_nis" plugin. It's easier to disable it from user config than to bring it back if deleted by the NixOS module. (Also, I have APC UPS.) |
|
I guess the only thing strange in that diff is that the hddtemp_smartctl and meminfo plugins no longer work after applying this PR. |
Add a new patch (adding_sconfdir_munin-node.patch) to be able to configure the location of plugin-conf.d (otherwise it has to be configured at build time). This patch is very similar to the existing 'adding_servicedir_munin-node.patch'.
|
@bjornfor I found two issues. Both hddtemp_smartctl and meminfo work for me now. Some plugins should run as root, hddtemp and meminfo happen to be such plugins. Added plugin level configuration to solve this. Without the wrappers, munin uses user-level state directories. This was broken with the preserve_environment.patch, so I updated it. Some plugins are still broken (e.g., the munin plugin), but as far as I can tell there are no more regressions on the two systems I tested. (Btw. I don't know why this worked before. :/) |
|
@orbekk: Great! Can you squash that last commit so each commit in the PR represents a good state? And |
|
@GrahamcOfBorg test munin |
There was a problem hiding this comment.
Success for system: x86_64-linux
one: exit status 1
syncing
one: running command: sync
one: exit status 0
test script finished in 123.80s
cleaning up
killing one (pid 143)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/tmp/nix-build-vm-test-run-munin.drv-0/vde1.ctl': Directory not empty
/nix/store/rz107zr4n2y0x0q8xlj75y82ng2j8b23-vm-test-run-munin
The munin-node service used wrapProgram to inject environment variables. This doesn't work because munin plugins depend on argv[0], which is overwritten when the executable is a script with a shebang line (example below). This commit removes the wrappers and instead passes the required environment variables to munin-node. Eliminating the wrappers resulted in some broken plugins, e.g., meminfo and hddtemp_smartctl. That was fixed with the per-plugin configuration. Example: The plugin if_eth0 is a symlink to /.../plugins/if_, which uses $0 to determine that it should monitor traffic on the eth0 interface. if_ is a wrapped program, and runs `exec -a "$0" .if_-wrapped` .if_-wrapped has a "#!/nix/.../bash" line, which results in bash changing $0, and as a result the plugin thinks my interface is called "-wrapped".
|
@bjornfor Done. I'd also like to make a few extra configuration variables for Munin, but I can send a follow-up request for that. |
|
Applied to master with bd3e49a and 3 parents. Thanks! I'll backport this to release-17.09 after a round of testing. |
|
Backported to NixOS 17.09 with 340f306 and 3 parents. |
|
thanks. |
Motivation for this change
The munin-node service used wrapProgram to inject environment variables.
This doesn't work because munin plugins depend on argv[0], which is
overwritten when the executable is a script with a shebang line.
Instead, rely on environment variables passed to the systemd units. This
requires another minor patch to munin.
Also fixes #23049 by disabling the
apc_nis plugin.
Things done
The munin.nix test passes.
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)