[Observability] Alarm on clustermgtd not running

[gmarciani]

Description of changes

1. Add permission cloudwatch:PutMetricData to the head node policy so that clustermgtd is able to emit metrics.
2. Add alarm $ClusterName-HeadNode-ClustermgtdHeartbeat, which goes into alarm when the metric ClustermgtdHeartbeat is missing for more than 10 minutes, which is the worst case time that clustermgtd is expected to be stopped during a cluster update.
3. Add the alarm above to the Head Node composite alarm
4. Surface the above alarm in cluster dashboard within the existing section "Head Node Alarms"
5. Surface the clustermgtd heartbeat to the cluster dashboard widget "Daemons Heartbeats"
6. Extended integ test test_monitoring so verify clustermgtd metrics are collected.

All the changes are applied only when Slurm is used (not AWS Batch) because at this point we are only pushing metrics from clustermgtd, which is deployed only with Slurm scheduler.

This PR is coupled with aws/aws-parallelcluster-node#685. In particular:

1. the current PR adds the permission required by clustermgtd to emit the metric.
2. the current PR adds an alarm that required the metric ClustermgtdHeartbeat to be emitted by clustermgtd

UX

Q&A

1. Why you did not extend the integ test test_monitoring.py to cover the new alarm details and dashboard?
   I extend the integ test to verify that the metric is collected. However, there is no point in extending the assertions around the alarm and dashboard details because they are already covered by unit tests. On the opposite, we should instead remove the asserttions around alarm settings and dashboard settings from the integ test (will do in a follow up PR)
2. Why not using a metric filter on clustermgtd logs to signal that it is running, rather than alarming an explicit metric posted by clustermgtd?
   • Reduced point of failure: with an explicit put_metric we rely only on CW Metrics; whereas with metric_Filters we rely also on Cw Logs
   • Robustness: metric_filters rely on logs formatting, which is ephemeral by definition.
   • Cluster Config: in pcluster you can toggle logs and alarms independently. If we go with metric_filters we would couple the two concepts and a user that disabled logs will end up disabling alarms as well.
3. Isn't the alarm name too long?
   No. The name is $clusterName-HeadNode-Clustermgtd.
   The max length for an alarm name is 255 chars.
   The max length for cluster name is 60 chars.
   The suffix -HeadNode-Clustermgtd is 21 chars, so we are far away from the limit.
4. Why only Slurm scheduler?
   In this Pr we are introducing the alarm for clustermgtd, which runs only when the scheduler is Slurm (not AWS Batch).
5. With this configuration the alarm goes red after 10 minutes of clustermgtd not running, which is ok. However it also requires 10 minutes of clustermgtd running to get back to green. Why not getting back to green at first positive datapoint?
   I agree that the alarm should recover faster. Ideally even 3 good datapoints would be enough to recover. However there is a technical blocker: CW does not support asymmetric thresholds within a single alarm. If you need 10 missing datapoints to go red, you need 10 good datapoints to recover. To implement asymmetric threshold we need to create a composite alarm made by one alarm driving the red state and the other one driving the recovery. This would overcomplicate the solution and the user experience.

Tests

• Unit tests (updated to cover the changes in this PR)
• Manual validation: created cluster and verified that clustermgtd is able to emit the metric and that the cluster dashboard shows as expected both the alarm and the metrics. Verified that by stopping clustermgtd the alarm goes red after the expected 10 minutes
• [SUCCESS] Integration test test_monitoring

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[gmarciani]

NOTE FOR THE REVIEWER
The refactoring of how alarms are defined here was required because clustermgtd heartbeat introduces an alarm that has different needs that the other alarms (threshold, missing data, observation period); so the logic must be changed to accommodate more flexibility.

[gmarciani]

NOTES FOR THE REVIEWER
We introduced here a dependency of head node alarms to the head node wait condition. This is needed because clustermgtd alarm is meant to go red on missing heartbeats. However, during cluster creation it is expected to not have such heartbeat. So it makes no sense to start alarming before the head node configuration has completed.

[himani2411]

I understand the Alarm dependency for the new ClustermgtdHeartbeat Alarm but why add dependency for all the alarms?

[gmarciani]

This is a good point.

My reasoning is:

1. value: there is no reason to alarm on the head node until the head node completes its setup.
2. simplicity: it is easier to define a dependency that applies to all the head node alarms than having different dependency rules.

So, un less there is a real value in created head node alarms before it completes its setup, there is not reason to have different behaviors.

Do you agree with that?

[himani2411]

Yes agreed!

[gmarciani]

NOTES FOR THE REVIEWER
Before this change we used to leave the treat_missing_data unspecified.
The default value when unspecified is MISSING. I wanted to set it here to make clearer what is the expected behavior.

[himani2411]

Yup I checked already the default