Skip to content

Comments

Remove availableReplicas check for old DecommissionController#545

Merged
andrewstucki merged 1 commit intomainfrom
fix-decommissioner-for-relaxed-healthchecks
Mar 20, 2025
Merged

Remove availableReplicas check for old DecommissionController#545
andrewstucki merged 1 commit intomainfrom
fix-decommissioner-for-relaxed-healthchecks

Conversation

@andrewstucki
Copy link
Contributor

So, this was causing our e2e v2 tests to fail the decommissioning test.

The logic basically says, "make sure I have 0 available replicas before attempting a decommission" -- in other words every broker must be marked as "not ready" for a decommission to happen with the old decommission controller. This isn't the way that the new decommissioner works and I'm amenable to plumbing in the new decommissioner sooner than later, but this is to just get our decommissioning tests to pass as this guard should be unnecessary and never will actually happen with the relaxed health checks (i.e. we'll never be in a state where all nodes are marked unhealthy since they tolerate potential node_down states).

Copy link
Contributor

@chrisseto chrisseto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This controller is so fragile 😬

@andrewstucki andrewstucki merged commit bb95984 into main Mar 20, 2025
12 checks passed
andrewstucki added a commit that referenced this pull request Mar 20, 2025
(cherry picked from commit bb95984)
@andrewstucki
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
release/v2.3.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

RafalKorepta pushed a commit that referenced this pull request Mar 21, 2025
@RafalKorepta RafalKorepta deleted the fix-decommissioner-for-relaxed-healthchecks branch March 28, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants