remove clusterConfig.faultTolerantExecution.configOverrides property

Author: razvan

CHANGELOG.md

@@ -9,6 +9,10 @@ All notable changes to this project will be documented in this file.
 - Support for fault-tolerant execution ([#779]).
 - Support for the client spooling protocol ([#793]).
 
+### Removed
+
+- Cluster wide FTE specific `configOverrides` has been removed in favor of the "classic" role/group overrides ([#793]).
+
 [#779]: https://github.com/stackabletech/trino-operator/pull/779
 [#793]: https://github.com/stackabletech/trino-operator/pull/793
 

deploy/helm/trino-operator/crds/crds.yaml

@@ -299,12 +299,6 @@ spec:
                                 - required:
                                     - local
                               properties:
-                                configOverrides:
-                                  additionalProperties:
-                                    type: string
-                                  default: {}
-                                  description: The `configOverrides` allow overriding arbitrary exchange manager properties.
-                                  type: object
                                 encryptionEnabled:
                                   description: Whether to enable encryption of spooling data.
                                   nullable: true
@@ -561,12 +555,6 @@ spec:
                                 - required:
                                     - local
                               properties:
-                                configOverrides:
-                                  additionalProperties:
-                                    type: string
-                                  default: {}
-                                  description: The `configOverrides` allow overriding arbitrary exchange manager properties.
-                                  type: object
                                 encryptionEnabled:
                                   description: Whether to enable encryption of spooling data.
                                   nullable: true

rust/operator-binary/src/controller.rs

@@ -888,28 +888,33 @@ fn build_rolegroup_config_map(
                     );
                 }
             }
+            PropertyNameKind::File(file_name) if file_name == EXCHANGE_MANAGER_PROPERTIES => {
+                // Add exchange manager properties from resolved fault tolerant execution configuration
+                if let Some(resolved_fte) = resolved_fte_config {
+                    dynamic_resolved_config = resolved_fte
+                        .exchange_manager_properties
+                        .iter()
+                        .map(|(k, v)| (k.clone(), Some(v.clone())))
+                        .collect();
+                }
+
+                // Override automatic properties with user provided configuration for the spooling protocol
+                dynamic_resolved_config.extend(transformed_config);
+
+                if !dynamic_resolved_config.is_empty() {
+                    cm_conf_data.insert(
+                        file_name.to_string(),
+                        to_java_properties_string(dynamic_resolved_config.iter())
+                            .with_context(|_| FailedToWriteJavaPropertiesSnafu)?,
+                    );
+                }
+            }
             _ => {}
         }
     }
 
     cm_conf_data.insert(JVM_CONFIG.to_string(), jvm_config.to_string());
 
-    // Add exchange manager properties from resolved fault tolerant execution configuration
-    if let Some(resolved_fte) = resolved_fte_config {
-        if !resolved_fte.exchange_manager_properties.is_empty() {
-            let exchange_props_with_options: BTreeMap<String, Option<String>> = resolved_fte
-                .exchange_manager_properties
-                .iter()
-                .map(|(k, v)| (k.clone(), Some(v.clone())))
-                .collect();
-            cm_conf_data.insert(
-                EXCHANGE_MANAGER_PROPERTIES.to_string(),
-                to_java_properties_string(exchange_props_with_options.iter())
-                    .with_context(|_| FailedToWriteJavaPropertiesSnafu)?,
-            );
-        }
-    }
-
     let jvm_sec_props: BTreeMap<String, Option<String>> = config
         .get(&PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()))
         .cloned()
@@ -1437,6 +1442,7 @@ fn validated_product_config(
         PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()),
         PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()),
         PropertyNameKind::File(SPOOLING_MANAGER_PROPERTIES.to_string()),
+        PropertyNameKind::File(EXCHANGE_MANAGER_PROPERTIES.to_string()),
     ];
 
     let coordinator_role = TrinoRole::Coordinator;
@@ -1953,6 +1959,7 @@ mod tests {
             PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()),
             PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()),
             PropertyNameKind::File(SPOOLING_MANAGER_PROPERTIES.to_string()),
+            PropertyNameKind::File(EXCHANGE_MANAGER_PROPERTIES.to_string()),
         ];
         let validated_config = validate_all_roles_and_groups_config(
             // The Trino version is a single number like 396.

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

@@ -5,7 +5,7 @@
 //!
 //! Based on the Trino documentation: <https://trino.io/docs/current/admin/fault-tolerant-execution.html>
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
 
 use serde::{Deserialize, Serialize};
 use snafu::Snafu;
@@ -124,10 +124,6 @@ pub struct ExchangeManagerConfig {
     /// Backend-specific configuration.
     #[serde(flatten)]
     pub backend: ExchangeManagerBackend,
-
-    /// The `configOverrides` allow overriding arbitrary exchange manager properties.
-    #[serde(default)]
-    pub config_overrides: HashMap<String, String>,
 }
 
 #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
@@ -440,8 +436,6 @@ impl ResolvedFaultTolerantExecutionConfig {
                     );
                 }
             }
-
-            exchange_manager_properties.extend(exchange_config.config_overrides.clone());
         }
 
         let mut resolved_config = Self {
@@ -562,6 +556,7 @@ impl ResolvedFaultTolerantExecutionConfig {
 
 #[cfg(test)]
 mod tests {
+
     use super::*;
 
     #[tokio::test]
@@ -625,7 +620,6 @@ mod tests {
                 backend: ExchangeManagerBackend::Local(LocalExchangeConfig {
                     base_directories: vec!["/tmp/exchange".to_string()],
                 }),
-                config_overrides: HashMap::new(),
             }),
         });
 
@@ -705,7 +699,6 @@ mod tests {
                     max_error_retries: Some(5),
                     upload_part_size: Some(Quantity("10Mi".to_string())),
                 }),
-                config_overrides: std::collections::HashMap::new(),
             },
         });
 
@@ -787,59 +780,4 @@ mod tests {
             Some(&"8".to_string())
         );
     }
-
-    #[tokio::test]
-    async fn test_exchange_manager_config_overrides() {
-        let mut config_overrides = HashMap::new();
-        config_overrides.insert("custom.property".to_string(), "custom-value".to_string());
-        config_overrides.insert(
-            "exchange.s3.upload.part-size".to_string(),
-            "overridden-value".to_string(),
-        );
-
-        let config = FaultTolerantExecutionConfig::Task(TaskRetryConfig {
-            retry_attempts_per_task: Some(2),
-            retry_initial_delay: None,
-            retry_max_delay: None,
-            retry_delay_scale_factor: None,
-            exchange_deduplication_buffer_size: None,
-            exchange_manager: ExchangeManagerConfig {
-                encryption_enabled: None,
-                sink_buffer_pool_min_size: None,
-                sink_buffers_per_partition: None,
-                sink_max_file_size: None,
-                source_concurrent_readers: None,
-                backend: ExchangeManagerBackend::S3(S3ExchangeConfig {
-                    base_directories: vec!["s3://my-bucket/exchange".to_string()],
-                    connection: stackable_operator::crd::s3::v1alpha1::InlineConnectionOrReference::Reference(
-                        "test-s3-connection".to_string()
-                    ),
-                    iam_role: None,
-                    external_id: None,
-                    max_error_retries: None,
-                    upload_part_size: Some(Quantity("10Mi".to_string())),
-                }),
-                config_overrides,
-            },
-        });
-
-        let fte_config =
-            ResolvedFaultTolerantExecutionConfig::from_config(&config, None, "default")
-                .await
-                .unwrap();
-
-        assert_eq!(
-            fte_config
-                .exchange_manager_properties
-                .get("custom.property"),
-            Some(&"custom-value".to_string())
-        );
-
-        assert_eq!(
-            fte_config
-                .exchange_manager_properties
-                .get("exchange.s3.upload.part-size"),
-            Some(&"overridden-value".to_string())
-        );
-    }
 }