zalando-incubator / es-operator

Kubernetes Operator for Elasticsearch
353 stars 44 forks source link

Implement context cancellation checking in retry mechanism for node drain #405

Closed A-Kamaee closed 5 months ago

A-Kamaee commented 5 months ago

Implement context cancellation checking in retry mechanism for node drain

Issue : #404

Description

This commit integrates context cancellation checks into the retry mechanism of the waitForEmptyEsNode function. By incorporating this check, the function can now immediately respond to context cancellations and stop retry attempts.

The context check is implemented directly in the retry condition, which allows for an immediate halt if the context is canceled, without proceeding to unnecessary retry attempts that would fail due to the context being expired or canceled.

Types of Changes

Review

List of tasks the reviewer must do to review the PR

otrosien commented 5 months ago

👍

mikkeloscar commented 5 months ago

Generally it does make sense to handle the context here. The real motivation as described in #404 is to avoid drain blocking up-scaling which is reasonable. We should just be conscious that this creates another edge case where too often scaling could lead to a drain never succeeding. Whether this is realistic to happen in practice I'm not sure, but just want to mention it to keep in mind. In the perfect world we would do drain and scaling independent of each other, but that would require fundamentally changing the operator logic.

mikkeloscar commented 5 months ago

:+1: