Add AWS Health event support for QP mode #510
Conversation
AustinSiu
force-pushed
the
qp-health-events
branch
from
b312b87
to
f80fb41
Compare
AustinSiu
changed the title
| } | ||
|
|
||
| // processInterruptionEvents takes interruption event wrappers and sends interruption events to the passed-in channel | ||
| func (m SQSMonitor) processInterruptionEvents(interruptionEventWrappers []InterruptionEventWrapper, message *sqs.Message) error { |
There was a problem hiding this comment.
nice improvement in the error handling-- much cleaner 👍
AustinSiu
force-pushed
the
qp-health-events
branch
from
5ad08ec
to
23a7c6f
Compare
|
|
||
| $ aws events put-rule \ | ||
| --name MyK8sScheduledChangeRule \ | ||
| --event-pattern "{\"source\": [\"aws.health\"],\"detail-type\": [\"AWS Health Event\"]}" |
There was a problem hiding this comment.
Is there a reason this event pattern isn't more specific? Should this be limited to EC2 scheduled changes since the code ignores all other events?
{
"source": [
"aws.health"
],
"detail-type": [
"AWS Health Event"
],
"detail": {
"service": [
"EC2"
],
"eventTypeCategory": [
"scheduledChange"
]
}
}Apologies for commenting on a closed PR. I'm trying to upgrade NTH and need to understand the new event rules that should be added.
There was a problem hiding this comment.
You should also add "region": ["us-east-1"] or whatever region you're using for the cluster. Since AWS Health events are global and at least want the filter to EC2 scheduled changes for the region you're using.
There was a problem hiding this comment.
@gabegorelick can you open an issue with your thoughts on how we should constrain the rule? Also, if you'd like to PR an update to the README and run some tests, that would be great!
Please do not merge, WIPDescription of changes: Introduce new event handler for AWS Health Scheduled Change events. AWS Health events can specify more than 1 host, which differs from currently-supported events. To support this, I'm introducing a wrapper around interruption events and related errors to more-easily manage the list.
Related issue: #293
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.