openstreetmap / mod_tile

Renders map tiles with mapnik and serves them using apache
http://wiki.openstreetmap.org/wiki/Mod_tile
GNU General Public License v2.0
288 stars 190 forks source link

Munin mod_tile_* hardcoded URL #418

Closed zenonp closed 3 months ago

zenonp commented 6 months ago

All four munin mod_tile scripts use "data=$(wget -q http://localhost/mod_tile -O -)". There are many cases where this cannot work, e.g. "Listen 10.22.33.44" in httpd.conf, mod_tile running in a virtual host, mod_tile only being accessible by https, or https with a self-signed certificate.

Changing the wget command in the scripts is easy, but if they have been installed by rpm or deb such changes will be lost at the first update of the munin plugin. A much better solution would be to add a configuration file /etc/munin/plugin-conf.d/mod_tile and let the scripts read the URL and any additional wget arguments (e.g. --no-check-certificate) from there.

hummeltech commented 6 months ago

Yes, I see that. Although Munin usage has been discontinued (https://github.com/openstreetmap/chef/commit/1751b31885773d33dcf3efb396f198f1d10a2221) by OSM, perhaps something like this https://github.com/openstreetmap/mod_tile/compare/master...hummeltech:MuninScripts will do the trick for you. Thanks

zenonp commented 6 months ago

Munin usage has been discontinued

I understand the principle of not maintaining code here that is not actually being used by OSMF, but it's a pity nevertheless. The plugin has been packaged by Debian and Fedora for a long time, so its removal will leave orphans all over the place. For my part, for a new installation, I dismissed prometheus and turned to munin as soon as I saw this. Do you know whether anyone else is taking over the munin plugin maintainance?

perhaps something like this will do the trick

Thank you. Especially for patching something that has already been obsoleted.

hummeltech commented 6 months ago

Sorry about that, I didn't mean to imply that these scripts were actually going to be removed from here as I'm sure there are still plenty of Munin users. The problem I was trying to convey is that I don't have much familiarity with Munin and that these scripts will likely not be getting many updates in the future. I will submit a pull request with the above patch for you to look over and test out and if the solution works for you, I'd be happy to merge it in.

I can see absolutely no reason that it would break any existing deployments.

Thanks

pnorman commented 6 months ago

I understand the principle of not maintaining code here that is not actually being used by OSMF, but it's a pity nevertheless

There is a lot of code here that is not used by the OSMF, I wouldn't say that's a huge consideration.

zenonp commented 6 months ago

The misunderstanding is entirely my fault. I just glanced at 1751b31 and, without even looking at what file that was and where, concluded that the munin plugins had already been removed. My only defence is that I hadn't had my morning coffee yet.

I'll test the dynamically configured scripts and report back in a couple of days.