fix queue-processor rbn draining by default #478
Conversation
| @@ -80,9 +80,11 @@ IMDS Processor Mode allows for a fine-grained configuration of IMDS paths that a | |||
| - `enableRebalanceMonitoring` | |||
| - `enableScheduledEventDraining` | |||
|
|
|||
| By default, IMDS mode will only Cordon in response to a Rebalance Recommendation event (all other events are Cordoned and Drained). Cordon is the default for a rebalance event because it's not known if an ASG is being utilized and if that ASG is configured to replace the instance on a rebalance event. If you are using an ASG w/ rebalance recommendations enabled, then you can set the `enableRebalanceDraining` flag to true to perform a Cordon and Drain when a rebalance event is received. | |||
There was a problem hiding this comment.
very clear 👍
I think adding another row for this "configuration granularity" would also be helpful in the feature table. Not required for this PR so sending 🚀
There was a problem hiding this comment.
I went back-and-forth on adding to the feature table. I think it actually makes sense to leave it off. The configuration granularity is available in queue-processor mode as well, you just have to do it via the EventBridge rules rather than command line flags to NTH. For example, you could not create an EventBridge rule for rebalance recommendations and have rebalance recs turned off on your ASG to ignore those. The cordon only action isn't possible on its own, but if cordon only logic is desired for a specific purpose, it would be fairly easy to setup with lifecycle hook timeouts and increasing the pod termination grace period. It's probably never optimal to cordon-only without another system taking further action at some point. We only do it for safety so that you don't drain workloads that are not going to be backfilled.
There was a problem hiding this comment.
ah so just change the row name to helm configuration granularity, but actually these are fair points 👍 I'm good leaving it off
| @@ -153,7 +153,7 @@ GET_ATTRS_SQS_CMD="awslocal sqs get-queue-attributes --queue-url ${queue_url} -- | |||
|
|
|||
| cordoned=0 | |||
| tainted=0 | |||
| not_evicted=0 | |||
| evicted=0 | |||
Issue #, if available:
N/A
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.