refactor: consolidatev2 logging

Author: maltesander

rust/operator-binary/src/command.rs

@@ -1,9 +1,9 @@
 use stackable_operator::{
-    product_logging::{
-        framework::{create_vector_shutdown_file_command, remove_vector_shutdown_file_command},
-        spec::{ContainerLogConfig, ContainerLogConfigChoice},
+    product_logging::framework::{
+        create_vector_shutdown_file_command, remove_vector_shutdown_file_command,
     },
     utils::COMMON_BASH_TRAP_FUNCTIONS,
+    v2::product_logging::framework::ValidatedContainerLogConfigChoice,
 };
 
 use crate::{
@@ -36,10 +36,7 @@ pub fn container_prepare_args(
     let mut args = vec![];
 
     // Copy custom logging provided `log.properties` to rw config
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Custom(_)),
-    }) = merged_config.logging.containers.get(&Container::Trino)
-    {
+    if let ValidatedContainerLogConfigChoice::Custom(_) = merged_config.logging.trino_container {
         // copy config files to a writeable empty folder
         args.push(format!(
             "echo copying {STACKABLE_LOG_CONFIG_DIR}/{LOG_PROPERTIES} {rw_conf}/{LOG_PROPERTIES}",

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

@@ -13,7 +13,9 @@ pub fn build(rg: &TrinoRoleGroupConfig) -> BTreeMap<String, String> {
 
     // 1. No defaults
     // 2. Automatic per-container logger levels
-    if let Some(per_container) = super::logging::get_log_property_map(&rg.config.logging) {
+    if let Some(per_container) =
+        super::logging::get_log_property_map(&rg.config.logging.trino_container)
+    {
         props.extend(per_container);
     }
 

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

@@ -1,21 +1,11 @@
+use std::collections::BTreeMap;
+
 use stackable_operator::{
-    kube::runtime::reflector::ObjectRef,
-    product_logging::{
-        framework::create_vector_config,
-        spec::{
-            AutomaticContainerLogConfig, ContainerLogConfig, ContainerLogConfigChoice, LogLevel,
-            Logging,
-        },
-    },
-    role_utils::RoleGroupRef,
+    product_logging::spec::{AutomaticContainerLogConfig, LogLevel},
+    v2::product_logging::framework::ValidatedContainerLogConfigChoice,
 };
 use strum::Display;
 
-use crate::{
-    controller::ValidatedCluster,
-    crd::{Container, TrinoRole},
-};
-
 #[derive(Display)]
 #[strum(serialize_all = "lowercase")]
 pub enum TrinoLogLevel {
@@ -38,63 +28,31 @@ impl From<LogLevel> for TrinoLogLevel {
     }
 }
 
-/// Return the `log.properties` content as a typed `BTreeMap`.
+/// Return the `log.properties` content as a typed `BTreeMap` for the (validated) Trino container.
+///
+/// Returns `None` when the Trino container uses a custom log ConfigMap (in which case the operator
+/// does not generate `log.properties`).
 pub fn get_log_property_map(
-    logging: &Logging<Container>,
-) -> Option<std::collections::BTreeMap<String, String>> {
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = logging.containers.get(&Container::Trino)
-    {
-        let map = log_config
-            .loggers
-            .iter()
-            .map(|(logger, config)| {
-                let log_level = TrinoLogLevel::from(config.level);
-                let key = if logger == AutomaticContainerLogConfig::ROOT_LOGGER {
-                    // ROOT logger maps to an empty key in log.properties (=LEVEL).
-                    String::new()
-                } else {
-                    logger.clone()
-                };
-                (key, log_level.to_string())
-            })
-            .collect();
-        Some(map)
-    } else {
-        None
-    }
-}
-
-/// Return the vector toml configuration
-pub fn get_vector_toml(
-    cluster: &ValidatedCluster,
-    role: &TrinoRole,
-    role_group_name: &str,
-    logging: &Logging<Container>,
-) -> Option<String> {
-    let vector_log_config = if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = logging.containers.get(&Container::Vector)
-    {
-        Some(log_config)
-    } else {
-        None
-    };
-
-    if logging.enable_vector_agent {
-        // The legacy `create_vector_config` still requires a `RoleGroupRef`, but ignores it
-        // entirely when rendering the config. We construct a throwaway one here so the rest of the
-        // operator no longer has to thread `RoleGroupRef` around.
-        // TODO(step-5): drop this once logging moves to the `v2` framework (`include_str!`-based
-        // `vector.yaml`), which does not need a `RoleGroupRef`.
-        let rolegroup_ref = RoleGroupRef {
-            cluster: ObjectRef::from_obj(cluster),
-            role: role.to_string(),
-            role_group: role_group_name.to_owned(),
-        };
-        Some(create_vector_config(&rolegroup_ref, vector_log_config))
-    } else {
-        None
+    trino_container: &ValidatedContainerLogConfigChoice,
+) -> Option<BTreeMap<String, String>> {
+    match trino_container {
+        ValidatedContainerLogConfigChoice::Automatic(log_config) => {
+            let map = log_config
+                .loggers
+                .iter()
+                .map(|(logger, config)| {
+                    let log_level = TrinoLogLevel::from(config.level);
+                    let key = if logger == AutomaticContainerLogConfig::ROOT_LOGGER {
+                        // ROOT logger maps to an empty key in log.properties (=LEVEL).
+                        String::new()
+                    } else {
+                        logger.clone()
+                    };
+                    (key, log_level.to_string())
+                })
+                .collect();
+            Some(map)
+        }
+        ValidatedContainerLogConfigChoice::Custom(_) => None,
     }
 }

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

@@ -11,6 +11,7 @@ pub mod exchange_manager_properties;
 pub mod log_properties;
 pub mod logging;
 pub mod node_properties;
+pub mod product_logging;
 pub mod security_properties;
 pub mod spooling_manager_properties;
 

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

@@ -0,0 +1,28 @@
+//! The Vector agent configuration (`vector.yaml`) assembled into the rolegroup `ConfigMap`.
+
+/// The Vector agent configuration (`vector.yaml`).
+///
+/// It is templated with environment variables (`${LOG_DIR}`, `${NAMESPACE}`, …) that the
+/// `v2` Vector container injects at runtime, so the same file content is used for every
+/// rolegroup.
+const VECTOR_CONFIG: &str = include_str!("vector.yaml");
+
+/// Returns the Vector agent config (`vector.yaml`) content.
+pub fn vector_config_file_content() -> String {
+    VECTOR_CONFIG.to_owned()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_vector_config_file_content() {
+        let content = vector_config_file_content();
+        assert!(!content.is_empty());
+        // The airlift source must be present (Trino's main log format) ...
+        assert!(content.contains("files_airlift"));
+        // ... while the aggregator sink must reference the injected address.
+        assert!(content.contains("${VECTOR_AGGREGATOR_ADDRESS}"));
+    }
+}

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

@@ -7,7 +7,7 @@ use stackable_operator::{
     builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
     k8s_openapi::api::core::v1::ConfigMap,
     kvp::ObjectLabels,
-    product_logging,
+    product_logging::framework::VECTOR_CONFIG_FILE,
     utils::cluster_info::KubernetesClusterInfo,
     v2::config_file_writer::to_java_properties_string,
 };
@@ -18,7 +18,7 @@ use crate::{
         ValidatedCluster,
         build::properties::{
             ConfigFileName, access_control_properties, config_properties,
-            exchange_manager_properties, log_properties, logging::get_vector_toml, node_properties,
+            exchange_manager_properties, log_properties, node_properties, product_logging,
             security_properties, spooling_manager_properties,
         },
     },
@@ -180,11 +180,12 @@ pub fn build_rolegroup_config_map(
     .context(BuildJvmConfigSnafu)?;
     data.insert(JVM_CONFIG.to_string(), jvm_config);
 
-    // 9. Vector sidecar toml if enabled.
-    if let Some(vector_toml) = get_vector_toml(cluster, role, role_group_name, &rg.config.logging) {
+    // 9. Vector agent config (`vector.yaml`) if the Vector agent is enabled. The file is templated
+    // with environment variables injected by the v2 Vector container at runtime.
+    if rg.config.logging.enable_vector_agent {
         data.insert(
-            product_logging::framework::VECTOR_CONFIG_FILE.to_string(),
-            vector_toml,
+            VECTOR_CONFIG_FILE.to_string(),
+            product_logging::vector_config_file_content(),
         );
     }
 

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

@@ -1,6 +1,6 @@
 //! Builds the per-rolegroup [`StatefulSet`] that runs a Trino role group.
 
-use std::{collections::BTreeMap, convert::Infallible};
+use std::{collections::BTreeMap, convert::Infallible, str::FromStr};
 
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
@@ -31,16 +31,13 @@ use stackable_operator::{
         apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString},
     },
     kvp::{Annotation, Annotations, Labels},
-    product_logging::{
-        self,
-        framework::LoggingError,
-        spec::{
-            ConfigMapLogConfig, ContainerLogConfig, ContainerLogConfigChoice,
-            CustomContainerLogConfig,
-        },
-    },
+    product_logging,
     shared::time::Duration,
-    v2::builder::pod::container::EnvVarSet,
+    v2::{
+        builder::pod::container::EnvVarSet,
+        product_logging::framework::{ValidatedContainerLogConfigChoice, vector_container},
+        types::kubernetes::{ContainerName, VolumeName},
+    },
 };
 
 use crate::{
@@ -59,16 +56,23 @@ use crate::{
     crd::{
         APP_NAME, CONFIG_DIR_NAME, Container, ENV_INTERNAL_SECRET, ENV_SPOOLING_SECRET, HTTP_PORT,
         HTTP_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT,
-        METRICS_PORT_NAME, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR,
-        STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR,
-        STACKABLE_TLS_STORE_PASSWORD, TrinoRole, v1alpha1,
+        METRICS_PORT_NAME, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR,
+        STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
+        STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD,
+        TrinoRole, v1alpha1,
     },
     trino_controller::{
         MAX_PREPARE_LOG_FILE_SIZE, STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR,
         build_recommended_labels, shared_internal_secret_name, shared_spooling_secret_name,
     },
 };
 
+stackable_operator::constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector");
+// The Vector agent reads its `vector.yaml` from the rolegroup ConfigMap (mounted as the "config"
+// volume) and writes its state under the shared "log" volume.
+stackable_operator::constant!(VECTOR_LOG_CONFIG_VOLUME_NAME: VolumeName = "config");
+stackable_operator::constant!(VECTOR_LOG_VOLUME_NAME: VolumeName = "log");
+
 #[derive(Snafu, Debug)]
 pub enum Error {
     #[snafu(display("missing secret lifetime"))]
@@ -88,12 +92,6 @@ pub enum Error {
         container_name: String,
     },
 
-    #[snafu(display("vector agent is enabled but vector aggregator ConfigMap is missing"))]
-    VectorAggregatorConfigMapMissing,
-
-    #[snafu(display("failed to build vector container"))]
-    BuildVectorContainer { source: LoggingError },
-
     #[snafu(display("failed to configure graceful shutdown"))]
     GracefulShutdown {
         source: build::graceful_shutdown::Error,
@@ -263,9 +261,8 @@ pub fn build_rolegroup_statefulset(
     )?;
 
     let mut prepare_args = vec![];
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = merged_config.logging.containers.get(&Container::Prepare)
+    if let ValidatedContainerLogConfigChoice::Automatic(log_config) =
+        &merged_config.logging.prepare_container
     {
         prepare_args.push(product_logging::framework::capture_shell_output(
             STACKABLE_LOG_DIR,
@@ -380,60 +377,33 @@ pub fn build_rolegroup_statefulset(
     // add password-update container if required
     trino_authentication_config.add_authentication_containers(trino_role, &mut pod_builder);
 
-    if let Some(ContainerLogConfig {
-        choice:
-            Some(ContainerLogConfigChoice::Custom(CustomContainerLogConfig {
-                custom: ConfigMapLogConfig { config_map },
-            })),
-    }) = merged_config.logging.containers.get(&Container::Trino)
-    {
-        pod_builder
-            .add_volume(Volume {
-                name: "log-config".to_string(),
-                config_map: Some(ConfigMapVolumeSource {
-                    name: config_map.into(),
-                    ..ConfigMapVolumeSource::default()
-                }),
-                ..Volume::default()
-            })
-            .context(AddVolumeSnafu)?;
-    } else {
-        pod_builder
-            .add_volume(Volume {
-                name: "log-config".to_string(),
-                config_map: Some(ConfigMapVolumeSource {
-                    name: config_map_name.clone(),
-                    ..ConfigMapVolumeSource::default()
-                }),
-                ..Volume::default()
-            })
-            .context(AddVolumeSnafu)?;
-    }
+    // The log-config volume mounts either the rolegroup ConfigMap (which carries the automatic
+    // `log.properties`) or a user-provided custom ConfigMap, depending on the validated choice.
+    let log_config_volume_config_map = match &merged_config.logging.trino_container {
+        ValidatedContainerLogConfigChoice::Custom(config_map) => config_map.to_string(),
+        ValidatedContainerLogConfigChoice::Automatic(_) => config_map_name.clone(),
+    };
+    pod_builder
+        .add_volume(Volume {
+            name: "log-config".to_string(),
+            config_map: Some(ConfigMapVolumeSource {
+                name: log_config_volume_config_map,
+                ..ConfigMapVolumeSource::default()
+            }),
+            ..Volume::default()
+        })
+        .context(AddVolumeSnafu)?;
 
-    if merged_config.logging.enable_vector_agent {
-        match &trino.spec.cluster_config.vector_aggregator_config_map_name {
-            Some(vector_aggregator_config_map_name) => {
-                pod_builder.add_container(
-                    product_logging::framework::vector_container(
-                        resolved_product_image,
-                        "config",
-                        "log",
-                        merged_config.logging.containers.get(&Container::Vector),
-                        ResourceRequirementsBuilder::new()
-                            .with_cpu_request("250m")
-                            .with_cpu_limit("500m")
-                            .with_memory_request("128Mi")
-                            .with_memory_limit("128Mi")
-                            .build(),
-                        vector_aggregator_config_map_name,
-                    )
-                    .context(BuildVectorContainerSnafu)?,
-                );
-            }
-            None => {
-                VectorAggregatorConfigMapMissingSnafu.fail()?;
-            }
-        }
+    if let Some(vector_log_config) = &merged_config.logging.vector_container {
+        pod_builder.add_container(vector_container(
+            &VECTOR_CONTAINER_NAME,
+            resolved_product_image,
+            vector_log_config,
+            &resource_names,
+            &VECTOR_LOG_CONFIG_VOLUME_NAME,
+            &VECTOR_LOG_VOLUME_NAME,
+            EnvVarSet::new(),
+        ));
     }
 
     let metadata = ObjectMetaBuilder::new()