Skip to content

Commit 8be7586

Browse files
committed
refactor: introduce ValidatedRoleConfig
1 parent 7736b5e commit 8be7586

3 files changed

Lines changed: 34 additions & 17 deletions

File tree

rust/operator-binary/src/controller.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ use stackable_operator::{
2727
},
2828
kvp::Labels,
2929
logging::controller::ReconcilerError,
30-
role_utils::GenericRoleConfig,
3130
shared::time::Duration,
3231
status::condition::{
3332
compute_conditions, operations::ClusterOperationsConditionBuilder,
@@ -119,17 +118,21 @@ pub struct ValidatedCluster {
119118
/// value.
120119
pub product_version: ProductVersion,
121120
pub cluster_config: ValidatedClusterConfig,
121+
/// Per-role configuration (e.g. the Pod disruption budget), keyed by role.
122+
pub role_configs: BTreeMap<KafkaRole, ValidatedRoleConfig>,
122123
pub role_group_configs: BTreeMap<KafkaRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
123124
}
124125

125126
impl ValidatedCluster {
127+
#[allow(clippy::too_many_arguments)]
126128
pub fn new(
127129
name: ClusterName,
128130
namespace: NamespaceName,
129131
uid: Uid,
130132
cluster_domain: DomainName,
131133
image: ResolvedProductImage,
132134
cluster_config: ValidatedClusterConfig,
135+
role_configs: BTreeMap<KafkaRole, ValidatedRoleConfig>,
133136
role_group_configs: BTreeMap<KafkaRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
134137
) -> Self {
135138
// `app_version_label_value` is constructed to be a valid label value, so it is also a
@@ -150,6 +153,7 @@ impl ValidatedCluster {
150153
image,
151154
product_version,
152155
cluster_config,
156+
role_configs,
153157
role_group_configs,
154158
}
155159
}
@@ -365,6 +369,15 @@ impl ValidatedClusterConfig {
365369
}
366370
}
367371

372+
/// Per-role configuration extracted during validation.
373+
///
374+
/// Resolved from the raw [`v1alpha1::KafkaCluster`] spec during validation so the reconcile loop
375+
/// never has to read it. Kafka's `GenericRoleConfig` only carries the Pod disruption budget.
376+
#[derive(Clone, Debug)]
377+
pub struct ValidatedRoleConfig {
378+
pub pdb: stackable_operator::commons::pdb::PdbConfig,
379+
}
380+
368381
/// Lets [`ValidatedCluster`] act as the owner [`Resource`] for child objects, so owner
369382
/// references are built from it (via the captured `metadata`) rather than the raw CR.
370383
impl Resource for ValidatedCluster {
@@ -706,11 +719,8 @@ pub async fn reconcile_kafka(
706719
);
707720
}
708721

709-
let role_cfg = kafka.role_config(kafka_role);
710-
if let Some(GenericRoleConfig {
711-
pod_disruption_budget: pdb,
712-
}) = role_cfg
713-
&& let Some(pdb) = build_pdb(pdb, &validated_cluster, kafka_role)
722+
if let Some(role_config) = validated_cluster.role_configs.get(kafka_role)
723+
&& let Some(pdb) = build_pdb(&role_config.pdb, &validated_cluster, kafka_role)
714724
{
715725
cluster_resources
716726
.add(client, pdb)

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ use stackable_operator::{
2929

3030
use crate::{
3131
controller::{
32-
RoleGroupName, ValidatedCluster, ValidatedClusterConfig, ValidatedRoleGroupConfig,
33-
dereference::DereferencedObjects,
32+
RoleGroupName, ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig,
33+
ValidatedRoleGroupConfig, dereference::DereferencedObjects,
3434
},
3535
crd::{
3636
self, CONTAINER_IMAGE_BASE_NAME,
@@ -228,6 +228,7 @@ pub fn validate(
228228
.vector_aggregator_config_map_name
229229
.clone();
230230

231+
let mut role_configs: BTreeMap<KafkaRole, ValidatedRoleConfig> = BTreeMap::new();
231232
let mut role_group_configs: BTreeMap<
232233
KafkaRole,
233234
BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>,
@@ -246,10 +247,16 @@ pub fn validate(
246247
validate_broker_logging,
247248
&vector_aggregator_config_map_name,
248249
)?;
250+
role_configs.insert(
251+
KafkaRole::Broker,
252+
ValidatedRoleConfig {
253+
pdb: broker_role.role_config.pod_disruption_budget.clone(),
254+
},
255+
);
249256
role_group_configs.insert(KafkaRole::Broker, broker_groups);
250257

251-
// Controllers are optional: ZooKeeper-mode clusters have none, and `controller_role()`
252-
// errors when `controllers` is unset, which would stop their reconciliation.
258+
// Controllers are optional: ZooKeeper-mode clusters have none, in which case they are simply
259+
// absent from both maps and not reconciled.
253260
if let Some(controller_role) = kafka.spec.controllers.as_ref() {
254261
let controller_groups = validate_role_group_configs(
255262
controller_role,
@@ -260,6 +267,12 @@ pub fn validate(
260267
validate_controller_logging,
261268
&vector_aggregator_config_map_name,
262269
)?;
270+
role_configs.insert(
271+
KafkaRole::Controller,
272+
ValidatedRoleConfig {
273+
pdb: controller_role.role_config.pod_disruption_budget.clone(),
274+
},
275+
);
263276
role_group_configs.insert(KafkaRole::Controller, controller_groups);
264277
}
265278

@@ -292,6 +305,7 @@ pub fn validate(
292305
.broker_id_pod_config_map_name
293306
.clone(),
294307
},
308+
role_configs,
295309
role_group_configs,
296310
))
297311
}

rust/operator-binary/src/crd/mod.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,6 @@ impl v1alpha1::KafkaCluster {
321321
})
322322
}
323323

324-
pub fn role_config(&self, role: &KafkaRole) -> Option<&GenericRoleConfig> {
325-
match role {
326-
KafkaRole::Broker => self.spec.brokers.as_ref().map(|b| &b.role_config),
327-
KafkaRole::Controller => self.spec.controllers.as_ref().map(|b| &b.role_config),
328-
}
329-
}
330-
331324
pub fn broker_role(&self) -> Result<&BrokerRole, Error> {
332325
self.spec.brokers.as_ref().context(MissingRoleSnafu {
333326
role: KafkaRole::Broker.to_string(),

0 commit comments

Comments
 (0)