refactor: move logging & discovery

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -56,12 +56,10 @@ use crate::{
     authentication::DruidAuthenticationConfig,
     crd::{
         APP_NAME, CommonRoleGroupConfig, Container, DRUID_CONFIG_DIRECTORY, DeepStorageSpec,
-        DruidClusterStatus, DruidRole, HDFS_CONFIG_DIRECTORY, LOG_CONFIG_DIRECTORY,
-        MAX_DRUID_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, OPERATOR_NAME,
-        RW_CONFIG_DIRECTORY, STACKABLE_LOG_DIR, build_recommended_labels,
-        security::DruidTlsSecurity, v1alpha1,
+        DruidClusterStatus, DruidRole, HDFS_CONFIG_DIRECTORY, LOG_CONFIG_DIRECTORY, METRICS_PORT,
+        METRICS_PORT_NAME, OPERATOR_NAME, RW_CONFIG_DIRECTORY, STACKABLE_LOG_DIR,
+        build_recommended_labels, security::DruidTlsSecurity, v1alpha1,
     },
-    discovery::{self, build_discovery_configmaps},
     internal_secret::create_shared_internal_secret,
     listener::{
         LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc,
@@ -75,6 +73,9 @@ mod build;
 mod dereference;
 mod validate;
 
+use build::discovery::{self, build_discovery_configmaps};
+use build::properties::logging::MAX_DRUID_LOG_FILES_SIZE;
+
 use validate::DruidRoleGroupConfig;
 
 pub const DRUID_CONTROLLER_NAME: &str = "druidcluster";
@@ -442,15 +443,10 @@ pub async fn reconcile_druid(
 
                 if *druid_role == DruidRole::Router {
                     // discovery
-                    for discovery_cm in build_discovery_configmaps(
-                        druid,
-                        druid,
-                        &validated_cluster.image,
-                        &validated_cluster.cluster_config.druid_tls_security,
-                        listener,
-                    )
-                    .await
-                    .context(BuildDiscoveryConfigSnafu)?
+                    for discovery_cm in
+                        build_discovery_configmaps(&validated_cluster, druid, listener)
+                            .await
+                            .context(BuildDiscoveryConfigSnafu)?
                     {
                         cluster_resources
                             .add(client, discovery_cm)

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

@@ -1,4 +1,5 @@
 //! Build steps that turn a `ValidatedCluster` into Kubernetes resources.
 
 pub mod config_map;
+pub mod discovery;
 pub mod properties;

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

@@ -18,29 +18,39 @@ use stackable_operator::{
     crd::s3,
     database_connections::drivers::jdbc::JdbcDatabaseConnection as _,
     k8s_openapi::api::core::v1::{ConfigMap, EnvVar},
+    product_logging::framework::VECTOR_CONFIG_FILE,
     role_utils::RoleGroupRef,
 };
 
 use crate::{
     config::jvm::construct_jvm_args,
     controller::{
         DRUID_CONTROLLER_NAME,
-        build::properties::{ConfigFileName, writer::to_java_properties_string},
+        build::properties::{
+            ConfigFileName,
+            logging::{build_log4j2_config, build_vector_config},
+            writer::to_java_properties_string,
+        },
         validate::{DruidRoleGroupConfig, ValidatedCluster},
     },
-    crd::{
-        AUTH_AUTHORIZER_OPA_URI, DS_BUCKET, DruidRole, EXTENSIONS_LOADLIST, S3_ACCESS_KEY,
-        S3_ENDPOINT_URL, S3_PATH_STYLE_ACCESS, S3_SECRET_KEY, ZOOKEEPER_CONNECTION_STRING,
-        build_recommended_labels, build_string_list, v1alpha1,
-    },
+    crd::{DruidRole, build_recommended_labels, build_string_list, v1alpha1},
     extensions::get_extension_list,
-    product_logging::extend_role_group_config_map,
 };
 
 // jvm.config is built by `config::jvm`, not a properties builder, so it is not part
 // of `ConfigFileName`.
 const JVM_CONFIG: &str = "jvm.config";
 
+// Druid `runtime.properties` config-property keys assembled into the rolegroup ConfigMap here.
+const EXTENSIONS_LOADLIST: &str = "druid.extensions.loadList";
+const ZOOKEEPER_CONNECTION_STRING: &str = "druid.zk.service.host";
+const DS_BUCKET: &str = "druid.storage.bucket";
+const S3_ENDPOINT_URL: &str = "druid.s3.endpoint.url";
+const S3_ACCESS_KEY: &str = "druid.s3.accessKey";
+const S3_SECRET_KEY: &str = "druid.s3.secretKey";
+const S3_PATH_STYLE_ACCESS: &str = "druid.s3.enablePathStyleAccess";
+const AUTH_AUTHORIZER_OPA_URI: &str = "druid.auth.authorizer.OpaAuthorizer.opaUri";
+
 #[derive(Snafu, Debug)]
 #[allow(clippy::enum_variant_names)]
 pub enum Error {
@@ -90,12 +100,6 @@ pub enum Error {
         source: crate::authentication::Error,
     },
 
-    #[snafu(display("failed to add the logging configuration to the ConfigMap [{cm_name}]"))]
-    InvalidLoggingConfig {
-        source: crate::product_logging::Error,
-        cm_name: String,
-    },
-
     #[snafu(display("invalid metadata database connection"))]
     InvalidMetadataDatabaseConnection {
         source: stackable_operator::database_connections::Error,
@@ -314,14 +318,13 @@ pub fn build_rolegroup_config_map(
         config_map_builder.add_data(filename, file_content);
     }
 
-    extend_role_group_config_map(
-        rolegroup,
-        &rg.merged_config.logging,
-        &mut config_map_builder,
-    )
-    .context(InvalidLoggingConfigSnafu {
-        cm_name: rolegroup.object_name(),
-    })?;
+    if let Some(log4j2_config) = build_log4j2_config(&rg.merged_config.logging) {
+        config_map_builder.add_data(ConfigFileName::Log4j2Properties.to_string(), log4j2_config);
+    }
+
+    if let Some(vector_config) = build_vector_config(rolegroup, &rg.merged_config.logging) {
+        config_map_builder.add_data(VECTOR_CONFIG_FILE, vector_config);
+    }
 
     config_map_builder
         .build()

rust/operator-binary/src/discovery.rs …binary/src/controller/build/discovery.rsrust/operator-binary/src/discovery.rs renamed to rust/operator-binary/src/controller/build/discovery.rs rust/operator-binary/src/discovery.rs renamed to rust/operator-binary/src/controller/build/discovery.rs

@@ -4,24 +4,23 @@
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
-    commons::product_image_selection::ResolvedProductImage,
     crd::listener::v1alpha1::Listener,
     k8s_openapi::api::core::v1::ConfigMap,
-    kube::{Resource, ResourceExt, runtime::reflector::ObjectRef},
+    kube::{Resource, ResourceExt},
 };
 
 use crate::{
     DRUID_CONTROLLER_NAME,
-    crd::{DruidRole, build_recommended_labels, security::DruidTlsSecurity, v1alpha1},
+    controller::validate::ValidatedCluster,
+    crd::{DruidRole, build_recommended_labels},
     listener::build_listener_connection_string,
 };
 
 #[derive(Snafu, Debug)]
 pub enum Error {
-    #[snafu(display("object {} is missing metadata to build owner reference", druid))]
+    #[snafu(display("object is missing metadata to build owner reference"))]
     ObjectMissingMetadataForOwnerRef {
         source: stackable_operator::builder::meta::Error,
-        druid: ObjectRef<v1alpha1::DruidCluster>,
     },
 
     #[snafu(display("failed to get service FQDN"))]
@@ -41,37 +40,28 @@ pub enum Error {
     ListenerConfiguration { source: crate::listener::Error },
 }
 
-/// Builds discovery [`ConfigMap`]s for connecting to a [`v1alpha1::DruidCluster`].
+/// Builds discovery [`ConfigMap`]s for connecting to a Druid cluster.
 pub async fn build_discovery_configmaps(
-    druid: &v1alpha1::DruidCluster,
+    cluster: &ValidatedCluster,
     owner: &impl Resource<DynamicType = ()>,
-    resolved_product_image: &ResolvedProductImage,
-    druid_tls_security: &DruidTlsSecurity,
     listener: Listener,
 ) -> Result<Vec<ConfigMap>, Error> {
     let name = owner.name_unchecked();
     Ok(vec![build_discovery_configmap(
-        druid,
-        owner,
-        resolved_product_image,
-        druid_tls_security,
-        &name,
-        listener,
+        cluster, owner, &name, listener,
     )?])
 }
 
-/// Build a discovery [`ConfigMap`] containing information about how to connect to a certain [`v1alpha1::DruidCluster`].
+/// Build a discovery [`ConfigMap`] containing information about how to connect to a certain Druid cluster.
 fn build_discovery_configmap(
-    druid: &v1alpha1::DruidCluster,
+    cluster: &ValidatedCluster,
     owner: &impl Resource<DynamicType = ()>,
-    resolved_product_image: &ResolvedProductImage,
-    druid_tls_security: &DruidTlsSecurity,
     name: &str,
     listener: Listener,
 ) -> Result<ConfigMap, Error> {
     let router_host = build_listener_connection_string(
         listener,
-        druid_tls_security,
+        &cluster.cluster_config.druid_tls_security,
         &DruidRole::Router.to_string(),
     )
     .context(ListenerConfigurationSnafu)?;
@@ -84,16 +74,14 @@ fn build_discovery_configmap(
     ConfigMapBuilder::new()
         .metadata(
             ObjectMetaBuilder::new()
-                .name_and_namespace(druid)
+                .name_and_namespace(owner)
                 .name(name)
                 .ownerreference_from_resource(owner, None, Some(true))
-                .with_context(|_| ObjectMissingMetadataForOwnerRefSnafu {
-                    druid: ObjectRef::from_obj(druid),
-                })?
+                .context(ObjectMissingMetadataForOwnerRefSnafu)?
                 .with_recommended_labels(&build_recommended_labels(
-                    druid,
+                    owner,
                     DRUID_CONTROLLER_NAME,
-                    &resolved_product_image.app_version_label_value,
+                    &cluster.image.app_version_label_value,
                     &DruidRole::Router.to_string(),
                     "discovery",
                 ))

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

@@ -0,0 +1,77 @@
+//! Builds the log4j2 and Vector logging configuration for the rolegroup `ConfigMap`.
+
+use stackable_operator::{
+    memory::{BinaryMultiple, MemoryQuantity},
+    product_logging::{
+        self,
+        spec::{ContainerLogConfig, ContainerLogConfigChoice, Logging},
+    },
+    role_utils::RoleGroupRef,
+};
+
+use crate::crd::{Container, STACKABLE_LOG_DIR, v1alpha1};
+
+const CONSOLE_CONVERSION_PATTERN: &str = "%d{ISO8601} %p [%t] %c - %m%n";
+
+/// File that the Druid log4j2 config writes its rolling log output to.
+const DRUID_LOG_FILE: &str = "druid.log4j2.xml";
+
+/// Maximum size of all Druid log files combined, used both for the log4j2 rollover configuration
+/// and to size the log volume in the controller.
+pub const MAX_DRUID_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity {
+    value: 10.0,
+    unit: BinaryMultiple::Mebi,
+};
+
+/// Renders the `log4j2.properties` content for the Druid container.
+///
+/// Returns `None` when the container uses a custom log ConfigMap rather than the operator's
+/// automatic logging configuration.
+pub fn build_log4j2_config(logging: &Logging<Container>) -> Option<String> {
+    if let Some(ContainerLogConfig {
+        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
+    }) = logging.containers.get(&Container::Druid)
+    {
+        Some(product_logging::framework::create_log4j2_config(
+            &format!(
+                "{STACKABLE_LOG_DIR}/{container}",
+                container = Container::Druid
+            ),
+            DRUID_LOG_FILE,
+            MAX_DRUID_LOG_FILES_SIZE
+                .scale_to(BinaryMultiple::Mebi)
+                .floor()
+                .value as u32,
+            CONSOLE_CONVERSION_PATTERN,
+            log_config,
+        ))
+    } else {
+        None
+    }
+}
+
+/// Renders the Vector agent config (`vector.yaml`).
+///
+/// Returns `None` when the Vector agent is disabled for this role group.
+pub fn build_vector_config(
+    rolegroup: &RoleGroupRef<v1alpha1::DruidCluster>,
+    logging: &Logging<Container>,
+) -> Option<String> {
+    if !logging.enable_vector_agent {
+        return None;
+    }
+
+    let vector_log_config = if let Some(ContainerLogConfig {
+        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
+    }) = logging.containers.get(&Container::Vector)
+    {
+        Some(log_config)
+    } else {
+        None
+    };
+
+    Some(product_logging::framework::create_vector_config(
+        rolegroup,
+        vector_log_config,
+    ))
+}

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

@@ -2,14 +2,21 @@
 
 pub mod writer;
 
+pub mod logging;
 pub mod runtime_properties;
 pub mod security_properties;
 
 /// The names of the operator-written Druid config files assembled into the rolegroup ConfigMap.
 #[derive(Clone, Copy, Debug, strum::Display)]
+// The shared `Properties` suffix mirrors the actual on-disk file names; it is not redundant naming.
+#[allow(clippy::enum_variant_names)]
 pub enum ConfigFileName {
     #[strum(serialize = "runtime.properties")]
     RuntimeProperties,
     #[strum(serialize = "security.properties")]
     SecurityProperties,
+    /// `log4j2.properties` is rendered by the logging framework rather than a properties builder,
+    /// but it is still an operator-written file assembled into the rolegroup ConfigMap.
+    #[strum(serialize = "log4j2.properties")]
+    Log4j2Properties,
 }

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

@@ -21,9 +21,8 @@ use crate::{
         dereference::DereferencedObjects,
     },
     crd::{
-        CommonRoleGroupConfig, DruidConfigOverrides, DruidRole, INDEXER_JAVA_OPTS,
-        STACKABLE_TRUST_STORE, STACKABLE_TRUST_STORE_PASSWORD, build_string_list,
-        security::DruidTlsSecurity, v1alpha1,
+        CommonRoleGroupConfig, DruidConfigOverrides, DruidRole, STACKABLE_TRUST_STORE,
+        STACKABLE_TRUST_STORE_PASSWORD, build_string_list, security::DruidTlsSecurity, v1alpha1,
     },
 };
 
@@ -92,6 +91,8 @@ fn key_value_overrides(
     let kv = match file {
         ConfigFileName::RuntimeProperties => overrides.runtime_properties.as_ref(),
         ConfigFileName::SecurityProperties => overrides.security_properties.as_ref(),
+        // log4j2.properties is rendered by the logging framework and accepts no key/value overrides.
+        ConfigFileName::Log4j2Properties => None,
     };
     kv.map(
         stackable_operator::config_overrides::KeyValueConfigOverrides::as_product_config_overrides,
@@ -156,6 +157,8 @@ fn build_role_group_config(
     }
 }
 
+const INDEXER_JAVA_OPTS: &str = "druid.indexer.runner.javaOptsArray";
+
 /// The `druid.indexer.runner.javaOptsArray` entry that `MiddleManagerConfigFragment::compute_files`
 /// adds for *every* file (runtime.properties and security.properties).
 fn middlemanager_indexer_java_opts() -> (String, Option<String>) {

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

@@ -7,10 +7,12 @@ use stackable_operator::{
     memory::{BinaryMultiple, MemoryQuantity},
 };
 
-use crate::crd::{
-    PROCESSING_BUFFER_SIZE_BYTES, PROCESSING_NUM_MERGE_BUFFERS, PROCESSING_NUM_THREADS,
-    storage::HistoricalStorage,
-};
+use crate::crd::storage::HistoricalStorage;
+
+// Druid historical processing config-property keys, only used for the memory calculations here.
+const PROCESSING_BUFFER_SIZE_BYTES: &str = "druid.processing.buffer.sizeBytes";
+const PROCESSING_NUM_MERGE_BUFFERS: &str = "druid.processing.numMergeBuffers";
+const PROCESSING_NUM_THREADS: &str = "druid.processing.numThreads";
 
 static MIN_HEAP_RATIO: f32 = 0.75;