test: add test coverage for beat replicas, superset config and vector path

Author: maltesander

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

@@ -270,3 +270,105 @@ fn rolegroup_properties(
 
     properties
 }
+
+#[cfg(test)]
+mod tests {
+    use stackable_operator::{
+        commons::{affinity::StackableAffinity, resources::Resources},
+        product_logging::spec::AutomaticContainerLogConfig,
+        v2::{
+            config_overrides::KeyValueConfigOverrides,
+            product_logging::framework::ValidatedContainerLogConfigChoice,
+        },
+    };
+
+    use super::{DEFAULT_ROW_LIMIT, DEFAULT_WEBSERVER_TIMEOUT, rolegroup_properties};
+    use crate::{
+        controller::{ValidatedLogging, ValidatedSupersetConfig},
+        crd::{SupersetConfigOptions, SupersetRole, v1alpha1::SupersetConfigOverrides},
+    };
+
+    /// Builds a [`ValidatedSupersetConfig`] with only the fields that affect `rolegroup_properties`
+    /// set; everything else defaults.
+    fn validated_config(
+        row_limit: Option<i32>,
+        webserver_timeout: Option<u32>,
+    ) -> ValidatedSupersetConfig {
+        ValidatedSupersetConfig {
+            affinity: StackableAffinity::default(),
+            graceful_shutdown_timeout: None,
+            logging: ValidatedLogging {
+                superset_container: ValidatedContainerLogConfigChoice::Automatic(
+                    AutomaticContainerLogConfig::default(),
+                ),
+                vector_container: None,
+                enable_vector_agent: false,
+            },
+            resources: Resources::default(),
+            row_limit,
+            webserver_timeout,
+        }
+    }
+
+    fn row_limit_key() -> String {
+        SupersetConfigOptions::RowLimit.to_string()
+    }
+
+    fn webserver_timeout_key() -> String {
+        SupersetConfigOptions::SupersetWebserverTimeout.to_string()
+    }
+
+    /// The `ROW_LIMIT`/`SUPERSET_WEBSERVER_TIMEOUT` defaults are only emitted for the `Node` role.
+    #[test]
+    fn rolegroup_properties_defaults_are_node_only() {
+        let worker = rolegroup_properties(
+            &SupersetRole::Worker,
+            &validated_config(None, None),
+            &SupersetConfigOverrides::default(),
+        );
+        assert!(!worker.contains_key(&row_limit_key()));
+        assert!(!worker.contains_key(&webserver_timeout_key()));
+
+        let node = rolegroup_properties(
+            &SupersetRole::Node,
+            &validated_config(None, None),
+            &SupersetConfigOverrides::default(),
+        );
+        assert_eq!(
+            node.get(&row_limit_key()),
+            Some(&DEFAULT_ROW_LIMIT.to_string())
+        );
+        assert_eq!(
+            node.get(&webserver_timeout_key()),
+            Some(&DEFAULT_WEBSERVER_TIMEOUT.to_string())
+        );
+    }
+
+    /// A typed `row_limit`/`webserver_timeout` overrides the Node default.
+    #[test]
+    fn rolegroup_properties_typed_fields_override_defaults() {
+        let node = rolegroup_properties(
+            &SupersetRole::Node,
+            &validated_config(Some(10), Some(600)),
+            &SupersetConfigOverrides::default(),
+        );
+        assert_eq!(node.get(&row_limit_key()), Some(&"10".to_string()));
+        assert_eq!(node.get(&webserver_timeout_key()), Some(&"600".to_string()));
+    }
+
+    /// `configOverrides` are applied last and win over both the default and the typed field.
+    #[test]
+    fn rolegroup_properties_config_overrides_win() {
+        let mut superset_config_py = KeyValueConfigOverrides::default();
+        superset_config_py
+            .overrides
+            .insert(row_limit_key(), "99".to_string());
+
+        let node = rolegroup_properties(
+            &SupersetRole::Node,
+            &validated_config(Some(10), None),
+            &SupersetConfigOverrides { superset_config_py },
+        );
+        assert_eq!(node.get(&row_limit_key()), Some(&"99".to_string()));
+    }
+}

rust/operator-binary/src/controller/build/resource/deployment.rs

@@ -85,24 +85,11 @@ pub fn build_rolegroup_deployment(
             worker_liveness_probe(),
             rolegroup_config.replicas,
         ),
-        SupersetRole::Beat => {
-            // Beat must only ever run a single instance, so the replica count is never left
-            // unset (no HorizontalPodAutoscaler): an explicit `0` is honoured (to stop Beat),
-            // any value `> 1` is clamped to `1`, and an unset value defaults to `1`.
-            let replicas = match rolegroup_config.replicas {
-                Some(0) => Some(0),
-                Some(replicas) if replicas > 1 => {
-                    tracing::warn! {"replicas for role `beat` set to greater `1`. Multiple beat instances are not allowed. Setting to `1` replica."}
-                    Some(1)
-                }
-                _ => Some(1),
-            };
-            (
-                format!("{CELERY_APP_INVOCATION} beat --pidfile {CELERY_BEAT_PIDFILE}"),
-                beat_liveness_probe(),
-                replicas,
-            )
-        }
+        SupersetRole::Beat => (
+            format!("{CELERY_APP_INVOCATION} beat --pidfile {CELERY_BEAT_PIDFILE}"),
+            beat_liveness_probe(),
+            beat_replicas(rolegroup_config.replicas),
+        ),
         SupersetRole::Node => {
             unreachable!("the `Node` role is deployed as a StatefulSet, not a Deployment")
         }
@@ -232,3 +219,39 @@ fn beat_liveness_probe() -> Probe {
         ..Default::default()
     }
 }
+
+/// Computes the replica count for the Celery `beat` role.
+///
+/// Beat is a singleton scheduler, so it must never run more than one instance: an explicit `0` is
+/// honoured (to stop Beat), any value `> 1` is clamped to `1` (with a warning), and an unset value
+/// defaults to `1`. The result is always `Some`, so no HorizontalPodAutoscaler can own the count.
+fn beat_replicas(requested: Option<u16>) -> Option<u16> {
+    match requested {
+        Some(0) => Some(0),
+        Some(replicas) if replicas > 1 => {
+            tracing::warn!(
+                "replicas for role `beat` set to greater `1`. Multiple beat instances are not allowed. Setting to `1` replica."
+            );
+            Some(1)
+        }
+        _ => Some(1),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::beat_replicas;
+
+    #[test]
+    fn beat_replicas_clamps_to_a_single_instance() {
+        // An unset replica count defaults to a single instance.
+        assert_eq!(beat_replicas(None), Some(1));
+        // An explicit `0` is honoured so Beat can be stopped.
+        assert_eq!(beat_replicas(Some(0)), Some(0));
+        // A single instance is kept as-is.
+        assert_eq!(beat_replicas(Some(1)), Some(1));
+        // Anything greater than one is clamped down to a single instance.
+        assert_eq!(beat_replicas(Some(2)), Some(1));
+        assert_eq!(beat_replicas(Some(100)), Some(1));
+    }
+}

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

@@ -255,9 +255,17 @@ pub fn validate_cluster(
 #[cfg(test)]
 mod tests {
 
-    use stackable_operator::utils::yaml_from_str_singleton_map;
+    use std::str::FromStr;
 
-    use super::validate_cluster;
+    use stackable_operator::{
+        product_logging::spec::{
+            AutomaticContainerLogConfig, ContainerLogConfig, ContainerLogConfigChoice, Logging,
+        },
+        utils::yaml_from_str_singleton_map,
+        v2::types::kubernetes::ConfigMapName,
+    };
+
+    use super::{Error, validate_cluster, validate_logging};
     use crate::{
         controller::dereference::DereferencedObjects,
         crd::{
@@ -281,6 +289,49 @@ mod tests {
         }
     }
 
+    /// Builds a [`Logging`] with automatic log configuration for the Superset and Vector containers.
+    fn automatic_logging(enable_vector_agent: bool) -> Logging<v1alpha1::Container> {
+        let automatic = || ContainerLogConfig {
+            choice: Some(ContainerLogConfigChoice::Automatic(
+                AutomaticContainerLogConfig::default(),
+            )),
+        };
+        Logging {
+            enable_vector_agent,
+            containers: [
+                (v1alpha1::Container::Superset, automatic()),
+                (v1alpha1::Container::Vector, automatic()),
+            ]
+            .into(),
+        }
+    }
+
+    /// The Vector aggregator discovery ConfigMap name is required exactly when the Vector agent is
+    /// enabled, and a Vector container is configured only in that case.
+    #[test]
+    fn validate_logging_requires_vector_aggregator_only_when_vector_enabled() {
+        // Vector enabled without an aggregator ConfigMap name fails validation up-front.
+        assert!(matches!(
+            validate_logging(&automatic_logging(true), &None),
+            Err(Error::MissingVectorAggregatorConfigMapName)
+        ));
+
+        // Vector enabled with an aggregator name configures a Vector container.
+        let aggregator = Some(
+            ConfigMapName::from_str("vector-aggregator-discovery").expect("valid ConfigMap name"),
+        );
+        let validated = validate_logging(&automatic_logging(true), &aggregator)
+            .expect("logging should validate");
+        assert!(validated.enable_vector_agent);
+        assert!(validated.vector_container.is_some());
+
+        // Vector disabled needs no aggregator name and configures no Vector container.
+        let validated =
+            validate_logging(&automatic_logging(false), &None).expect("logging should validate");
+        assert!(!validated.enable_vector_agent);
+        assert!(validated.vector_container.is_none());
+    }
+
     /// Characterises the `superset_config.py` override resolution: role-level overrides are merged
     /// with role-group overrides, the role group winning on shared keys, and unique keys from both
     /// levels surviving.