From fed139be68bb218635f8a037472b8f3d4762bb05 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 31 Oct 2024 13:32:49 +0100 Subject: [PATCH 01/51] Add draft test for composite alarm WIP, didn't run yet against AWS so no snapshot yet. --- .../services/cloudwatch/test_cloudwatch.py | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 54341d8c4fe2b..bd6a2579d5818 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -988,6 +988,151 @@ def test_set_alarm(self, sns_create_topic, sqs_create_queue, aws_client, cleanup describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_name]) snapshot.match("reset-alarm", describe_alarm) + @markers.aws.validated + @pytest.mark.skipif(is_old_provider(), reason="New test for v2 provider") + def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_client, cleanups, snapshot): + snapshot.add_transformer(snapshot.transform.cloudwatch_api()) + + # create topics for state 'ALARM' and 'OK' of the composite alarm + topic_name_alarm = f"topic-{short_uid()}" + topic_name_ok = f"topic-{short_uid()}" + + sns_topic_alarm = sns_create_topic(Name=topic_name_alarm) + topic_arn_alarm = sns_topic_alarm["TopicArn"] + sns_topic_ok = sns_create_topic(Name=topic_name_ok) + topic_arn_ok = sns_topic_ok["TopicArn"] + snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) + snapshot.add_transformer(snapshot.transform.regex(topic_arn_ok, "")) + + # create queues for 'ALARM' and 'OK' of the composite alarm (will receive sns messages) + uid = short_uid() + queue_url_alarm = sqs_create_queue(QueueName=f"AlarmQueue-{uid}") + queue_url_ok = sqs_create_queue(QueueName=f"OKQueue-{uid}") + + arn_queue_alarm = aws_client.sqs.get_queue_attributes( + QueueUrl=queue_url_alarm, AttributeNames=["QueueArn"] + )["Attributes"]["QueueArn"] + arn_queue_ok = aws_client.sqs.get_queue_attributes( + QueueUrl=queue_url_ok, AttributeNames=["QueueArn"] + )["Attributes"]["QueueArn"] + aws_client.sqs.set_queue_attributes( + QueueUrl=queue_url_alarm, + Attributes={"Policy": get_sqs_policy(arn_queue_alarm, topic_arn_alarm)}, + ) + aws_client.sqs.set_queue_attributes( + QueueUrl=queue_url_ok, Attributes={"Policy": get_sqs_policy(arn_queue_ok, topic_arn_ok)} + ) + + # put metric alarms that would be parts of a composite one + snapshot.add_transformer(TransformerUtility.key_value("MetricName")) + + def _put_metric_alarm(alarm_name: str): + metric_name = "CPUUtilization" + namespace = "AWS/EC2" + aws_client.cloudwatch.put_metric_alarm( + AlarmName=alarm_name, + MetricName=metric_name, + Namespace=namespace, + EvaluationPeriods=1, + Period=10, + Statistic="Sum", + ComparisonOperator="GreaterThanThreshold", + Threshold=30, + ) + cleanups.append(lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[alarm_name])) + + alarm_1_name = "simple-alarm-1" + alarm_2_name = "simple-alarm-2" + + _put_metric_alarm(alarm_1_name) + _put_metric_alarm(alarm_2_name) + + # put composite alarm that is triggered when either of metric alarms is triggered. + composite_alarm_name="composite-alarm" + composite_alarm_description="composite alarm description" + + aws_client.cloudwatch.put_composite_alarm( + AlarmName=composite_alarm_name, + AlarmDescription=composite_alarm_description, + AlarmRule=f'ALARM("{alarm_1_name}") OR ALARM("{alarm_2_name}")', + OKActions=[topic_arn_ok], + AlarmActions=[topic_arn_alarm], + ) + cleanups.append( + lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[composite_alarm_name]) + ) + + # trigger alarm 1 - composite one should also go into ALARM state + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" + ) + + retry( + check_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + expected_queue_url=queue_url_alarm, + expected_topic_arn=topic_arn_alarm, + expected_new="ALARM" + expected_reason=state_reason, #TODO define state reason for composite alarm + alarm_name=composite_alarm_name, + alarm_description=composite_alarm_description, + expected_trigger=expected_trigger, #TODO define expected trigger for composite alarm + ) + describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) + snapshot.match("triggered-alarm", describe_alarm) + + # trigger OK for alarm 1 - composite one should also go back to OK + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" + ) + + retry( + check_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + expected_queue_url=queue_url_ok, + expected_topic_arn=topic_arn_ok, + expected_new="OK", + expected_reason=state_reason, #TODO define state reason for composite alarm + alarm_name=composite_alarm_name, + alarm_description=composite_alarm_description, + expected_trigger=expected_trigger, #TODO define expected trigger for composite alarm + ) + describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) + snapshot.match("reset-alarm", describe_alarm) + + # trigger alarm 2 - composite one should go again into ALARM state + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" + ) + + retry( + check_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + expected_queue_url=queue_url_alarm, + expected_topic_arn=topic_arn_alarm, + expected_new="ALARM" + expected_reason = state_reason, # TODO define state reason for composite alarm + alarm_name = composite_alarm_name, + alarm_description = composite_alarm_description, + expected_trigger = expected_trigger, # TODO define expected trigger for composite alarm + ) + describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) + snapshot.match("triggered-alarm", describe_alarm) + + # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" + ) + # TODO check that alarm state hasn't changed, probably with describe_alarm_history + # take into account that composite alarm runs on schedule + + @markers.aws.validated @markers.snapshot.skip_snapshot_verify( paths=[ From 11b6a7aedac0a6d6b3407f74fb7fa58de1a30b6d Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 6 Nov 2024 12:32:33 +0100 Subject: [PATCH 02/51] Comment out further test cases for now Focus on making test pass in AWS mode. --- .../services/cloudwatch/test_cloudwatch.py | 96 +++++++++---------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index bd6a2579d5818..fd0b66b2ec9dc 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1083,54 +1083,54 @@ def _put_metric_alarm(alarm_name: str): describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) snapshot.match("triggered-alarm", describe_alarm) - # trigger OK for alarm 1 - composite one should also go back to OK - aws_client.cloudwatch.set_alarm_state( - AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" - ) - - retry( - check_message, - retries=PUBLICATION_RETRIES, - sleep_before=1, - sqs_client=aws_client.sqs, - expected_queue_url=queue_url_ok, - expected_topic_arn=topic_arn_ok, - expected_new="OK", - expected_reason=state_reason, #TODO define state reason for composite alarm - alarm_name=composite_alarm_name, - alarm_description=composite_alarm_description, - expected_trigger=expected_trigger, #TODO define expected trigger for composite alarm - ) - describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) - snapshot.match("reset-alarm", describe_alarm) - - # trigger alarm 2 - composite one should go again into ALARM state - aws_client.cloudwatch.set_alarm_state( - AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" - ) - - retry( - check_message, - retries=PUBLICATION_RETRIES, - sleep_before=1, - sqs_client=aws_client.sqs, - expected_queue_url=queue_url_alarm, - expected_topic_arn=topic_arn_alarm, - expected_new="ALARM" - expected_reason = state_reason, # TODO define state reason for composite alarm - alarm_name = composite_alarm_name, - alarm_description = composite_alarm_description, - expected_trigger = expected_trigger, # TODO define expected trigger for composite alarm - ) - describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) - snapshot.match("triggered-alarm", describe_alarm) - - # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change - aws_client.cloudwatch.set_alarm_state( - AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" - ) - # TODO check that alarm state hasn't changed, probably with describe_alarm_history - # take into account that composite alarm runs on schedule + # # trigger OK for alarm 1 - composite one should also go back to OK + # aws_client.cloudwatch.set_alarm_state( + # AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" + # ) + # + # retry( + # check_message, + # retries=PUBLICATION_RETRIES, + # sleep_before=1, + # sqs_client=aws_client.sqs, + # expected_queue_url=queue_url_ok, + # expected_topic_arn=topic_arn_ok, + # expected_new="OK", + # expected_reason=state_reason, #TODO define state reason for composite alarm + # alarm_name=composite_alarm_name, + # alarm_description=composite_alarm_description, + # expected_trigger=expected_trigger, #TODO define expected trigger for composite alarm + # ) + # describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) + # snapshot.match("reset-alarm", describe_alarm) + # + # # trigger alarm 2 - composite one should go again into ALARM state + # aws_client.cloudwatch.set_alarm_state( + # AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" + # ) + # + # retry( + # check_message, + # retries=PUBLICATION_RETRIES, + # sleep_before=1, + # sqs_client=aws_client.sqs, + # expected_queue_url=queue_url_alarm, + # expected_topic_arn=topic_arn_alarm, + # expected_new="ALARM" + # expected_reason = state_reason, # TODO define state reason for composite alarm + # alarm_name = composite_alarm_name, + # alarm_description = composite_alarm_description, + # expected_trigger = expected_trigger, # TODO define expected trigger for composite alarm + # ) + # describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) + # snapshot.match("triggered-alarm", describe_alarm) + # + # # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change + # aws_client.cloudwatch.set_alarm_state( + # AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" + # ) + # # TODO check that alarm state hasn't changed, probably with describe_alarm_history + # # take into account that composite alarm runs on schedule @markers.aws.validated From 5bbbd9f90a99c27f3bd7cd72e5c17239d712dbc1 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 6 Nov 2024 12:36:24 +0100 Subject: [PATCH 03/51] Inline metric alarm variables --- tests/aws/services/cloudwatch/test_cloudwatch.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index fd0b66b2ec9dc..cbea83e6309bb 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1027,12 +1027,10 @@ def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_c snapshot.add_transformer(TransformerUtility.key_value("MetricName")) def _put_metric_alarm(alarm_name: str): - metric_name = "CPUUtilization" - namespace = "AWS/EC2" aws_client.cloudwatch.put_metric_alarm( AlarmName=alarm_name, - MetricName=metric_name, - Namespace=namespace, + MetricName="CPUUtilization", + Namespace="AWS/EC2", EvaluationPeriods=1, Period=10, Statistic="Sum", From f9937d2202b193525aaa1b6096001b115254a7de Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 6 Nov 2024 14:56:38 +0100 Subject: [PATCH 04/51] Add missing comma --- tests/aws/services/cloudwatch/test_cloudwatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index cbea83e6309bb..e36454c3310f3 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1072,7 +1072,7 @@ def _put_metric_alarm(alarm_name: str): sqs_client=aws_client.sqs, expected_queue_url=queue_url_alarm, expected_topic_arn=topic_arn_alarm, - expected_new="ALARM" + expected_new="ALARM", expected_reason=state_reason, #TODO define state reason for composite alarm alarm_name=composite_alarm_name, alarm_description=composite_alarm_description, From 09e83e845cad337ae4f58b0758435c89df15c56d Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 6 Nov 2024 14:57:36 +0100 Subject: [PATCH 05/51] Use unique alarm names --- tests/aws/services/cloudwatch/test_cloudwatch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index e36454c3310f3..abde198dd2cf3 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1039,14 +1039,14 @@ def _put_metric_alarm(alarm_name: str): ) cleanups.append(lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[alarm_name])) - alarm_1_name = "simple-alarm-1" - alarm_2_name = "simple-alarm-2" + alarm_1_name = f"simple-alarm-1-{short_uid()}" + alarm_2_name = f"simple-alarm-2-{short_uid()}" _put_metric_alarm(alarm_1_name) _put_metric_alarm(alarm_2_name) # put composite alarm that is triggered when either of metric alarms is triggered. - composite_alarm_name="composite-alarm" + composite_alarm_name=f"composite-alarm-{short_uid()}" composite_alarm_description="composite alarm description" aws_client.cloudwatch.put_composite_alarm( From 21d48c0091364a14ca34a98c7d478d9aed8a64b4 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 7 Nov 2024 11:51:24 +0100 Subject: [PATCH 06/51] Use more descriptive topic names --- tests/aws/services/cloudwatch/test_cloudwatch.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index abde198dd2cf3..d0a7377a21c7a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -994,8 +994,8 @@ def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_c snapshot.add_transformer(snapshot.transform.cloudwatch_api()) # create topics for state 'ALARM' and 'OK' of the composite alarm - topic_name_alarm = f"topic-{short_uid()}" - topic_name_ok = f"topic-{short_uid()}" + topic_name_alarm = f"topic-alarm-{short_uid()}" + topic_name_ok = f"topic-ok-{short_uid()}" sns_topic_alarm = sns_create_topic(Name=topic_name_alarm) topic_arn_alarm = sns_topic_alarm["TopicArn"] @@ -1005,9 +1005,8 @@ def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_c snapshot.add_transformer(snapshot.transform.regex(topic_arn_ok, "")) # create queues for 'ALARM' and 'OK' of the composite alarm (will receive sns messages) - uid = short_uid() - queue_url_alarm = sqs_create_queue(QueueName=f"AlarmQueue-{uid}") - queue_url_ok = sqs_create_queue(QueueName=f"OKQueue-{uid}") + queue_url_alarm = sqs_create_queue(QueueName=f"AlarmQueue-{short_uid()}") + queue_url_ok = sqs_create_queue(QueueName=f"OKQueue-{short_uid()}") arn_queue_alarm = aws_client.sqs.get_queue_attributes( QueueUrl=queue_url_alarm, AttributeNames=["QueueArn"] From 7685da5dc6d380b3c8b0de8b82d468223174c9e1 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 7 Nov 2024 11:52:41 +0100 Subject: [PATCH 07/51] Subscribe SQS queu to alarm SNS topic --- tests/aws/services/cloudwatch/test_cloudwatch.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index d0a7377a21c7a..fdc96acab535b 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1022,6 +1022,22 @@ def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_c QueueUrl=queue_url_ok, Attributes={"Policy": get_sqs_policy(arn_queue_ok, topic_arn_ok)} ) + # subscribe to SQS + subscription_alarm = aws_client.sns.subscribe( + TopicArn=topic_arn_alarm, Protocol="sqs", Endpoint=arn_queue_alarm + ) + cleanups.append( + lambda: aws_client.sns.unsubscribe( + SubscriptionArn=subscription_alarm["SubscriptionArn"] + ) + ) + subscription_ok = aws_client.sns.subscribe( + TopicArn=topic_arn_ok, Protocol="sqs", Endpoint=arn_queue_ok + ) + cleanups.append( + lambda: aws_client.sns.unsubscribe(SubscriptionArn=subscription_ok["SubscriptionArn"]) + ) + # put metric alarms that would be parts of a composite one snapshot.add_transformer(TransformerUtility.key_value("MetricName")) From c50889d7d7f2ce1d09d79815ae32df1160c4772a Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 7 Nov 2024 11:58:49 +0100 Subject: [PATCH 08/51] Check that composite alarm is saved --- tests/aws/services/cloudwatch/test_cloudwatch.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index fdc96acab535b..96f43005b1e5f 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1064,10 +1064,12 @@ def _put_metric_alarm(alarm_name: str): composite_alarm_name=f"composite-alarm-{short_uid()}" composite_alarm_description="composite alarm description" + composite_alarm_rule = f'ALARM("{alarm_1_name}") OR ALARM("{alarm_2_name}")' + aws_client.cloudwatch.put_composite_alarm( AlarmName=composite_alarm_name, AlarmDescription=composite_alarm_description, - AlarmRule=f'ALARM("{alarm_1_name}") OR ALARM("{alarm_2_name}")', + AlarmRule=composite_alarm_rule, OKActions=[topic_arn_ok], AlarmActions=[topic_arn_alarm], ) @@ -1075,6 +1077,13 @@ def _put_metric_alarm(alarm_name: str): lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[composite_alarm_name]) ) + composite_alarms_list = aws_client.cloudwatch.describe_alarms( + AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] + ) + alarm = composite_alarms_list["CompositeAlarms"][0] + assert alarm["AlarmName"] == composite_alarm_name + assert alarm["AlarmRule"] == composite_alarm_rule + # trigger alarm 1 - composite one should also go into ALARM state aws_client.cloudwatch.set_alarm_state( AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" From 0d47210a099c485ad0de458163152bb4235c1a33 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 7 Nov 2024 12:07:22 +0100 Subject: [PATCH 09/51] Add composite alarm message check Use TriggeringChildren property to verify that composite alarm was raised using expected source. --- .../services/cloudwatch/test_cloudwatch.py | 48 +++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 96f43005b1e5f..ee5ad334bbbe2 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1060,9 +1060,12 @@ def _put_metric_alarm(alarm_name: str): _put_metric_alarm(alarm_1_name) _put_metric_alarm(alarm_2_name) + alarm_1_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_1_name])["MetricAlarms"][0]["AlarmArn"] + alarm_2_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_2_name])["MetricAlarms"][0]["AlarmArn"] + # put composite alarm that is triggered when either of metric alarms is triggered. - composite_alarm_name=f"composite-alarm-{short_uid()}" - composite_alarm_description="composite alarm description" + composite_alarm_name = f"composite-alarm-{short_uid()}" + composite_alarm_description = "composite alarm description" composite_alarm_rule = f'ALARM("{alarm_1_name}") OR ALARM("{alarm_2_name}")' @@ -1090,20 +1093,18 @@ def _put_metric_alarm(alarm_name: str): ) retry( - check_message, + check_composite_alarm_message, retries=PUBLICATION_RETRIES, sleep_before=1, sqs_client=aws_client.sqs, - expected_queue_url=queue_url_alarm, + queue_url=queue_url_alarm, expected_topic_arn=topic_arn_alarm, - expected_new="ALARM", - expected_reason=state_reason, #TODO define state reason for composite alarm alarm_name=composite_alarm_name, alarm_description=composite_alarm_description, - expected_trigger=expected_trigger, #TODO define expected trigger for composite alarm + expected_state="ALARM", + expected_triggering_child_arn=alarm_1_arn, + expected_triggering_child_state="ALARM", ) - describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) - snapshot.match("triggered-alarm", describe_alarm) # # trigger OK for alarm 1 - composite one should also go back to OK # aws_client.cloudwatch.set_alarm_state( @@ -2880,6 +2881,35 @@ def _sqs_messages_snapshot(expected_state, sqs_client, sqs_queue, snapshot, iden snapshot.match(f"{identifier}-sqs-msg", found_msg) +def check_composite_alarm_message( + sqs_client, + queue_url, + expected_topic_arn, + alarm_name, + alarm_description, + expected_state, + expected_triggering_child_arn, + expected_triggering_child_state, +): + receive_result = sqs_client.receive_message(QueueUrl=queue_url) + LOG.info(f"receive SQS message: {receive_result}") + message = None + for msg in receive_result["Messages"]: + body = json.loads(msg["Body"]) + if body["TopicArn"] == expected_topic_arn: + message = json.loads(body["Message"]) + receipt_handle = msg["ReceiptHandle"] + sqs_client.delete_message(QueueUrl=queue_url, ReceiptHandle=receipt_handle) + break + assert message["NewStateValue"] == expected_state + assert message["AlarmName"] == alarm_name + assert message["AlarmDescription"] == alarm_description + triggering_child_alarm = message["TriggeringChildren"][0] + assert triggering_child_alarm["Arn"] == expected_triggering_child_arn + assert triggering_child_alarm["State"]["Value"] == expected_triggering_child_state + return message + + def check_message( sqs_client, expected_queue_url, From 7735a5d76ae1376a3a784f03b852ce90c490e4ff Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 7 Nov 2024 12:44:28 +0100 Subject: [PATCH 10/51] Record a snapshot of composite alarm Still needs some more transformers --- .../services/cloudwatch/test_cloudwatch.py | 12 +++++-- .../cloudwatch/test_cloudwatch.snapshot.json | 35 +++++++++++++++++++ .../test_cloudwatch.validation.json | 3 ++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index ee5ad334bbbe2..3a2b804490112 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1083,9 +1083,9 @@ def _put_metric_alarm(alarm_name: str): composite_alarms_list = aws_client.cloudwatch.describe_alarms( AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) - alarm = composite_alarms_list["CompositeAlarms"][0] - assert alarm["AlarmName"] == composite_alarm_name - assert alarm["AlarmRule"] == composite_alarm_rule + composite_alarm = composite_alarms_list["CompositeAlarms"][0] + assert composite_alarm["AlarmName"] == composite_alarm_name + assert composite_alarm["AlarmRule"] == composite_alarm_rule # trigger alarm 1 - composite one should also go into ALARM state aws_client.cloudwatch.set_alarm_state( @@ -1106,6 +1106,12 @@ def _put_metric_alarm(alarm_name: str): expected_triggering_child_state="ALARM", ) + composite_alarms_list = aws_client.cloudwatch.describe_alarms( + AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] + ) + composite_alarm_in_alarm_caused_by_alarm_1 = composite_alarms_list["CompositeAlarms"][0] + snapshot.match("composite-alarm-in-alarm-when-alarm-1-is-in-alarm", composite_alarm_in_alarm_caused_by_alarm_1) + # # trigger OK for alarm 1 - composite one should also go back to OK # aws_client.cloudwatch.set_alarm_state( # AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index eb6e15e65e461..4220927a6dd9a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2103,5 +2103,40 @@ } } } + }, + "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { + "recorded-date": "07-11-2024, 11:29:50", + "recorded-content": { + "composite-alarm-in-alarm-when-alarm-1-is-in-alarm": { + "ActionsEnabled": true, + "AlarmActions": [ + "arn::sns::111111111111:" + ], + "AlarmArn": "arn::cloudwatch::111111111111:alarm:", + "AlarmConfigurationUpdatedTimestamp": "timestamp", + "AlarmDescription": "composite alarm description", + "AlarmName": "", + "AlarmRule": "ALARM(\"\") OR ALARM(\"simple-alarm-2-7a20e452\")", + "InsufficientDataActions": [], + "OKActions": [ + "" + ], + "StateReason": "arn::cloudwatch::111111111111:alarm: transitioned to ALARM at Thursday 07 November, 2024 11:29:", + "StateReasonData": { + "triggeringAlarms": [ + { + "arn": "arn::cloudwatch::111111111111:alarm:", + "state": { + "value": "ALARM", + "timestamp": "date" + } + } + ] + }, + "StateTransitionedTimestamp": "timestamp", + "StateUpdatedTimestamp": "timestamp", + "StateValue": "ALARM" + } + } } } diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 31b537546082c..2dc99a2069c78 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -67,5 +67,8 @@ }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_store_tags": { "last_validated_date": "2024-09-02T14:03:31+00:00" + }, + "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { + "last_validated_date": "2024-11-07T11:29:50+00:00" } } From d056955f2bcaecfc0260a226255a6579dfdbfd2d Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 7 Nov 2024 16:12:31 +0100 Subject: [PATCH 11/51] Use metric alarms ARN in composite alarm rule Both name and ARN can be used according to composite alarm docs. First iteration of Localstack implementation will support only reference by ARN. --- tests/aws/services/cloudwatch/test_cloudwatch.py | 8 +++++--- .../aws/services/cloudwatch/test_cloudwatch.snapshot.json | 6 +++--- .../services/cloudwatch/test_cloudwatch.validation.json | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 3a2b804490112..2f22cbf5e0559 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1039,8 +1039,6 @@ def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_c ) # put metric alarms that would be parts of a composite one - snapshot.add_transformer(TransformerUtility.key_value("MetricName")) - def _put_metric_alarm(alarm_name: str): aws_client.cloudwatch.put_metric_alarm( AlarmName=alarm_name, @@ -1067,7 +1065,7 @@ def _put_metric_alarm(alarm_name: str): composite_alarm_name = f"composite-alarm-{short_uid()}" composite_alarm_description = "composite alarm description" - composite_alarm_rule = f'ALARM("{alarm_1_name}") OR ALARM("{alarm_2_name}")' + composite_alarm_rule = f'ALARM("{alarm_1_arn}") OR ALARM("{alarm_2_arn}")' aws_client.cloudwatch.put_composite_alarm( AlarmName=composite_alarm_name, @@ -1106,6 +1104,10 @@ def _put_metric_alarm(alarm_name: str): expected_triggering_child_state="ALARM", ) + # state reason is a text with dates, for now stubbing it out because + # composite alarm reason can be checked via TriggeringChildren property + snapshot.add_transformer(snapshot.transform.key_value("StateReason")) + composite_alarms_list = aws_client.cloudwatch.describe_alarms( AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index 4220927a6dd9a..95f52de55d226 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,7 +2105,7 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "07-11-2024, 11:29:50", + "recorded-date": "07-11-2024, 15:07:28", "recorded-content": { "composite-alarm-in-alarm-when-alarm-1-is-in-alarm": { "ActionsEnabled": true, @@ -2116,12 +2116,12 @@ "AlarmConfigurationUpdatedTimestamp": "timestamp", "AlarmDescription": "composite alarm description", "AlarmName": "", - "AlarmRule": "ALARM(\"\") OR ALARM(\"simple-alarm-2-7a20e452\")", + "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:simple-alarm-2-cc82e41a\")", "InsufficientDataActions": [], "OKActions": [ "" ], - "StateReason": "arn::cloudwatch::111111111111:alarm: transitioned to ALARM at Thursday 07 November, 2024 11:29:", + "StateReason": "arn::cloudwatch::111111111111:alarm: transitioned to ALARM at Thursday 07 November, 2024 15:07:", "StateReasonData": { "triggeringAlarms": [ { diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 2dc99a2069c78..74cb0d7c7c5cf 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-07T11:29:50+00:00" + "last_validated_date": "2024-11-07T15:07:28+00:00" } } From c4151c37c2db07b2c372228a29421c6824f904ae Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Fri, 8 Nov 2024 16:20:46 +0100 Subject: [PATCH 12/51] Use only necessary transformers cloudwatch_api() one doesn't contain anything relevant for the test and only makes transforming more complicated. Explicitly adding only specific transformers needed for the test in question. --- tests/aws/services/cloudwatch/test_cloudwatch.py | 15 ++++++++++----- .../cloudwatch/test_cloudwatch.snapshot.json | 16 ++++++++-------- .../cloudwatch/test_cloudwatch.validation.json | 2 +- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 2f22cbf5e0559..eeb37be46e287 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -991,7 +991,6 @@ def test_set_alarm(self, sns_create_topic, sqs_create_queue, aws_client, cleanup @markers.aws.validated @pytest.mark.skipif(is_old_provider(), reason="New test for v2 provider") def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_client, cleanups, snapshot): - snapshot.add_transformer(snapshot.transform.cloudwatch_api()) # create topics for state 'ALARM' and 'OK' of the composite alarm topic_name_alarm = f"topic-alarm-{short_uid()}" @@ -1001,8 +1000,6 @@ def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_c topic_arn_alarm = sns_topic_alarm["TopicArn"] sns_topic_ok = sns_create_topic(Name=topic_name_ok) topic_arn_ok = sns_topic_ok["TopicArn"] - snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) - snapshot.add_transformer(snapshot.transform.regex(topic_arn_ok, "")) # create queues for 'ALARM' and 'OK' of the composite alarm (will receive sns messages) queue_url_alarm = sqs_create_queue(QueueName=f"AlarmQueue-{short_uid()}") @@ -1104,9 +1101,17 @@ def _put_metric_alarm(alarm_name: str): expected_triggering_child_state="ALARM", ) - # state reason is a text with dates, for now stubbing it out because - # composite alarm reason can be checked via TriggeringChildren property + # add necessary transformers for the snapshot + + # StateReason is a text with formatted dates inside it. For now stubbing it out fully because + # composite alarm reason can be checked via StateReasonData property which is simpler to check + # as its properties reference ARN and state of individual alarms without putting them all into a piece of text. snapshot.add_transformer(snapshot.transform.key_value("StateReason")) + snapshot.add_transformer(snapshot.transform.regex(composite_alarm_name, "")) + snapshot.add_transformer(snapshot.transform.regex(alarm_1_name, "")) + snapshot.add_transformer(snapshot.transform.regex(alarm_2_name, "")) + snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) + snapshot.add_transformer(snapshot.transform.regex(topic_name_ok, "")) composite_alarms_list = aws_client.cloudwatch.describe_alarms( AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index 95f52de55d226..04096e108a0b7 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,27 +2105,27 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "07-11-2024, 15:07:28", + "recorded-date": "08-11-2024, 15:18:19", "recorded-content": { "composite-alarm-in-alarm-when-alarm-1-is-in-alarm": { "ActionsEnabled": true, "AlarmActions": [ - "arn::sns::111111111111:" + "arn::sns::111111111111:" ], - "AlarmArn": "arn::cloudwatch::111111111111:alarm:", + "AlarmArn": "arn::cloudwatch::111111111111:alarm:", "AlarmConfigurationUpdatedTimestamp": "timestamp", "AlarmDescription": "composite alarm description", - "AlarmName": "", - "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:simple-alarm-2-cc82e41a\")", + "AlarmName": "", + "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:\")", "InsufficientDataActions": [], "OKActions": [ - "" + "arn::sns::111111111111:" ], - "StateReason": "arn::cloudwatch::111111111111:alarm: transitioned to ALARM at Thursday 07 November, 2024 15:07:", + "StateReason": "", "StateReasonData": { "triggeringAlarms": [ { - "arn": "arn::cloudwatch::111111111111:alarm:", + "arn": "arn::cloudwatch::111111111111:alarm:", "state": { "value": "ALARM", "timestamp": "date" diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 74cb0d7c7c5cf..f967aba8e450a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-07T15:07:28+00:00" + "last_validated_date": "2024-11-08T15:18:19+00:00" } } From 97ca92ce260d9b67fcd81a957b3ee4696629f7da Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Fri, 8 Nov 2024 16:48:29 +0100 Subject: [PATCH 13/51] Add composite alarms store Leaving current `alarms` property intact for metric alarms intact, without renaming. Most AWS commands like describe-alarms only return metric alarms if composite alarm type is not specified explicitly as a parameter. Therefore it is reasonable to keep the store name as is and add a separate store for composite ones. --- localstack-core/localstack/services/cloudwatch/models.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudwatch/models.py b/localstack-core/localstack/services/cloudwatch/models.py index eb8315ca26fcd..5a7095f1800cd 100644 --- a/localstack-core/localstack/services/cloudwatch/models.py +++ b/localstack-core/localstack/services/cloudwatch/models.py @@ -84,9 +84,12 @@ class CloudWatchStore(BaseStore): # maps resource ARN to tags TAGS: TaggingService = CrossRegionAttribute(default=TaggingService) - # maps resource ARN to alarms + # maps resource ARN to metric alarms alarms: Dict[str, LocalStackAlarm] = LocalAttribute(default=dict) + # maps resource ARN to composite alarms + composite_alarms: Dict[str, LocalStackAlarm] = LocalAttribute(default=dict) + # Contains all the Alarm Histories. Per documentation, an alarm history is retained even if the alarm is deleted, # making it necessary to save this at store level histories: List[Dict] = LocalAttribute(default=list) From ae871e7c52d7337b65ca97b72797306ca2636b4b Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Fri, 8 Nov 2024 17:01:30 +0100 Subject: [PATCH 14/51] Revert "Add composite alarms store" There is already some logic for composite alarms in functions like describe alarms, I will first check if it can be stretched a bit more before introducing a separate store. This reverts commit b7580eb8edd6367a10011b44a405a3ad04a9bd5c. --- localstack-core/localstack/services/cloudwatch/models.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/models.py b/localstack-core/localstack/services/cloudwatch/models.py index 5a7095f1800cd..eb8315ca26fcd 100644 --- a/localstack-core/localstack/services/cloudwatch/models.py +++ b/localstack-core/localstack/services/cloudwatch/models.py @@ -84,12 +84,9 @@ class CloudWatchStore(BaseStore): # maps resource ARN to tags TAGS: TaggingService = CrossRegionAttribute(default=TaggingService) - # maps resource ARN to metric alarms + # maps resource ARN to alarms alarms: Dict[str, LocalStackAlarm] = LocalAttribute(default=dict) - # maps resource ARN to composite alarms - composite_alarms: Dict[str, LocalStackAlarm] = LocalAttribute(default=dict) - # Contains all the Alarm Histories. Per documentation, an alarm history is retained even if the alarm is deleted, # making it necessary to save this at store level histories: List[Dict] = LocalAttribute(default=list) From 432202d8ad7fd7411c331ab7bf5a72bccfca7d3e Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Fri, 8 Nov 2024 17:32:40 +0100 Subject: [PATCH 15/51] Set default attributes for a composite alarm For now it is a copy of a metric default attributes, minus Dimensions. --- .../localstack/services/cloudwatch/models.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/models.py b/localstack-core/localstack/services/cloudwatch/models.py index eb8315ca26fcd..2d049a4e15c0b 100644 --- a/localstack-core/localstack/services/cloudwatch/models.py +++ b/localstack-core/localstack/services/cloudwatch/models.py @@ -52,8 +52,20 @@ def __init__(self, account_id: str, region: str, alarm: CompositeAlarm): self.set_default_attributes() def set_default_attributes(self): - # TODO - pass + current_time = datetime.datetime.utcnow() + self.alarm["AlarmArn"] = arns.cloudwatch_alarm_arn( + self.alarm["AlarmName"], account_id=self.account_id, region_name=self.region + ) + self.alarm["AlarmConfigurationUpdatedTimestamp"] = current_time + self.alarm.setdefault("ActionsEnabled", True) + self.alarm.setdefault("OKActions", []) + self.alarm.setdefault("AlarmActions", []) + self.alarm.setdefault("InsufficientDataActions", []) + self.alarm["StateValue"] = StateValue.INSUFFICIENT_DATA + self.alarm["StateReason"] = "Unchecked: Initial alarm creation" + self.alarm["StateUpdatedTimestamp"] = current_time + self.alarm["StateTransitionedTimestamp"] = current_time + class LocalStackDashboard: From 6a9b0cf214a14c16d3cfc0056d8025fe0c4eda99 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Fri, 8 Nov 2024 17:59:01 +0100 Subject: [PATCH 16/51] Remove composite-to-metric alarm conversion Also adds sepearate scheduling function, a stub for now. --- .../services/cloudwatch/alarm_scheduler.py | 3 +++ .../services/cloudwatch/provider_v2.py | 22 ++++++------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py b/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py index 2b0675f121450..dc8024fbb5891 100644 --- a/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py +++ b/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py @@ -81,6 +81,9 @@ def on_error(e): self.scheduled_alarms[alarm_arn] = task + def schedule_composite_alarm(self, alarm_arn: str) -> None: + pass + def delete_scheduler_for_alarm(self, alarm_arn: str) -> None: """ Deletes the recurring scheduler for an alarm diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index c606d65040575..09ac55d17b811 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -81,6 +81,7 @@ LocalStackAlarm, LocalStackDashboard, LocalStackMetricAlarm, + LocalStackCompositeAlarm, cloudwatch_stores, ) from localstack.services.edge import ROUTER @@ -454,21 +455,12 @@ def put_metric_alarm(self, context: RequestContext, request: PutMetricAlarmInput @handler("PutCompositeAlarm", expand=False) def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlarmInput) -> None: - composite_to_metric_alarm = { - "AlarmName": request.get("AlarmName"), - "AlarmDescription": request.get("AlarmDescription"), - "AlarmActions": request.get("AlarmActions", []), - "OKActions": request.get("OKActions", []), - "InsufficientDataActions": request.get("InsufficientDataActions", []), - "ActionsEnabled": request.get("ActionsEnabled", True), - "AlarmRule": request.get("AlarmRule"), - "Tags": request.get("Tags", []), - } - self.put_metric_alarm(context=context, request=composite_to_metric_alarm) - - LOG.warning( - "Composite Alarms configuration is not yet supported, alarm state will not be evaluated" - ) + with _STORE_LOCK: + store = self.get_store(context.account_id, context.region) + composite_alarm = LocalStackCompositeAlarm(context.account_id, context.region, {**request}) + alarm_arn = composite_alarm.alarm["AlarmArn"] + store.alarms[alarm_arn] = composite_alarm + self.alarm_scheduler.schedule_composite_alarm(alarm_arn) def describe_alarms( self, From 0ee5919ca3827d705adb8719466cc43cb13fe0c5 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sat, 9 Nov 2024 10:15:50 +0100 Subject: [PATCH 17/51] Add evaluation of composite alarm state The evaluation of composite alarms is invoked when a metric alarm state changes because composite alarm state can only change if its child metric alarm changes. This way there is no need to involve scheduler, moreover, there is nothing in documentation about composite alarms evaluation period. So far parent-child relation is not implemented, the simplest approach is to evaluate all composite alarms if any metric alarm changes its state. Later this can be optimized if needed. --- .../services/cloudwatch/provider_v2.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 09ac55d17b811..5c76e47e1df1f 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -354,6 +354,8 @@ def set_alarm_state( state_reason_data, ) + self._evaluate_composite_alarms(context) + if not alarm.alarm["ActionsEnabled"]: return if state_value == "OK": @@ -836,6 +838,30 @@ def _get_timestamp(input: dict): history = [h for h in history if (date := _get_timestamp(h)) and date <= end_date] return DescribeAlarmHistoryOutput(AlarmHistoryItems=history) + def _evaluate_composite_alarms(self, context: RequestContext): + store = self.get_store(context.account_id, context.region) + alarms = list(store.alarms.values()) + composite_alarms = [a for a in alarms if isinstance(a, LocalStackCompositeAlarm)] + for composite_alarm in composite_alarms: + new_state_value = StateValue.OK + alarm_rule = composite_alarm.alarm["AlarmRule"] + # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied + metric_alarm_arns = re.findall(r'\("([^"]*)"\)', alarm_rule) # regexp for everything within (" ") + for metric_alarm_arn in metric_alarm_arns: + metric_alarm = store.alarms.get(metric_alarm_arn) + if metric_alarm.alarm["StateValue"] == StateValue.ALARM: + new_state_value = StateValue.ALARM + break + + old_state_value = composite_alarm.alarm["StateValue"] + + if old_state_value == new_state_value: + return + + state_reason = "" #TODO format the reason + state_reason_data = {} #TODO fill in reason data + self._update_state(context,composite_alarm, new_state_value, state_reason, state_reason_data) + def create_metric_data_query_from_alarm(alarm: LocalStackMetricAlarm): # TODO may need to be adapted for other use cases From 39cc78601ad2656692bc7bc38c2e104ba943247c Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sat, 9 Nov 2024 10:17:28 +0100 Subject: [PATCH 18/51] Invoke composite alarm actions Starting with SNS support only. --- .../services/cloudwatch/provider_v2.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 5c76e47e1df1f..cc755b146eb4a 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -862,6 +862,32 @@ def _evaluate_composite_alarms(self, context: RequestContext): state_reason_data = {} #TODO fill in reason data self._update_state(context,composite_alarm, new_state_value, state_reason, state_reason_data) + if not composite_alarm.alarm["ActionsEnabled"]: + return + if new_state_value == "OK": + actions = composite_alarm.alarm["OKActions"] + elif new_state_value == "ALARM": + actions = composite_alarm.alarm["AlarmActions"] + else: + actions = composite_alarm.alarm["InsufficientDataActions"] + for action in actions: + data = arns.parse_arn(action) + if data["service"] == "sns": + service = connect_to( + region_name=data["region"], aws_access_key_id=data["account"] + ).sns + # TODO verify message for composite alarm + subject = f"""{new_state_value}: "{composite_alarm.alarm["AlarmName"]}" in {context.region}""" + message = create_message_response_update_state_sns(composite_alarm, old_state_value) + service.publish(TopicArn=action, Subject=subject, Message=message) + else: + # TODO: support other actions + LOG.warning( + "Action for service %s not implemented, action '%s' will not be triggered.", + data["service"], + action, + ) + def create_metric_data_query_from_alarm(alarm: LocalStackMetricAlarm): # TODO may need to be adapted for other use cases From 9edf9341f61fd33465a723fb50072cce4bdc8564 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sat, 9 Nov 2024 13:19:22 +0100 Subject: [PATCH 19/51] Create SNS message response for composite alarm --- .../services/cloudwatch/provider_v2.py | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index cc755b146eb4a..6bba53ef8d5b9 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -843,6 +843,7 @@ def _evaluate_composite_alarms(self, context: RequestContext): alarms = list(store.alarms.values()) composite_alarms = [a for a in alarms if isinstance(a, LocalStackCompositeAlarm)] for composite_alarm in composite_alarms: + triggering_alarm = None new_state_value = StateValue.OK alarm_rule = composite_alarm.alarm["AlarmRule"] # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied @@ -850,6 +851,7 @@ def _evaluate_composite_alarms(self, context: RequestContext): for metric_alarm_arn in metric_alarm_arns: metric_alarm = store.alarms.get(metric_alarm_arn) if metric_alarm.alarm["StateValue"] == StateValue.ALARM: + triggering_alarm = metric_alarm new_state_value = StateValue.ALARM break @@ -878,7 +880,7 @@ def _evaluate_composite_alarms(self, context: RequestContext): ).sns # TODO verify message for composite alarm subject = f"""{new_state_value}: "{composite_alarm.alarm["AlarmName"]}" in {context.region}""" - message = create_message_response_update_state_sns(composite_alarm, old_state_value) + message = create_message_response_update_composite_alarm_state_sns(composite_alarm, triggering_alarm, old_state_value) service.publish(TopicArn=action, Subject=subject, Message=message) else: # TODO: support other actions @@ -942,7 +944,7 @@ def create_message_response_update_state_lambda( return json.dumps(response, cls=JSONEncoder) -def create_message_response_update_state_sns(alarm, old_state): +def create_message_response_update_state_sns(alarm: LocalStackMetricAlarm, old_state): _alarm = alarm.alarm response = { "AWSAccountId": alarm.account_id, @@ -996,3 +998,39 @@ def create_message_response_update_state_sns(alarm, old_state): response["Trigger"] = details return json.dumps(response, cls=JSONEncoder) + +def create_message_response_update_composite_alarm_state_sns( + composite_alarm: LocalStackCompositeAlarm, + triggering_alarm: LocalStackMetricAlarm, + old_state, +): + _alarm = composite_alarm.alarm + response = { + "AWSAccountId": composite_alarm.account_id, + "AlarmName": _alarm["AlarmName"], + "AlarmDescription": _alarm.get("AlarmDescription"), + "AlarmRule": _alarm.get("AlarmRule"), + "OldStateValue": old_state, + "NewStateValue": _alarm["StateValue"], + "NewStateReason": _alarm["StateReason"], + "StateChangeTime": _alarm["StateUpdatedTimestamp"], + # the long-name for 'region' should be used - as we don't have it, we use the short name + # which needs to be slightly changed to make snapshot tests work + "Region": composite_alarm.region.replace("-", " ").capitalize(), + "AlarmArn": _alarm["AlarmArn"], + "OKActions": _alarm.get("OKActions", []), + "AlarmActions": _alarm.get("AlarmActions", []), + "InsufficientDataActions": _alarm.get("InsufficientDataActions", []), + } + + triggering_children = [{ + "Arn": triggering_alarm.alarm.get("AlarmArn"), + "State": { + "Value": triggering_alarm.alarm["StateValue"], + "Timestamp": triggering_alarm.alarm["StateUpdatedTimestamp"], + } + }] + + response["TriggeringChildren"] = triggering_children + + return json.dumps(response, cls=JSONEncoder) From c59f723daea07a8009529ff5733adfffd8ddcaa0 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sat, 9 Nov 2024 13:40:18 +0100 Subject: [PATCH 20/51] Add state reason data for composite alarm --- .../localstack/services/cloudwatch/provider_v2.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 6bba53ef8d5b9..86f8fb18117b0 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -760,7 +760,7 @@ def _update_state( old_state_reason = alarm.alarm["StateReason"] store = self.get_store(context.account_id, context.region) current_time = datetime.datetime.now() - if state_reason_data: + if state_reason_data and isinstance(alarm, LocalStackMetricAlarm): state_reason_data["version"] = HISTORY_VERSION history_data = { "version": HISTORY_VERSION, @@ -861,7 +861,17 @@ def _evaluate_composite_alarms(self, context: RequestContext): return state_reason = "" #TODO format the reason - state_reason_data = {} #TODO fill in reason data + state_reason_data = { + "triggeringAlarms": [ + { + "arn": triggering_alarm.alarm.get("AlarmArn"), + "state": { + "value": triggering_alarm.alarm.get("StateValue"), + "timestamp": timestamp_millis(triggering_alarm.alarm.get("StateTransitionedTimestamp")), + } + } + ] + } self._update_state(context,composite_alarm, new_state_value, state_reason, state_reason_data) if not composite_alarm.alarm["ActionsEnabled"]: From 3614be7a7152902f6e5f63ad235139578ae51a96 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sat, 9 Nov 2024 15:42:12 +0100 Subject: [PATCH 21/51] Remove scheduler stub for a composite alarm Eventually composite alarms are evaluated on metric alarm state change instead of schedule. --- .../localstack/services/cloudwatch/alarm_scheduler.py | 3 --- localstack-core/localstack/services/cloudwatch/provider_v2.py | 1 - 2 files changed, 4 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py b/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py index dc8024fbb5891..2b0675f121450 100644 --- a/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py +++ b/localstack-core/localstack/services/cloudwatch/alarm_scheduler.py @@ -81,9 +81,6 @@ def on_error(e): self.scheduled_alarms[alarm_arn] = task - def schedule_composite_alarm(self, alarm_arn: str) -> None: - pass - def delete_scheduler_for_alarm(self, alarm_arn: str) -> None: """ Deletes the recurring scheduler for an alarm diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 86f8fb18117b0..aa2eeb743308a 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -462,7 +462,6 @@ def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlar composite_alarm = LocalStackCompositeAlarm(context.account_id, context.region, {**request}) alarm_arn = composite_alarm.alarm["AlarmArn"] store.alarms[alarm_arn] = composite_alarm - self.alarm_scheduler.schedule_composite_alarm(alarm_arn) def describe_alarms( self, From 235462a7937f5426f41d86c17aec8d0ee2d068ae Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sat, 9 Nov 2024 16:52:05 +0100 Subject: [PATCH 22/51] Format state reason for composite alarm Also, changes some usages of deprecated utcnow() --- .../localstack/services/cloudwatch/models.py | 5 +++-- .../services/cloudwatch/provider_v2.py | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/models.py b/localstack-core/localstack/services/cloudwatch/models.py index 2d049a4e15c0b..87981b5174f71 100644 --- a/localstack-core/localstack/services/cloudwatch/models.py +++ b/localstack-core/localstack/services/cloudwatch/models.py @@ -1,4 +1,5 @@ import datetime +from datetime import timezone from typing import Dict, List from localstack.aws.api.cloudwatch import CompositeAlarm, DashboardBody, MetricAlarm, StateValue @@ -24,7 +25,7 @@ def __init__(self, account_id: str, region: str, alarm: MetricAlarm): self.set_default_attributes() def set_default_attributes(self): - current_time = datetime.datetime.utcnow() + current_time = datetime.datetime.now(timezone.utc) self.alarm["AlarmArn"] = arns.cloudwatch_alarm_arn( self.alarm["AlarmName"], account_id=self.account_id, region_name=self.region ) @@ -52,7 +53,7 @@ def __init__(self, account_id: str, region: str, alarm: CompositeAlarm): self.set_default_attributes() def set_default_attributes(self): - current_time = datetime.datetime.utcnow() + current_time = datetime.datetime.now(timezone.utc) self.alarm["AlarmArn"] = arns.cloudwatch_alarm_arn( self.alarm["AlarmName"], account_id=self.account_id, region_name=self.region ) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index aa2eeb743308a..df8f37cd3f6b9 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -4,6 +4,7 @@ import re import threading import uuid +from datetime import timezone from typing import List from localstack.aws.api import CommonServiceException, RequestContext, handler @@ -340,7 +341,7 @@ def set_alarm_state( if old_state == state_value: return - alarm.alarm["StateTransitionedTimestamp"] = datetime.datetime.now() + alarm.alarm["StateTransitionedTimestamp"] = datetime.datetime.now(timezone.utc) # update startDate (=last ALARM date) - should only update when a new alarm is triggered # the date is only updated if we have a reason-data, which is set by an alarm if state_reason_data: @@ -859,14 +860,21 @@ def _evaluate_composite_alarms(self, context: RequestContext): if old_state_value == new_state_value: return - state_reason = "" #TODO format the reason + triggering_alarm_arn = triggering_alarm.alarm.get("AlarmArn") + triggering_alarm_state = triggering_alarm.alarm.get("StateValue") + triggering_alarm_state_change_timestamp = triggering_alarm.alarm.get("StateTransitionedTimestamp") + state_reason_formatted_timestamp = triggering_alarm_state_change_timestamp.strftime( + "%A %d %B, %Y %H:%M:%S %Z") + state_reason = (f"{triggering_alarm_arn} " + f"transitioned to {triggering_alarm_state} " + f"at {state_reason_formatted_timestamp}") state_reason_data = { "triggeringAlarms": [ { - "arn": triggering_alarm.alarm.get("AlarmArn"), + "arn": triggering_alarm_arn, "state": { - "value": triggering_alarm.alarm.get("StateValue"), - "timestamp": timestamp_millis(triggering_alarm.alarm.get("StateTransitionedTimestamp")), + "value": triggering_alarm_state, + "timestamp": timestamp_millis(triggering_alarm_state_change_timestamp), } } ] From 5c5a71b37de9482646d4e0e7d69b2aed9dec1aa3 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 11:29:22 +0100 Subject: [PATCH 23/51] Move transformers setup before test cases run So that test cases code is grouped better --- tests/aws/services/cloudwatch/test_cloudwatch.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index eeb37be46e287..34e9784b93c50 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1082,6 +1082,19 @@ def _put_metric_alarm(alarm_name: str): assert composite_alarm["AlarmName"] == composite_alarm_name assert composite_alarm["AlarmRule"] == composite_alarm_rule + + # add necessary transformers for the snapshot + + # StateReason is a text with formatted dates inside it. For now stubbing it out fully because + # composite alarm reason can be checked via StateReasonData property which is simpler to check + # as its properties reference ARN and state of individual alarms without putting them all into a piece of text. + snapshot.add_transformer(snapshot.transform.key_value("StateReason")) + snapshot.add_transformer(snapshot.transform.regex(composite_alarm_name, "")) + snapshot.add_transformer(snapshot.transform.regex(alarm_1_name, "")) + snapshot.add_transformer(snapshot.transform.regex(alarm_2_name, "")) + snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) + snapshot.add_transformer(snapshot.transform.regex(topic_name_ok, "")) + # trigger alarm 1 - composite one should also go into ALARM state aws_client.cloudwatch.set_alarm_state( AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" From fce2ec1f1a8a96cc95c2041fcd5f15a6b6b3c33b Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 11:30:20 +0100 Subject: [PATCH 24/51] Add composite alarm back to OK test case --- .../services/cloudwatch/provider_v2.py | 5 +- .../services/cloudwatch/test_cloudwatch.py | 58 ++++++++----------- .../cloudwatch/test_cloudwatch.snapshot.json | 32 +++++++++- .../test_cloudwatch.validation.json | 2 +- 4 files changed, 59 insertions(+), 38 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index df8f37cd3f6b9..95f2d513d85a2 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -355,7 +355,7 @@ def set_alarm_state( state_reason_data, ) - self._evaluate_composite_alarms(context) + self._evaluate_composite_alarms(context, alarm) if not alarm.alarm["ActionsEnabled"]: return @@ -838,12 +838,11 @@ def _get_timestamp(input: dict): history = [h for h in history if (date := _get_timestamp(h)) and date <= end_date] return DescribeAlarmHistoryOutput(AlarmHistoryItems=history) - def _evaluate_composite_alarms(self, context: RequestContext): + def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): store = self.get_store(context.account_id, context.region) alarms = list(store.alarms.values()) composite_alarms = [a for a in alarms if isinstance(a, LocalStackCompositeAlarm)] for composite_alarm in composite_alarms: - triggering_alarm = None new_state_value = StateValue.OK alarm_rule = composite_alarm.alarm["AlarmRule"] # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 34e9784b93c50..c079761893488 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1114,45 +1114,37 @@ def _put_metric_alarm(alarm_name: str): expected_triggering_child_state="ALARM", ) - # add necessary transformers for the snapshot + composite_alarms_list = aws_client.cloudwatch.describe_alarms( + AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] + ) + composite_alarm_in_alarm_when_alarm_1_in_alarm = composite_alarms_list["CompositeAlarms"][0] + snapshot.match("composite-alarm-in-alarm-when-alarm-1-is-in-alarm", composite_alarm_in_alarm_when_alarm_1_in_alarm) - # StateReason is a text with formatted dates inside it. For now stubbing it out fully because - # composite alarm reason can be checked via StateReasonData property which is simpler to check - # as its properties reference ARN and state of individual alarms without putting them all into a piece of text. - snapshot.add_transformer(snapshot.transform.key_value("StateReason")) - snapshot.add_transformer(snapshot.transform.regex(composite_alarm_name, "")) - snapshot.add_transformer(snapshot.transform.regex(alarm_1_name, "")) - snapshot.add_transformer(snapshot.transform.regex(alarm_2_name, "")) - snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) - snapshot.add_transformer(snapshot.transform.regex(topic_name_ok, "")) + # trigger OK for alarm 1 - composite one should also go back to OK + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" + ) + + retry( + check_composite_alarm_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + queue_url=queue_url_ok, + expected_topic_arn=topic_arn_ok, + alarm_name=composite_alarm_name, + alarm_description=composite_alarm_description, + expected_state="OK", + expected_triggering_child_arn=alarm_1_arn, + expected_triggering_child_state="OK", + ) composite_alarms_list = aws_client.cloudwatch.describe_alarms( AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) - composite_alarm_in_alarm_caused_by_alarm_1 = composite_alarms_list["CompositeAlarms"][0] - snapshot.match("composite-alarm-in-alarm-when-alarm-1-is-in-alarm", composite_alarm_in_alarm_caused_by_alarm_1) + composite_alarm_in_ok_when_alarm_1_back_to_ok = composite_alarms_list["CompositeAlarms"][0] + snapshot.match("composite-alarm-in-ok-when-alarm-1-is-back-to-ok", composite_alarm_in_ok_when_alarm_1_back_to_ok) - # # trigger OK for alarm 1 - composite one should also go back to OK - # aws_client.cloudwatch.set_alarm_state( - # AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" - # ) - # - # retry( - # check_message, - # retries=PUBLICATION_RETRIES, - # sleep_before=1, - # sqs_client=aws_client.sqs, - # expected_queue_url=queue_url_ok, - # expected_topic_arn=topic_arn_ok, - # expected_new="OK", - # expected_reason=state_reason, #TODO define state reason for composite alarm - # alarm_name=composite_alarm_name, - # alarm_description=composite_alarm_description, - # expected_trigger=expected_trigger, #TODO define expected trigger for composite alarm - # ) - # describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) - # snapshot.match("reset-alarm", describe_alarm) - # # # trigger alarm 2 - composite one should go again into ALARM state # aws_client.cloudwatch.set_alarm_state( # AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index 04096e108a0b7..dc74b72016e8c 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,7 +2105,7 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "08-11-2024, 15:18:19", + "recorded-date": "11-11-2024, 10:26:28", "recorded-content": { "composite-alarm-in-alarm-when-alarm-1-is-in-alarm": { "ActionsEnabled": true, @@ -2136,6 +2136,36 @@ "StateTransitionedTimestamp": "timestamp", "StateUpdatedTimestamp": "timestamp", "StateValue": "ALARM" + }, + "composite-alarm-in-ok-when-alarm-1-is-back-to-ok": { + "ActionsEnabled": true, + "AlarmActions": [ + "arn::sns::111111111111:" + ], + "AlarmArn": "arn::cloudwatch::111111111111:alarm:", + "AlarmConfigurationUpdatedTimestamp": "timestamp", + "AlarmDescription": "composite alarm description", + "AlarmName": "", + "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:\")", + "InsufficientDataActions": [], + "OKActions": [ + "arn::sns::111111111111:" + ], + "StateReason": "", + "StateReasonData": { + "triggeringAlarms": [ + { + "arn": "arn::cloudwatch::111111111111:alarm:", + "state": { + "value": "OK", + "timestamp": "date" + } + } + ] + }, + "StateTransitionedTimestamp": "timestamp", + "StateUpdatedTimestamp": "timestamp", + "StateValue": "OK" } } } diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index f967aba8e450a..59913751184fe 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-08T15:18:19+00:00" + "last_validated_date": "2024-11-11T10:26:28+00:00" } } From e1fb6a24fc1a3e572fc843872df7bd3cbee1f1d4 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 11:38:47 +0100 Subject: [PATCH 25/51] Add composite alarm in ALARM because of alarm-2 test case --- .../services/cloudwatch/test_cloudwatch.py | 45 ++++++++++--------- .../cloudwatch/test_cloudwatch.snapshot.json | 32 ++++++++++++- .../test_cloudwatch.validation.json | 2 +- 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index c079761893488..dee470de2027a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1145,26 +1145,31 @@ def _put_metric_alarm(alarm_name: str): composite_alarm_in_ok_when_alarm_1_back_to_ok = composite_alarms_list["CompositeAlarms"][0] snapshot.match("composite-alarm-in-ok-when-alarm-1-is-back-to-ok", composite_alarm_in_ok_when_alarm_1_back_to_ok) - # # trigger alarm 2 - composite one should go again into ALARM state - # aws_client.cloudwatch.set_alarm_state( - # AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" - # ) - # - # retry( - # check_message, - # retries=PUBLICATION_RETRIES, - # sleep_before=1, - # sqs_client=aws_client.sqs, - # expected_queue_url=queue_url_alarm, - # expected_topic_arn=topic_arn_alarm, - # expected_new="ALARM" - # expected_reason = state_reason, # TODO define state reason for composite alarm - # alarm_name = composite_alarm_name, - # alarm_description = composite_alarm_description, - # expected_trigger = expected_trigger, # TODO define expected trigger for composite alarm - # ) - # describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[composite_alarm_name]) - # snapshot.match("triggered-alarm", describe_alarm) + # trigger alarm 2 - composite one should go again into ALARM state + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" + ) + + retry( + check_composite_alarm_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + queue_url=queue_url_alarm, + expected_topic_arn=topic_arn_alarm, + alarm_name=composite_alarm_name, + alarm_description=composite_alarm_description, + expected_state="ALARM", + expected_triggering_child_arn=alarm_2_arn, + expected_triggering_child_state="ALARM", + ) + + composite_alarms_list = aws_client.cloudwatch.describe_alarms( + AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] + ) + composite_alarm_in_alarm_when_alarm_2_in_alarm = composite_alarms_list["CompositeAlarms"][0] + snapshot.match("composite-alarm-in-alarm-when-alarm-2-is-in-alarm", + composite_alarm_in_alarm_when_alarm_2_in_alarm) # # # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change # aws_client.cloudwatch.set_alarm_state( diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index dc74b72016e8c..baee2fb2ba0bb 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,7 +2105,7 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "11-11-2024, 10:26:28", + "recorded-date": "11-11-2024, 10:37:42", "recorded-content": { "composite-alarm-in-alarm-when-alarm-1-is-in-alarm": { "ActionsEnabled": true, @@ -2166,6 +2166,36 @@ "StateTransitionedTimestamp": "timestamp", "StateUpdatedTimestamp": "timestamp", "StateValue": "OK" + }, + "composite-alarm-in-alarm-when-alarm-2-is-in-alarm": { + "ActionsEnabled": true, + "AlarmActions": [ + "arn::sns::111111111111:" + ], + "AlarmArn": "arn::cloudwatch::111111111111:alarm:", + "AlarmConfigurationUpdatedTimestamp": "timestamp", + "AlarmDescription": "composite alarm description", + "AlarmName": "", + "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:\")", + "InsufficientDataActions": [], + "OKActions": [ + "arn::sns::111111111111:" + ], + "StateReason": "", + "StateReasonData": { + "triggeringAlarms": [ + { + "arn": "arn::cloudwatch::111111111111:alarm:", + "state": { + "value": "ALARM", + "timestamp": "date" + } + } + ] + }, + "StateTransitionedTimestamp": "timestamp", + "StateUpdatedTimestamp": "timestamp", + "StateValue": "ALARM" } } } diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 59913751184fe..5b40155a6a5fe 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-11T10:26:28+00:00" + "last_validated_date": "2024-11-11T10:37:42+00:00" } } From 505d10d6151b3fb8da188ab500fbd0ef9f9165be Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 12:04:52 +0100 Subject: [PATCH 26/51] Add composite alarm not changed by second trigger test case --- .../services/cloudwatch/test_cloudwatch.py | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index dee470de2027a..9de4cf9ffe82a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1170,13 +1170,22 @@ def _put_metric_alarm(alarm_name: str): composite_alarm_in_alarm_when_alarm_2_in_alarm = composite_alarms_list["CompositeAlarms"][0] snapshot.match("composite-alarm-in-alarm-when-alarm-2-is-in-alarm", composite_alarm_in_alarm_when_alarm_2_in_alarm) - # - # # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change - # aws_client.cloudwatch.set_alarm_state( - # AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" - # ) - # # TODO check that alarm state hasn't changed, probably with describe_alarm_history - # # take into account that composite alarm runs on schedule + + # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" + ) + + composite_alarms_list = aws_client.cloudwatch.describe_alarms( + AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] + ) + composite_alarm_not_changed_by_second_trigger = composite_alarms_list["CompositeAlarms"][0] + # It is not possible to reuse the snapshot within the same test, + # therefore checking that alarm reason hasn't changed instead of recording new snapshot + state_reason_data = json.loads(composite_alarm_not_changed_by_second_trigger["StateReasonData"]) + triggering_alarms = state_reason_data["triggeringAlarms"] + assert len(triggering_alarms) == 1 + assert triggering_alarms[0]["arn"] == alarm_2_arn @markers.aws.validated From 20f97d328c1a32f14a4eb46015eae42cb087fd2a Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 12:13:18 +0100 Subject: [PATCH 27/51] Remove SQS message logging in test --- tests/aws/services/cloudwatch/test_cloudwatch.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 9de4cf9ffe82a..4a3085ddf4fe5 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -2924,7 +2924,6 @@ def check_composite_alarm_message( expected_triggering_child_state, ): receive_result = sqs_client.receive_message(QueueUrl=queue_url) - LOG.info(f"receive SQS message: {receive_result}") message = None for msg in receive_result["Messages"]: body = json.loads(msg["Body"]) From e9a83cb491599e5aa15868ca8005800e5feccbbb Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 12:14:23 +0100 Subject: [PATCH 28/51] Format code --- .../localstack/services/cloudwatch/models.py | 1 - .../services/cloudwatch/provider_v2.py | 56 ++++++++++++------- .../services/cloudwatch/test_cloudwatch.py | 55 +++++++++++------- 3 files changed, 71 insertions(+), 41 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/models.py b/localstack-core/localstack/services/cloudwatch/models.py index 87981b5174f71..a1246569f4f97 100644 --- a/localstack-core/localstack/services/cloudwatch/models.py +++ b/localstack-core/localstack/services/cloudwatch/models.py @@ -68,7 +68,6 @@ def set_default_attributes(self): self.alarm["StateTransitionedTimestamp"] = current_time - class LocalStackDashboard: region: str account_id: str diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 95f2d513d85a2..6be64580af03c 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -80,9 +80,9 @@ from localstack.services.cloudwatch.models import ( CloudWatchStore, LocalStackAlarm, + LocalStackCompositeAlarm, LocalStackDashboard, LocalStackMetricAlarm, - LocalStackCompositeAlarm, cloudwatch_stores, ) from localstack.services.edge import ROUTER @@ -460,7 +460,9 @@ def put_metric_alarm(self, context: RequestContext, request: PutMetricAlarmInput def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlarmInput) -> None: with _STORE_LOCK: store = self.get_store(context.account_id, context.region) - composite_alarm = LocalStackCompositeAlarm(context.account_id, context.region, {**request}) + composite_alarm = LocalStackCompositeAlarm( + context.account_id, context.region, {**request} + ) alarm_arn = composite_alarm.alarm["AlarmArn"] store.alarms[alarm_arn] = composite_alarm @@ -846,7 +848,9 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): new_state_value = StateValue.OK alarm_rule = composite_alarm.alarm["AlarmRule"] # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied - metric_alarm_arns = re.findall(r'\("([^"]*)"\)', alarm_rule) # regexp for everything within (" ") + metric_alarm_arns = re.findall( + r'\("([^"]*)"\)', alarm_rule + ) # regexp for everything within (" ") for metric_alarm_arn in metric_alarm_arns: metric_alarm = store.alarms.get(metric_alarm_arn) if metric_alarm.alarm["StateValue"] == StateValue.ALARM: @@ -861,12 +865,17 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): triggering_alarm_arn = triggering_alarm.alarm.get("AlarmArn") triggering_alarm_state = triggering_alarm.alarm.get("StateValue") - triggering_alarm_state_change_timestamp = triggering_alarm.alarm.get("StateTransitionedTimestamp") + triggering_alarm_state_change_timestamp = triggering_alarm.alarm.get( + "StateTransitionedTimestamp" + ) state_reason_formatted_timestamp = triggering_alarm_state_change_timestamp.strftime( - "%A %d %B, %Y %H:%M:%S %Z") - state_reason = (f"{triggering_alarm_arn} " - f"transitioned to {triggering_alarm_state} " - f"at {state_reason_formatted_timestamp}") + "%A %d %B, %Y %H:%M:%S %Z" + ) + state_reason = ( + f"{triggering_alarm_arn} " + f"transitioned to {triggering_alarm_state} " + f"at {state_reason_formatted_timestamp}" + ) state_reason_data = { "triggeringAlarms": [ { @@ -874,11 +883,13 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): "state": { "value": triggering_alarm_state, "timestamp": timestamp_millis(triggering_alarm_state_change_timestamp), - } + }, } ] } - self._update_state(context,composite_alarm, new_state_value, state_reason, state_reason_data) + self._update_state( + context, composite_alarm, new_state_value, state_reason, state_reason_data + ) if not composite_alarm.alarm["ActionsEnabled"]: return @@ -896,7 +907,9 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): ).sns # TODO verify message for composite alarm subject = f"""{new_state_value}: "{composite_alarm.alarm["AlarmName"]}" in {context.region}""" - message = create_message_response_update_composite_alarm_state_sns(composite_alarm, triggering_alarm, old_state_value) + message = create_message_response_update_composite_alarm_state_sns( + composite_alarm, triggering_alarm, old_state_value + ) service.publish(TopicArn=action, Subject=subject, Message=message) else: # TODO: support other actions @@ -1015,10 +1028,11 @@ def create_message_response_update_state_sns(alarm: LocalStackMetricAlarm, old_s return json.dumps(response, cls=JSONEncoder) + def create_message_response_update_composite_alarm_state_sns( - composite_alarm: LocalStackCompositeAlarm, - triggering_alarm: LocalStackMetricAlarm, - old_state, + composite_alarm: LocalStackCompositeAlarm, + triggering_alarm: LocalStackMetricAlarm, + old_state, ): _alarm = composite_alarm.alarm response = { @@ -1039,13 +1053,15 @@ def create_message_response_update_composite_alarm_state_sns( "InsufficientDataActions": _alarm.get("InsufficientDataActions", []), } - triggering_children = [{ - "Arn": triggering_alarm.alarm.get("AlarmArn"), - "State": { - "Value": triggering_alarm.alarm["StateValue"], - "Timestamp": triggering_alarm.alarm["StateUpdatedTimestamp"], + triggering_children = [ + { + "Arn": triggering_alarm.alarm.get("AlarmArn"), + "State": { + "Value": triggering_alarm.alarm["StateValue"], + "Timestamp": triggering_alarm.alarm["StateUpdatedTimestamp"], + }, } - }] + ] response["TriggeringChildren"] = triggering_children diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 4a3085ddf4fe5..a27771966ebe6 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -990,8 +990,9 @@ def test_set_alarm(self, sns_create_topic, sqs_create_queue, aws_client, cleanup @markers.aws.validated @pytest.mark.skipif(is_old_provider(), reason="New test for v2 provider") - def test_trigger_composite_alarm(self, sns_create_topic, sqs_create_queue, aws_client, cleanups, snapshot): - + def test_trigger_composite_alarm( + self, sns_create_topic, sqs_create_queue, aws_client, cleanups, snapshot + ): # create topics for state 'ALARM' and 'OK' of the composite alarm topic_name_alarm = f"topic-alarm-{short_uid()}" topic_name_ok = f"topic-ok-{short_uid()}" @@ -1055,8 +1056,12 @@ def _put_metric_alarm(alarm_name: str): _put_metric_alarm(alarm_1_name) _put_metric_alarm(alarm_2_name) - alarm_1_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_1_name])["MetricAlarms"][0]["AlarmArn"] - alarm_2_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_2_name])["MetricAlarms"][0]["AlarmArn"] + alarm_1_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_1_name])[ + "MetricAlarms" + ][0]["AlarmArn"] + alarm_2_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_2_name])[ + "MetricAlarms" + ][0]["AlarmArn"] # put composite alarm that is triggered when either of metric alarms is triggered. composite_alarm_name = f"composite-alarm-{short_uid()}" @@ -1082,14 +1087,15 @@ def _put_metric_alarm(alarm_name: str): assert composite_alarm["AlarmName"] == composite_alarm_name assert composite_alarm["AlarmRule"] == composite_alarm_rule - # add necessary transformers for the snapshot # StateReason is a text with formatted dates inside it. For now stubbing it out fully because # composite alarm reason can be checked via StateReasonData property which is simpler to check # as its properties reference ARN and state of individual alarms without putting them all into a piece of text. snapshot.add_transformer(snapshot.transform.key_value("StateReason")) - snapshot.add_transformer(snapshot.transform.regex(composite_alarm_name, "")) + snapshot.add_transformer( + snapshot.transform.regex(composite_alarm_name, "") + ) snapshot.add_transformer(snapshot.transform.regex(alarm_1_name, "")) snapshot.add_transformer(snapshot.transform.regex(alarm_2_name, "")) snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) @@ -1118,7 +1124,10 @@ def _put_metric_alarm(alarm_name: str): AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) composite_alarm_in_alarm_when_alarm_1_in_alarm = composite_alarms_list["CompositeAlarms"][0] - snapshot.match("composite-alarm-in-alarm-when-alarm-1-is-in-alarm", composite_alarm_in_alarm_when_alarm_1_in_alarm) + snapshot.match( + "composite-alarm-in-alarm-when-alarm-1-is-in-alarm", + composite_alarm_in_alarm_when_alarm_1_in_alarm, + ) # trigger OK for alarm 1 - composite one should also go back to OK aws_client.cloudwatch.set_alarm_state( @@ -1143,7 +1152,10 @@ def _put_metric_alarm(alarm_name: str): AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) composite_alarm_in_ok_when_alarm_1_back_to_ok = composite_alarms_list["CompositeAlarms"][0] - snapshot.match("composite-alarm-in-ok-when-alarm-1-is-back-to-ok", composite_alarm_in_ok_when_alarm_1_back_to_ok) + snapshot.match( + "composite-alarm-in-ok-when-alarm-1-is-back-to-ok", + composite_alarm_in_ok_when_alarm_1_back_to_ok, + ) # trigger alarm 2 - composite one should go again into ALARM state aws_client.cloudwatch.set_alarm_state( @@ -1168,8 +1180,10 @@ def _put_metric_alarm(alarm_name: str): AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) composite_alarm_in_alarm_when_alarm_2_in_alarm = composite_alarms_list["CompositeAlarms"][0] - snapshot.match("composite-alarm-in-alarm-when-alarm-2-is-in-alarm", - composite_alarm_in_alarm_when_alarm_2_in_alarm) + snapshot.match( + "composite-alarm-in-alarm-when-alarm-2-is-in-alarm", + composite_alarm_in_alarm_when_alarm_2_in_alarm, + ) # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change aws_client.cloudwatch.set_alarm_state( @@ -1182,12 +1196,13 @@ def _put_metric_alarm(alarm_name: str): composite_alarm_not_changed_by_second_trigger = composite_alarms_list["CompositeAlarms"][0] # It is not possible to reuse the snapshot within the same test, # therefore checking that alarm reason hasn't changed instead of recording new snapshot - state_reason_data = json.loads(composite_alarm_not_changed_by_second_trigger["StateReasonData"]) + state_reason_data = json.loads( + composite_alarm_not_changed_by_second_trigger["StateReasonData"] + ) triggering_alarms = state_reason_data["triggeringAlarms"] assert len(triggering_alarms) == 1 assert triggering_alarms[0]["arn"] == alarm_2_arn - @markers.aws.validated @markers.snapshot.skip_snapshot_verify( paths=[ @@ -2914,14 +2929,14 @@ def _sqs_messages_snapshot(expected_state, sqs_client, sqs_queue, snapshot, iden def check_composite_alarm_message( - sqs_client, - queue_url, - expected_topic_arn, - alarm_name, - alarm_description, - expected_state, - expected_triggering_child_arn, - expected_triggering_child_state, + sqs_client, + queue_url, + expected_topic_arn, + alarm_name, + alarm_description, + expected_state, + expected_triggering_child_arn, + expected_triggering_child_state, ): receive_result = sqs_client.receive_message(QueueUrl=queue_url) message = None From 168514bec5be7922d75c63039e25cdde13422eee Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Mon, 11 Nov 2024 12:37:40 +0100 Subject: [PATCH 29/51] Remove implemented TODO --- localstack-core/localstack/services/cloudwatch/provider_v2.py | 1 - 1 file changed, 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 6be64580af03c..80332411221c3 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -905,7 +905,6 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): service = connect_to( region_name=data["region"], aws_access_key_id=data["account"] ).sns - # TODO verify message for composite alarm subject = f"""{new_state_value}: "{composite_alarm.alarm["AlarmName"]}" in {context.region}""" message = create_message_response_update_composite_alarm_state_sns( composite_alarm, triggering_alarm, old_state_value From b5af73b6fce85133e5f98f1f68e28d2b214bd4cc Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Tue, 12 Nov 2024 11:58:54 +0100 Subject: [PATCH 30/51] Extract method for evaluating single composite alarm Improves readability and fixes a bug with returning from all alarms evaluation if state of one of them hasn't changed - function complexity needs to be reduced as there are too many break and return statements. --- .../services/cloudwatch/provider_v2.py | 140 +++++++++--------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 80332411221c3..370c688de63b1 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -845,78 +845,78 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): alarms = list(store.alarms.values()) composite_alarms = [a for a in alarms if isinstance(a, LocalStackCompositeAlarm)] for composite_alarm in composite_alarms: - new_state_value = StateValue.OK - alarm_rule = composite_alarm.alarm["AlarmRule"] - # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied - metric_alarm_arns = re.findall( - r'\("([^"]*)"\)', alarm_rule - ) # regexp for everything within (" ") - for metric_alarm_arn in metric_alarm_arns: - metric_alarm = store.alarms.get(metric_alarm_arn) - if metric_alarm.alarm["StateValue"] == StateValue.ALARM: - triggering_alarm = metric_alarm - new_state_value = StateValue.ALARM - break - - old_state_value = composite_alarm.alarm["StateValue"] - - if old_state_value == new_state_value: - return + self._evaluate_composite_alarm(composite_alarm, context, triggering_alarm) - triggering_alarm_arn = triggering_alarm.alarm.get("AlarmArn") - triggering_alarm_state = triggering_alarm.alarm.get("StateValue") - triggering_alarm_state_change_timestamp = triggering_alarm.alarm.get( - "StateTransitionedTimestamp" - ) - state_reason_formatted_timestamp = triggering_alarm_state_change_timestamp.strftime( - "%A %d %B, %Y %H:%M:%S %Z" - ) - state_reason = ( - f"{triggering_alarm_arn} " - f"transitioned to {triggering_alarm_state} " - f"at {state_reason_formatted_timestamp}" - ) - state_reason_data = { - "triggeringAlarms": [ - { - "arn": triggering_alarm_arn, - "state": { - "value": triggering_alarm_state, - "timestamp": timestamp_millis(triggering_alarm_state_change_timestamp), - }, - } - ] - } - self._update_state( - context, composite_alarm, new_state_value, state_reason, state_reason_data - ) - - if not composite_alarm.alarm["ActionsEnabled"]: - return - if new_state_value == "OK": - actions = composite_alarm.alarm["OKActions"] - elif new_state_value == "ALARM": - actions = composite_alarm.alarm["AlarmActions"] + def _evaluate_composite_alarm(self, composite_alarm, context, triggering_alarm): + store = self.get_store(context.account_id, context.region) + new_state_value = StateValue.OK + alarm_rule = composite_alarm.alarm["AlarmRule"] + # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied + metric_alarm_arns = re.findall( + r'\("([^"]*)"\)', alarm_rule + ) # regexp for everything within (" ") + for metric_alarm_arn in metric_alarm_arns: + metric_alarm = store.alarms.get(metric_alarm_arn) + if metric_alarm.alarm["StateValue"] == StateValue.ALARM: + triggering_alarm = metric_alarm + new_state_value = StateValue.ALARM + break + old_state_value = composite_alarm.alarm["StateValue"] + if old_state_value == new_state_value: + return + triggering_alarm_arn = triggering_alarm.alarm.get("AlarmArn") + triggering_alarm_state = triggering_alarm.alarm.get("StateValue") + triggering_alarm_state_change_timestamp = triggering_alarm.alarm.get( + "StateTransitionedTimestamp" + ) + state_reason_formatted_timestamp = triggering_alarm_state_change_timestamp.strftime( + "%A %d %B, %Y %H:%M:%S %Z" + ) + state_reason = ( + f"{triggering_alarm_arn} " + f"transitioned to {triggering_alarm_state} " + f"at {state_reason_formatted_timestamp}" + ) + state_reason_data = { + "triggeringAlarms": [ + { + "arn": triggering_alarm_arn, + "state": { + "value": triggering_alarm_state, + "timestamp": timestamp_millis(triggering_alarm_state_change_timestamp), + }, + } + ] + } + self._update_state( + context, composite_alarm, new_state_value, state_reason, state_reason_data + ) + if not composite_alarm.alarm["ActionsEnabled"]: + return + if new_state_value == "OK": + actions = composite_alarm.alarm["OKActions"] + elif new_state_value == "ALARM": + actions = composite_alarm.alarm["AlarmActions"] + else: + actions = composite_alarm.alarm["InsufficientDataActions"] + for action in actions: + data = arns.parse_arn(action) + if data["service"] == "sns": + service = connect_to( + region_name=data["region"], aws_access_key_id=data["account"] + ).sns + subject = f"""{new_state_value}: "{composite_alarm.alarm["AlarmName"]}" in {context.region}""" + message = create_message_response_update_composite_alarm_state_sns( + composite_alarm, triggering_alarm, old_state_value + ) + service.publish(TopicArn=action, Subject=subject, Message=message) else: - actions = composite_alarm.alarm["InsufficientDataActions"] - for action in actions: - data = arns.parse_arn(action) - if data["service"] == "sns": - service = connect_to( - region_name=data["region"], aws_access_key_id=data["account"] - ).sns - subject = f"""{new_state_value}: "{composite_alarm.alarm["AlarmName"]}" in {context.region}""" - message = create_message_response_update_composite_alarm_state_sns( - composite_alarm, triggering_alarm, old_state_value - ) - service.publish(TopicArn=action, Subject=subject, Message=message) - else: - # TODO: support other actions - LOG.warning( - "Action for service %s not implemented, action '%s' will not be triggered.", - data["service"], - action, - ) + # TODO: support other actions + LOG.warning( + "Action for service %s not implemented, action '%s' will not be triggered.", + data["service"], + action, + ) def create_metric_data_query_from_alarm(alarm: LocalStackMetricAlarm): From b15cabaf5c629b06adf53b7e470251118ce79837 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Tue, 12 Nov 2024 12:03:16 +0100 Subject: [PATCH 31/51] Extract method for for running composite alarm actions Improves readability and reduces return statements from the original method. --- .../localstack/services/cloudwatch/provider_v2.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 370c688de63b1..85c7093e29ae8 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -845,9 +845,9 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): alarms = list(store.alarms.values()) composite_alarms = [a for a in alarms if isinstance(a, LocalStackCompositeAlarm)] for composite_alarm in composite_alarms: - self._evaluate_composite_alarm(composite_alarm, context, triggering_alarm) + self._evaluate_composite_alarm(context, composite_alarm, triggering_alarm) - def _evaluate_composite_alarm(self, composite_alarm, context, triggering_alarm): + def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): store = self.get_store(context.account_id, context.region) new_state_value = StateValue.OK alarm_rule = composite_alarm.alarm["AlarmRule"] @@ -891,11 +891,14 @@ def _evaluate_composite_alarm(self, composite_alarm, context, triggering_alarm): self._update_state( context, composite_alarm, new_state_value, state_reason, state_reason_data ) - if not composite_alarm.alarm["ActionsEnabled"]: - return - if new_state_value == "OK": + if composite_alarm.alarm["ActionsEnabled"]: + self._run_composite_alarm_actions(context, composite_alarm, old_state_value, triggering_alarm) + + def _run_composite_alarm_actions(self, context, composite_alarm, old_state_value, triggering_alarm): + new_state_value = composite_alarm.alarm["StateValue"] + if new_state_value == StateValue.OK: actions = composite_alarm.alarm["OKActions"] - elif new_state_value == "ALARM": + elif new_state_value == StateValue.ALARM: actions = composite_alarm.alarm["AlarmActions"] else: actions = composite_alarm.alarm["InsufficientDataActions"] From 91d09da34b8025a9e612fce9fb2453af70093db7 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Tue, 12 Nov 2024 12:19:12 +0100 Subject: [PATCH 32/51] Apply format and lint --- .../localstack/services/cloudwatch/provider_v2.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 85c7093e29ae8..c0adb9d8b9252 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -892,9 +892,13 @@ def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): context, composite_alarm, new_state_value, state_reason, state_reason_data ) if composite_alarm.alarm["ActionsEnabled"]: - self._run_composite_alarm_actions(context, composite_alarm, old_state_value, triggering_alarm) + self._run_composite_alarm_actions( + context, composite_alarm, old_state_value, triggering_alarm + ) - def _run_composite_alarm_actions(self, context, composite_alarm, old_state_value, triggering_alarm): + def _run_composite_alarm_actions( + self, context, composite_alarm, old_state_value, triggering_alarm + ): new_state_value = composite_alarm.alarm["StateValue"] if new_state_value == StateValue.OK: actions = composite_alarm.alarm["OKActions"] From c12a1884eb2d03bfdea7a43de15ebcb220a0df04 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Tue, 12 Nov 2024 12:30:02 +0100 Subject: [PATCH 33/51] Validate if metric alarms from alarm rule exist --- .../services/cloudwatch/provider_v2.py | 19 ++++++++++++++----- .../services/cloudwatch/test_cloudwatch.py | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index c0adb9d8b9252..c5a330d500698 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -463,6 +463,15 @@ def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlar composite_alarm = LocalStackCompositeAlarm( context.account_id, context.region, {**request} ) + missing_alarms = [] + for metric_alarm_arn in self._get_alarm_arns(composite_alarm.alarm["AlarmRule"]): + if metric_alarm_arn not in store.alarms: + missing_alarms.append(metric_alarm_arn) + if missing_alarms: + raise ValidationError( + f"Could not save the composite alarm as alarms [{', '.join(missing_alarms)}] " + f"in the alarm rule do not exist" + ) alarm_arn = composite_alarm.alarm["AlarmArn"] store.alarms[alarm_arn] = composite_alarm @@ -850,12 +859,8 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): store = self.get_store(context.account_id, context.region) new_state_value = StateValue.OK - alarm_rule = composite_alarm.alarm["AlarmRule"] # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied - metric_alarm_arns = re.findall( - r'\("([^"]*)"\)', alarm_rule - ) # regexp for everything within (" ") - for metric_alarm_arn in metric_alarm_arns: + for metric_alarm_arn in self._get_alarm_arns(composite_alarm.alarm["AlarmRule"]): metric_alarm = store.alarms.get(metric_alarm_arn) if metric_alarm.alarm["StateValue"] == StateValue.ALARM: triggering_alarm = metric_alarm @@ -896,6 +901,10 @@ def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): context, composite_alarm, old_state_value, triggering_alarm ) + def _get_alarm_arns(self, composite_alarm_rule): + # regexp for everything within (" ") + return re.findall(r'\("([^"]*)"\)', composite_alarm_rule) + def _run_composite_alarm_actions( self, context, composite_alarm, old_state_value, triggering_alarm ): diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index a27771966ebe6..9b731b503d1eb 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -598,6 +598,21 @@ def test_describe_alarms_converts_date_format_correctly(self, aws_client, cleanu assert isinstance(alarm["AlarmConfigurationUpdatedTimestamp"], datetime) assert isinstance(alarm["StateUpdatedTimestamp"], datetime) + @markers.aws.validated + @pytest.mark.skipif(is_old_provider(), reason="not supported by the old provider") + def test_put_composite_alarm_validation(self, aws_client): + with pytest.raises(Exception) as ex: + aws_client.cloudwatch.put_composite_alarm( + AlarmName="alarm_name", + AlarmRule='ALARM("metric-alarm-arn")', + ) + err = ex.value.response["Error"] + assert err["Code"] == "ValidationError" + assert ( + err["Message"] + == "Could not save the composite alarm as alarms [metric-alarm-arn] in the alarm rule do not exist" + ) + @markers.aws.validated def test_put_composite_alarm_describe_alarms(self, aws_client, cleanups): composite_alarm_name = f"composite-a-{short_uid()}" From f0afeb00e37fcf1ac4c2db64bd2962a6efdb7f3c Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Tue, 12 Nov 2024 17:21:20 +0100 Subject: [PATCH 34/51] Validate if each rule operand starts with ALARM --- .../localstack/services/cloudwatch/provider_v2.py | 12 +++++++++++- tests/aws/services/cloudwatch/test_cloudwatch.py | 13 +++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index c5a330d500698..81f5fe3e38d9e 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -463,8 +463,18 @@ def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlar composite_alarm = LocalStackCompositeAlarm( context.account_id, context.region, {**request} ) + + alarm_rule = composite_alarm.alarm["AlarmRule"] + + alarms_from_rule = [alarm.strip() for alarm in alarm_rule.split("OR")] + for alarm in alarms_from_rule: + if not alarm.startswith("ALARM"): + raise ValidationError( + f"Error in alarm rule condition '{alarm}': Only ALARM expression is supported" + ) + missing_alarms = [] - for metric_alarm_arn in self._get_alarm_arns(composite_alarm.alarm["AlarmRule"]): + for metric_alarm_arn in self._get_alarm_arns(alarm_rule): if metric_alarm_arn not in store.alarms: missing_alarms.append(metric_alarm_arn) if missing_alarms: diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 9b731b503d1eb..4c6500140f502 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -601,6 +601,19 @@ def test_describe_alarms_converts_date_format_correctly(self, aws_client, cleanu @markers.aws.validated @pytest.mark.skipif(is_old_provider(), reason="not supported by the old provider") def test_put_composite_alarm_validation(self, aws_client): + # each logical expression operand should start with ALARM() + with pytest.raises(Exception) as ex: + aws_client.cloudwatch.put_composite_alarm( + AlarmName="alarm_name", + AlarmRule='ALARM("metric-alarm-arn-1") OR OK("metric-alarm-arn-2")', + ) + err = ex.value.response["Error"] + assert err["Code"] == "ValidationError" + assert ( + err["Message"] + == "Error in alarm rule condition 'OK(\"metric-alarm-arn-2\")': Only ALARM expression is supported" + ) + with pytest.raises(Exception) as ex: aws_client.cloudwatch.put_composite_alarm( AlarmName="alarm_name", From b4a4ccfed9f43b54ec6c5415a014c15e91471fa0 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Tue, 12 Nov 2024 17:45:20 +0100 Subject: [PATCH 35/51] Add clarifying comment --- localstack-core/localstack/services/cloudwatch/provider_v2.py | 1 + 1 file changed, 1 insertion(+) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 81f5fe3e38d9e..9f7020ff734c8 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -781,6 +781,7 @@ def _update_state( old_state_reason = alarm.alarm["StateReason"] store = self.get_store(context.account_id, context.region) current_time = datetime.datetime.now() + # version is not present in state reason data for composite alarm, hence the check if state_reason_data and isinstance(alarm, LocalStackMetricAlarm): state_reason_data["version"] = HISTORY_VERSION history_data = { From 51f07f44b77c4918af1287e83f8cb892215e2c20 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 09:23:08 +0100 Subject: [PATCH 36/51] Update limitations in provider class description --- .../localstack/services/cloudwatch/provider_v2.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 9f7020ff734c8..fd24e6d346068 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -147,7 +147,10 @@ class CloudwatchProvider(CloudwatchApi, ServiceLifecycleHook): Cloudwatch provider. LIMITATIONS: - - no alarm rule evaluation + - simplified composite alarm rule evaluation: + - only OR operator is supported + - only ALARM expression is supported + - only metric alarms can be included in the rule and they should be referenced by ARN only """ def __init__(self): From 23b2185255d6fad3a496e48f03949fa0a6e7ff31 Mon Sep 17 00:00:00 2001 From: Misha Tiurin <650819+tiurin@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:27:20 +0100 Subject: [PATCH 37/51] Add typing to create_message_response_update_state_sns signature Co-authored-by: Simon Walker --- localstack-core/localstack/services/cloudwatch/provider_v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index fd24e6d346068..8233a93614c3c 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -1002,7 +1002,7 @@ def create_message_response_update_state_lambda( return json.dumps(response, cls=JSONEncoder) -def create_message_response_update_state_sns(alarm: LocalStackMetricAlarm, old_state): +def create_message_response_update_state_sns(alarm: LocalStackMetricAlarm, old_state: StateValue): _alarm = alarm.alarm response = { "AWSAccountId": alarm.account_id, From deea700338bc3ee767e22fdca37c38800d9d7b08 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 10:33:10 +0100 Subject: [PATCH 38/51] Add typing to create_message_response_update_composite_alarm_state_sns signature --- localstack-core/localstack/services/cloudwatch/provider_v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 8233a93614c3c..46497fdce114c 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -1061,7 +1061,7 @@ def create_message_response_update_state_sns(alarm: LocalStackMetricAlarm, old_s def create_message_response_update_composite_alarm_state_sns( composite_alarm: LocalStackCompositeAlarm, triggering_alarm: LocalStackMetricAlarm, - old_state, + old_state: StateValue, ): _alarm = composite_alarm.alarm response = { From 2898beb76da0c0adc141696bcb119e2c27a8a896 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 11:45:40 +0100 Subject: [PATCH 39/51] Fix test_put_composite_alarm_describe_alarms It relied on the alarm name reference in composite alarm rule which is not supported yet. Changing it to ARN reference. Alarm names support should be added as the next step though, it is not too difficult and is very natural to use them in alarm rule description. --- tests/aws/services/cloudwatch/test_cloudwatch.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 4c6500140f502..e691619ead10e 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -632,7 +632,6 @@ def test_put_composite_alarm_describe_alarms(self, aws_client, cleanups): alarm_name = f"a-{short_uid()}" metric_name = "something" namespace = f"test-ns-{short_uid()}" - alarm_rule = f'ALARM("{alarm_name}")' aws_client.cloudwatch.put_metric_alarm( AlarmName=alarm_name, Namespace=namespace, @@ -644,6 +643,12 @@ def test_put_composite_alarm_describe_alarms(self, aws_client, cleanups): Threshold=30, ) cleanups.append(lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[alarm_name])) + + metric_alarm_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_name])[ + "MetricAlarms" + ][0]["AlarmArn"] + alarm_rule = f'ALARM("{metric_alarm_arn}")' + aws_client.cloudwatch.put_composite_alarm( AlarmName=composite_alarm_name, AlarmRule=alarm_rule, From f717ef1c3a3b8fc08da5900149033ea7a65b9722 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 14:26:44 +0100 Subject: [PATCH 40/51] Verify put_composite_alarm response with snapshot Co-authored-by: Simon Walker --- tests/aws/services/cloudwatch/test_cloudwatch.py | 3 ++- .../aws/services/cloudwatch/test_cloudwatch.snapshot.json | 8 +++++++- .../services/cloudwatch/test_cloudwatch.validation.json | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index e691619ead10e..58257aebd5286 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1102,7 +1102,7 @@ def _put_metric_alarm(alarm_name: str): composite_alarm_rule = f'ALARM("{alarm_1_arn}") OR ALARM("{alarm_2_arn}")' - aws_client.cloudwatch.put_composite_alarm( + put_composite_alarm_response = aws_client.cloudwatch.put_composite_alarm( AlarmName=composite_alarm_name, AlarmDescription=composite_alarm_description, AlarmRule=composite_alarm_rule, @@ -1112,6 +1112,7 @@ def _put_metric_alarm(alarm_name: str): cleanups.append( lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[composite_alarm_name]) ) + snapshot.match("put-composite-alarm", put_composite_alarm_response) composite_alarms_list = aws_client.cloudwatch.describe_alarms( AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index baee2fb2ba0bb..3d72f28430a93 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,8 +2105,14 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "11-11-2024, 10:37:42", + "recorded-date": "13-11-2024, 13:20:25", "recorded-content": { + "put-composite-alarm": { + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 200 + } + }, "composite-alarm-in-alarm-when-alarm-1-is-in-alarm": { "ActionsEnabled": true, "AlarmActions": [ diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 5b40155a6a5fe..02f3189f25fca 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-11T10:37:42+00:00" + "last_validated_date": "2024-11-13T13:20:25+00:00" } } From 4cf2275555af9d541f9e53c526881775d48adbc5 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 14:38:51 +0100 Subject: [PATCH 41/51] Add test fixture TODOs --- tests/aws/services/cloudwatch/test_cloudwatch.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 58257aebd5286..8b16238bbae5a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1035,6 +1035,7 @@ def test_trigger_composite_alarm( sns_topic_ok = sns_create_topic(Name=topic_name_ok) topic_arn_ok = sns_topic_ok["TopicArn"] + # TODO extract SNS-to-SQS into a fixture # create queues for 'ALARM' and 'OK' of the composite alarm (will receive sns messages) queue_url_alarm = sqs_create_queue(QueueName=f"AlarmQueue-{short_uid()}") queue_url_ok = sqs_create_queue(QueueName=f"OKQueue-{short_uid()}") @@ -1070,6 +1071,7 @@ def test_trigger_composite_alarm( ) # put metric alarms that would be parts of a composite one + #TODO extract put metric alarm and associated cleanups into a fixture def _put_metric_alarm(alarm_name: str): aws_client.cloudwatch.put_metric_alarm( AlarmName=alarm_name, From 525fb732f3fef92db546dc22d82cdb4788a1a459 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 15:20:05 +0100 Subject: [PATCH 42/51] Apply format --- tests/aws/services/cloudwatch/test_cloudwatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 8b16238bbae5a..ca22476aad94a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1071,7 +1071,7 @@ def test_trigger_composite_alarm( ) # put metric alarms that would be parts of a composite one - #TODO extract put metric alarm and associated cleanups into a fixture + # TODO extract put metric alarm and associated cleanups into a fixture def _put_metric_alarm(alarm_name: str): aws_client.cloudwatch.put_metric_alarm( AlarmName=alarm_name, From e47fba9aa5ca671024207034fe47f57e18faac3b Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Wed, 13 Nov 2024 16:19:22 +0100 Subject: [PATCH 43/51] Add TODO for store lock management improvements --- localstack-core/localstack/services/cloudwatch/provider_v2.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 46497fdce114c..09289d89e8a69 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -864,6 +864,8 @@ def _get_timestamp(input: dict): return DescribeAlarmHistoryOutput(AlarmHistoryItems=history) def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): + # TODO either pass store as a parameter or acquire RLock (with _STORE_LOCK:) + # everything works ok now but better ensure protection of critical section in front of future changes store = self.get_store(context.account_id, context.region) alarms = list(store.alarms.values()) composite_alarms = [a for a in alarms if isinstance(a, LocalStackCompositeAlarm)] From 6fd4f9de9fa20de1e100313baf350cf41d338f71 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 14 Nov 2024 08:50:40 +0100 Subject: [PATCH 44/51] Log a warning when alarm rule has unsupported expressions https://github.com/localstack/localstack/pull/11828#discussion_r1838441628 --- .../localstack/services/cloudwatch/provider_v2.py | 11 ++++++----- tests/aws/services/cloudwatch/test_cloudwatch.py | 13 ------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 09289d89e8a69..4a37ac4a77894 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -469,11 +469,12 @@ def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlar alarm_rule = composite_alarm.alarm["AlarmRule"] - alarms_from_rule = [alarm.strip() for alarm in alarm_rule.split("OR")] - for alarm in alarms_from_rule: - if not alarm.startswith("ALARM"): - raise ValidationError( - f"Error in alarm rule condition '{alarm}': Only ALARM expression is supported" + alarms_conditions = [alarm.strip() for alarm in alarm_rule.split("OR")] + for alarm_condition in alarms_conditions: + if not alarm_condition.startswith("ALARM"): + LOG.warning( + "Unsupported expression in alarm rule condition %s: Only ALARM expression is supported by Localstack as of now", + alarm_condition, ) missing_alarms = [] diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index ca22476aad94a..6f58a56153ef4 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -601,19 +601,6 @@ def test_describe_alarms_converts_date_format_correctly(self, aws_client, cleanu @markers.aws.validated @pytest.mark.skipif(is_old_provider(), reason="not supported by the old provider") def test_put_composite_alarm_validation(self, aws_client): - # each logical expression operand should start with ALARM() - with pytest.raises(Exception) as ex: - aws_client.cloudwatch.put_composite_alarm( - AlarmName="alarm_name", - AlarmRule='ALARM("metric-alarm-arn-1") OR OK("metric-alarm-arn-2")', - ) - err = ex.value.response["Error"] - assert err["Code"] == "ValidationError" - assert ( - err["Message"] - == "Error in alarm rule condition 'OK(\"metric-alarm-arn-2\")': Only ALARM expression is supported" - ) - with pytest.raises(Exception) as ex: aws_client.cloudwatch.put_composite_alarm( AlarmName="alarm_name", From 603824158a0a7f986220c1104b2d778e93bb59a7 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 14 Nov 2024 10:21:30 +0100 Subject: [PATCH 45/51] Ignore alarm rule when it has unsupported expressions https://github.com/localstack/localstack/pull/11828#discussion_r1838441628 --- .../services/cloudwatch/provider_v2.py | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 4a37ac4a77894..4f18342f80969 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -469,13 +469,8 @@ def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlar alarm_rule = composite_alarm.alarm["AlarmRule"] - alarms_conditions = [alarm.strip() for alarm in alarm_rule.split("OR")] - for alarm_condition in alarms_conditions: - if not alarm_condition.startswith("ALARM"): - LOG.warning( - "Unsupported expression in alarm rule condition %s: Only ALARM expression is supported by Localstack as of now", - alarm_condition, - ) + rule_expression_validation_result = self._validate_alarm_rule_expression(alarm_rule) + [LOG.warning(w) for w in rule_expression_validation_result] missing_alarms = [] for metric_alarm_arn in self._get_alarm_arns(alarm_rule): @@ -875,9 +870,16 @@ def _evaluate_composite_alarms(self, context: RequestContext, triggering_alarm): def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): store = self.get_store(context.account_id, context.region) + alarm_rule = composite_alarm.alarm["AlarmRule"] + rule_expression_validation = self._validate_alarm_rule_expression(alarm_rule) + if rule_expression_validation: + LOG.warning( + "Alarm rule contains unsupported expressions and will not be evaluated: %s", + rule_expression_validation, + ) new_state_value = StateValue.OK # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied - for metric_alarm_arn in self._get_alarm_arns(composite_alarm.alarm["AlarmRule"]): + for metric_alarm_arn in self._get_alarm_arns(alarm_rule): metric_alarm = store.alarms.get(metric_alarm_arn) if metric_alarm.alarm["StateValue"] == StateValue.ALARM: triggering_alarm = metric_alarm @@ -918,6 +920,16 @@ def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): context, composite_alarm, old_state_value, triggering_alarm ) + def _validate_alarm_rule_expression(self, alarm_rule): + validation_result = [] + alarms_conditions = [alarm.strip() for alarm in alarm_rule.split("OR")] + for alarm_condition in alarms_conditions: + if not alarm_condition.startswith("ALARM"): + validation_result.append( + f"Unsupported expression in alarm rule condition {alarm_condition}: Only ALARM expression is supported by Localstack as of now" + ) + return validation_result + def _get_alarm_arns(self, composite_alarm_rule): # regexp for everything within (" ") return re.findall(r'\("([^"]*)"\)', composite_alarm_rule) From 8cc084814b765d2b04403fe988f962a0ea111b63 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 14 Nov 2024 14:56:16 +0100 Subject: [PATCH 46/51] Add helper inner methods to check alarm action result Avoid repetitive code. --- .../services/cloudwatch/test_cloudwatch.py | 72 +++++++++++-------- .../test_cloudwatch.validation.json | 2 +- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 6f58a56153ef4..1600a212a452b 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1124,21 +1124,51 @@ def _put_metric_alarm(alarm_name: str): snapshot.add_transformer(snapshot.transform.regex(topic_name_alarm, "")) snapshot.add_transformer(snapshot.transform.regex(topic_name_ok, "")) + # helper methods to verify that correct message landed in correct SQS queue + # for ALARM and OK state changes respectively + + def _check_composite_alarm_alarm_message( + expected_triggering_child_arn, + expected_triggering_child_state, + ): + retry( + check_composite_alarm_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + queue_url=queue_url_alarm, + expected_topic_arn=topic_arn_alarm, + alarm_name=composite_alarm_name, + alarm_description=composite_alarm_description, + expected_state="ALARM", + expected_triggering_child_arn=expected_triggering_child_arn, + expected_triggering_child_state=expected_triggering_child_state, + ) + + def _check_composite_alarm_ok_message( + expected_triggering_child_arn, + expected_triggering_child_state, + ): + retry( + check_composite_alarm_message, + retries=PUBLICATION_RETRIES, + sleep_before=1, + sqs_client=aws_client.sqs, + queue_url=queue_url_ok, + expected_topic_arn=topic_arn_ok, + alarm_name=composite_alarm_name, + alarm_description=composite_alarm_description, + expected_state="OK", + expected_triggering_child_arn=expected_triggering_child_arn, + expected_triggering_child_state=expected_triggering_child_state, + ) + # trigger alarm 1 - composite one should also go into ALARM state aws_client.cloudwatch.set_alarm_state( AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" ) - retry( - check_composite_alarm_message, - retries=PUBLICATION_RETRIES, - sleep_before=1, - sqs_client=aws_client.sqs, - queue_url=queue_url_alarm, - expected_topic_arn=topic_arn_alarm, - alarm_name=composite_alarm_name, - alarm_description=composite_alarm_description, - expected_state="ALARM", + _check_composite_alarm_alarm_message( expected_triggering_child_arn=alarm_1_arn, expected_triggering_child_state="ALARM", ) @@ -1157,16 +1187,7 @@ def _put_metric_alarm(alarm_name: str): AlarmName=alarm_1_name, StateValue="OK", StateReason="resetting alarm 1" ) - retry( - check_composite_alarm_message, - retries=PUBLICATION_RETRIES, - sleep_before=1, - sqs_client=aws_client.sqs, - queue_url=queue_url_ok, - expected_topic_arn=topic_arn_ok, - alarm_name=composite_alarm_name, - alarm_description=composite_alarm_description, - expected_state="OK", + _check_composite_alarm_ok_message( expected_triggering_child_arn=alarm_1_arn, expected_triggering_child_state="OK", ) @@ -1185,16 +1206,7 @@ def _put_metric_alarm(alarm_name: str): AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" ) - retry( - check_composite_alarm_message, - retries=PUBLICATION_RETRIES, - sleep_before=1, - sqs_client=aws_client.sqs, - queue_url=queue_url_alarm, - expected_topic_arn=topic_arn_alarm, - alarm_name=composite_alarm_name, - alarm_description=composite_alarm_description, - expected_state="ALARM", + _check_composite_alarm_alarm_message( expected_triggering_child_arn=alarm_2_arn, expected_triggering_child_state="ALARM", ) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 02f3189f25fca..74297d66d6830 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-13T13:20:25+00:00" + "last_validated_date": "2024-11-14T13:45:48+00:00" } } From 346cf75dfc1a7d7ef9045900b1992c4e51decbf3 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 14 Nov 2024 15:02:57 +0100 Subject: [PATCH 47/51] Add test when 2nd alarm from the rule is back to OK --- .../services/cloudwatch/test_cloudwatch.py | 22 +++++++++++++ .../cloudwatch/test_cloudwatch.snapshot.json | 32 ++++++++++++++++++- .../test_cloudwatch.validation.json | 2 +- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 1600a212a452b..aafbca5c027bd 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1220,7 +1220,29 @@ def _check_composite_alarm_ok_message( composite_alarm_in_alarm_when_alarm_2_in_alarm, ) + # trigger OK for alarm 2 - composite one should also go back to OK + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_2_name, StateValue="OK", StateReason="resetting alarm 2" + ) + + _check_composite_alarm_ok_message( + expected_triggering_child_arn=alarm_2_arn, + expected_triggering_child_state="OK", + ) + + composite_alarms_list = aws_client.cloudwatch.describe_alarms( + AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] + ) + composite_alarm_in_ok_when_alarm_2_back_to_ok = composite_alarms_list["CompositeAlarms"][0] + snapshot.match( + "composite-alarm-in-ok-when-alarm-2-is-back-to-ok", + composite_alarm_in_ok_when_alarm_2_back_to_ok, + ) + # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change + aws_client.cloudwatch.set_alarm_state( + AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" + ) aws_client.cloudwatch.set_alarm_state( AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" ) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index 3d72f28430a93..76ec0f994ea9a 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,7 +2105,7 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "13-11-2024, 13:20:25", + "recorded-date": "14-11-2024, 13:59:36", "recorded-content": { "put-composite-alarm": { "ResponseMetadata": { @@ -2202,6 +2202,36 @@ "StateTransitionedTimestamp": "timestamp", "StateUpdatedTimestamp": "timestamp", "StateValue": "ALARM" + }, + "composite-alarm-in-ok-when-alarm-2-is-back-to-ok": { + "ActionsEnabled": true, + "AlarmActions": [ + "arn::sns::111111111111:" + ], + "AlarmArn": "arn::cloudwatch::111111111111:alarm:", + "AlarmConfigurationUpdatedTimestamp": "timestamp", + "AlarmDescription": "composite alarm description", + "AlarmName": "", + "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:\")", + "InsufficientDataActions": [], + "OKActions": [ + "arn::sns::111111111111:" + ], + "StateReason": "", + "StateReasonData": { + "triggeringAlarms": [ + { + "arn": "arn::cloudwatch::111111111111:alarm:", + "state": { + "value": "OK", + "timestamp": "date" + } + } + ] + }, + "StateTransitionedTimestamp": "timestamp", + "StateUpdatedTimestamp": "timestamp", + "StateValue": "OK" } } } diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 74297d66d6830..7527bb9f90066 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-14T13:45:48+00:00" + "last_validated_date": "2024-11-14T13:59:36+00:00" } } From 384979c652a81cf4aded420b7eacddf180424738 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 14 Nov 2024 15:29:33 +0100 Subject: [PATCH 48/51] Add snapshot for both alarms from rule are triggered test case --- .../services/cloudwatch/test_cloudwatch.py | 20 ++++++------ .../cloudwatch/test_cloudwatch.snapshot.json | 32 ++++++++++++++++++- .../test_cloudwatch.validation.json | 2 +- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index aafbca5c027bd..1c626e8f76189 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1239,26 +1239,24 @@ def _check_composite_alarm_ok_message( composite_alarm_in_ok_when_alarm_2_back_to_ok, ) - # trigger alarm 1 while alarm 2 is triggered - composite one shouldn't change + # trigger alarm 2 while alarm 1 is triggered - composite one shouldn't change aws_client.cloudwatch.set_alarm_state( - AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" + AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" ) aws_client.cloudwatch.set_alarm_state( - AlarmName=alarm_1_name, StateValue="ALARM", StateReason="trigger alarm 1" + AlarmName=alarm_2_name, StateValue="ALARM", StateReason="trigger alarm 2" ) composite_alarms_list = aws_client.cloudwatch.describe_alarms( AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) - composite_alarm_not_changed_by_second_trigger = composite_alarms_list["CompositeAlarms"][0] - # It is not possible to reuse the snapshot within the same test, - # therefore checking that alarm reason hasn't changed instead of recording new snapshot - state_reason_data = json.loads( - composite_alarm_not_changed_by_second_trigger["StateReasonData"] + composite_alarm_is_triggered_by_alarm_1_and_then_not_changed_by_alarm_2 = ( + composite_alarms_list["CompositeAlarms"][0] + ) + snapshot.match( + "composite-alarm-is-triggered-by-alarm-1-and-then-unchanged-by-alarm-2", + composite_alarm_is_triggered_by_alarm_1_and_then_not_changed_by_alarm_2, ) - triggering_alarms = state_reason_data["triggeringAlarms"] - assert len(triggering_alarms) == 1 - assert triggering_alarms[0]["arn"] == alarm_2_arn @markers.aws.validated @markers.snapshot.skip_snapshot_verify( diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json index 76ec0f994ea9a..6563131e1e53f 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json @@ -2105,7 +2105,7 @@ } }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "recorded-date": "14-11-2024, 13:59:36", + "recorded-date": "14-11-2024, 14:25:30", "recorded-content": { "put-composite-alarm": { "ResponseMetadata": { @@ -2232,6 +2232,36 @@ "StateTransitionedTimestamp": "timestamp", "StateUpdatedTimestamp": "timestamp", "StateValue": "OK" + }, + "composite-alarm-is-triggered-by-alarm-1-and-then-unchanged-by-alarm-2": { + "ActionsEnabled": true, + "AlarmActions": [ + "arn::sns::111111111111:" + ], + "AlarmArn": "arn::cloudwatch::111111111111:alarm:", + "AlarmConfigurationUpdatedTimestamp": "timestamp", + "AlarmDescription": "composite alarm description", + "AlarmName": "", + "AlarmRule": "ALARM(\"arn::cloudwatch::111111111111:alarm:\") OR ALARM(\"arn::cloudwatch::111111111111:alarm:\")", + "InsufficientDataActions": [], + "OKActions": [ + "arn::sns::111111111111:" + ], + "StateReason": "", + "StateReasonData": { + "triggeringAlarms": [ + { + "arn": "arn::cloudwatch::111111111111:alarm:", + "state": { + "value": "ALARM", + "timestamp": "date" + } + } + ] + }, + "StateTransitionedTimestamp": "timestamp", + "StateUpdatedTimestamp": "timestamp", + "StateValue": "ALARM" } } } diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json index 7527bb9f90066..35c96909b8d23 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.validation.json +++ b/tests/aws/services/cloudwatch/test_cloudwatch.validation.json @@ -69,6 +69,6 @@ "last_validated_date": "2024-09-02T14:03:31+00:00" }, "tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_trigger_composite_alarm": { - "last_validated_date": "2024-11-14T13:59:36+00:00" + "last_validated_date": "2024-11-14T14:25:30+00:00" } } From 5080f12963eabffa70aed2f8b16f5fc55afe06fa Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Thu, 14 Nov 2024 15:54:43 +0100 Subject: [PATCH 49/51] Add TODO for describe-composite-alarm snapshot --- tests/aws/services/cloudwatch/test_cloudwatch.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 1c626e8f76189..02086a2a5a9f0 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -1107,6 +1107,10 @@ def _put_metric_alarm(alarm_name: str): AlarmNames=[composite_alarm_name], AlarmTypes=["CompositeAlarm"] ) composite_alarm = composite_alarms_list["CompositeAlarms"][0] + # TODO snapshot.match("describe-composite-alarm", composite_alarm) instead of asserts + # right now the lack of parity for initial composite alarm evaluation prevents from checking snapshot. + # Namely, for initial evaluation after alarm creation all child alarms + # should be included as triggering alarms assert composite_alarm["AlarmName"] == composite_alarm_name assert composite_alarm["AlarmRule"] == composite_alarm_rule From cf51bbe52e0a6b464defb88170c4da82c0f96ec2 Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Fri, 15 Nov 2024 16:38:31 +0100 Subject: [PATCH 50/51] Skip rule evaluation on missing alarms As discussed in the (PR)[https://github.com/localstack/localstack/pull/11828#discussion_r1838441628], partial behaviour should not hurt parity. In this case, because evaluation by name is not yet implemented, it is better to let user save an alarm that is not fully supported by Localstack but skip this alarm evaluation. --- .../services/cloudwatch/provider_v2.py | 17 +++++++---------- .../aws/services/cloudwatch/test_cloudwatch.py | 15 --------------- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/localstack-core/localstack/services/cloudwatch/provider_v2.py b/localstack-core/localstack/services/cloudwatch/provider_v2.py index 4f18342f80969..2d6bce7b47a1e 100644 --- a/localstack-core/localstack/services/cloudwatch/provider_v2.py +++ b/localstack-core/localstack/services/cloudwatch/provider_v2.py @@ -468,19 +468,9 @@ def put_composite_alarm(self, context: RequestContext, request: PutCompositeAlar ) alarm_rule = composite_alarm.alarm["AlarmRule"] - rule_expression_validation_result = self._validate_alarm_rule_expression(alarm_rule) [LOG.warning(w) for w in rule_expression_validation_result] - missing_alarms = [] - for metric_alarm_arn in self._get_alarm_arns(alarm_rule): - if metric_alarm_arn not in store.alarms: - missing_alarms.append(metric_alarm_arn) - if missing_alarms: - raise ValidationError( - f"Could not save the composite alarm as alarms [{', '.join(missing_alarms)}] " - f"in the alarm rule do not exist" - ) alarm_arn = composite_alarm.alarm["AlarmArn"] store.alarms[alarm_arn] = composite_alarm @@ -877,10 +867,17 @@ def _evaluate_composite_alarm(self, context, composite_alarm, triggering_alarm): "Alarm rule contains unsupported expressions and will not be evaluated: %s", rule_expression_validation, ) + return new_state_value = StateValue.OK # assuming that a rule consists only of ALARM evaluations of metric alarms, with OR logic applied for metric_alarm_arn in self._get_alarm_arns(alarm_rule): metric_alarm = store.alarms.get(metric_alarm_arn) + if not metric_alarm: + LOG.warning( + "Alarm rule won't be evaluated as there is no alarm with ARN %s", + metric_alarm_arn, + ) + return if metric_alarm.alarm["StateValue"] == StateValue.ALARM: triggering_alarm = metric_alarm new_state_value = StateValue.ALARM diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index 02086a2a5a9f0..e70cdbb40b134 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -598,21 +598,6 @@ def test_describe_alarms_converts_date_format_correctly(self, aws_client, cleanu assert isinstance(alarm["AlarmConfigurationUpdatedTimestamp"], datetime) assert isinstance(alarm["StateUpdatedTimestamp"], datetime) - @markers.aws.validated - @pytest.mark.skipif(is_old_provider(), reason="not supported by the old provider") - def test_put_composite_alarm_validation(self, aws_client): - with pytest.raises(Exception) as ex: - aws_client.cloudwatch.put_composite_alarm( - AlarmName="alarm_name", - AlarmRule='ALARM("metric-alarm-arn")', - ) - err = ex.value.response["Error"] - assert err["Code"] == "ValidationError" - assert ( - err["Message"] - == "Could not save the composite alarm as alarms [metric-alarm-arn] in the alarm rule do not exist" - ) - @markers.aws.validated def test_put_composite_alarm_describe_alarms(self, aws_client, cleanups): composite_alarm_name = f"composite-a-{short_uid()}" From afe714a1887e50a3dfe442849e45205cda4ebf0d Mon Sep 17 00:00:00 2001 From: Misha Tiurin Date: Sun, 17 Nov 2024 11:54:13 +0100 Subject: [PATCH 51/51] Revert "Fix test_put_composite_alarm_describe_alarms" This reverts commit f77b41800b9d34d2755cd18f57a2d7a803d5ae4b. --- tests/aws/services/cloudwatch/test_cloudwatch.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/aws/services/cloudwatch/test_cloudwatch.py b/tests/aws/services/cloudwatch/test_cloudwatch.py index e70cdbb40b134..c9554034dd3b1 100644 --- a/tests/aws/services/cloudwatch/test_cloudwatch.py +++ b/tests/aws/services/cloudwatch/test_cloudwatch.py @@ -604,6 +604,7 @@ def test_put_composite_alarm_describe_alarms(self, aws_client, cleanups): alarm_name = f"a-{short_uid()}" metric_name = "something" namespace = f"test-ns-{short_uid()}" + alarm_rule = f'ALARM("{alarm_name}")' aws_client.cloudwatch.put_metric_alarm( AlarmName=alarm_name, Namespace=namespace, @@ -615,12 +616,6 @@ def test_put_composite_alarm_describe_alarms(self, aws_client, cleanups): Threshold=30, ) cleanups.append(lambda: aws_client.cloudwatch.delete_alarms(AlarmNames=[alarm_name])) - - metric_alarm_arn = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_name])[ - "MetricAlarms" - ][0]["AlarmArn"] - alarm_rule = f'ALARM("{metric_alarm_arn}")' - aws_client.cloudwatch.put_composite_alarm( AlarmName=composite_alarm_name, AlarmRule=alarm_rule,