refactor: consolidate containers & logging

Author: maltesander

Cargo.lock

@@ -3,13 +3,17 @@
 use serde::{Deserialize, Serialize};
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
-    product_logging::spec::{ContainerLogConfig, ContainerLogConfigChoice, LogLevel},
-    v2::config_overrides::JsonConfigOverrides,
+    product_logging::spec::LogLevel,
+    v2::{
+        config_overrides::JsonConfigOverrides,
+        product_logging::framework::ValidatedContainerLogConfigChoice,
+    },
 };
 
 use super::ConfigFileName;
 use crate::{
-    crd::{Container, OpaConfig, OpaConfigOverrides},
+    controller::ValidatedOpaConfig,
+    crd::{Container, OpaConfigOverrides},
     opa_controller::OPA_STACKABLE_SERVICE_NAME,
 };
 
@@ -29,12 +33,14 @@ pub enum Error {
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 /// Builds the OPA `config.json` from the operator defaults and the merged user `configOverrides`.
-pub fn build(merged_config: &OpaConfig, config_overrides: &OpaConfigOverrides) -> Result<String> {
+pub fn build(
+    merged_config: &ValidatedOpaConfig,
+    config_overrides: &OpaConfigOverrides,
+) -> Result<String> {
     let mut decision_logging_enabled = DEFAULT_DECISION_LOGGING_ENABLED;
 
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = merged_config.logging.containers.get(&Container::Opa)
+    if let Some(ValidatedContainerLogConfigChoice::Automatic(log_config)) =
+        merged_config.logging.containers.get(&Container::Opa)
         && let Some(config) = log_config.loggers.get("decision")
     {
         decision_logging_enabled = config.level != LogLevel::NONE;
@@ -151,8 +157,7 @@ mod tests {
             .values()
             .next()
             .expect("the default role group should exist");
-        let rendered =
-            build(&rg.config.config, &rg.config.config_overrides).expect("config.json builds");
+        let rendered = build(&rg.config, &rg.config_overrides).expect("config.json builds");
         serde_json::from_str(&rendered).expect("config.json should be valid JSON")
     }
 

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

@@ -97,7 +97,7 @@ mod tests {
             .next()
             .expect("the default role group should exist");
 
-        build_rolegroup_config_map(&validated, role_group_name, &rg.config, None)
+        build_rolegroup_config_map(&validated, role_group_name, rg, None)
             .expect("the config map should build")
     }
 

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

@@ -40,18 +40,17 @@ use stackable_operator::{
     product_logging::{
         self,
         framework::{create_vector_shutdown_file_command, remove_vector_shutdown_file_command},
-        spec::{
-            AppenderConfig, AutomaticContainerLogConfig, ContainerLogConfig,
-            ContainerLogConfigChoice, LogLevel,
-        },
+        spec::{AppenderConfig, AutomaticContainerLogConfig, LogLevel},
     },
     utils::{COMMON_BASH_TRAP_FUNCTIONS, cluster_info::KubernetesClusterInfo},
     v2::{
         builder::{
             meta::ownerreference_from_resource,
             pod::container::{EnvVarSet, new_container_builder},
         },
-        product_logging::framework::{STACKABLE_LOG_DIR, vector_container},
+        product_logging::framework::{
+            STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container,
+        },
         types::{
             common::Port,
             kubernetes::{ContainerName, VolumeName},
@@ -61,8 +60,10 @@ use stackable_operator::{
 
 use super::service::{self, APP_PORT, APP_PORT_NAME};
 use crate::{
-    controller::{RoleGroupName, ValidatedCluster, ValidatedRoleGroup, build},
-    crd::{Container, DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT, OpaConfig, user_info_fetcher},
+    controller::{
+        OpaRoleGroupConfig, RoleGroupName, ValidatedCluster, ValidatedOpaConfig, build,
+    },
+    crd::{Container, DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT, user_info_fetcher},
     operations::graceful_shutdown::add_graceful_shutdown_config,
 };
 
@@ -237,14 +238,14 @@ fn http_liveness_probe(path: &str, port: IntOrString, scheme: Option<String>) ->
 pub fn build_server_rolegroup_daemonset(
     cluster: &ValidatedCluster,
     role_group_name: &RoleGroupName,
-    role_group: &ValidatedRoleGroup,
+    role_group: &OpaRoleGroupConfig,
     opa_bundle_builder_image: &str,
     user_info_fetcher_image: &str,
     service_account: &ServiceAccount,
     cluster_info: &KubernetesClusterInfo,
 ) -> Result<DaemonSet> {
     let resolved_product_image = &cluster.image;
-    let rolegroup_config = &role_group.config;
+    let rolegroup_config = role_group;
     // All overrides were already merged (role group over role over defaults) in the validate step.
     let merged_config = &rolegroup_config.config;
 
@@ -567,7 +568,7 @@ pub fn build_server_rolegroup_daemonset(
 
     // The Vector logging config was validated up-front (see `ValidatedLogging`); a `Some` here means
     // the Vector agent is enabled and the aggregator discovery ConfigMap name is valid.
-    if let Some(vector_log_config) = &role_group.logging.vector_container {
+    if let Some(vector_log_config) = &merged_config.logging.vector_container {
         pb.add_container(vector_container(
             &container_name(&Container::Vector),
             resolved_product_image,
@@ -661,7 +662,7 @@ fn add_stackable_rust_cli_env_vars(
 }
 
 fn build_opa_start_command(
-    merged_config: &OpaConfig,
+    merged_config: &ValidatedOpaConfig,
     container_name: &str,
     tls_enabled: bool,
     cli_overrides: &BTreeMap<String, String>,
@@ -671,9 +672,8 @@ fn build_opa_start_command(
     let mut server_log_level = DEFAULT_SERVER_LOG_LEVEL;
     let mut decision_log_level = DEFAULT_DECISION_LOG_LEVEL;
 
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = merged_config.logging.containers.get(&Container::Opa)
+    if let Some(ValidatedContainerLogConfigChoice::Automatic(log_config)) =
+        merged_config.logging.containers.get(&Container::Opa)
     {
         if let Some(AppenderConfig {
             level: Some(log_level),
@@ -749,14 +749,15 @@ fn build_opa_start_command(
     }
 }
 
-fn build_bundle_builder_start_command(merged_config: &OpaConfig, container_name: &str) -> String {
+fn build_bundle_builder_start_command(
+    merged_config: &ValidatedOpaConfig,
+    container_name: &str,
+) -> String {
     let mut console_logging_off = false;
 
     // We need to check if the console logging is deactivated (NONE)
     // This will result in not using `tee` later on in the start command
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = merged_config
+    if let Some(ValidatedContainerLogConfigChoice::Automatic(log_config)) = merged_config
         .logging
         .containers
         .get(&Container::BundleBuilder)
@@ -812,12 +813,11 @@ fn build_bundle_builder_start_command(merged_config: &OpaConfig, container_name:
 ///
 /// Context: https://docs.stackable.tech/home/stable/concepts/logging/
 fn sidecar_container_log_level(
-    merged_config: &OpaConfig,
+    merged_config: &ValidatedOpaConfig,
     sidecar_container: &Container,
 ) -> build::properties::product_logging::BundleBuilderLogLevel {
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = merged_config.logging.containers.get(sidecar_container)
+    if let Some(ValidatedContainerLogConfigChoice::Automatic(log_config)) =
+        merged_config.logging.containers.get(sidecar_container)
         && let Some(logger) = log_config
             .loggers
             .get(AutomaticContainerLogConfig::ROOT_LOGGER)
@@ -828,11 +828,13 @@ fn sidecar_container_log_level(
     build::properties::product_logging::BundleBuilderLogLevel::Info
 }
 
-fn build_prepare_start_command(merged_config: &OpaConfig, container_name: &str) -> Vec<String> {
+fn build_prepare_start_command(
+    merged_config: &ValidatedOpaConfig,
+    container_name: &str,
+) -> Vec<String> {
     let mut prepare_container_args = vec![];
-    if let Some(ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    }) = merged_config.logging.containers.get(&Container::Prepare)
+    if let Some(ValidatedContainerLogConfigChoice::Automatic(log_config)) =
+        merged_config.logging.containers.get(&Container::Prepare)
     {
         prepare_container_args.push(product_logging::framework::capture_shell_output(
             STACKABLE_LOG_DIR,

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

@@ -6,9 +6,14 @@ use std::{collections::BTreeMap, str::FromStr};
 // Re-exported so the rest of the controller refers to `crate::controller::RoleGroupName`.
 pub use stackable_operator::v2::types::operator::RoleGroupName;
 use stackable_operator::{
-    commons::product_image_selection::ResolvedProductImage,
+    commons::{
+        affinity::StackableAffinity,
+        product_image_selection::ResolvedProductImage,
+        resources::{NoRuntimeLimits, Resources},
+    },
     kube::{Resource as KubeResource, api::ObjectMeta},
     kvp::Labels,
+    shared::time::Duration,
     v2::{
         HasName, HasUid, NameIsValidLabelValue,
         kvp::label::{recommended_labels, role_group_selector, role_selector},
@@ -25,8 +30,8 @@ use stackable_operator::{
 
 use crate::{
     crd::{
-        APP_NAME, OPERATOR_NAME, OpaConfig, OpaConfigOverrides, OpaRole, user_info_fetcher,
-        v1alpha2,
+        APP_NAME, OPERATOR_NAME, OpaConfig, OpaConfigOverrides, OpaRole, OpaStorageConfig,
+        user_info_fetcher, v1alpha2,
     },
     opa_controller::OPA_CONTROLLER_NAME,
 };
@@ -49,7 +54,7 @@ pub struct ValidatedCluster {
     pub uid: Uid,
     pub image: ResolvedProductImage,
     pub cluster_config: ValidatedClusterConfig,
-    pub role_group_configs: BTreeMap<OpaRole, BTreeMap<RoleGroupName, ValidatedRoleGroup>>,
+    pub role_group_configs: BTreeMap<OpaRole, BTreeMap<RoleGroupName, OpaRoleGroupConfig>>,
 }
 
 impl ValidatedCluster {
@@ -59,7 +64,7 @@ impl ValidatedCluster {
         uid: Uid,
         image: ResolvedProductImage,
         cluster_config: ValidatedClusterConfig,
-        role_group_configs: BTreeMap<OpaRole, BTreeMap<RoleGroupName, ValidatedRoleGroup>>,
+        role_group_configs: BTreeMap<OpaRole, BTreeMap<RoleGroupName, OpaRoleGroupConfig>>,
     ) -> Self {
         let metadata = ObjectMeta {
             name: Some(name.to_string()),
@@ -205,19 +210,26 @@ pub struct ValidatedClusterConfig {
     pub listener_class: v1alpha2::CurrentlySupportedListenerClasses,
 }
 
-/// The validated configuration of a single role group.
-///
-/// All override kinds (`config`, `configOverrides`, `envOverrides`, `cliOverrides`, `podOverrides`)
-/// are merged once by [`with_validated_config`](stackable_operator::v2::role_utils::with_validated_config),
-/// with the role group winning over the role, which wins over the operator defaults.
-///
-/// Note: `replicas` is carried by the framework type but unused here — OPA runs as a `DaemonSet`
-/// (one Pod per node).
-pub type OpaRoleGroupConfig = RoleGroupConfig<OpaConfig, GenericCommonConfig, OpaConfigOverrides>;
-
-/// A single validated role group: the merged [`OpaRoleGroupConfig`] plus its validated logging
-/// configuration (produced together by the validate step, so the build steps never re-validate).
-pub struct ValidatedRoleGroup {
-    pub config: OpaRoleGroupConfig,
+/// The validated, merged configuration of a single OPA role group.
+pub type OpaRoleGroupConfig =
+    RoleGroupConfig<ValidatedOpaConfig, GenericCommonConfig, OpaConfigOverrides>;
+
+/// A validated OPA config: the merged [`OpaConfig`].
+#[derive(Clone, Debug, PartialEq)]
+pub struct ValidatedOpaConfig {
+    pub resources: Resources<OpaStorageConfig, NoRuntimeLimits>,
     pub logging: validate::ValidatedLogging,
+    pub affinity: StackableAffinity,
+    pub graceful_shutdown_timeout: Option<Duration>,
+}
+
+impl ValidatedOpaConfig {
+    pub(crate) fn from_merged(merged: OpaConfig, logging: validate::ValidatedLogging) -> Self {
+        Self {
+            resources: merged.resources,
+            logging,
+            affinity: merged.affinity,
+            graceful_shutdown_timeout: merged.graceful_shutdown_timeout,
+        }
+    }
 }