refactor: add namespace, ui to ValidatedCluster, cleanup raw ZkCluster references

Author: maltesander

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

@@ -6,8 +6,7 @@ use stackable_operator::{
 
 use crate::crd::{
     JMX_METRICS_PORT, LoggingFramework, STACKABLE_CONFIG_DIR, STACKABLE_LOG_CONFIG_DIR,
-    ZookeeperServerRoleType,
-    v1alpha1::{ZookeeperCluster, ZookeeperConfig},
+    ZookeeperServerRoleType, logging_framework, v1alpha1::ZookeeperConfig,
 };
 
 const JAVA_HEAP_FACTOR: f32 = 0.8;
@@ -37,12 +36,11 @@ pub enum Error {
 
 /// All JVM arguments.
 fn construct_jvm_args(
-    zk: &ZookeeperCluster,
     role: &ZookeeperServerRoleType,
     role_group: &str,
     product_version: &str,
 ) -> Result<Vec<String>, Error> {
-    let logging_framework = zk.logging_framework(product_version);
+    let logging_framework = logging_framework(product_version);
 
     let jvm_args = vec![
         format!("-Djava.security.properties={STACKABLE_CONFIG_DIR}/{JVM_SECURITY_PROPERTIES_FILE}"),
@@ -72,12 +70,11 @@ fn construct_jvm_args(
 /// Arguments that go into `SERVER_JVMFLAGS`, so *not* the heap settings (which you can get using
 /// [`construct_zk_server_heap_env`]).
 pub fn construct_non_heap_jvm_args(
-    zk: &ZookeeperCluster,
     role: &ZookeeperServerRoleType,
     role_group: &str,
     product_version: &str,
 ) -> Result<String, Error> {
-    let mut jvm_args = construct_jvm_args(zk, role, role_group, product_version)?;
+    let mut jvm_args = construct_jvm_args(role, role_group, product_version)?;
     jvm_args.retain(|arg| !is_heap_jvm_argument(arg));
 
     Ok(jvm_args.join(" "))
@@ -110,7 +107,7 @@ fn is_heap_jvm_argument(jvm_argument: &str) -> bool {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::crd::ZookeeperRole;
+    use crate::crd::{ZookeeperRole, v1alpha1::ZookeeperCluster};
 
     #[test]
     fn test_construct_jvm_arguments_defaults() {
@@ -128,13 +125,9 @@ mod tests {
                 replicas: 1
         "#;
         let (zookeeper, merged_config, role, rolegroup) = construct_boilerplate(input);
-        let non_heap_jvm_args = construct_non_heap_jvm_args(
-            &zookeeper,
-            &role,
-            &rolegroup,
-            zookeeper.spec.image.product_version(),
-        )
-        .expect("test: function must pass");
+        let non_heap_jvm_args =
+            construct_non_heap_jvm_args(&role, &rolegroup, zookeeper.spec.image.product_version())
+                .expect("test: function must pass");
         let zk_server_heap_env =
             construct_zk_server_heap_env(&merged_config).expect("test: function must pass");
 
@@ -180,13 +173,9 @@ mod tests {
                     - -Dhttps.proxyPort=1234
         "#;
         let (zookeeper, merged_config, role, rolegroup) = construct_boilerplate(input);
-        let non_heap_jvm_args = construct_non_heap_jvm_args(
-            &zookeeper,
-            &role,
-            &rolegroup,
-            zookeeper.spec.image.product_version(),
-        )
-        .expect("test: function must pass");
+        let non_heap_jvm_args =
+            construct_non_heap_jvm_args(&role, &rolegroup, zookeeper.spec.image.product_version())
+                .expect("test: function must pass");
         let zk_server_heap_env =
             construct_zk_server_heap_env(&merged_config).expect("test: function must pass");
 

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

@@ -448,22 +448,23 @@ impl ZookeeperPodRef {
     }
 }
 
-impl v1alpha1::ZookeeperCluster {
-    pub fn logging_framework(&self, product_version: &str) -> LoggingFramework {
-        let zookeeper_versions_with_log4j = [
-            "1.", "2.", "3.0.", "3.1.", "3.2.", "3.3.", "3.4.", "3.5.", "3.6.", "3.7.",
-        ];
-
-        if zookeeper_versions_with_log4j
-            .into_iter()
-            .any(|prefix| product_version.starts_with(prefix))
-        {
-            LoggingFramework::LOG4J
-        } else {
-            LoggingFramework::LOGBACK
-        }
+/// Returns the [`LoggingFramework`] used by the given ZooKeeper `product_version`.
+pub fn logging_framework(product_version: &str) -> LoggingFramework {
+    let zookeeper_versions_with_log4j = [
+        "1.", "2.", "3.0.", "3.1.", "3.2.", "3.3.", "3.4.", "3.5.", "3.6.", "3.7.",
+    ];
+
+    if zookeeper_versions_with_log4j
+        .into_iter()
+        .any(|prefix| product_version.starts_with(prefix))
+    {
+        LoggingFramework::LOG4J
+    } else {
+        LoggingFramework::LOGBACK
     }
+}
 
+impl v1alpha1::ZookeeperCluster {
     /// The fully-qualified domain name of the role-level [Listener]
     ///
     /// [Listener]: stackable_operator::crd::listener::v1alpha1::Listener

rust/operator-binary/src/zk_controller.rs

@@ -377,10 +377,8 @@ pub async fn reconcile_zk(
         .context(BuildServiceSnafu)?;
         let rg_configmap = build::config_map::build_server_rolegroup_config_map(
             &validated_cluster,
-            &zk_role,
             &rolegroup,
             rolegroup_config,
-            zk,
         )
         .context(BuildRoleGroupConfigMapSnafu {
             rolegroup: rolegroup.clone(),
@@ -453,7 +451,7 @@ pub async fn reconcile_zk(
     // We don't /need/ stability, but it's still nice to avoid spurious changes where possible.
     let mut discovery_hash = FnvHasher::with_key(0);
     let discovery_cm = build::discovery::build_discovery_configmap(
-        zk,
+        &validated_cluster,
         ZK_CONTROLLER_NAME,
         applied_listener,
         None,
@@ -679,7 +677,6 @@ fn build_server_rolegroup_statefulset(
         .add_env_var(
             "SERVER_JVMFLAGS",
             construct_non_heap_jvm_args(
-                zk,
                 role,
                 &rolegroup_ref.role_group,
                 &resolved_product_image.product_version,
@@ -898,8 +895,11 @@ pub fn error_policy(
 #[cfg(test)]
 mod tests {
     use stackable_operator::{
-        commons::networking::DomainName, k8s_openapi::api::core::v1::ConfigMap,
-        role_utils::JavaCommonConfig, utils::cluster_info::KubernetesClusterInfo,
+        commons::networking::DomainName,
+        k8s_openapi::api::core::v1::ConfigMap,
+        role_utils::JavaCommonConfig,
+        utils::cluster_info::KubernetesClusterInfo,
+        v2::controller_utils::{get_cluster_name, get_namespace, get_uid},
     };
 
     use super::*;
@@ -1054,7 +1054,7 @@ mod tests {
     fn build_config_map(zookeeper_yaml: &str) -> ConfigMap {
         let mut zookeeper: v1alpha1::ZookeeperCluster =
             serde_yaml::from_str(zookeeper_yaml).expect("illegal test input");
-        zookeeper.metadata.uid = Some("42".to_owned());
+        zookeeper.metadata.uid = Some("c27b3971-ca72-42c1-80a4-abdfc1db0ddd".to_owned());
         zookeeper.metadata.namespace = Some("default".to_owned());
         let cluster_info = KubernetesClusterInfo {
             cluster_domain: DomainName::try_from("cluster.local").unwrap(),
@@ -1100,25 +1100,25 @@ mod tests {
             })
             .collect();
 
-        let validated_cluster = ValidatedCluster {
-            name: zookeeper.name_any(),
+        let validated_cluster = ValidatedCluster::new(
+            get_cluster_name(&zookeeper).unwrap(),
+            get_namespace(&zookeeper).unwrap(),
+            get_uid(&zookeeper).unwrap(),
             image,
-            cluster_config: ValidatedClusterConfig {
+            ValidatedClusterConfig {
                 zookeeper_security,
                 server_addresses,
             },
             role_group_configs,
-        };
+        );
 
         let rolegroup_ref = zookeeper.server_rolegroup_ref("default");
         let rolegroup_config = &validated_cluster.role_group_configs[&zk_role]["default"];
 
         build::config_map::build_server_rolegroup_config_map(
             &validated_cluster,
-            &zk_role,
             &rolegroup_ref,
             rolegroup_config,
-            &zookeeper,
         )
         .unwrap()
     }

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

@@ -12,11 +12,14 @@ use stackable_operator::{
     builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
     k8s_openapi::api::core::v1::ConfigMap,
     role_utils::RoleGroupRef,
-    v2::config_file_writer::{PropertiesWriterError, to_java_properties_string},
+    v2::{
+        builder::meta::ownerreference_from_resource,
+        config_file_writer::{PropertiesWriterError, to_java_properties_string},
+    },
 };
 
 use crate::{
-    crd::{ZookeeperRole, v1alpha1},
+    crd::{logging_framework, v1alpha1},
     utils::build_recommended_labels,
     zk_controller::{
         ZK_CONTROLLER_NAME,
@@ -36,22 +39,11 @@ pub enum Error {
         rolegroup: RoleGroupRef<v1alpha1::ZookeeperCluster>,
     },
 
-    #[snafu(display("object is missing metadata to build owner reference"))]
-    ObjectMissingMetadataForOwnerRef {
-        source: stackable_operator::builder::meta::Error,
-    },
-
     #[snafu(display("failed to build object meta data"))]
     ObjectMeta {
         source: stackable_operator::builder::meta::Error,
     },
 
-    #[snafu(display("failed to add the logging configuration to the ConfigMap [{cm_name}]"))]
-    InvalidLoggingConfig {
-        source: logging::Error,
-        cm_name: String,
-    },
-
     #[snafu(display("failed to build ConfigMap for {rolegroup}"))]
     BuildConfigMap {
         source: stackable_operator::builder::configmap::Error,
@@ -61,16 +53,14 @@ pub enum Error {
 
 type Result<T, E = Error> = std::result::Result<T, E>;
 
-/// Builds the rolegroup [`ConfigMap`].
+/// Builds the rolegroup [`ConfigMap`] entirely from the [`ValidatedCluster`].
 ///
-/// `owner` is the [`v1alpha1::ZookeeperCluster`] and is used solely for the owner
-/// reference and object metadata (name, namespace, labels).
+/// The owner reference and object metadata (name, namespace, labels) are derived
+/// from `cluster`, which mirrors the raw [`v1alpha1::ZookeeperCluster`] metadata.
 pub fn build_server_rolegroup_config_map(
     cluster: &ValidatedCluster,
-    role: &ZookeeperRole,
     rolegroup_ref: &RoleGroupRef<v1alpha1::ZookeeperCluster>,
     rolegroup_config: &ZookeeperRoleGroupConfig,
-    owner: &v1alpha1::ZookeeperCluster,
 ) -> Result<ConfigMap> {
     let mut data: BTreeMap<String, String> = BTreeMap::new();
 
@@ -95,27 +85,20 @@ pub fn build_server_rolegroup_config_map(
     );
 
     // logback.xml / log4j.properties and vector.yaml
-    data.extend(
-        logging::build(
-            owner,
-            role.clone(),
-            rolegroup_ref,
-            &cluster.image.product_version,
-        )
-        .context(InvalidLoggingConfigSnafu {
-            cm_name: rolegroup_ref.object_name(),
-        })?,
-    );
+    data.extend(logging::build(
+        &rolegroup_config.config.logging,
+        logging_framework(&cluster.image.product_version),
+        rolegroup_ref,
+    ));
 
     ConfigMapBuilder::new()
         .metadata(
             ObjectMetaBuilder::new()
-                .name_and_namespace(owner)
+                .name_and_namespace(cluster)
                 .name(rolegroup_ref.object_name())
-                .ownerreference_from_resource(owner, None, Some(true))
-                .context(ObjectMissingMetadataForOwnerRefSnafu)?
+                .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
                 .with_recommended_labels(&build_recommended_labels(
-                    owner,
+                    cluster,
                     ZK_CONTROLLER_NAME,
                     &cluster.image.app_version_label_value,
                     &rolegroup_ref.role,

rust/operator-binary/src/zk_controller/build/properties/logging.rs

@@ -4,17 +4,16 @@
 
 use std::collections::BTreeMap;
 
-use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     memory::{BinaryMultiple, MemoryQuantity},
     product_logging::{
         self,
-        spec::{ContainerLogConfig, ContainerLogConfigChoice},
+        spec::{ContainerLogConfig, ContainerLogConfigChoice, Logging},
     },
     role_utils::RoleGroupRef,
 };
 
-use crate::crd::{LoggingFramework, STACKABLE_LOG_DIR, ZookeeperRole, v1alpha1};
+use crate::crd::{LoggingFramework, STACKABLE_LOG_DIR, v1alpha1};
 
 /// The logback config file name (when the product uses the LOGBACK framework).
 pub const LOGBACK_CONFIG_FILE: &str = "logback.xml";
@@ -29,30 +28,22 @@ pub const MAX_ZK_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity {
     unit: BinaryMultiple::Mebi,
 };
 
-#[derive(Snafu, Debug)]
-pub enum Error {
-    #[snafu(display("crd validation failure"))]
-    CrdValidationFailure { source: crate::crd::Error },
-}
-
-type Result<T, E = Error> = std::result::Result<T, E>;
-
 const CONSOLE_CONVERSION_PATTERN: &str = "%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L] - %m%n";
 
 /// Builds the logging-related ConfigMap entries (product log config and the
 /// Vector agent config) for a role group.
+///
+/// `logging` is the merged [`Logging`] from the role group's validated config and
+/// `framework` selects the product log config format (see [`logging_framework`]).
+///
+/// [`logging_framework`]: crate::crd::logging_framework
 pub fn build(
-    zk: &v1alpha1::ZookeeperCluster,
-    role: ZookeeperRole,
+    logging: &Logging<v1alpha1::Container>,
+    framework: LoggingFramework,
     rolegroup: &RoleGroupRef<v1alpha1::ZookeeperCluster>,
-    product_version: &str,
-) -> Result<BTreeMap<String, String>> {
+) -> BTreeMap<String, String> {
     let mut data = BTreeMap::new();
 
-    let logging = zk
-        .logging(&role, rolegroup)
-        .context(CrdValidationFailureSnafu)?;
-
     if let Some(ContainerLogConfig {
         choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
     }) = logging.containers.get(&v1alpha1::Container::Zookeeper)
@@ -63,7 +54,7 @@ pub fn build(
             .floor()
             .value as u32;
 
-        match zk.logging_framework(product_version) {
+        match framework {
             LoggingFramework::LOG4J => {
                 data.insert(
                     LOG4J_CONFIG_FILE.to_string(),
@@ -108,5 +99,5 @@ pub fn build(
         );
     }
 
-    Ok(data)
+    data
 }