Skip to content

Commit 20959a3

Browse files
razvanmaltesander
andauthored
refactor: move server config props from cmd line to config files (#911)
* chore: move server properties from the command line to the configuration file * fix 3.7 deployments and tests * remove controller listener from broker config * move jaas properties from the cmd line to config file * remove comment * update changelog * remove typo * small tweaks * revert brainfuck * Revert "remove controller listener from broker config" This reverts commit 01b1a05. * Apply suggestions from code review Co-authored-by: Malte Sander <[email protected]> * Update rust/operator-binary/src/crd/mod.rs Co-authored-by: Malte Sander <[email protected]> * remove commented out code * rearange config map properties --------- Co-authored-by: Malte Sander <[email protected]>
1 parent 49d5092 commit 20959a3

File tree

12 files changed

+383
-238
lines changed

12 files changed

+383
-238
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
### Changed
8+
9+
- Refactor: move server configuration properties from the command line to configuration files. ([#911]).
10+
11+
[#911]: https://github.com/stackabletech/kafka-operator/pull/911
12+
713
## [25.11.0] - 2025-11-07
814

915
## [25.11.0-rc1] - 2025-11-06

rust/operator-binary/src/config/command.rs

Lines changed: 25 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,7 @@ use stackable_operator::{
99
use crate::{
1010
crd::{
1111
KafkaPodDescriptor, STACKABLE_CONFIG_DIR, STACKABLE_KERBEROS_KRB5_PATH,
12-
STACKABLE_LISTENER_BOOTSTRAP_DIR, STACKABLE_LISTENER_BROKER_DIR,
13-
listener::{KafkaListenerConfig, KafkaListenerName, node_address_cmd},
14-
role::{
15-
KAFKA_ADVERTISED_LISTENERS, KAFKA_CONTROLLER_QUORUM_BOOTSTRAP_SERVERS,
16-
KAFKA_CONTROLLER_QUORUM_VOTERS, KAFKA_LISTENER_SECURITY_PROTOCOL_MAP, KAFKA_LISTENERS,
17-
KAFKA_NODE_ID, KAFKA_NODE_ID_OFFSET, KafkaRole, broker::BROKER_PROPERTIES_FILE,
18-
controller::CONTROLLER_PROPERTIES_FILE,
19-
},
12+
role::{broker::BROKER_PROPERTIES_FILE, controller::CONTROLLER_PROPERTIES_FILE},
2013
security::KafkaTlsSecurity,
2114
v1alpha1,
2215
},
@@ -28,8 +21,6 @@ pub fn broker_kafka_container_commands(
2821
kafka: &v1alpha1::KafkaCluster,
2922
cluster_id: &str,
3023
controller_descriptors: Vec<KafkaPodDescriptor>,
31-
kafka_listeners: &KafkaListenerConfig,
32-
opa_connect_string: Option<&str>,
3324
kafka_security: &KafkaTlsSecurity,
3425
product_version: &str,
3526
) -> String {
@@ -51,88 +42,45 @@ pub fn broker_kafka_container_commands(
5142
true => format!("export KERBEROS_REALM=$(grep -oP 'default_realm = \\K.*' {STACKABLE_KERBEROS_KRB5_PATH})"),
5243
false => "".to_string(),
5344
},
54-
broker_start_command = broker_start_command(kafka, cluster_id, controller_descriptors, kafka_listeners, opa_connect_string, kafka_security, product_version),
45+
broker_start_command = broker_start_command(kafka, cluster_id, controller_descriptors, product_version),
5546
}
5647
}
5748

5849
fn broker_start_command(
5950
kafka: &v1alpha1::KafkaCluster,
6051
cluster_id: &str,
6152
controller_descriptors: Vec<KafkaPodDescriptor>,
62-
kafka_listeners: &KafkaListenerConfig,
63-
opa_connect_string: Option<&str>,
64-
kafka_security: &KafkaTlsSecurity,
6553
product_version: &str,
6654
) -> String {
67-
let opa_config = match opa_connect_string {
68-
None => "".to_string(),
69-
Some(opa_connect_string) => {
70-
format!(" --override \"opa.authorizer.url={opa_connect_string}\"")
71-
}
72-
};
73-
74-
let jaas_config = match kafka_security.has_kerberos_enabled() {
75-
true => {
76-
formatdoc! {"
77-
--override \"{client_jaas_config}=com.sun.security.auth.module.Krb5LoginModule required useKeyTab=true storeKey=true isInitiator=false keyTab=\\\"/stackable/kerberos/keytab\\\" principal=\\\"{service_name}/{broker_address}@$KERBEROS_REALM\\\";\" \
78-
--override \"{bootstrap_jaas_config}=com.sun.security.auth.module.Krb5LoginModule required useKeyTab=true storeKey=true isInitiator=false keyTab=\\\"/stackable/kerberos/keytab\\\" principal=\\\"{service_name}/{bootstrap_address}@$KERBEROS_REALM\\\";\"
79-
",
80-
client_jaas_config = KafkaListenerName::Client.listener_gssapi_sasl_jaas_config(),
81-
bootstrap_jaas_config = KafkaListenerName::Bootstrap.listener_gssapi_sasl_jaas_config(),
82-
service_name = KafkaRole::Broker.kerberos_service_name(),
83-
broker_address = node_address_cmd(STACKABLE_LISTENER_BROKER_DIR),
84-
bootstrap_address = node_address_cmd(STACKABLE_LISTENER_BOOTSTRAP_DIR),
85-
}
86-
}
87-
false => "".to_string(),
88-
};
89-
90-
let client_port = kafka_security.client_port();
91-
92-
// TODO: The properties file from the configmap is copied to the /tmp folder and appended with dynamic properties
93-
// This should be improved:
94-
// - mount emptyDir as readWriteConfig
95-
// - use config-utils for proper replacements?
96-
// - should we print the adapted properties file at startup?
9755
if kafka.is_controller_configured() {
9856
formatdoc! {"
99-
export REPLICA_ID=$(echo \"$POD_NAME\" | grep -oE '[0-9]+$')
57+
POD_INDEX=$(echo \"$POD_NAME\" | grep -oE '[0-9]+$')
58+
export REPLICA_ID=$((POD_INDEX+NODE_ID_OFFSET))
59+
10060
cp {config_dir}/{properties_file} /tmp/{properties_file}
61+
config-utils template /tmp/{properties_file}
10162
102-
echo \"{KAFKA_NODE_ID}=$((REPLICA_ID + ${KAFKA_NODE_ID_OFFSET}))\" >> /tmp/{properties_file}
103-
echo \"{KAFKA_CONTROLLER_QUORUM_BOOTSTRAP_SERVERS}={bootstrap_servers}\" >> /tmp/{properties_file}
104-
echo \"{KAFKA_LISTENERS}={listeners}\" >> /tmp/{properties_file}
105-
echo \"{KAFKA_ADVERTISED_LISTENERS}={advertised_listeners}\" >> /tmp/{properties_file}
106-
echo \"{KAFKA_LISTENER_SECURITY_PROTOCOL_MAP}={listener_security_protocol_map}\" >> /tmp/{properties_file}
107-
echo \"{KAFKA_CONTROLLER_QUORUM_VOTERS}={controller_quorum_voters}\" >> /tmp/{properties_file}
63+
cp {config_dir}/jaas.properties /tmp/jaas.properties
64+
config-utils template /tmp/jaas.properties
10865
10966
bin/kafka-storage.sh format --cluster-id {cluster_id} --config /tmp/{properties_file} --ignore-formatted {initial_controller_command}
110-
bin/kafka-server-start.sh /tmp/{properties_file} {opa_config}{jaas_config} &
67+
bin/kafka-server-start.sh /tmp/{properties_file} &
11168
",
11269
config_dir = STACKABLE_CONFIG_DIR,
11370
properties_file = BROKER_PROPERTIES_FILE,
114-
bootstrap_servers = to_bootstrap_servers(&controller_descriptors, client_port),
115-
listeners = kafka_listeners.listeners(),
116-
advertised_listeners = kafka_listeners.advertised_listeners(),
117-
listener_security_protocol_map = kafka_listeners.listener_security_protocol_map(),
118-
controller_quorum_voters = to_quorum_voters(&controller_descriptors, client_port),
119-
initial_controller_command = initial_controllers_command(&controller_descriptors, product_version, client_port),
71+
initial_controller_command = initial_controllers_command(&controller_descriptors, product_version),
12072
}
12173
} else {
12274
formatdoc! {"
123-
bin/kafka-server-start.sh {config_dir}/{properties_file} \
124-
--override \"zookeeper.connect=$ZOOKEEPER\" \
125-
--override \"{KAFKA_LISTENERS}={listeners}\" \
126-
--override \"{KAFKA_ADVERTISED_LISTENERS}={advertised_listeners}\" \
127-
--override \"{KAFKA_LISTENER_SECURITY_PROTOCOL_MAP}={listener_security_protocol_map}\" \
128-
{opa_config} \
129-
{jaas_config} \
130-
&",
75+
cp {config_dir}/{properties_file} /tmp/{properties_file}
76+
config-utils template /tmp/{properties_file}
77+
78+
cp {config_dir}/jaas.properties /tmp/jaas.properties
79+
config-utils template /tmp/jaas.properties
80+
81+
bin/kafka-server-start.sh /tmp/{properties_file} &",
13182
config_dir = STACKABLE_CONFIG_DIR,
13283
properties_file = BROKER_PROPERTIES_FILE,
133-
listeners = kafka_listeners.listeners(),
134-
advertised_listeners = kafka_listeners.advertised_listeners(),
135-
listener_security_protocol_map = kafka_listeners.listener_security_protocol_map(),
13684
}
13785
}
13886
}
@@ -182,31 +130,20 @@ wait_for_termination()
182130
pub fn controller_kafka_container_command(
183131
cluster_id: &str,
184132
controller_descriptors: Vec<KafkaPodDescriptor>,
185-
kafka_listeners: &KafkaListenerConfig,
186-
kafka_security: &KafkaTlsSecurity,
187133
product_version: &str,
188134
) -> String {
189-
let client_port = kafka_security.client_port();
190-
191-
// TODO: The properties file from the configmap is copied to the /tmp folder and appended with dynamic properties
192-
// This should be improved:
193-
// - mount emptyDir as readWriteConfig
194-
// - use config-utils for proper replacements?
195-
// - should we print the adapted properties file at startup?
196135
formatdoc! {"
197136
{BASH_TRAP_FUNCTIONS}
198137
{remove_vector_shutdown_file_command}
199138
prepare_signal_handlers
200139
containerdebug --output={STACKABLE_LOG_DIR}/containerdebug-state.json --loop &
201140
202-
export REPLICA_ID=$(echo \"$POD_NAME\" | grep -oE '[0-9]+$')
141+
POD_INDEX=$(echo \"$POD_NAME\" | grep -oE '[0-9]+$')
142+
export REPLICA_ID=$((POD_INDEX+NODE_ID_OFFSET))
143+
203144
cp {config_dir}/{properties_file} /tmp/{properties_file}
204145
205-
echo \"{KAFKA_NODE_ID}=$((REPLICA_ID + ${KAFKA_NODE_ID_OFFSET}))\" >> /tmp/{properties_file}
206-
echo \"{KAFKA_CONTROLLER_QUORUM_BOOTSTRAP_SERVERS}={bootstrap_servers}\" >> /tmp/{properties_file}
207-
echo \"{KAFKA_LISTENERS}={listeners}\" >> /tmp/{properties_file}
208-
echo \"{KAFKA_LISTENER_SECURITY_PROTOCOL_MAP}={listener_security_protocol_map}\" >> /tmp/{properties_file}
209-
echo \"{KAFKA_CONTROLLER_QUORUM_VOTERS}={controller_quorum_voters}\" >> /tmp/{properties_file}
146+
config-utils template /tmp/{properties_file}
210147
211148
bin/kafka-storage.sh format --cluster-id {cluster_id} --config /tmp/{properties_file} --ignore-formatted {initial_controller_command}
212149
bin/kafka-server-start.sh /tmp/{properties_file} &
@@ -217,64 +154,28 @@ pub fn controller_kafka_container_command(
217154
remove_vector_shutdown_file_command = remove_vector_shutdown_file_command(STACKABLE_LOG_DIR),
218155
config_dir = STACKABLE_CONFIG_DIR,
219156
properties_file = CONTROLLER_PROPERTIES_FILE,
220-
bootstrap_servers = to_bootstrap_servers(&controller_descriptors, client_port),
221-
listeners = to_listeners(client_port),
222-
listener_security_protocol_map = to_listener_security_protocol_map(kafka_listeners),
223-
initial_controller_command = initial_controllers_command(&controller_descriptors, product_version, client_port),
224-
controller_quorum_voters = to_quorum_voters(&controller_descriptors, client_port),
157+
initial_controller_command = initial_controllers_command(&controller_descriptors, product_version),
225158
create_vector_shutdown_file_command = create_vector_shutdown_file_command(STACKABLE_LOG_DIR)
226159
}
227160
}
228161

229-
fn to_listeners(port: u16) -> String {
230-
// The environment variables are set in the statefulset of the controller
231-
format!(
232-
"{listener_name}://$POD_NAME.$ROLEGROUP_HEADLESS_SERVICE_NAME.$NAMESPACE.svc.$CLUSTER_DOMAIN:{port}",
233-
listener_name = KafkaListenerName::Controller
234-
)
235-
}
236-
237-
fn to_listener_security_protocol_map(kafka_listeners: &KafkaListenerConfig) -> String {
238-
kafka_listeners
239-
.listener_security_protocol_map_for_listener(&KafkaListenerName::Controller)
240-
.unwrap_or("".to_string())
241-
}
242-
243-
fn to_initial_controllers(controller_descriptors: &[KafkaPodDescriptor], port: u16) -> String {
244-
controller_descriptors
245-
.iter()
246-
.map(|desc| desc.as_voter(port))
247-
.collect::<Vec<String>>()
248-
.join(",")
249-
}
250-
251-
// TODO: This can be removed once 3.7.2 is removed. Used in command.rs.
252-
fn to_quorum_voters(controller_descriptors: &[KafkaPodDescriptor], port: u16) -> String {
253-
controller_descriptors
254-
.iter()
255-
.map(|desc| desc.as_quorum_voter(port))
256-
.collect::<Vec<String>>()
257-
.join(",")
258-
}
259-
260-
fn to_bootstrap_servers(controller_descriptors: &[KafkaPodDescriptor], port: u16) -> String {
162+
fn to_initial_controllers(controller_descriptors: &[KafkaPodDescriptor]) -> String {
261163
controller_descriptors
262164
.iter()
263-
.map(|desc| format!("{fqdn}:{port}", fqdn = desc.fqdn()))
165+
.map(|desc| desc.as_voter())
264166
.collect::<Vec<String>>()
265167
.join(",")
266168
}
267169

268170
fn initial_controllers_command(
269171
controller_descriptors: &[KafkaPodDescriptor],
270172
product_version: &str,
271-
client_port: u16,
272173
) -> String {
273174
match product_version.starts_with("3.7") {
274175
true => "".to_string(),
275176
false => format!(
276177
"--initial-controllers {initial_controllers}",
277-
initial_controllers = to_initial_controllers(controller_descriptors, client_port),
178+
initial_controllers = to_initial_controllers(controller_descriptors),
278179
),
279180
}
280181
}

rust/operator-binary/src/config/node_id_hasher.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ use stackable_operator::role_utils::RoleGroupRef;
22

33
use crate::crd::v1alpha1::KafkaCluster;
44

5+
/// The Kafka node.id needs to be unique across the Kafka cluster.
6+
/// This function generates an integer that is stable for a given role group
7+
/// regardless if broker or controllers.
8+
/// This integer is then added to the pod index to compute the final node.id
9+
/// The node.id is only set and used in Kraft mode.
10+
/// Warning: this is not safe from collisions.
511
pub fn node_id_hash32_offset(rolegroup_ref: &RoleGroupRef<KafkaCluster>) -> u32 {
612
let hash = fnv_hash32(&format!(
713
"{role}-{rolegroup}",

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,6 @@ impl KafkaListenerName {
9999
listener_name = self.to_string().to_lowercase()
100100
)
101101
}
102-
103-
pub fn listener_gssapi_sasl_jaas_config(&self) -> String {
104-
format!(
105-
"listener.name.{listener_name}.gssapi.sasl.jaas.config",
106-
listener_name = self.to_string().to_lowercase()
107-
)
108-
}
109102
}
110103

111104
#[derive(Debug)]
@@ -330,21 +323,29 @@ pub fn get_kafka_listener_config(
330323
})
331324
}
332325

333-
pub fn node_address_cmd(directory: &str) -> String {
326+
pub fn node_address_cmd_env(directory: &str) -> String {
334327
format!("$(cat {directory}/default-address/address)")
335328
}
336329

337-
pub fn node_port_cmd(directory: &str, port_name: &str) -> String {
330+
pub fn node_port_cmd_env(directory: &str, port_name: &str) -> String {
338331
format!("$(cat {directory}/default-address/ports/{port_name})")
339332
}
340333

334+
pub fn node_address_cmd(directory: &str) -> String {
335+
format!("${{file:UTF-8:{directory}/default-address/address}}")
336+
}
337+
338+
pub fn node_port_cmd(directory: &str, port_name: &str) -> String {
339+
format!("${{file:UTF-8:{directory}/default-address/ports/{port_name}}}")
340+
}
341+
341342
pub fn pod_fqdn(
342343
kafka: &v1alpha1::KafkaCluster,
343344
sts_service_name: &str,
344345
cluster_info: &KubernetesClusterInfo,
345346
) -> Result<String, KafkaListenerError> {
346347
Ok(format!(
347-
"$POD_NAME.{sts_service_name}.{namespace}.svc.{cluster_domain}",
348+
"${{env:POD_NAME}}.{sts_service_name}.{namespace}.svc.{cluster_domain}",
348349
namespace = kafka.namespace().context(ObjectHasNoNamespaceSnafu)?,
349350
cluster_domain = cluster_info.cluster_domain
350351
))

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,15 @@ impl v1alpha1::KafkaCluster {
254254
})
255255
}
256256

257-
/// List all pod descriptors of a provided role expected to form the cluster.
258-
///
257+
/// List pod descriptors for a given role and all its rolegroups.
258+
/// If no role is provided, pod descriptors for all roles (and all groups) are listed.
259259
/// We try to predict the pods here rather than looking at the current cluster state in order to
260260
/// avoid instance churn.
261261
pub fn pod_descriptors(
262262
&self,
263-
requested_kafka_role: &KafkaRole,
263+
requested_kafka_role: Option<&KafkaRole>,
264264
cluster_info: &KubernetesClusterInfo,
265+
client_port: u16,
265266
) -> Result<Vec<KafkaPodDescriptor>, Error> {
266267
let namespace = self.metadata.namespace.clone().context(NoNamespaceSnafu)?;
267268
let mut pod_descriptors = Vec::new();
@@ -289,17 +290,19 @@ impl v1alpha1::KafkaCluster {
289290
}
290291
};
291292

292-
// only return descriptors for selected role
293-
if current_role == *requested_kafka_role {
293+
// If no specific role is requested, or the current role matches the requested one, add pod descriptors
294+
if requested_kafka_role.is_none() || Some(&current_role) == requested_kafka_role {
294295
for replica in 0..replicas {
295296
pod_descriptors.push(KafkaPodDescriptor {
296297
namespace: namespace.clone(),
298+
role: current_role.to_string(),
297299
role_group_service_name: rolegroup_ref
298300
.rolegroup_headless_service_name(),
299301
role_group_statefulset_name: rolegroup_ref.object_name(),
300302
replica,
301303
cluster_domain: cluster_info.cluster_domain.clone(),
302304
node_id: node_id_hash_offset + u32::from(replica),
305+
client_port,
303306
});
304307
}
305308
}
@@ -348,6 +351,8 @@ pub struct KafkaPodDescriptor {
348351
replica: u16,
349352
cluster_domain: DomainName,
350353
node_id: u32,
354+
pub role: String,
355+
pub client_port: u16,
351356
}
352357

353358
impl KafkaPodDescriptor {
@@ -376,18 +381,20 @@ impl KafkaPodDescriptor {
376381
/// * controller-0 is the replica's host,
377382
/// * 1234 is the replica's port.
378383
// NOTE(@maltesander): Even though the used Uuid states to be type 4 it does not work... 0000000000-00000000000 works...
379-
pub fn as_voter(&self, port: u16) -> String {
384+
pub fn as_voter(&self) -> String {
380385
format!(
381386
"{node_id}@{fqdn}:{port}:0000000000-{node_id:0>11}",
382387
node_id = self.node_id,
388+
port = self.client_port,
383389
fqdn = self.fqdn(),
384390
)
385391
}
386392

387-
pub fn as_quorum_voter(&self, port: u16) -> String {
393+
pub fn as_quorum_voter(&self) -> String {
388394
format!(
389395
"{node_id}@{fqdn}:{port}",
390396
node_id = self.node_id,
397+
port = self.client_port,
391398
fqdn = self.fqdn(),
392399
)
393400
}

0 commit comments

Comments
 (0)