Skip to content

Commit a7a7686

Browse files
committed
refactor: use ValidatedCluster in statefulset
1 parent dc1ea69 commit a7a7686

5 files changed

Lines changed: 50 additions & 95 deletions

File tree

rust/operator-binary/src/controller.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,10 +339,19 @@ pub struct ValidatedClusterConfig {
339339
pub authorization_config: Option<KafkaAuthorizationConfig>,
340340
pub metadata_manager: MetadataManager,
341341

342-
/// Whether the operator must not generate broker ids itself, because the user
343-
/// supplied a `broker_id_pod_config_map_name`. Resolved from the raw spec during
344-
/// validation so the config-map builder never has to read it.
345-
pub disable_broker_id_generation: bool,
342+
/// The discovery `ConfigMap` providing the ZooKeeper connection string, if the cluster
343+
/// is connected to a ZooKeeper ensemble. Resolved from the raw spec during validation so
344+
/// the build steps never have to read it.
345+
pub zookeeper_config_map_name: Option<String>,
346+
347+
/// The `ConfigMap` mapping pods to broker ids, if the user supplied one. Resolved from the
348+
/// raw spec during validation so the build steps never have to read it.
349+
pub broker_id_pod_config_map_name: Option<String>,
350+
351+
/// The discovery `ConfigMap` providing the Vector aggregator address, if Vector log
352+
/// aggregation is configured. Resolved from the raw spec during validation so the build
353+
/// steps never have to read it.
354+
pub vector_aggregator_config_map_name: Option<String>,
346355
}
347356

348357
impl ValidatedClusterConfig {
@@ -351,6 +360,12 @@ impl ValidatedClusterConfig {
351360
self.metadata_manager == MetadataManager::KRaft
352361
}
353362

363+
/// Whether the operator must not generate broker ids itself, because the user supplied a
364+
/// `broker_id_pod_config_map_name`.
365+
pub fn disable_broker_id_generation(&self) -> bool {
366+
self.broker_id_pod_config_map_name.is_some()
367+
}
368+
354369
/// The OPA connect string, if OPA authorization is configured.
355370
pub fn opa_connect(&self) -> Option<&str> {
356371
self.authorization_config
@@ -644,7 +659,6 @@ pub async fn reconcile_kafka(
644659

645660
let rg_statefulset = match kafka_role {
646661
KafkaRole::Broker => build_broker_rolegroup_statefulset(
647-
kafka,
648662
kafka_role,
649663
rolegroup_name,
650664
&validated_cluster,
@@ -653,7 +667,6 @@ pub async fn reconcile_kafka(
653667
)
654668
.context(BuildStatefulsetSnafu)?,
655669
KafkaRole::Controller => build_controller_rolegroup_statefulset(
656-
kafka,
657670
kafka_role,
658671
rolegroup_name,
659672
&validated_cluster,

rust/operator-binary/src/controller/build/properties/broker_properties.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ pub fn build(
7777
// so we disable automatic id generation.
7878
// This check ensures that existing clusters running in ZooKeeper mode do not
7979
// suddenly break after the introduction of this change.
80-
if cluster_config.disable_broker_id_generation {
80+
if cluster_config.disable_broker_id_generation() {
8181
result.extend([
8282
(
8383
"broker.id.generation.enable".to_string(),

rust/operator-binary/src/controller/build/resource/statefulset.rs

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -57,25 +57,20 @@ use crate::{
5757
node_id_hasher::node_id_hash32_offset,
5858
},
5959
crd::{
60-
self, BROKER_ID_POD_MAP_DIR, KAFKA_HEAP_OPTS, LISTENER_BOOTSTRAP_VOLUME_NAME,
60+
BROKER_ID_POD_MAP_DIR, KAFKA_HEAP_OPTS, LISTENER_BOOTSTRAP_VOLUME_NAME,
6161
LISTENER_BROKER_VOLUME_NAME, LOG_DIRS_VOLUME_NAME, METRICS_PORT, METRICS_PORT_NAME,
62-
MetadataManager, STACKABLE_CONFIG_DIR, STACKABLE_DATA_DIR,
63-
STACKABLE_LISTENER_BOOTSTRAP_DIR, STACKABLE_LISTENER_BROKER_DIR, STACKABLE_LOG_CONFIG_DIR,
64-
STACKABLE_LOG_DIR,
62+
STACKABLE_CONFIG_DIR, STACKABLE_DATA_DIR, STACKABLE_LISTENER_BOOTSTRAP_DIR,
63+
STACKABLE_LISTENER_BROKER_DIR, STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR,
6564
role::{
6665
AnyConfig, KAFKA_NODE_ID_OFFSET, KafkaRole, broker::BrokerContainer,
6766
controller::ControllerContainer,
6867
},
6968
security::KafkaTlsSecurity,
70-
v1alpha1,
7169
},
7270
};
7371

7472
#[derive(Snafu, Debug)]
7573
pub enum Error {
76-
#[snafu(display("invalid metadata manager"))]
77-
InvalidMetadataManager { source: crate::crd::Error },
78-
7974
#[snafu(display("failed to add kerberos config"))]
8075
AddKerberosConfig {
8176
source: crate::controller::build::kerberos::Error,
@@ -128,9 +123,6 @@ pub enum Error {
128123
#[snafu(display("missing secret lifetime"))]
129124
MissingSecretLifetime,
130125

131-
#[snafu(display("failed to retrieve rolegroup replicas"))]
132-
RoleGroupReplicas { source: crd::role::Error },
133-
134126
#[snafu(display("vector agent is enabled but vector aggregator ConfigMap is missing"))]
135127
VectorAggregatorConfigMapMissing,
136128
}
@@ -140,7 +132,6 @@ pub enum Error {
140132
/// The [`Pod`](`stackable_operator::k8s_openapi::api::core::v1::Pod`)s are accessible through the corresponding
141133
/// [`Service`](`stackable_operator::k8s_openapi::api::core::v1::Service`) from [`build_rolegroup_headless_service`](`crate::controller::build::resource::service::build_rolegroup_headless_service`).
142134
pub fn build_broker_rolegroup_statefulset(
143-
kafka: &v1alpha1::KafkaCluster,
144135
kafka_role: &KafkaRole,
145136
role_group_name: &RoleGroupName,
146137
validated_cluster: &ValidatedCluster,
@@ -213,7 +204,9 @@ pub fn build_broker_rolegroup_statefulset(
213204

214205
let mut env = Vec::<EnvVar>::from(validated_rg.env_overrides.clone());
215206

216-
if let Some(zookeeper_config_map_name) = &kafka.spec.cluster_config.zookeeper_config_map_name {
207+
if let Some(zookeeper_config_map_name) =
208+
&validated_cluster.cluster_config.zookeeper_config_map_name
209+
{
217210
env.push(EnvVar {
218211
name: "ZOOKEEPER".to_string(),
219212
value_from: Some(EnvVarSource {
@@ -240,10 +233,6 @@ pub fn build_broker_rolegroup_statefulset(
240233
..EnvVar::default()
241234
});
242235

243-
let metadata_manager = kafka
244-
.effective_metadata_manager()
245-
.context(InvalidMetadataManagerSnafu)?;
246-
247236
cb_kafka
248237
.image_from_product_image(resolved_product_image)
249238
.command(vec![
@@ -254,7 +243,7 @@ pub fn build_broker_rolegroup_statefulset(
254243
"-c".to_string(),
255244
])
256245
.args(vec![broker_kafka_container_commands(
257-
metadata_manager == MetadataManager::KRaft,
246+
validated_cluster.cluster_config.is_kraft_mode(),
258247
// we need controller pods
259248
validated_cluster
260249
.pod_descriptors(Some(&KafkaRole::Controller))
@@ -355,8 +344,9 @@ pub fn build_broker_rolegroup_statefulset(
355344
.context(AddListenerVolumeSnafu)?;
356345
}
357346

358-
if let Some(broker_id_config_map_name) =
359-
&kafka.spec.cluster_config.broker_id_pod_config_map_name
347+
if let Some(broker_id_config_map_name) = &validated_cluster
348+
.cluster_config
349+
.broker_id_pod_config_map_name
360350
{
361351
pod_builder
362352
.add_volume(
@@ -381,7 +371,7 @@ pub fn build_broker_rolegroup_statefulset(
381371

382372
add_vector_container(
383373
&mut pod_builder,
384-
kafka,
374+
validated_cluster,
385375
resolved_product_image,
386376
merged_config,
387377
)?;
@@ -411,10 +401,7 @@ pub fn build_broker_rolegroup_statefulset(
411401
.build(),
412402
spec: Some(StatefulSetSpec {
413403
pod_management_policy: Some("Parallel".to_string()),
414-
replicas: kafka_role
415-
.replicas(kafka, role_group_name.as_ref())
416-
.context(RoleGroupReplicasSnafu)?
417-
.map(i32::from),
404+
replicas: Some(i32::from(validated_rg.replicas)),
418405
selector: LabelSelector {
419406
match_labels: Some(
420407
validated_cluster
@@ -434,7 +421,6 @@ pub fn build_broker_rolegroup_statefulset(
434421

435422
/// The controller rolegroup [`StatefulSet`] runs the rolegroup, as configured by the administrator.
436423
pub fn build_controller_rolegroup_statefulset(
437-
kafka: &v1alpha1::KafkaCluster,
438424
kafka_role: &KafkaRole,
439425
role_group_name: &RoleGroupName,
440426
validated_cluster: &ValidatedCluster,
@@ -500,7 +486,9 @@ pub fn build_controller_rolegroup_statefulset(
500486
});
501487

502488
// Controllers need the ZooKeeper connection string for migration
503-
if let Some(zookeeper_config_map_name) = &kafka.spec.cluster_config.zookeeper_config_map_name {
489+
if let Some(zookeeper_config_map_name) =
490+
&validated_cluster.cluster_config.zookeeper_config_map_name
491+
{
504492
env.push(EnvVar {
505493
name: "ZOOKEEPER".to_string(),
506494
value_from: Some(EnvVarSource {
@@ -606,7 +594,7 @@ pub fn build_controller_rolegroup_statefulset(
606594

607595
add_vector_container(
608596
&mut pod_builder,
609-
kafka,
597+
validated_cluster,
610598
resolved_product_image,
611599
merged_config,
612600
)?;
@@ -636,10 +624,7 @@ pub fn build_controller_rolegroup_statefulset(
636624
type_: Some("RollingUpdate".to_string()),
637625
..StatefulSetUpdateStrategy::default()
638626
}),
639-
replicas: kafka_role
640-
.replicas(kafka, role_group_name.as_ref())
641-
.context(RoleGroupReplicasSnafu)?
642-
.map(i32::from),
627+
replicas: Some(i32::from(validated_rg.replicas)),
643628
selector: LabelSelector {
644629
match_labels: Some(
645630
validated_cluster
@@ -795,13 +780,16 @@ fn add_common_pod_config(
795780
/// configured on the cluster.
796781
fn add_vector_container(
797782
pod_builder: &mut PodBuilder,
798-
kafka: &v1alpha1::KafkaCluster,
783+
validated_cluster: &ValidatedCluster,
799784
resolved_product_image: &ResolvedProductImage,
800785
merged_config: &AnyConfig,
801786
) -> Result<(), Error> {
802787
// Add vector container after kafka container to keep the defaulting into kafka container
803788
if merged_config.vector_logging_enabled() {
804-
match &kafka.spec.cluster_config.vector_aggregator_config_map_name {
789+
match &validated_cluster
790+
.cluster_config
791+
.vector_aggregator_config_map_name
792+
{
805793
Some(vector_aggregator_config_map_name) => {
806794
pod_builder.add_container(
807795
product_logging::framework::vector_container(

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,17 @@ pub fn validate(
180180
kafka_security,
181181
authorization_config: dereferenced_objects.authorization_config,
182182
metadata_manager,
183-
disable_broker_id_generation: kafka
183+
zookeeper_config_map_name: kafka.spec.cluster_config.zookeeper_config_map_name.clone(),
184+
broker_id_pod_config_map_name: kafka
184185
.spec
185186
.cluster_config
186187
.broker_id_pod_config_map_name
187-
.is_some(),
188+
.clone(),
189+
vector_aggregator_config_map_name: kafka
190+
.spec
191+
.cluster_config
192+
.vector_aggregator_config_map_name
193+
.clone(),
188194
},
189195
role_group_configs,
190196
))

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

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ pub mod controller;
55
use std::{borrow::Cow, ops::Deref};
66

77
use serde::{Deserialize, Serialize};
8-
use snafu::{OptionExt, ResultExt, Snafu};
98
use stackable_operator::{
109
commons::resources::{NoRuntimeLimits, Resources},
1110
product_logging::spec::ContainerLogConfig,
@@ -64,18 +63,6 @@ pub const KAFKA_LISTENER_SECURITY_PROTOCOL_MAP: &str = "listener.security.protoc
6463
/// For example: localhost:9092,localhost:9093,localhost:9094.
6564
pub const KAFKA_CONTROLLER_QUORUM_BOOTSTRAP_SERVERS: &str = "controller.quorum.bootstrap.servers";
6665

67-
#[derive(Snafu, Debug)]
68-
pub enum Error {
69-
#[snafu(display("the Kafka role [{role}] is missing from spec"))]
70-
MissingRole {
71-
source: crate::crd::Error,
72-
role: String,
73-
},
74-
75-
#[snafu(display("missing role group {rolegroup:?} for role {role:?}"))]
76-
MissingRoleGroup { role: String, rolegroup: String },
77-
}
78-
7966
#[derive(
8067
Clone,
8168
Debug,
@@ -113,45 +100,6 @@ impl KafkaRole {
113100
pub fn kerberos_service_name(&self) -> &'static str {
114101
"kafka"
115102
}
116-
117-
pub fn replicas(
118-
&self,
119-
kafka: &v1alpha1::KafkaCluster,
120-
rolegroup: &str,
121-
) -> Result<Option<u16>, Error> {
122-
let replicas = match self {
123-
Self::Broker => {
124-
kafka
125-
.broker_role()
126-
.with_context(|_| MissingRoleSnafu {
127-
role: self.to_string(),
128-
})?
129-
.role_groups
130-
.get(rolegroup)
131-
.with_context(|| MissingRoleGroupSnafu {
132-
role: self.to_string(),
133-
rolegroup: rolegroup.to_string(),
134-
})?
135-
.replicas
136-
}
137-
Self::Controller => {
138-
kafka
139-
.controller_role()
140-
.with_context(|_| MissingRoleSnafu {
141-
role: self.to_string(),
142-
})?
143-
.role_groups
144-
.get(rolegroup)
145-
.with_context(|| MissingRoleGroupSnafu {
146-
role: self.to_string(),
147-
rolegroup: rolegroup.to_string(),
148-
})?
149-
.replicas
150-
}
151-
};
152-
153-
Ok(replicas)
154-
}
155103
}
156104

157105
/// Configuration for a role and rolegroup of an unknown type.

0 commit comments

Comments
 (0)