Add check_curl to ITL

[RincewindsHat]

Adding check_curl from Monitoring Plugins to the ITL

[Al2Klimov]

And the documentation?

[RincewindsHat]

Added some documentation

[Al2Klimov]

Have a look at the failed checks.

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Lorenz Kästle.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Lorenz Kästle.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[hydrapolic]

This would be nice as check_http is going to be removed monitoring-plugins/monitoring-plugins#1806

[oxzi]

The following options from the man page are missing in the CheckCommand:

• --continue-after-certificate
• -4 and -6
• --haproxy-protocol
• ---cookie-jar=FILE

Furthermore, as already mentioned by @hydrapolic, if the monitoring-plugins project (or you in specific) aims to obsolete the good old check_http plugin, please add a comment to the ITL documentation of check_http, stating that this plugin will be deprecated and check_curl should be used instead.

[RincewindsHat]

The following options from the man page are missing in the CheckCommand:

* `--continue-after-certificate`

* `-4` and `-6`

* `--haproxy-protocol`

* `---cookie-jar=FILE`

I did add those options (they were added after I opened this PR...., except for -4 and -6 which I somehow forgot halfway)

Furthermore, as already mentioned by @hydrapolic, if the monitoring-plugins project (or you in specific) aims to obsolete the good old check_http plugin, please add a comment to the ITL documentation of check_http, stating that this plugin will be deprecated and check_curl should be used instead.

This is slightly tricky, check_http is going to be deprecated in the monitoring-plugins which is the one you get on debianish and suseish systems (and others).
This is probably not true for the check_http from the nagios-plugins.

[oxzi]

The following options from the man page are missing in the CheckCommand:

* `--continue-after-certificate`

* `-4` and `-6`

* `--haproxy-protocol`

* `---cookie-jar=FILE`

I did add those options (they were added after I opened this PR...., except for -4 and -6 which I somehow forgot halfway)

Thanks for keeping it up to date. I know, long lived (or long ignored) PRs are keeping one's patience to the test.

Furthermore, as already mentioned by @hydrapolic, if the monitoring-plugins project (or you in specific) aims to obsolete the good old check_http plugin, please add a comment to the ITL documentation of check_http, stating that this plugin will be deprecated and check_curl should be used instead.

This is slightly tricky, check_http is going to be deprecated in the monitoring-plugins which is the one you get on debianish and suseish systems (and others). This is probably not true for the check_http from the nagios-plugins.

Valid point. I tend to forget the multiple implementations. Maybe add a short note stating that users of the monitoring-plugins should take a look at check_curl. My train of thought was that doing so might ease the deprecation process for you.

[oxzi]

I just imported the CheckCommand into a playground setup to replace the "http" CheckCommand. Thereby I experienced some issues, as commented inline. However, with the named workarounds or posted patches, it worked for me :)

[oxzi]

Could you please rebase this PR branch against the current master. Doing so should please the CI.

[oxzi]

I hope this was my last round of comments and we can finish this long march.

Regardless of my nagging, I want to say thanks again and have even replaced the check_http with your check_curl for my private monitoring.

[oxzi]

Thanks again for all your work. For me, this looks good now.

@yhabteab and @Al2Klimov, you have also taken a look in the past. Thus, would at least one of you take another look?

Btw, I would suggest to "squash-and-merge" this PR, as, at least imho, not all commits are needed in the master branch.

[yhabteab]

The --extra-opts option is still missing!

Edit: Apart from that why do I get this warning on Debian? WARNING: check_curl is experimental. Please use check_http if you need a stable version.

[oxzi]

The --extra-opts option is still missing!

I am not quite sure how useful the monitoring plugins extra-opts are for an Icinga 2 CheckCommand, but it seems like (some) other CheckCommands are implementing those as well.

icinga2/itl/command-plugins.conf

Lines 1986 to 1988 in 894d6aa

	"--extra-opts" = {
	value = "$apt_extra_opts$"
	description = "Read options from an ini file."

Edit: Apart from that why do I get this warning on Debian? WARNING: check_curl is experimental. Please use check_http if you need a stable version.

Depends on your Debian package version, I guess. Unlike in sid, the monitoring-plugin package versions are quite old.

https://packages.debian.org/search?keywords=monitoring-plugins

I tested it with the latest monitoring-plugins version, which works fine.

[RincewindsHat]

The --extra-opts option is still missing!

I guess I forgot about it, since I haven't ever seen anyone use it (in any of the MP plugins).
Added it now.

[oxzi]

Edit: Apart from that why do I get this warning on Debian? WARNING: check_curl is experimental. Please use check_http if you need a stable version.

Depends on your Debian package version, I guess. Unlike in sid, the monitoring-plugin package versions are quite old.

https://packages.debian.org/search?keywords=monitoring-plugins

I tested it with the latest monitoring-plugins version, which works fine.

I thought that the check plugin returned a WARNING, but I missed the part in the --help output, which seems to be a forgotten relict. As I just wanted to create an upstream issue about it, I found out that @RincewindsHat just resolved it: monitoring-plugins/monitoring-plugins#2017

[yhabteab]

Should be fine, I guess!

http:

WARNING - Certificate 'Yonas Icinga' expires in 189 day(s) (Fri Mar  7 14:31:58 2025 +0000).
HTTP WARNING: HTTP/1.1 200 OK - 11031 bytes in 0.007 second response time |time=0.006887s;;;0.000000;10.000000 size=11031B;;;0;

curl:

WARNING - Certificate 'Yonas Icinga' expires in 189 day(s) (Fri Mar  7 14:31:58 2025 +0000).
HTTP OK: HTTP/1.1 200 OK - 11031 bytes in 0.008 second response time |time=0.007655s;;;0.000000;10.000000 size=11031B;;;0;

almost two years old