Skip to content

Commit 33a0b1b

Browse files
maltesandermaltesander
committed
fix / improve todos
1 parent d8144db commit 33a0b1b

5 files changed

Lines changed: 31 additions & 23 deletions

File tree

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

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,11 @@ fn broker_start_command(
8989

9090
let client_port = kafka_security.client_port();
9191

92-
// TODO: copy to tmp? mount readwrite folder?
93-
// TODO: do "cat /tmp/{properties_file}" ?
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?
9497
if kafka.is_controller_configured() {
9598
formatdoc! {"
9699
export REPLICA_ID=$(echo \"$POD_NAME\" | grep -oE '[0-9]+$')
@@ -143,7 +146,11 @@ pub fn controller_kafka_container_command(
143146
) -> String {
144147
let client_port = kafka_security.client_port();
145148

146-
// TODO: copy to tmp? mount readwrite folder?
149+
// TODO: The properties file from the configmap is copied to the /tmp folder and appended with dynamic properties
150+
// This should be improved:
151+
// - mount emptyDir as readWriteConfig
152+
// - use config-utils for proper replacements?
153+
// - should we print the adapted properties file at startup?
147154
formatdoc! {"
148155
{COMMON_BASH_TRAP_FUNCTIONS}
149156
{remove_vector_shutdown_file_command}
@@ -165,16 +172,24 @@ pub fn controller_kafka_container_command(
165172
wait_for_termination $!
166173
{create_vector_shutdown_file_command}
167174
",
168-
remove_vector_shutdown_file_command = remove_vector_shutdown_file_command(STACKABLE_LOG_DIR),
169-
config_dir = STACKABLE_CONFIG_DIR,
170-
properties_file = CONTROLLER_PROPERTIES_FILE,
171-
bootstrap_servers = to_bootstrap_servers(&controller_descriptors, client_port),
172-
listeners = to_listeners(client_port),
173-
listener_security_protocol_map = to_listener_security_protocol_map(kafka_listeners),
174-
initial_controller_command = initial_controllers_command(&controller_descriptors, product_version, client_port),
175-
controller_quorum_voters = to_quorum_voters(&controller_descriptors, client_port),
176-
create_vector_shutdown_file_command = create_vector_shutdown_file_command(STACKABLE_LOG_DIR)
177-
}
175+
remove_vector_shutdown_file_command = remove_vector_shutdown_file_command(STACKABLE_LOG_DIR),
176+
config_dir = STACKABLE_CONFIG_DIR,
177+
properties_file = CONTROLLER_PROPERTIES_FILE,
178+
bootstrap_servers = to_bootstrap_servers(&controller_descriptors, client_port),
179+
listeners = to_listeners(client_port),
180+
listener_security_protocol_map = to_listener_security_protocol_map(kafka_listeners),
181+
initial_controller_command = initial_controllers_command(&controller_descriptors, product_version, client_port),
182+
controller_quorum_voters = to_quorum_voters(&controller_descriptors, client_port),
183+
create_vector_shutdown_file_command = create_vector_shutdown_file_command(STACKABLE_LOG_DIR)
184+
185+
186+
controller.quorum.bootstrap.servers=test-kafka-controller-default-0.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093,test-kafka-controller-default-1.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093,test-kafka-controller-default-2.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093
187+
listeners=CONTROLLER://test-kafka-controller-default-1.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093
188+
listener.security.protocol.map=CONTROLLER:SSL
189+
controller.quorum.voters=2110489703@test-kafka-controller-default-0.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093,2110489704@test-kafka-controller-default-1.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093,2110489705@test-kafka-controller-default-2.test-kafka-controller-default.kuttl-test-cute-ghoul.svc.cluster.local:9093
190+
191+
192+
}
178193
}
179194

180195
fn to_listeners(port: u16) -> String {
@@ -199,6 +214,7 @@ fn to_initial_controllers(controller_descriptors: &[KafkaPodDescriptor], port: u
199214
.join(",")
200215
}
201216

217+
// TODO: This can be removed once 3.7.2 is removed. Used in command.rs.
202218
fn to_quorum_voters(controller_descriptors: &[KafkaPodDescriptor], port: u16) -> String {
203219
controller_descriptors
204220
.iter()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ impl KafkaPodDescriptor {
372372
/// See: <https://github.com/apache/kafka/blob/c5169ca805bd03d870a5bcd49744dcc34891cf15/clients/src/main/java/org/apache/kafka/common/Uuid.java#L29>
373373
/// * controller-0 is the replica's host,
374374
/// * 1234 is the replica's port.
375-
// TODO(@maltesander): Even though the used Uuid states to be type 4 it does not work... 0000000000-00000000000 works...
375+
// NOTE(@maltesander): Even though the used Uuid states to be type 4 it does not work... 0000000000-00000000000 works...
376376
pub fn as_voter(&self, port: u16) -> String {
377377
format!(
378378
"{node_id}@{fqdn}:{port}:0000000000-{node_id:0>11}",

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ impl KafkaRole {
133133

134134
/// A Kerberos principal has three parts, with the form username/fully.qualified.domain.name@YOUR-REALM.COM.
135135
/// but is similar to HBase).
136-
// TODO: split into broker / controller?
137136
pub fn kerberos_service_name(&self) -> &'static str {
138137
"kafka"
139138
}

‎rust/operator-binary/src/crd/security.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,6 @@ impl KafkaTlsSecurity {
548548
// Kerberos
549549
if self.has_kerberos_enabled() {
550550
config.insert("sasl.enabled.mechanisms".to_string(), "GSSAPI".to_string());
551-
// TODO: what service name?
552551
config.insert(
553552
"sasl.kerberos.service.name".to_string(),
554553
KafkaRole::Broker.kerberos_service_name().to_string(),
@@ -617,10 +616,9 @@ impl KafkaTlsSecurity {
617616
// Kerberos
618617
if self.has_kerberos_enabled() {
619618
config.insert("sasl.enabled.mechanisms".to_string(), "GSSAPI".to_string());
620-
// TODO: what service name?
621619
config.insert(
622620
"sasl.kerberos.service.name".to_string(),
623-
KafkaRole::Broker.kerberos_service_name().to_string(),
621+
KafkaRole::Controller.kerberos_service_name().to_string(),
624622
);
625623
config.insert(
626624
"sasl.mechanism.inter.broker.protocol".to_string(),

‎rust/operator-binary/src/resource/statefulset.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -825,11 +825,6 @@ pub fn build_controller_rolegroup_statefulset(
825825
add_graceful_shutdown_config(merged_config, &mut pod_builder).context(GracefulShutdownSnafu)?;
826826

827827
let mut pod_template = pod_builder.build_template();
828-
let pod_template_spec = pod_template.spec.get_or_insert_with(PodSpec::default);
829-
830-
// Don't run kcat pod as PID 1, to ensure that default signal handlers apply
831-
// TODO: we need that?
832-
pod_template_spec.share_process_namespace = Some(true);
833828

834829
pod_template.merge_from(
835830
kafka_role

0 commit comments

Comments
 (0)