Page MenuHomePhabricator

Evaluate "drop in" replacement for nrpe scripts
Open, Needs TriagePublic

Description

In the process of deprecating/removing icinga we have a few scripts invoked via nrpe (i.e. locally on the host, as opposed on the icinga host). It would be convenient to at least evaluate if we can ease the icinga nrpe migration story by being able to use the nrpe scripts more or less unchanged.

Problem statement

Icinga checks can contain arbitrary logic since the execution model is "exec the program and alert based on its exit status". Over time we have written, deployed and tested checks with complex logic to ensure services work as expected.

As we migrate away from Icinga and onto Prometheus/Alertmanager some of these checks can be rewritten to be based on service metrics instead. However for some we can't do it now (e.g. no service metrics yet, no engineering resources, etc) and thus we have to support an "icinga compatibility layer".

In practical terms this means for puppet users to be able to run an icinga check on their hosts and get an alertmanager alert back if something goes wrong (i.e. exit status != 0). To support the current use cases the alert should also include the first line of stdout/stderr.

Scope

Checks with significant and complex logic are in scope to be used by the compatibility layer. Simple checks or checks with modern replacements are not in scope. (e.g. check_procs nowadays is covered by systemd metrics and SystemdUnitFailed alert or check_http is covered by blackbox checks and their alerts).

Something else to consider is checks for which we use icinga's event_handler, i.e. raid checks where we use the event handler to gather more information/context to be included in the task description. The options below do not currently cater for said use case which we'll have to think about separately.

Implementation options

1. Dedicated exporter

With this option we have the following:

  1. https://github.com/canonical/nrpe_exporter deployed to the host, which talks to the local nrpe server
  2. puppet builds the exporter configuration, configuring how to run each script
  3. puppet configures prometheus to scrape said script
  4. puppet deploys a corresponding prometheus alert for metrics
Pros
  1. nrpe exporter is already written and tested
  2. the nrpe::monitor_service puppet code can stay in place as-is, since we would be essentially talking to nrpe-server instead of icinga
Cons
  1. nrpe-server keeps running on the hosts, which is literally remote command execution
  2. command output is not reported in metrics and thus in the alert
  3. we need to scrape (i.e. run the nrpe commands) at most every five minutes to have continuous metrics on prometheus

2. Using node-exporter textfile wrapper

With this solution we develop a wrapper to execute the given script and capture its exit status, and stdout/stderr. The metrics are written in Prometheus format on the filesystem for node-exporter to pick up. A systemd timer periodically executes the wrapper + script.

Since the stdout/stderr are arbitrary strings they will need to be included as label values, and space on label values is limited. For this reason the idea with this solution is to include the first line of stderr/stdout as label value, for example:

check_exit_status{name="check_foo"} 1
check_output{name="check_foo", stdout="first line of stdout", stderr="first line of stderr"} 1
Pros
  1. nrpe is out of the picture
  2. scripts can be executed with any cadence (i.e. not max every 5 minutes)
  3. stdout/stderr can be included in alert text
Cons
  1. some (not a lot) code to be written and tested for said wrapper
  2. more changes at puppet level
  3. potentially high cardinality metrics if output changes often

See T384472: Candidate nrpe checks for compatibility layer icinga/prometheus/alertmanager for a list/audit of candidates checks/scripts

3. Modify the remaining check_ scripts to emit metrics to a push gateway

This option offers the same pros and cons as option 2 with the benefit of not needing a wrapper.

Pros
  1. nrpe is out of the picture
  2. scripts can be executed with any cadence (i.e. not max every 5 minutes)
  3. stdout/stderr can be included in alert text
  4. Doesn't need a wrapper.
Cons
  1. some (not a lot) code to be written and tested for said wrapper
  2. more changes at puppet level
  3. potentially high cardinality metrics if output changes often

Related Objects

Event Timeline

While reflecting on the options above, an option 3 comes to mind where we would modify the check_ scripts that will remain to emit metrics to a push gateway. Pros and cons are mostly cut/paste from option 2, with a pro of not needing a wrapper.

Could you clarify a bit for the various options how the stdout/err of the script will be stored/exposed in the alert to the user? It's not fully clear to me how that will work, in particular for cases with larger output.

I suggest discarding option 1 due to security concerns. The engineering effort required to implement option 2 or option 3 (proposed by Keith) is worthwhile in the long run to mitigate the risks associated with having RCE enabled on the hosts.

I'm for option 3, especially if it comes with a "staleness" indicator we can leverage. Otherwise, option 2 seems sound.

I'd prefer to mitigate the cardinality concerns by not sending std(out|err) to metrics as part of the solution. Logs are better suited for such things. Since alerts must come with a dashboard and/or runbook links, the textual output feels less important than the indicator itself.

Could you clarify a bit for the various options how the stdout/err of the script will be stored/exposed in the alert to the user? It's not fully clear to me how that will work, in particular for cases with larger output.

I clarified the option 2 for stdout/stderr handling, I'm assuming you are referring to raid checks with larger output and I've added a note to that effect too. What do you think ?

While reflecting on the options above, an option 3 comes to mind where we would modify the check_ scripts that will remain to emit metrics to a push gateway. Pros and cons are mostly cut/paste from option 2, with a pro of not needing a wrapper.

The biggest "con" I see with this solution is that the scripts vary wildly in both programming language and complexity. To give an example, we would have to change shell scripts to also emit metrics to the push gateway, I think on balance the effort required for the wrapper is less than having to inspect, change and test every script (in the order of ~30)

I'm for option 3, especially if it comes with a "staleness" indicator we can leverage. Otherwise, option 2 seems sound.

I'd prefer to mitigate the cardinality concerns by not sending std(out|err) to metrics as part of the solution. Logs are better suited for such things. Since alerts must come with a dashboard and/or runbook links, the textual output feels less important than the indicator itself.

I clarified a bit stdout/stderr handling in metrics (i.e. including only the first line), and I understand what you are saying re: cardinality, I'm hoping the cases where stdout/stderr is required are limited (or none, ideally!) and for the rest we can rely on logs instead.

Could you clarify a bit for the various options how the stdout/err of the script will be stored/exposed in the alert to the user? It's not fully clear to me how that will work, in particular for cases with larger output.

I clarified the option 2 for stdout/stderr handling, I'm assuming you are referring to raid checks with larger output and I've added a note to that effect too. What do you think ?

Thanks for the clarification @fgiunchedi. I agree we need to find a separate solution for things where large output is required somewhere.
As for the first line of stdout/err, I'd suggest to also cut it to X chars/bytes as one line could potentially be very long :)

Personally it seems a bit of a stretch to use labels for stdout/err, and I'm wondering if whatever solution we will find for the ones with large outputs might be best suited also for those where smaller outputs are needed. But I'll leave that to your team to decide what's best.

As for the "large output" kind of script problem one possible option could be to pair it with a project we've talked about in I/F but never get to do it: the possibility to trigger some cookbooks via some HTTP api/UI both for scheduling and auto-remediation capabilities. This could be a third use case, the reporting triggered by some alerts.

How this could look like at high level:

  • Convert the existing scripts to just emit one metric with the status and no output. For example we could use the nagios check exit code as value (0, 1, 2, 3)
  • Have alerts setup based on those metrics that trigger a webhook
  • Have an internal HTTP endpoint where the webhook can authenticate and trigger a run of a cookbook that could either re-run the check script on the host with some parameter to get the full output, or have the script save the status on a file and have the cookbook retrieve it, etc... At that point the cookbook could use the output anywhere else (phabricator task or paste, email, etc...)

There are a bunch of implementation specs to be decided on the cookbook API side (auth, execution, concurrency management, failures management), but is something that we'd need to investigate and decide anyway if we go ahead with the project also for the other use cases.
If this is something you're interested in we can surely find some time to discuss it between observability and infrastructure foundations also for planning purposes.

How this could look like at high level:

  • Convert the existing scripts to just emit one metric with the status and no output. For example we could use the nagios check exit code as value (0, 1, 2, 3)
  • Have alerts setup based on those metrics that trigger a webhook
  • Have an internal HTTP endpoint where the webjook can authenticate and trigger a run of a cookbook that could either re-run the check script on the host with some parameter to get the full output, or have the script save the status on a file and have the cookbook retrieve it, etc... At that point the cookbook could use the output anywhere else (phabricator task or paste, email, etc...)

I don't know if this would work for all cases, but I like this, and certainly this would work for (for example) failed db backups. Icinga with no oncall alerts work ok for me, but I don't want a failed backup to create more than an email or a phab ticket.

In general, I like the "leaving the scripts alone" approach but building some small infra around it so that npre and anything purely icinga related can go away. It is a lot of work, but it would be more work to redo 20+ python, perl and go scripts with a lot of custom and subtle business logic, many times (speaking about mine) written poorly because I was under lot of pressure to get something to alert. That way icinga can go away, and big rewrites can happen naturally without blockers. I am 100% sure that we rewrite quickly, we will end up in a worse state (gaps that had been filled over the years, lost cases, etc.). With this I don't mean those are un-rewritable, but that it should be done in a more natural way "this check is no longer suitable for its use/doesn't scale anymore".

I would like to mention also these needs: T321808#10230641 I believe the prometheus workflow is intended for services (specially important/big ones, such as k8s, mysql, cassandra, backups) to integrate that as part of the service, but it would be nice to have some kind of solution/wrapper for one-off checks [of course, with the prometheus backend, but an easier "user" setup]- some of which you mentioned already.

As for the "large output" kind of script problem one possible option could be to pair it with a project we've talked about in I/F but never get to do it: the possibility to trigger some cookbooks via some HTTP api/UI both for scheduling and auto-remediation capabilities. This could be a third use case, the reporting triggered by some alerts.

How this could look like at high level:

  • Convert the existing scripts to just emit one metric with the status and no output. For example we could use the nagios check exit code as value (0, 1, 2, 3)
  • Have alerts setup based on those metrics that trigger a webhook
  • Have an internal HTTP endpoint where the webjook can authenticate and trigger a run of a cookbook that could either re-run the check script on the host with some parameter to get the full output, or have the script save the status on a file and have the cookbook retrieve it, etc... At that point the cookbook could use the output anywhere else (phabricator task or paste, email, etc...)

There are a bunch of implementation specs to be decided on the cookbook API side (auth, execution, concurrency management, failures management), but is something that we'd need to investigate and decide anyway if we go ahead with the project also for the other use cases.
If this is something you're interested in we can surely find some time to discuss it between observability and infrastructure foundations also for planning purposes.

Thank you for the feedback, I definitely like the idea of being able to run cookbooks in reaction to alerts! It does sound to me like the most future-proof solution in this case, since as you mentioned the project is something that you have talked about already. We'll discuss at the team level and update

In general, I like the "leaving the scripts alone" approach but building some small infra around it so that npre and anything purely icinga related can go away. It is a lot of work, but it would be more work to redo 20+ python, perl and go scripts with a lot of custom and subtle business logic, many times (speaking about mine) written poorly because I was under lot of pressure to get something to alert. That way icinga can go away, and big rewrites can happen naturally without blockers. I am 100% sure that we rewrite quickly, we will end up in a worse state (gaps that had been filled over the years, lost cases, etc.). With this I don't mean those are un-rewritable, but that it should be done in a more natural way "this check is no longer suitable for its use/doesn't scale anymore".

Agreed leaving the scripts alone is the best option in my mind also for the reasons you described.

I would like to mention also these needs: T321808#10230641 I believe the prometheus workflow is intended for services (specially important/big ones, such as k8s, mysql, cassandra, backups) to integrate that as part of the service, but it would be nice to have some kind of solution/wrapper for one-off checks [of course, with the prometheus backend, but an easier "user" setup]- some of which you mentioned already.

Indeed, one of the ideas of this task is also to have a good story at the puppet level for such one off scripts

If this is something you're interested in we can surely find some time to discuss it between observability and infrastructure foundations also for planning purposes.

FYI I've broken out this particular thing/project in T384837 since it is fairly stand alone wrt the requirements of this task

Change #1167691 had a related patch set uploaded (by Jcrespo; author: Jcrespo):

[operations/puppet@production] prometheus: Proof of concept of a nrpe to prometheus translation wrapper

https://gerrit.wikimedia.org/r/1167691

In T395446: Evaluate which solution we could adopt as a drop-in replacement for NRPE (and start prototyping), a MVP has been developed for option 2 where a wrapper triggered by a systemd timer runs the NRPE script, and it produces a node exporter file with metrics (based on the plugin exit code) in the form of:

# HELP nagios_nrpe_check_result Result of Nagios-style check script execution
# TYPE nagios_nrpe_check_result gauge
nagios_nrpe_check_result{alert_rule_hash="fa946df0fbcb1b4c79a46a85bedc5ce7",check_name="check_ferm_active",severity="info",status="OK"} 1.0
nagios_nrpe_check_result{alert_rule_hash="fa946df0fbcb1b4c79a46a85bedc5ce7",check_name="check_ferm_active",severity="warning",status="WARNING"} 0.0
nagios_nrpe_check_result{alert_rule_hash="fa946df0fbcb1b4c79a46a85bedc5ce7",check_name="check_ferm_active",severity="critical",status="CRITICAL"} 0.0
nagios_nrpe_check_result{alert_rule_hash="fa946df0fbcb1b4c79a46a85bedc5ce7",check_name="check_ferm_active",severity="info",status="UNKNOWN"} 0.0
# HELP nagios_nrpe_check_config_error True if errors occurred during check initialization.
# TYPE nagios_nrpe_check_config_error gauge
nagios_nrpe_check_config_error{alert_rule_hash="fa946df0fbcb1b4c79a46a85bedc5ce7",check_name="check_ferm_active"} 0.0

where the rule hash is computed as md5("${title}${for}m${summary}${dashboard_link}${notes_url}${alertmanager_team}"), which are the parameters passed to the nrpe::monitor_service resource. Further details on the alert_rule_hash will be provided later.

The plugin output is sent to the Logstash cluster in an ECS-compatible format.

Alert rules are automatically generated using prometheus::alert::rule:

groups:
- name: nrpechecks
  rules:
  - for: 32m
    expr: (nagios_nrpe_check_result{alert_rule_hash="fa946df0fbcb1b4c79a46a85bedc5ce7",check_name="check_ferm_active",
      status=~"(WARNING|CRITICAL)", severity=~"(warning|critical)"} > 0) * on (instance)
      group_left (team) role_owner
    alert: check_ferm_active
    labels:
      team: observability
      severity: info
    annotations:
      logs: https://logstash.wikimedia.org/app/dashboards#/view/2d343ac0-6df8-11f0-8e08-7fab0da52b33?_g=(filters:!((query:(match_phrase:(event.module:check_ferm_active))),(query:(match_phrase:(host.name:{{$labels.instance|stripPort}})))))
      runbook: https://wikitech.wikimedia.org/wiki/Monitoring/check_ferm
      summary: Check whether ferm is active by checking the default input chain
      dashboard: TODO
      description: Check whether ferm is active by checking the default input chain
  - for: 60m
    expr: (nagios_nrpe_check_result{alert_rule_hash="d45add008615cbb9112143fc8a956f24",check_name="check_ferm_active",
      status=~"(WARNING|CRITICAL)", severity=~"(warning|critical)"} > 0) * on (instance)
      group_left (team) role_owner
    alert: check_ferm_active
    labels:
      team: observability
      severity: info
    annotations:
      logs: https://logstash.wikimedia.org/app/dashboards#/view/2d343ac0-6df8-11f0-8e08-7fab0da52b33?_g=(filters:!((query:(match_phrase:(event.module:check_ferm_active))),(query:(match_phrase:(host.name:{{$labels.instance|stripPort}})))))
      runbook: https://wikitech.wikimedia.org/wiki/Monitoring/check_ferm
      summary: Check whether ferm is active by checking the default input chain
      dashboard: TODO
      description: Check whether ferm is active by checking the default input chain

Such resources are deduplicated and can therefore be defined multiple times, resulting in a single instance of the final alert per set of parameters. The previously mentioned alert_rule_hash is then used to match the alert parameters with the corresponding metric. This allows having multiple checks of the same type, for example, with different for: parameters on different roles/profiles.

Icinga uses two different intervals: the check interval and the retry interval. The first is used when everything is working fine, while the latter is applied when the initial check attempt fails.
Prometheus rules do not have this concept of two separate intervals, so the for parameter has been mapped to represent the maximum amount of time after which Icinga would have definitively identified an alert: $_for = $check_interval + (($retries - 1) * $retry_interval).
The systemd timer interval is set to ensure that the wrapper is executed at least $retries times within the for: window.

Every alert reports a pre-formatted link to an OpenSearch view where the plugin output can be retrieved.

The recipients of the alerts are determined by the role_owner metric, but this can be overridden using the alertmanager_team parameter.

The severity is computed according to the previous Icinga thresholds, and a status of CRITICAL triggers a page severity if the nrpe::monitor_service is instantiated with the $critical parameter set to true.

Any unmanaged exception in the script is caught, and the cache file is consequently removed. This allows the use of the absent/up PromQL functions. After removal, the exception is raised again to mark the unit as failed.