v2: refactor managed decommission #288
Conversation
|
The refactor to the controller ended up being pretty heavy but the logic is largely unchanged with the exception of |
chrisseto
force-pushed
the
chris/p/managed-decom-client-factory
branch
from
b591c32 to
a36efbc
Compare
| // NB: Deleting this Pod will take an unexpectedly long time as the | ||
| // pre-stop hook will "spin" due to not handling decommissioned brokers. |
There was a problem hiding this comment.
I don't understand this comment. I'm under impression that podEvict is called after Redpanda is successfully decommissioned. Maybe the problem is that maintenance mode is not achievable if particular Redpanda is no longer part of the Cluster?
There was a problem hiding this comment.
Maybe the problem is that maintenance mode is not achievable if particular Redpanda is no longer part of the Cluster?
Exactly this! You can't set maintenance mode on a broker that's not part of a cluster. The admin API returns a 404.
| Repository: ptr.To("redpandadata/redpanda"), // Override the default to make use of the docker-io image cache. | ||
| }, | ||
| Config: &redpandav1alpha2.Config{}, | ||
| Image: &redpandav1alpha2.RedpandaImage{}, |
There was a problem hiding this comment.
Why an override was removed?
There was a problem hiding this comment.
It got added by accident 😓 I've actually added it back in and updated the comment. This way we won't inflate our own metrics :P
| Image: &redpandav1alpha2.RedpandaImage{ | ||
| Repository: ptr.To("redpandadata/redpanda"), // Override the default to make use of the docker-io image cache. | ||
| }, | ||
| Config: &redpandav1alpha2.Config{}, |
There was a problem hiding this comment.
Why empty Config struct is added?
There was a problem hiding this comment.
Oh, sorry that leaked in from another PR. It's so that I can set config later on without having to worry about nil pointers. Added a comment.
chrisseto
force-pushed
the
chris/p/managed-decom-client-factory
branch
5 times, most recently
from
7c8d7be to
12205e4
Compare
Prior to this commit the managed decommission controller was utilizing patterns akin to the node watcher and decommission controller for connecting to the admin API. This made it difficult to test via the `RedpandaControllerSuite` test which was desirable due to the flakey nature (especially when considering a later commit) of it's corresponding kuttl based test. This commit refactors the manged decommission controller to leverage the `ClientFactory` struct used in the redpanda controller which makes it possible to test in the faster and more easily debuggable `RedpandaControllerSuite`.
chrisseto
force-pushed
the
chris/p/managed-decom-client-factory
branch
from
12205e4 to
908182b
Compare
Prior to this commit the managed decommission controller was utilizing patterns akin to the node watcher and decommission controller for connecting to the admin API. This made it difficult to test via the
RedpandaControllerSuitetest which was desirable due to the flakey nature (especially when considering a later commit) of it's corresponding kuttl based test.This commit refactors the manged decommission controller to leverage the
ClientFactorystruct used in the redpanda controller which makes it possible to test in the faster and more easily debuggableRedpandaControllerSuite.