Skip to content

Commit bf87de4

Browse files
authored
refactor: Extract apply and update status steps (#856)
* move object_meta into the build step * derive the version label instead of hardcoding the dev suffix * extract apply and update_status steps * look up the applied Router listener declaratively * clean-up * changelog * added listener look-up test, minor improvements * destructure the resources exhaustively in apply() * include listener in dereferencing step so that its status/addresses can be inspected before building discovery config map * add debug statement for CM miss * split internal secret into static and migration parts * fix log name and split listener name function * consolidate ingress derivation * ensure stateful sets are applied after the secret * rework internal secret handling so an unchanged secret results in a no-op apply * discovery ConfigMap is no longer orphan-deleted while the listener has no ingress address
1 parent 6a0d366 commit bf87de4

16 files changed

Lines changed: 1138 additions & 562 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ All notable changes to this project will be documented in this file.
1111
- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
1212
functions and carry the full set of recommended labels ([#846]).
1313
- Bump stackable-operator to 0.114.0 ([#855]).
14+
- The reconciler now applies resources and derives the cluster status in discrete
15+
apply and update_status steps ([#856]).
1416
- All product containers now run with `securityContext.runAsNonRoot` set to `true` to improve security ([#860]).
1517

1618
### Fixed
@@ -22,6 +24,7 @@ All notable changes to this project will be documented in this file.
2224
[#841]: https://github.com/stackabletech/druid-operator/pull/841
2325
[#846]: https://github.com/stackabletech/druid-operator/pull/846
2426
[#855]: https://github.com/stackabletech/druid-operator/pull/855
27+
[#856]: https://github.com/stackabletech/druid-operator/pull/856
2528
[#860]: https://github.com/stackabletech/druid-operator/pull/860
2629

2730
## [26.7.0] - 2026-07-21

‎deploy/helm/druid-operator/templates/clusterrole-operator.yaml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,9 @@ rules:
113113
# Required for startup condition
114114
- list
115115
- watch
116-
# Listener created per role group for external access. Applied via SSA and tracked for orphan
117-
# cleanup.
116+
# Listener created per externally reachable role for external access. Applied via SSA and
117+
# tracked for orphan cleanup. Watched by the controller, so a reconciliation is triggered once
118+
# the listener-operator writes the ingress address (needed for the discovery ConfigMap).
118119
- apiGroups:
119120
- listeners.stackable.tech
120121
resources:
@@ -125,6 +126,7 @@ rules:
125126
- get
126127
- list
127128
- patch
129+
- watch
128130
# Required to report reconciliation results and warnings back to the DruidCluster object.
129131
- apiGroups:
130132
- events.k8s.io

‎rust/operator-binary/src/controller.rs‎

Lines changed: 44 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Ensures that `Pod`s are configured and running for each [`DruidCluster`][v1alpha1]
22
//!
33
//! [v1alpha1]: v1alpha1::DruidCluster
4-
use std::{str::FromStr, sync::Arc};
4+
use std::{marker::PhantomData, str::FromStr, sync::Arc};
55

66
use const_format::concatcp;
77
use snafu::{ResultExt, Snafu};
@@ -11,7 +11,7 @@ use stackable_operator::{
1111
crd::listener::v1alpha1::Listener,
1212
k8s_openapi::api::{
1313
apps::v1::StatefulSet,
14-
core::v1::{ConfigMap, Service, ServiceAccount},
14+
core::v1::{ConfigMap, Secret, Service, ServiceAccount},
1515
policy::v1::PodDisruptionBudget,
1616
rbac::v1::RoleBinding,
1717
},
@@ -21,29 +21,22 @@ use stackable_operator::{
2121
},
2222
logging::controller::ReconcilerError,
2323
shared::time::Duration,
24-
status::condition::{
25-
compute_conditions, operations::ClusterOperationsConditionBuilder,
26-
statefulset::StatefulSetConditionBuilder,
27-
},
28-
v2::{
29-
cluster_resources::cluster_resources_new,
30-
types::operator::{ControllerName, OperatorName, ProductName},
31-
},
24+
v2::types::operator::{ControllerName, OperatorName, ProductName},
3225
};
3326
use strum::{EnumDiscriminants, IntoStaticStr};
3427

3528
use crate::{
36-
controller::build::resource::listener::{build_group_listener, group_listener_name},
37-
crd::{APP_NAME, DruidClusterStatus, DruidRole, OPERATOR_NAME, v1alpha1},
38-
internal_secret::create_shared_internal_secret,
29+
controller::{apply::Applier, update_status::update_status},
30+
crd::{APP_NAME, OPERATOR_NAME, v1alpha1},
3931
};
4032

33+
mod apply;
4134
mod build;
4235
mod dereference;
36+
mod migrate;
37+
mod update_status;
4338
pub(crate) mod validate;
4439

45-
use build::resource::discovery::{self, build_discovery_configmaps};
46-
4740
pub const DRUID_CONTROLLER_NAME: &str = "druidcluster";
4841
pub const FULL_CONTROLLER_NAME: &str = concatcp!(DRUID_CONTROLLER_NAME, '.', OPERATOR_NAME);
4942

@@ -70,68 +63,52 @@ pub struct Ctx {
7063
pub operator_environment: OperatorEnvironmentOptions,
7164
}
7265

73-
/// Every Kubernetes resource produced by the client-free [`build`](build::build) step.
66+
/// Marker for prepared Kubernetes resources which are not applied yet.
67+
pub struct Prepared;
68+
69+
/// Marker for applied Kubernetes resources.
70+
pub struct Applied;
71+
72+
/// Every Kubernetes resource produced by the build step.
7473
///
75-
/// The Router group `Listener` and the discovery `ConfigMap`s are intentionally *not* yet part of this
76-
/// bundle: the discovery `ConfigMap` derives from the *applied* Router listener's ingress address,
77-
/// so both are built and applied in the reconcile step instead.
78-
pub struct KubernetesResources {
74+
/// `T` is a marker that indicates if these resources are only [`Prepared`] or already [`Applied`].
75+
/// The marker is useful e.g. to ensure that the cluster status is updated based on the applied
76+
/// resources.
77+
pub struct KubernetesResources<T> {
7978
pub stateful_sets: Vec<StatefulSet>,
8079
pub services: Vec<Service>,
8180
pub listeners: Vec<Listener>,
8281
pub config_maps: Vec<ConfigMap>,
8382
pub pod_disruption_budgets: Vec<PodDisruptionBudget>,
8483
pub service_accounts: Vec<ServiceAccount>,
8584
pub role_bindings: Vec<RoleBinding>,
85+
pub internal_secret: Secret,
86+
pub status: PhantomData<T>,
8687
}
8788

8889
#[derive(Snafu, Debug, EnumDiscriminants)]
8990
#[strum_discriminants(derive(IntoStaticStr))]
9091
pub enum Error {
92+
#[snafu(display("failed to apply the Kubernetes resources"))]
93+
ApplyResources { source: apply::Error },
94+
9195
#[snafu(display("failed to build the Kubernetes resources"))]
9296
BuildResources { source: build::Error },
9397

94-
#[snafu(display("failed to apply Kubernetes resource"))]
95-
ApplyResource {
96-
source: stackable_operator::cluster_resources::Error,
97-
},
98-
9998
#[snafu(display("failed to dereference cluster objects"))]
10099
Dereference { source: dereference::Error },
101100

102-
#[snafu(display("failed to build discovery ConfigMap"))]
103-
BuildDiscoveryConfig { source: discovery::Error },
104-
105-
#[snafu(display("failed to apply discovery ConfigMap"))]
106-
ApplyDiscoveryConfig {
107-
source: stackable_operator::cluster_resources::Error,
108-
},
109-
110-
#[snafu(display("failed to apply cluster status"))]
111-
ApplyStatus {
112-
source: stackable_operator::client::Error,
113-
},
114-
115-
#[snafu(display("failed to delete orphaned resources"))]
116-
DeleteOrphanedResources {
117-
source: stackable_operator::cluster_resources::Error,
118-
},
101+
#[snafu(display("failed to update the cluster status"))]
102+
UpdateStatus { source: update_status::Error },
119103

120-
#[snafu(display("failed to retrieve secret for internal communications"))]
121-
FailedInternalSecretCreation {
122-
source: crate::internal_secret::Error,
123-
},
104+
#[snafu(display("failed to migrate the immutable internal secret"))]
105+
MigrateInternalSecret { source: migrate::Error },
124106

125107
#[snafu(display("DruidCluster object is invalid"))]
126108
InvalidDruidCluster {
127109
source: error_boundary::InvalidObject,
128110
},
129111

130-
#[snafu(display("failed to apply group listener"))]
131-
ApplyGroupListener {
132-
source: stackable_operator::cluster_resources::Error,
133-
},
134-
135112
#[snafu(display("failed to validate cluster"))]
136113
ValidateCluster { source: validate::Error },
137114
}
@@ -165,124 +142,28 @@ pub async fn reconcile_druid(
165142
validate::validate(druid, &dereferenced_objects, &ctx.operator_environment)
166143
.context(ValidateClusterSnafu)?;
167144

168-
let mut cluster_resources = cluster_resources_new(
169-
&product_name(),
170-
&operator_name(),
171-
&controller_name(),
172-
&validated_cluster.name,
173-
&validated_cluster.namespace,
174-
&validated_cluster.uid,
145+
let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?;
146+
147+
// Temporary migration step; remove together with the [`migrate`] module.
148+
migrate::delete_immutable_internal_secret(client, &validated_cluster)
149+
.await
150+
.context(MigrateInternalSecretSnafu)?;
151+
152+
let applier = Applier::new(
153+
client,
154+
&validated_cluster,
175155
ClusterResourceApplyStrategy::from(&druid.spec.cluster_operation),
176156
&druid.spec.object_overrides,
177157
);
178158

179-
// The internal secret is shared across all roles and role groups, so it only needs to be
180-
// created once per reconcile rather than inside the role loop below.
181-
create_shared_internal_secret(&validated_cluster, client, DRUID_CONTROLLER_NAME)
159+
let applied = applier
160+
.apply(resources)
182161
.await
183-
.context(FailedInternalSecretCreationSnafu)?;
162+
.context(ApplyResourcesSnafu)?;
184163

185-
let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?;
186-
187-
let mut ss_cond_builder = StatefulSetConditionBuilder::default();
188-
189-
for service_account in resources.service_accounts {
190-
cluster_resources
191-
.add(client, service_account)
192-
.await
193-
.context(ApplyResourceSnafu)?;
194-
}
195-
196-
for role_binding in resources.role_bindings {
197-
cluster_resources
198-
.add(client, role_binding)
199-
.await
200-
.context(ApplyResourceSnafu)?;
201-
}
202-
203-
// Apply order: everything a Pod mounts (ConfigMaps) must exist before the StatefulSets, so the
204-
// StatefulSets are applied last to prevent unnecessary Pod restarts.
205-
// See https://github.com/stackabletech/commons-operator/issues/111 for details.
206-
for service in resources.services {
207-
cluster_resources
208-
.add(client, service)
209-
.await
210-
.context(ApplyResourceSnafu)?;
211-
}
212-
for listener in resources.listeners {
213-
cluster_resources
214-
.add(client, listener)
215-
.await
216-
.context(ApplyResourceSnafu)?;
217-
}
218-
for config_map in resources.config_maps {
219-
cluster_resources
220-
.add(client, config_map)
221-
.await
222-
.context(ApplyResourceSnafu)?;
223-
}
224-
for pdb in resources.pod_disruption_budgets {
225-
cluster_resources
226-
.add(client, pdb)
227-
.await
228-
.context(ApplyResourceSnafu)?;
229-
}
230-
for stateful_set in resources.stateful_sets {
231-
ss_cond_builder.add(
232-
cluster_resources
233-
.add(client, stateful_set)
234-
.await
235-
.context(ApplyResourceSnafu)?,
236-
);
237-
}
238-
239-
// The Router group Listener and its discovery ConfigMaps are applied here rather than in the
240-
// build step: the discovery ConfigMap derives from the *applied* Router listener's ingress
241-
// address, which is only known after the Listener has been applied.
242-
if let Some(listener_class) = &validated_cluster
243-
.role_config(&DruidRole::Router)
244-
.listener_class
245-
&& let Some(listener_group_name) =
246-
group_listener_name(&validated_cluster, &DruidRole::Router)
247-
{
248-
let router_listener = build_group_listener(
249-
&validated_cluster,
250-
listener_class,
251-
listener_group_name,
252-
&DruidRole::Router,
253-
);
254-
255-
let listener = cluster_resources
256-
.add(client, router_listener)
257-
.await
258-
.context(ApplyGroupListenerSnafu)?;
259-
260-
for discovery_cm in build_discovery_configmaps(&validated_cluster, listener)
261-
.await
262-
.context(BuildDiscoveryConfigSnafu)?
263-
{
264-
cluster_resources
265-
.add(client, discovery_cm)
266-
.await
267-
.context(ApplyDiscoveryConfigSnafu)?;
268-
}
269-
}
270-
271-
let cluster_operation_cond_builder =
272-
ClusterOperationsConditionBuilder::new(&druid.spec.cluster_operation);
273-
274-
let status = DruidClusterStatus {
275-
conditions: compute_conditions(druid, &[&ss_cond_builder, &cluster_operation_cond_builder]),
276-
};
277-
278-
cluster_resources
279-
.delete_orphaned_resources(client)
280-
.await
281-
.context(DeleteOrphanedResourcesSnafu)?;
282-
client
283-
.apply_patch_status(OPERATOR_NAME, druid, &status)
164+
update_status(client, druid, &applied)
284165
.await
285-
.context(ApplyStatusSnafu)?;
166+
.context(UpdateStatusSnafu)?;
286167

287168
Ok(Action::await_change())
288169
}
@@ -305,12 +186,11 @@ mod test {
305186
use rstest::*;
306187
use stackable_operator::v2::types::operator::RoleGroupName;
307188

308-
use super::*;
309189
use crate::{
310190
controller::build::{
311191
properties::ConfigFileName, resource::config_map::build_rolegroup_config_map,
312192
},
313-
crd::PROP_SEGMENT_CACHE_LOCATIONS,
193+
crd::{DruidRole, PROP_SEGMENT_CACHE_LOCATIONS},
314194
};
315195

316196
#[rstest]

0 commit comments

Comments
 (0)