refactor: Extract build and apply steps in the zk- and znode-controller - #1069
Merged
Conversation
The discovery ConfigMap advertises the addresses that the listener operator publishes on the ZooKeeper role Listener, so it used to be built and applied inline in reconcile_zk, from the Listener that had just been applied. That Listener carries no addresses yet on the first reconciliation, so the controller relied on the reconciliation failing and being requeued five seconds later. Follow the pattern the OpenSearch operator already uses: dereference the role Listener, validate its addresses into the ValidatedCluster, and let build() emit an Option<ConfigMap>. The controller now watches Listeners, so the reconciliation that writes the discovery ConfigMap is triggered as soon as the addresses appear, and it also reruns when they change. The listener address extraction moves into a shared listener_addresses module, because the ZookeeperZnode controller needs it too. The operator ClusterRole gains the watch verb on listeners, which is required for the new watch.
reconcile_zk built the ClusterResources itself and applied every collection with its own inline loop, so the driver carried the apply order, the orphan deletion and the resource specific error variants. Extract all of that into an Applier, following the airflow and hbase operators. KubernetesResources is now marked as either Prepared or Applied, which makes it impossible to derive the cluster status from resources that were only built. apply() destructures the resource set exhaustively, so a new field fails to compile here instead of silently never being applied. Unlike the sibling operators, the apply module is declared in zk_controller.rs itself, so the Applier is imported without `self` to avoid a name collision with the module declaration.
The cluster conditions and the discovery hash were computed inline at the end of reconcile_zk, from resources that happened to be in scope. Move both into an update_status step, following the airflow and hbase operators. It takes KubernetesResources<Applied>, so the type system proves the status is derived from resources that were actually applied rather than merely built. The discovery hash, which the sibling operators do not have, becomes a private helper next to it. reconcile_zk is now the dereference, validate, build, apply and update_status pipeline and nothing else.
reconcile_apply created the ClusterResources, talked to ZooKeeper, fetched the role Listener, built the discovery ConfigMap and applied it, all inline, so the ZookeeperZnode controller was the only one left without a pipeline. Give it the same dereference, validate, build and apply structure as the ZookeeperCluster controller. The Listener fetch moves into the dereference step, where a missing Listener stays a non error so it can never block finalizer removal, and its addresses become a validated field on ValidatedZnode. Creating the znode inside the ZooKeeper ensemble lives in the apply step as a free function, next to the Applier, because it is a client side effect that the client free build() cannot perform. The discovery ConfigMap builders move to a shared discovery module, so the znode controller no longer reaches into the cluster controller's build tree. Unlike the cluster controller, the resources carry no Prepared or Applied marker: the ZookeeperZnode has no cluster conditions, so there is no status step the marker could protect.
12 tasks
maltesander
changed the title
adwk67
self-requested a review
August 12, 2026 09:07
adwk67
requested changes
Aug 12, 2026
adwk67
left a comment
adwk67
left a comment
Member
There was a problem hiding this comment.
discovery.rs has not test module: we could at least add a test asserting that a Listener with addresses produces a discovery CM with the right
ZOOKEEPER/ZOOKEEPER_HOSTS/ZOOKEEPER_CLIENT_PORT/ZOOKEEPER_CHROOT content.
Skipping the discovery ConfigMap while the role Listener publishes no addresses let the apply step delete the already published one as an orphan, breaking consumers that mount it. Before the pipeline extraction, the missing addresses aborted the run before delete_orphaned_resources() ran, so this only surfaced now. Follow the kafka-operator pattern instead: always write the ConfigMap, with an empty ZOOKEEPER value while no address is known. The Listener watch triggers a new run that fills the value in.
The znode controller had no tests at all. Add fixtures for it and cover both ways the role Listener can block validation: addresses published under an unexpected port name (ReadRoleListenerAddresses), and no addresses at all, either because the Listener does not exist yet or because the listener operator has not published its status (NoRoleListenerAddresses). The Listener fixtures move into a shared test_support module in listener_addresses, so the cluster controller's tests can use them too.
Add a try_validate variant that takes a role Listener, and cover the three outcomes: addresses published under an unexpected port name fail with ReadRoleListenerAddresses, published addresses reach the validated cluster sorted, and a missing or unpublished Listener validates to no addresses rather than an error, which is what lets the build step still write the discovery ConfigMap.
Assert the ZOOKEEPER, ZOOKEEPER_HOSTS, ZOOKEEPER_CLIENT_PORT and ZOOKEEPER_CHROOT content for both the cluster and the znode variant, that only ZOOKEEPER carries the chroot, and that a relative chroot is rejected. The empty-address case is covered too, pinning that the ConfigMap is written even while the role Listener publishes nothing. The Listener fixtures now publish the secure client port, which is what the role Listener serves while the fixture keeps TLS enabled.
…y-and-status-steps
adwk67
previously approved these changes
Aug 12, 2026
adwk67
left a comment
adwk67
left a comment
Member
There was a problem hiding this comment.
LGTM - just made the user-facing change clear in the changelog.
Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com>
adwk67
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Brings the ZooKeeper operator in line with the apply and update status steps merged in airflow-operator#828 and hbase-operator#787, with the opensearch-operator as the target shape.
Both controllers now run an explicit pipeline, with each step in its own submodule:
ZookeeperCluster:apply, update_statusZookeeperZnode: build, applyBehavioural changes
watchverb onlisteners.stackable.tech, which it did not have.Tests
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker