refactor: remove ownerrefs from builder functions

Author: maltesander

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

@@ -8,7 +8,7 @@ use stackable_operator::{
 
 use crate::crd::{
     METRICS_PORT, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD,
-    TrinoRoleType, v1alpha1,
+    v1alpha1,
 };
 
 const JVM_SECURITY_PROPERTIES: &str = "security.properties";
@@ -46,8 +46,8 @@ pub enum Error {
 pub fn jvm_config(
     product_version: u16,
     merged_config: &v1alpha1::TrinoConfig,
-    role: &TrinoRoleType,
-    role_group: &str,
+    role_jvm_argument_overrides: &JvmArgumentOverrides,
+    role_group_jvm_argument_overrides: &JvmArgumentOverrides,
 ) -> Result<String, Error> {
     let memory_unit = BinaryMultiple::Mebi;
     let heap_size = MemoryQuantity::try_from(
@@ -93,8 +93,18 @@ pub fn jvm_config(
     jvm_args.push("# Arguments from jvmArgumentOverrides".to_owned());
 
     let operator_generated = JvmArgumentOverrides::new_with_only_additions(jvm_args);
-    let merged_jvm_argument_overrides = role
-        .get_merged_jvm_argument_overrides(role_group, &operator_generated)
+
+    // Merge order mirrors `Role::get_merged_jvm_argument_overrides`:
+    // 1. operator-generated args are layered on top of the role-level overrides,
+    // 2. the role-group-level overrides are applied last.
+    // Note that this is not a purely additive merge, hence the unusual order.
+    let mut from_role = role_jvm_argument_overrides.clone();
+    from_role
+        .try_merge(&operator_generated)
+        .context(MergeJvmArgumentOverridesSnafu)?;
+    let mut merged_jvm_argument_overrides = role_group_jvm_argument_overrides.clone();
+    merged_jvm_argument_overrides
+        .try_merge(&from_role)
         .context(MergeJvmArgumentOverridesSnafu)?;
 
     Ok(merged_jvm_argument_overrides
@@ -264,13 +274,24 @@ mod tests {
         let merged_config = trino.merged_config(&role, &rolegroup_ref, &[]).unwrap();
         let coordinators = trino.role(&role).unwrap();
 
+        let role_jvm_argument_overrides = coordinators
+            .config
+            .product_specific_common_config
+            .jvm_argument_overrides
+            .clone();
+        let role_group_jvm_argument_overrides = coordinators.role_groups["default"]
+            .config
+            .product_specific_common_config
+            .jvm_argument_overrides
+            .clone();
+
         let product_version = trino.spec.image.product_version();
 
         jvm_config(
             u16::from_str(product_version).expect("trino version as u16"),
             &merged_config,
-            &coordinators,
-            "default",
+            &role_jvm_argument_overrides,
+            &role_group_jvm_argument_overrides,
         )
         .unwrap()
     }

rust/operator-binary/src/controller.rs

@@ -307,6 +307,12 @@ pub async fn reconcile_trino(
     let validated_cluster =
         validate::validate(trino, &dereferenced_objects, &ctx.operator_environment)
             .context(ValidateClusterSnafu)?;
+    tracing::debug!(
+        trino.name = %validated_cluster.name,
+        trino.namespace = %validated_cluster.namespace,
+        trino.uid = %validated_cluster.uid,
+        "Validated TrinoCluster"
+    );
 
     let mut cluster_resources = ClusterResources::new(
         APP_NAME,
@@ -403,7 +409,6 @@ pub async fn reconcile_trino(
                 &role_group_ref,
                 &client.kubernetes_cluster_info,
                 &role_group_service_recommended_labels,
-                trino,
             )
             .with_context(|_| BuildRoleGroupConfigMapSnafu {
                 rolegroup: role_group_ref.clone(),
@@ -1337,7 +1342,7 @@ mod tests {
             serde_yaml::with::singleton_map_recursive::deserialize(deserializer)
                 .expect("invalid test input");
         trino.metadata.namespace = Some("default".to_owned());
-        trino.metadata.uid = Some("42".to_owned());
+        trino.metadata.uid = Some("e6ac237d-a6d4-43a1-8135-f36506110912".to_owned());
 
         let cluster_info = KubernetesClusterInfo {
             cluster_domain: DomainName::try_from("cluster.local").unwrap(),
@@ -1385,7 +1390,6 @@ mod tests {
         });
 
         let derefs = DereferencedObjects {
-            namespace: "default".parse().unwrap(),
             resolved_authentication_classes: Vec::new(),
             catalog_definitions: Vec::new(),
             catalogs: Vec::new(),
@@ -1422,7 +1426,6 @@ mod tests {
             &rolegroup_ref,
             &cluster_info,
             &recommended_labels,
-            &trino,
         )
         .expect("build_rolegroup_config_map should succeed")
     }
@@ -1598,7 +1601,7 @@ mod tests {
         metadata:
           name: trino
           namespace: default
-          uid: "42"
+          uid: "e6ac237d-a6d4-43a1-8135-f36506110912"
         spec:
           image:
             productVersion: "479"
@@ -1626,7 +1629,6 @@ mod tests {
             serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap();
 
         let derefs = DereferencedObjects {
-            namespace: "default".parse().unwrap(),
             resolved_authentication_classes: Vec::new(),
             catalog_definitions: Vec::new(),
             catalogs: Vec::new(),

rust/operator-binary/src/controller/build/config_map.rs

@@ -54,11 +54,8 @@ pub enum Error {
         source: stackable_operator::builder::meta::Error,
     },
 
-    #[snafu(display("failed to resolve the {role} role"))]
-    ReadRole {
-        source: crate::crd::Error,
-        role: String,
-    },
+    #[snafu(display("missing JVM argument overrides for role {role}"))]
+    MissingRoleJvmArgumentOverrides { role: String },
 
     #[snafu(display("failed to build jvm.config"))]
     BuildJvmConfig { source: crate::config::jvm::Error },
@@ -72,7 +69,6 @@ pub fn build_rolegroup_config_map(
     rolegroup_ref: &RoleGroupRef<v1alpha1::TrinoCluster>,
     cluster_info: &KubernetesClusterInfo,
     recommended_labels: &ObjectLabels<'_, v1alpha1::TrinoCluster>,
-    owner_target: &v1alpha1::TrinoCluster,
 ) -> Result<ConfigMap> {
     let role_group_configs =
         cluster
@@ -174,14 +170,18 @@ pub fn build_rolegroup_config_map(
     }
 
     // 8. jvm.config.
-    let role_obj = owner_target.role(role).with_context(|_| ReadRoleSnafu {
-        role: role.to_string(),
-    })?;
+    let role_jvm_argument_overrides =
+        cluster
+            .role_jvm_argument_overrides
+            .get(role)
+            .with_context(|| MissingRoleJvmArgumentOverridesSnafu {
+                role: role.to_string(),
+            })?;
     let jvm_config = jvm::jvm_config(
         cluster.product_version,
         &rg.config,
-        &role_obj,
-        &rolegroup_ref.role_group,
+        role_jvm_argument_overrides,
+        &rg.product_specific_common_config.jvm_argument_overrides,
     )
     .context(BuildJvmConfigSnafu)?;
     data.insert(JVM_CONFIG.to_string(), jvm_config);
@@ -197,9 +197,9 @@ pub fn build_rolegroup_config_map(
     ConfigMapBuilder::new()
         .metadata(
             ObjectMetaBuilder::new()
-                .name_and_namespace(owner_target)
                 .name(rolegroup_ref.object_name())
-                .ownerreference_from_resource(owner_target, None, Some(true))
+                .namespace(cluster.namespace.to_string())
+                .ownerreference_from_resource(cluster, None, Some(true))
                 .context(MetadataSnafu)?
                 .with_recommended_labels(recommended_labels)
                 .context(MetadataSnafu)?

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

@@ -58,7 +58,6 @@ pub(crate) mod test_support {
     pub fn validated_cluster_from_yaml(yaml: &str) -> ValidatedCluster {
         let trino: v1alpha1::TrinoCluster = serde_yaml::from_str(yaml).expect("invalid test YAML");
         let derefs = DereferencedObjects {
-            namespace: "default".parse().unwrap(),
             resolved_authentication_classes: Vec::new(),
             catalog_definitions: Vec::new(),
             catalogs: Vec::new(),
@@ -81,7 +80,7 @@ pub(crate) mod test_support {
         metadata:
           name: simple-trino
           namespace: default
-          uid: "42"
+          uid: "e6ac237d-a6d4-43a1-8135-f36506110912"
         spec:
           image:
             productVersion: "479"

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

@@ -10,9 +10,7 @@ use std::{num::ParseIntError, str::FromStr};
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
-    client::Client,
-    kube::runtime::reflector::ObjectRef,
-    v2::{controller_utils::get_namespace, types::kubernetes::NamespaceName},
+    client::Client, kube::runtime::reflector::ObjectRef, v2::controller_utils::get_namespace,
 };
 
 use crate::{
@@ -76,9 +74,6 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 /// Kubernetes objects referenced from the TrinoCluster spec, already fetched (and, for now, partly
 /// validated by the existing helper functions).
 pub struct DereferencedObjects {
-    // Catalogs are dereferenced and require the namespace, so we check
-    // or validate the namespace here once and pass it to validate.
-    pub namespace: NamespaceName,
     pub resolved_authentication_classes: Vec<ResolvedAuthenticationClassRef>,
     pub catalog_definitions: Vec<catalog::v1alpha1::TrinoCatalog>,
     pub catalogs: Vec<CatalogConfig>,
@@ -160,7 +155,6 @@ pub async fn dereference(
     };
 
     Ok(DereferencedObjects {
-        namespace,
         resolved_authentication_classes,
         catalog_definitions,
         catalogs,