From c437078bfd7ee0fa1cc5e1d1b2d1c8efbeb31ea0 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 16 Sep 2026 17:05:03 +0200 Subject: [PATCH 1/4] refactor: role-builder --- .../src/controller/build/container.rs | 65 ++++---- .../src/controller/build/mod.rs | 78 ++-------- .../src/controller/build/resolve.rs | 139 ++++++++++++------ .../controller/build/resource/config_map.rs | 55 +++---- .../controller/build/resource/statefulset.rs | 55 +++---- .../controller/build/role_group_builder.rs | 104 +++++++++++++ rust/operator-binary/src/hdfs_controller.rs | 34 ++--- 7 files changed, 292 insertions(+), 238 deletions(-) create mode 100644 rust/operator-binary/src/controller/build/role_group_builder.rs diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 45e93d35..9c76e891 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -53,11 +53,9 @@ use stackable_operator::{ STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, VectorContainerLogConfig, vector_container, }, - role_utils::{JavaCommonConfig, RoleGroupConfig}, types::{ common::Port, kubernetes::{ConfigMapName, ContainerName, VolumeName}, - operator::RoleGroupName, }, }, }; @@ -67,7 +65,7 @@ use crate::{ controller::{ ValidatedCluster, build::{ - self, ResolvedRoleGroup, RoleGroupResolver, RoleSpecificValues, + self, ResolvedRoleGroup, RoleGroupBuilder, RoleSpecificValues, jvm::{self, construct_global_jvm_args, construct_role_specific_jvm_args}, kerberos::KERBEROS_CONTAINER_PATH, properties::product_logging::{ @@ -92,7 +90,6 @@ use crate::{ SERVICE_PORT_NAME_RPC, STACKABLE_ROOT_DATA_DIR, }, storage::DataNodeStorageConfig, - v1alpha1, }, }; @@ -213,17 +210,19 @@ impl ContainerConfig { /// Add all main, side and init containers as well as required volumes to the pod builder. /// - /// Every role-specific value is resolved by the caller into `resolved`; the role itself comes - /// from `C::ROLE`, the same `C` that produced it. - pub fn add_containers_and_volumes( + /// Everything about the role group comes from `resolved`, the role and the merged overrides + /// included, so there is nothing here to pair with the wrong role group. + pub(crate) fn add_containers_and_volumes( pb: &mut PodBuilder, - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rolegroup_config: &RoleGroupConfig, - resolved: &ResolvedRoleGroup, + builder: &RoleGroupBuilder, ) -> Result<(), Error> { - let role = &C::ROLE; + let RoleGroupBuilder { + cluster, + cluster_info, + role_group_name, + resolved, + } = builder; + let role = &builder.role(); let namenode_podrefs = build::pod_refs(cluster, &HdfsNodeRole::Name); // HDFS main container @@ -241,7 +240,6 @@ impl ContainerConfig { cluster, cluster_info, &resolved.logging.hdfs, - rolegroup_config, resolved, )?); @@ -355,7 +353,6 @@ impl ContainerConfig { cluster, cluster_info, zkfc, - rolegroup_config, resolved, )?); @@ -371,7 +368,6 @@ impl ContainerConfig { cluster, cluster_info, format_namenodes, - rolegroup_config, resolved, &namenode_podrefs, )?); @@ -388,7 +384,6 @@ impl ContainerConfig { cluster, cluster_info, format_zookeeper, - rolegroup_config, resolved, &namenode_podrefs, )?); @@ -408,7 +403,6 @@ impl ContainerConfig { cluster, cluster_info, wait_for_namenodes, - rolegroup_config, resolved, &namenode_podrefs, )?); @@ -491,15 +485,14 @@ impl ContainerConfig { /// - Namenode ZooKeeper fail over controller (ZKFC) /// - Datanode main process /// - Journalnode main process - fn main_container( + fn main_container( &self, cluster: &ValidatedCluster, cluster_info: &KubernetesClusterInfo, container_log_config: &ContainerLogConfig, - rolegroup_config: &RoleGroupConfig, - resolved: &ResolvedRoleGroup, + resolved: &ResolvedRoleGroup, ) -> Result { - let role = &C::ROLE; + let role = &resolved.role.node_role(); let mut cb = new_container_builder(self.container_name()); let resources = self.resources(&resolved.resources); @@ -507,7 +500,7 @@ impl ContainerConfig { cb.image_from_product_image(&cluster.image) .command(Self::command()) .args(self.args(cluster, cluster_info, role, container_log_config, &[])?) - .add_env_vars(self.env(cluster, role, rolegroup_config, resources.as_ref())?) + .add_env_vars(self.env(cluster, role, resolved, resources.as_ref())?) .add_volume_mounts(self.volume_mounts(cluster, &resolved.volume_claim_templates)) .context(AddVolumeMountSnafu)? .add_container_ports(self.container_ports(cluster)); @@ -539,16 +532,15 @@ impl ContainerConfig { /// Creates respective init containers for: /// - Namenode (format-namenodes, format-zookeeper) /// - Datanode (wait-for-namenodes) - fn init_container( + fn init_container( &self, cluster: &ValidatedCluster, cluster_info: &KubernetesClusterInfo, container_log_config: &ContainerLogConfig, - rolegroup_config: &RoleGroupConfig, - resolved: &ResolvedRoleGroup, + resolved: &ResolvedRoleGroup, namenode_podrefs: &[HdfsPodRef], ) -> Result { - let role = &C::ROLE; + let role = &resolved.role.node_role(); let mut cb = new_container_builder(self.container_name()); cb.image_from_product_image(&cluster.image) @@ -560,7 +552,7 @@ impl ContainerConfig { container_log_config, namenode_podrefs, )?) - .add_env_vars(self.env(cluster, role, rolegroup_config, None)?) + .add_env_vars(self.env(cluster, role, resolved, None)?) .add_volume_mounts(self.volume_mounts(cluster, &resolved.volume_claim_templates)) .context(AddVolumeMountSnafu)?; @@ -881,11 +873,11 @@ impl ContainerConfig { } /// Returns the container env variables. - fn env( + fn env( &self, cluster: &ValidatedCluster, role: &HdfsNodeRole, - rolegroup_config: &RoleGroupConfig, + resolved: &ResolvedRoleGroup, resources: Option<&ResourceRequirements>, ) -> Result, Error> { // Maps env var name to env var object. This allows env_overrides to work @@ -916,7 +908,7 @@ impl ContainerConfig { role_opts_name.clone(), EnvVar { name: role_opts_name, - value: Some(self.build_hadoop_opts(cluster, resources, rolegroup_config)?), + value: Some(self.build_hadoop_opts(cluster, resources, resolved)?), ..EnvVar::default() }, ); @@ -980,7 +972,8 @@ impl ContainerConfig { ); // Overrides need to come last - let mut env_override_vars: BTreeMap = rolegroup_config + let mut env_override_vars: BTreeMap = resolved + .merged .env_overrides .clone() .into_iter() @@ -1253,11 +1246,11 @@ impl ContainerConfig { } /// Build HADOOP_{*node}_OPTS for each namenode, datanodes and journalnodes. - fn build_hadoop_opts( + fn build_hadoop_opts( &self, cluster: &ValidatedCluster, resources: Option<&ResourceRequirements>, - rolegroup_config: &RoleGroupConfig, + resolved: &ResolvedRoleGroup, ) -> Result { match self { ContainerConfig::Hdfs { @@ -1267,9 +1260,7 @@ impl ContainerConfig { let config_dir = volume_mount_dirs.final_config(); construct_role_specific_jvm_args( role, - &rolegroup_config - .product_specific_common_config - .jvm_argument_overrides, + &resolved.merged.jvm_argument_overrides, cluster.has_kerberos_enabled(), resources, config_dir, diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 2eeb36d5..6f625274 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -59,6 +59,7 @@ pub mod opa; pub mod properties; pub mod resolve; pub mod resource; +pub mod role_group_builder; #[derive(Snafu, Debug)] pub enum Error { @@ -108,8 +109,10 @@ pub enum Error { }, } -pub(crate) use resolve::RoleGroupResolver; -pub use resolve::{ResolvedRoleGroup, RoleGroupLogging, RoleSpecificValues}; +pub(crate) use resolve::{ + ResolvedRoleGroup, RoleGroupLogging, RoleGroupResolver, RoleSpecificValues, +}; +pub(crate) use role_group_builder::RoleGroupBuilder; /// The resources of every role, accumulated one role at a time by [`build_role`]. #[derive(Default)] @@ -136,42 +139,15 @@ fn build_role( let role = &C::ROLE; for (role_group_name, rg_config) in role_group_configs { - build_role_group_services(cluster, role, role_group_name, &mut rg_resources.services)?; - - let selector_labels = rolegroup_selector_labels(cluster, role, role_group_name).context( - RoleGroupSelectorLabelsSnafu { - role: *role, - role_group: role_group_name.clone(), - }, - )?; - let resolved = rg_config.config.resolve(role_group_name, selector_labels)?; + let builder = RoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; - rg_resources.config_maps.push( - resource::config_map::build_rolegroup_config_map( - cluster, - cluster_info, - role_group_name, - rg_config, - &resolved, - ) - .context(ConfigMapSnafu { - role: *role, - role_group: role_group_name.clone(), - })?, - ); - rg_resources.stateful_sets.entry(C::ROLE).or_default().push( - resource::statefulset::build_rolegroup_statefulset( - cluster, - cluster_info, - role_group_name, - rg_config, - &resolved, - ) - .context(StatefulSetSnafu { - role: *role, - role_group: role_group_name.clone(), - })?, - ); + rg_resources.services.extend(builder.build_services()?); + rg_resources.config_maps.push(builder.build_config_map()?); + rg_resources + .stateful_sets + .entry(C::ROLE) + .or_default() + .push(builder.build_stateful_set()?); } if let Some(pdb) = resource::pdb::build_pdb(cluster, role) { @@ -250,34 +226,6 @@ pub fn build( }) } -/// Builds the two Services for one role group. Role-agnostic: it reads nothing from the role -/// config. -fn build_role_group_services( - cluster: &ValidatedCluster, - role: &HdfsNodeRole, - role_group_name: &RoleGroupName, - services: &mut Vec, -) -> Result<(), Error> { - services.push( - resource::service::rolegroup_headless_service(cluster, role, role_group_name).context( - ServiceSnafu { - role: *role, - role_group: role_group_name.clone(), - }, - )?, - ); - services.push( - resource::service::rolegroup_metrics_service(cluster, role, role_group_name).context( - ServiceSnafu { - role: *role, - role_group: role_group_name.clone(), - }, - )?, - ); - - Ok(()) -} - /// The replica count a role group gets when it does not set one: Kubernetes runs a single pod for /// a `StatefulSet` with `replicas: null`. pub(crate) const DEFAULT_REPLICAS: u16 = 1; diff --git a/rust/operator-binary/src/controller/build/resolve.rs b/rust/operator-binary/src/controller/build/resolve.rs index 66fcea75..0a2d0b19 100644 --- a/rust/operator-binary/src/controller/build/resolve.rs +++ b/rust/operator-binary/src/controller/build/resolve.rs @@ -1,24 +1,64 @@ -//! Resolving one role group into the values the shared builders cannot derive themselves. +//! Resolving one role group into everything the shared builders need. //! //! One [`RoleGroupResolver`] impl per role config type, so a role's resolution is written once. +//! [`RoleGroupResolver::resolve`] takes the whole [`RoleGroupConfig`] and returns a +//! [`ResolvedRoleGroup`] that no longer mentions the config type, so the builders take one +//! non-generic argument and read the role back out of it. -use std::{fmt::Display, marker::PhantomData}; +use std::fmt::Display; use snafu::ResultExt; use stackable_operator::{ - k8s_openapi::api::core::v1::{PersistentVolumeClaim, ResourceRequirements, Volume}, + k8s_openapi::api::core::v1::{ + PersistentVolumeClaim, PodTemplateSpec, ResourceRequirements, Volume, + }, kvp::Labels, product_logging::spec::{ContainerLogConfig, Logging}, - v2::types::operator::RoleGroupName, + v2::{ + builder::pod::container::EnvVarSet, + jvm_argument_overrides::JvmArgumentOverrides, + role_utils::{JavaCommonConfig, RoleGroupConfig}, + types::operator::RoleGroupName, + }, }; use super::{Error, ListenerVolumeSnafu, VolumeClaimTemplatesSnafu, container::ContainerConfig}; use crate::crd::{ CommonNodeConfig, DataNodeConfig, DataNodeContainer, HdfsNodeRole, JournalNodeConfig, JournalNodeContainer, NameNodeConfig, NameNodeContainer, - storage::DataNodeStorageConfigInnerType, + storage::DataNodeStorageConfigInnerType, v1alpha1, }; +/// The role group's merged values that the builders use verbatim: its replica count and the +/// override sets. Nothing here depends on the role, which is why it is carried alongside the +/// resolved values rather than among them. +/// +/// `cli_overrides` is deliberately absent: it is merged during validation but no builder reads it. +pub struct MergedRoleGroupConfig { + pub replicas: Option, + pub config_overrides: v1alpha1::HdfsConfigOverrides, + pub env_overrides: EnvVarSet, + pub pod_overrides: PodTemplateSpec, + pub jvm_argument_overrides: JvmArgumentOverrides, +} + +impl MergedRoleGroupConfig { + fn of( + rg_config: &RoleGroupConfig, + ) -> Self { + Self { + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + } + } +} + /// The log config of the two containers every role has. Containers only one role runs carry theirs /// in [`RoleSpecificValues`], which is the single place the role is decided. #[derive(Debug)] @@ -32,10 +72,9 @@ pub struct RoleGroupLogging { /// The values the shared builders cannot derive themselves, resolved by /// [`RoleGroupResolver::resolve`], which knows the role. /// -/// Every builder takes `RoleGroupConfig` and `ResolvedRoleGroup` together, so one role's -/// overrides and replica count cannot be paired with another role's resolved values: both are the -/// same `C` or they do not compile. -pub struct ResolvedRoleGroup { +/// The builders take this and nothing else about the role group, so there is no second argument to +/// pair with the wrong one. The role comes from [`RoleSpecificValues::node_role`]. +pub struct ResolvedRoleGroup { /// The selector labels of the role group's pods, also used as the `StatefulSet` selector and /// on its listener volume. /// @@ -55,10 +94,8 @@ pub struct ResolvedRoleGroup { pub role: RoleSpecificValues, /// The log config of each of the role group's containers. pub logging: RoleGroupLogging, - /// Ties the bundle to its config type. Needed because `C` appears in no other field, which on - /// its own does not compile (`E0392`). Private, so [`RoleGroupResolver::resolve`] is the only - /// constructor outside this module — a struct literal elsewhere is `E0451`. - _config: PhantomData, + /// The role group's replica count and overrides, carried through unchanged. + pub merged: MergedRoleGroupConfig, } /// Everything that exists for one role only: the containers that role runs, their log configs, and @@ -93,6 +130,15 @@ pub enum RoleSpecificValues { } impl RoleSpecificValues { + /// The role these values belong to. + pub fn node_role(&self) -> HdfsNodeRole { + match self { + Self::Journal => HdfsNodeRole::Journal, + Self::Name { .. } => HdfsNodeRole::Name, + Self::Data { .. } => HdfsNodeRole::Data, + } + } + /// The role group's ephemeral listener volume; only datanodes have one. pub fn listener_volume(&self) -> Option<&Volume> { match self { @@ -146,34 +192,35 @@ pub(crate) trait RoleGroupResolver: Sized { /// Resolves everything the shared builders cannot derive themselves. Takes the selector /// labels because two of the three roles need them to build their listener. fn resolve( - &self, + rg_config: &RoleGroupConfig, role_group_name: &RoleGroupName, selector_labels: Labels, - ) -> Result, Error>; + ) -> Result; } impl RoleGroupResolver for JournalNodeConfig { const ROLE: HdfsNodeRole = HdfsNodeRole::Journal; fn resolve( - &self, + rg_config: &RoleGroupConfig, _role_group_name: &RoleGroupName, selector_labels: Labels, - ) -> Result, Error> { + ) -> Result { + let config = &rg_config.config; let (hdfs, vector) = common_container_logging( - &self.logging, + &config.logging, JournalNodeContainer::Hdfs, JournalNodeContainer::Vector, ); Ok(ResolvedRoleGroup { selector_labels, - common: self.common.clone(), - resources: self.resources.clone().into(), - volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(self), + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), role: RoleSpecificValues::Journal, logging: RoleGroupLogging { hdfs, vector }, - _config: PhantomData, + merged: MergedRoleGroupConfig::of(rg_config), }) } } @@ -182,14 +229,15 @@ impl RoleGroupResolver for NameNodeConfig { const ROLE: HdfsNodeRole = HdfsNodeRole::Name; fn resolve( - &self, + rg_config: &RoleGroupConfig, role_group_name: &RoleGroupName, selector_labels: Labels, - ) -> Result, Error> { + ) -> Result { + let config = &rg_config.config; // Namenodes get their listener from a persistent volume claim template, for stable // per-pod identity, rather than from an ephemeral volume. let volume_claim_templates = - ContainerConfig::namenode_volume_claim_templates(self, &selector_labels).context( + ContainerConfig::namenode_volume_claim_templates(config, &selector_labels).context( VolumeClaimTemplatesSnafu { role: Self::ROLE, role_group: role_group_name.clone(), @@ -197,32 +245,32 @@ impl RoleGroupResolver for NameNodeConfig { )?; let (hdfs, vector) = common_container_logging( - &self.logging, + &config.logging, NameNodeContainer::Hdfs, NameNodeContainer::Vector, ); Ok(ResolvedRoleGroup { selector_labels, - common: self.common.clone(), - resources: self.resources.clone().into(), + common: config.common.clone(), + resources: config.resources.clone().into(), volume_claim_templates, role: RoleSpecificValues::Name { - zkfc: self + zkfc: config .logging .for_container(&NameNodeContainer::Zkfc) .into_owned(), - format_namenodes: self + format_namenodes: config .logging .for_container(&NameNodeContainer::FormatNameNodes) .into_owned(), - format_zookeeper: self + format_zookeeper: config .logging .for_container(&NameNodeContainer::FormatZooKeeper) .into_owned(), }, logging: RoleGroupLogging { hdfs, vector }, - _config: PhantomData, + merged: MergedRoleGroupConfig::of(rg_config), }) } } @@ -231,38 +279,39 @@ impl RoleGroupResolver for DataNodeConfig { const ROLE: HdfsNodeRole = HdfsNodeRole::Data; fn resolve( - &self, + rg_config: &RoleGroupConfig, role_group_name: &RoleGroupName, selector_labels: Labels, - ) -> Result, Error> { + ) -> Result { + let config = &rg_config.config; // Datanodes use an ephemeral listener volume, since they need no stable per-pod identity. - let listener_volume = ContainerConfig::datanode_listener_volume(self, &selector_labels) + let listener_volume = ContainerConfig::datanode_listener_volume(config, &selector_labels) .context(ListenerVolumeSnafu { - role: Self::ROLE, - role_group: role_group_name.clone(), - })?; + role: Self::ROLE, + role_group: role_group_name.clone(), + })?; let (hdfs, vector) = common_container_logging( - &self.logging, + &config.logging, DataNodeContainer::Hdfs, DataNodeContainer::Vector, ); Ok(ResolvedRoleGroup { selector_labels, - common: self.common.clone(), - resources: self.resources.clone().into(), - volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(self), + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), role: RoleSpecificValues::Data { listener_volume, - storage: self.resources.storage.clone(), - wait_for_namenodes: self + storage: config.resources.storage.clone(), + wait_for_namenodes: config .logging .for_container(&DataNodeContainer::WaitForNameNodes) .into_owned(), }, logging: RoleGroupLogging { hdfs, vector }, - _config: PhantomData, + merged: MergedRoleGroupConfig::of(rg_config), }) } } diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index f77ebf9c..ef9ee8ea 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -2,29 +2,16 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::configmap::ConfigMapBuilder, - k8s_openapi::api::core::v1::ConfigMap, - product_logging::framework::VECTOR_CONFIG_FILE, - utils::cluster_info::KubernetesClusterInfo, - v2::{ - config_file_writer::PropertiesWriterError, - role_utils::{JavaCommonConfig, RoleGroupConfig}, - types::operator::RoleGroupName, - }, + builder::configmap::ConfigMapBuilder, k8s_openapi::api::core::v1::ConfigMap, + product_logging::framework::VECTOR_CONFIG_FILE, v2::config_file_writer::PropertiesWriterError, }; -use crate::{ - controller::{ - ValidatedCluster, - build::{ - self, ResolvedRoleGroup, RoleGroupResolver, - properties::{ - ConfigFileName, core_site, hadoop_policy, hdfs_site, product_logging, - security_properties, ssl_client, ssl_server, - }, - }, +use crate::controller::build::{ + self, RoleGroupBuilder, + properties::{ + ConfigFileName, core_site, hadoop_policy, hdfs_site, product_logging, security_properties, + ssl_client, ssl_server, }, - crd::v1alpha1, }; #[derive(Snafu, Debug)] @@ -47,20 +34,18 @@ type Result = std::result::Result; /// Builds the [`ConfigMap`] of one role group. /// -/// Every role-specific value is resolved by the caller into `resolved`. The role comes from -/// `C::ROLE`, and `C`'s [`RoleGroupResolver`] bound ties it to `resolved`, so this cannot read one -/// role's `HdfsNodeRole` alongside another role's resolved values. The datanode storage -/// configuration comes from `resolved` rather than a separate parameter: taking it independently -/// would let a caller pass a datanode without its storage, which silently drops -/// `dfs.datanode.data.dir`. -pub fn build_rolegroup_config_map( - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rolegroup_config: &RoleGroupConfig, - resolved: &ResolvedRoleGroup, -) -> Result { - let role = C::ROLE; +/// Everything about the role group comes from `resolved`, the role and the merged overrides +/// included, so there is nothing here to pair with the wrong role group. The datanode storage +/// configuration comes from `resolved` for the same reason: taking it independently would let a +/// caller pass a datanode without its storage, which silently drops `dfs.datanode.data.dir`. +pub(crate) fn build_rolegroup_config_map(builder: &RoleGroupBuilder) -> Result { + let RoleGroupBuilder { + cluster, + cluster_info, + role_group_name, + resolved, + } = builder; + let role = builder.role(); tracing::info!( "Setting up ConfigMap for role {role} role group {role_group_name}", @@ -69,7 +54,7 @@ pub fn build_rolegroup_config_map( let metadata = build::rolegroup_metadata(cluster, &role, role_group_name); - let config_overrides = &rolegroup_config.config_overrides; + let config_overrides = &resolved.merged.config_overrides; let cluster_config = &cluster.cluster_config; let hdfs_site_xml = hdfs_site::build( diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index a4988c69..ebb2147d 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -9,23 +9,12 @@ use stackable_operator::{ apimachinery::pkg::apis::meta::v1::LabelSelector, }, kube::api::ObjectMeta, - utils::cluster_info::KubernetesClusterInfo, - v2::{ - role_utils::{JavaCommonConfig, RoleGroupConfig}, - types::operator::RoleGroupName, - }, }; -use crate::{ - controller::{ - ValidatedCluster, - build::{ - self, ResolvedRoleGroup, RoleGroupResolver, - container::{self, ContainerConfig}, - graceful_shutdown::{self, add_graceful_shutdown_config}, - }, - }, - crd::v1alpha1, +use crate::controller::build::{ + self, RoleGroupBuilder, + container::{self, ContainerConfig}, + graceful_shutdown::{self, add_graceful_shutdown_config}, }; #[derive(Snafu, Debug)] @@ -39,17 +28,18 @@ pub enum Error { /// Builds the [`StatefulSet`] of one role group. /// -/// Every role-specific value is resolved by the caller into `resolved`. The role comes from -/// `C::ROLE`, and `resolved` is [`ResolvedRoleGroup`](ResolvedRoleGroup), produced by that same -/// `C`'s [`RoleGroupResolver::resolve`], so it cannot disagree with `resolved`. -pub(crate) fn build_rolegroup_statefulset( - validated: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rolegroup_config: &RoleGroupConfig, - resolved: &ResolvedRoleGroup, +/// Everything about the role group comes from `resolved`, the role and the merged overrides +/// included, so there is nothing here to pair with the wrong role group. +pub(crate) fn build_rolegroup_statefulset( + builder: &RoleGroupBuilder, ) -> Result { - let role = &C::ROLE; + let RoleGroupBuilder { + cluster: validated, + role_group_name, + resolved, + .. + } = builder; + let role = &builder.role(); tracing::info!( "Setting up StatefulSet for role {role} role group {role_group_name}", @@ -82,26 +72,19 @@ pub(crate) fn build_rolegroup_statefulset( ); // Adds all containers and volumes to the pod builder. - ContainerConfig::add_containers_and_volumes( - &mut pb, - validated, - cluster_info, - role_group_name, - rolegroup_config, - resolved, - ) - .context(FailedToCreateContainerAndVolumeConfigurationSnafu)?; + ContainerConfig::add_containers_and_volumes(&mut pb, builder) + .context(FailedToCreateContainerAndVolumeConfigurationSnafu)?; add_graceful_shutdown_config(&resolved.common, &mut pb).context(GracefulShutdownSnafu)?; // The `podOverrides` were already merged (role <- role group) during validation // by the local-`framework` `with_validated_config`. let mut pod_template = pb.build_template(); - pod_template.merge_from(rolegroup_config.pod_overrides.clone()); + pod_template.merge_from(resolved.merged.pod_overrides.clone()); let statefulset_spec = StatefulSetSpec { pod_management_policy: Some("OrderedReady".to_string()), - replicas: rolegroup_config.replicas.map(i32::from), + replicas: resolved.merged.replicas.map(i32::from), selector: LabelSelector { match_labels: Some(resolved.selector_labels.clone().into()), ..LabelSelector::default() diff --git a/rust/operator-binary/src/controller/build/role_group_builder.rs b/rust/operator-binary/src/controller/build/role_group_builder.rs new file mode 100644 index 00000000..33c097c5 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group_builder.rs @@ -0,0 +1,104 @@ +//! Building the Kubernetes resources of one role group. +//! +//! [`RoleGroupBuilder::new`] resolves the role group once; every builder below reads that one +//! object, so none of them takes the role, the config or the resolved values as separate +//! arguments and there is nothing to pair with the wrong role group. + +use snafu::ResultExt; +use stackable_operator::{ + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Service}, + }, + utils::cluster_info::KubernetesClusterInfo, + v2::{ + role_utils::{JavaCommonConfig, RoleGroupConfig}, + types::operator::RoleGroupName, + }, +}; + +use super::{ + ConfigMapSnafu, Error, RoleGroupSelectorLabelsSnafu, ServiceSnafu, StatefulSetSnafu, + resolve::{ResolvedRoleGroup, RoleGroupResolver}, + resource, rolegroup_selector_labels, +}; +use crate::{ + controller::ValidatedCluster, + crd::{HdfsNodeRole, v1alpha1}, +}; + +/// One role group's resolved values plus the context every builder needs. +pub(crate) struct RoleGroupBuilder<'a> { + pub(crate) cluster: &'a ValidatedCluster, + pub(crate) cluster_info: &'a KubernetesClusterInfo, + pub(crate) role_group_name: RoleGroupName, + pub(crate) resolved: ResolvedRoleGroup, +} + +impl<'a> RoleGroupBuilder<'a> { + /// Resolves one role group. `C` is the role group's config type and appears here only: the + /// builder it returns names no config type, so nothing below this point is generic. + pub(crate) fn new( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &RoleGroupConfig, + ) -> Result { + let selector_labels = rolegroup_selector_labels(cluster, &C::ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { + role: C::ROLE, + role_group: role_group_name.clone(), + })?; + let resolved = C::resolve(rg_config, role_group_name, selector_labels)?; + + Ok(Self { + cluster, + cluster_info, + role_group_name: role_group_name.clone(), + resolved, + }) + } + + /// The role this role group belongs to, read back out of the resolved values. + pub(crate) fn role(&self) -> HdfsNodeRole { + self.resolved.role.node_role() + } + + /// The headless and metrics Services. Role-agnostic: neither reads the role config. + pub(crate) fn build_services(&self) -> Result, Error> { + let role = self.role(); + let context = || ServiceSnafu { + role, + role_group: self.role_group_name.clone(), + }; + + Ok(vec![ + resource::service::rolegroup_headless_service( + self.cluster, + &role, + &self.role_group_name, + ) + .with_context(|_| context())?, + resource::service::rolegroup_metrics_service( + self.cluster, + &role, + &self.role_group_name, + ) + .with_context(|_| context())?, + ]) + } + + pub(crate) fn build_config_map(&self) -> Result { + resource::config_map::build_rolegroup_config_map(self).context(ConfigMapSnafu { + role: self.role(), + role_group: self.role_group_name.clone(), + }) + } + + pub(crate) fn build_stateful_set(&self) -> Result { + resource::statefulset::build_rolegroup_statefulset(self).context(StatefulSetSnafu { + role: self.role(), + role_group: self.role_group_name.clone(), + }) + } +} diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index 7734aa21..a4063424 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -170,7 +170,6 @@ mod test { events::{Recorder, Reporter}, }, }, - kvp::Labels, utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; @@ -178,10 +177,8 @@ mod test { use super::*; use crate::{ HDFS_FULL_CONTROLLER_NAME, - controller::build::{RoleGroupResolver, container::ContainerConfig}, - test_support::{ - datanode_config, datanode_role_group_config, deserialize_cluster, validate_cluster, - }, + controller::build::{RoleGroupBuilder, container::ContainerConfig}, + test_support::{datanode_role_group_config, deserialize_cluster, validate_cluster}, }; #[test] @@ -224,25 +221,22 @@ spec: let validated_cluster = validate_cluster(&hdfs); let role_group_name = RoleGroupName::from_str("default").unwrap(); let role_group_config = datanode_role_group_config(&validated_cluster, &role_group_name); - // Resolved through the production path, so this test cannot drift from what the build - // step actually hands the container builder. - let resolved = datanode_config(&validated_cluster, &role_group_name) - .resolve(&role_group_name, Labels::new()) - .expect("the datanode role group should resolve"); - - let mut pb = PodBuilder::new(); - pb.metadata(ObjectMeta::default()); - ContainerConfig::add_containers_and_volumes( - &mut pb, + let cluster_info = KubernetesClusterInfo { + cluster_domain: DomainName::try_from("cluster.local").unwrap(), + }; + // Built through the production path, so this test cannot drift from what the build step + // actually hands the container builder. + let builder = RoleGroupBuilder::new( &validated_cluster, - &KubernetesClusterInfo { - cluster_domain: DomainName::try_from("cluster.local").unwrap(), - }, + &cluster_info, &role_group_name, role_group_config, - &resolved, ) - .unwrap(); + .expect("the datanode role group should resolve"); + + let mut pb = PodBuilder::new(); + pb.metadata(ObjectMeta::default()); + ContainerConfig::add_containers_and_volumes(&mut pb, &builder).unwrap(); let containers = pb.build().unwrap().spec.unwrap().containers; let env_vars = containers .iter() From b88dc5970207979099d68efa054248339226584d Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 22 Sep 2026 12:51:00 +0200 Subject: [PATCH 2/4] replace common role group builder with individual ones for each role --- .../src/controller/build/container.rs | 270 +++++---------- .../src/controller/build/mod.rs | 133 ++++---- .../build/properties/product_logging/mod.rs | 184 +++++----- .../src/controller/build/resolve.rs | 317 ------------------ .../controller/build/resource/config_map.rs | 78 +++-- .../controller/build/resource/statefulset.rs | 80 +++-- .../controller/build/role_group/datanode.rs | 151 +++++++++ .../build/role_group/journalnode.rs | 110 ++++++ .../src/controller/build/role_group/mod.rs | 163 +++++++++ .../controller/build/role_group/namenode.rs | 162 +++++++++ .../controller/build/role_group_builder.rs | 104 ------ rust/operator-binary/src/hdfs_controller.rs | 26 +- 12 files changed, 944 insertions(+), 834 deletions(-) delete mode 100644 rust/operator-binary/src/controller/build/resolve.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/datanode.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/journalnode.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/mod.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/namenode.rs delete mode 100644 rust/operator-binary/src/controller/build/role_group_builder.rs diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 9c76e891..83718931 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -65,16 +65,15 @@ use crate::{ controller::{ ValidatedCluster, build::{ - self, ResolvedRoleGroup, RoleGroupBuilder, RoleSpecificValues, + self, jvm::{self, construct_global_jvm_args, construct_role_specific_jvm_args}, kerberos::KERBEROS_CONTAINER_PATH, properties::product_logging::{ - FORMAT_NAMENODES_LOG4J_CONFIG_FILE, FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, - HDFS_LOG4J_CONFIG_FILE, MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, - MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, MAX_HDFS_LOG_FILE_SIZE, - MAX_WAIT_NAMENODES_LOG_FILE_SIZE, MAX_ZKFC_LOG_FILE_SIZE, - WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, ZKFC_LOG4J_CONFIG_FILE, + MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, + MAX_HDFS_LOG_FILE_SIZE, MAX_WAIT_NAMENODES_LOG_FILE_SIZE, MAX_ZKFC_LOG_FILE_SIZE, + log4j_config_file, }, + role_group::RoleGroupCommon, }, }, crd::{ @@ -208,43 +207,29 @@ impl ContainerConfig { const ZKFC_CONFIG_VOLUME_MOUNT_NAME: &'static str = "zkfc-config"; const ZKFC_LOG_VOLUME_MOUNT_NAME: &'static str = "zkfc-log-config"; - /// Add all main, side and init containers as well as required volumes to the pod builder. + /// Adds the containers every role runs — the `hdfs` main container and, when enabled, the + /// Vector sidecar — plus the volumes every role group needs, to the pod builder. /// - /// Everything about the role group comes from `resolved`, the role and the merged overrides - /// included, so there is nothing here to pair with the wrong role group. - pub(crate) fn add_containers_and_volumes( + /// The containers only one role runs are added by that role's builder in + /// [`role_group`](crate::controller::build::role_group). + pub(crate) fn add_hdfs_container_and_common_volumes( pb: &mut PodBuilder, - builder: &RoleGroupBuilder, + common: &RoleGroupCommon, ) -> Result<(), Error> { - let RoleGroupBuilder { - cluster, - cluster_info, - role_group_name, - resolved, - } = builder; - let role = &builder.role(); - let namenode_podrefs = build::pod_refs(cluster, &HdfsNodeRole::Name); + let cluster = common.cluster; + let role = &common.role; + let object_name = common.object_name(); + let resource_names = cluster.role_group_resource_names(role, &common.role_group_name); // HDFS main container let main_container_config = Self::from(*role); - let resource_names = cluster.role_group_resource_names(role, role_group_name); - let object_name = resource_names.qualified_role_group_name().to_string(); - pb.add_volumes(main_container_config.volumes( - &resolved.logging.hdfs, - resolved.role.listener_volume(), - &object_name, - )) - .context(AddVolumeSnafu)?; - pb.add_container(main_container_config.main_container( - cluster, - cluster_info, - &resolved.logging.hdfs, - resolved, - )?); + pb.add_volumes(main_container_config.volumes(&common.hdfs_logging, &object_name)) + .context(AddVolumeSnafu)?; + pb.add_container(main_container_config.main_container(common, &common.hdfs_logging)?); // Vector sidecar container. - if let Some(vector_logging) = &resolved.logging.vector { + if let Some(vector_logging) = &common.vector_logging { let vector_aggregator_config_map_name = cluster .cluster_config .logging @@ -277,7 +262,7 @@ impl ContainerConfig { log_config, vector_aggregator_config_map_name, }, - &cluster.role_group_resource_names(role, role_group_name), + &resource_names, &VECTOR_CONFIG_VOLUME_NAME, &VECTOR_LOG_VOLUME_NAME, EnvVarSet::new(), @@ -301,7 +286,7 @@ impl ContainerConfig { .with_format(SecretFormat::TlsPkcs12) .with_tls_pkcs12_password(TLS_STORE_PASSWORD) .with_auto_tls_cert_lifetime( - resolved + common .common .requested_secret_lifetime .context(MissingSecretLifetimeSnafu)?, @@ -336,78 +321,45 @@ impl ContainerConfig { .context(AddVolumeSnafu)?; } - // The role-specific containers and their log configs come from one enum, so a container - // is never built without the log config that belongs to it. - match &resolved.role { - RoleSpecificValues::Journal => {} - RoleSpecificValues::Name { - zkfc, - format_namenodes, - format_zookeeper, - } => { - // Zookeeper fail over container - let zkfc_container_config = Self::Zkfc; - pb.add_volumes(zkfc_container_config.volumes(zkfc, None, &object_name)) - .context(AddVolumeSnafu)?; - pb.add_container(zkfc_container_config.main_container( - cluster, - cluster_info, - zkfc, - resolved, - )?); - - // Format namenode init container - let format_namenodes_container_config = Self::FormatNameNodes; - pb.add_volumes(format_namenodes_container_config.volumes( - format_namenodes, - None, - &object_name, - )) - .context(AddVolumeSnafu)?; - pb.add_init_container(format_namenodes_container_config.init_container( - cluster, - cluster_info, - format_namenodes, - resolved, - &namenode_podrefs, - )?); - - // Format ZooKeeper init container - let format_zookeeper_container_config = Self::FormatZooKeeper; - pb.add_volumes(format_zookeeper_container_config.volumes( - format_zookeeper, - None, - &object_name, - )) - .context(AddVolumeSnafu)?; - pb.add_init_container(format_zookeeper_container_config.init_container( - cluster, - cluster_info, - format_zookeeper, - resolved, - &namenode_podrefs, - )?); - } - RoleSpecificValues::Data { - wait_for_namenodes, .. - } => { - // Wait for namenode init container - let wait_for_namenodes_container_config = Self::WaitForNameNodes; - pb.add_volumes(wait_for_namenodes_container_config.volumes( - wait_for_namenodes, - None, - &object_name, - )) - .context(AddVolumeSnafu)?; - pb.add_init_container(wait_for_namenodes_container_config.init_container( - cluster, - cluster_info, - wait_for_namenodes, - resolved, - &namenode_podrefs, - )?); - } - } + Ok(()) + } + + /// Adds this container to the pod as a side container, together with the volumes it needs. + /// + /// `container_log_config` is this container's own, passed by the role builder adding it. + pub(crate) fn add_as_side_container( + &self, + pb: &mut PodBuilder, + common: &RoleGroupCommon, + container_log_config: &ContainerLogConfig, + ) -> Result<(), Error> { + pb.add_volumes(self.volumes(container_log_config, &common.object_name())) + .context(AddVolumeSnafu)?; + pb.add_container(self.main_container(common, container_log_config)?); + + Ok(()) + } + + /// Adds this container to the pod as an init container, together with the volumes it needs. + /// + /// `container_log_config` is this container's own, passed by the role builder adding it. + pub(crate) fn add_as_init_container( + &self, + pb: &mut PodBuilder, + common: &RoleGroupCommon, + container_log_config: &ContainerLogConfig, + ) -> Result<(), Error> { + // `format-namenodes` and `wait-for-namenodes` address the namenodes by pod name; + // `format-zookeeper` ignores these. + let namenode_podrefs = build::pod_refs(common.cluster, &HdfsNodeRole::Name); + + pb.add_volumes(self.volumes(container_log_config, &common.object_name())) + .context(AddVolumeSnafu)?; + pb.add_init_container(self.init_container( + common, + container_log_config, + &namenode_podrefs, + )?); Ok(()) } @@ -487,21 +439,19 @@ impl ContainerConfig { /// - Journalnode main process fn main_container( &self, - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, + common: &RoleGroupCommon, container_log_config: &ContainerLogConfig, - resolved: &ResolvedRoleGroup, ) -> Result { - let role = &resolved.role.node_role(); + let cluster = common.cluster; let mut cb = new_container_builder(self.container_name()); - let resources = self.resources(&resolved.resources); + let resources = self.resources(&common.resources); cb.image_from_product_image(&cluster.image) .command(Self::command()) - .args(self.args(cluster, cluster_info, role, container_log_config, &[])?) - .add_env_vars(self.env(cluster, role, resolved, resources.as_ref())?) - .add_volume_mounts(self.volume_mounts(cluster, &resolved.volume_claim_templates)) + .args(self.args(common, container_log_config, &[])?) + .add_env_vars(self.env(common, resources.as_ref())?) + .add_volume_mounts(self.volume_mounts(cluster, &common.volume_claim_templates)) .context(AddVolumeMountSnafu)? .add_container_ports(self.container_ports(cluster)); @@ -534,32 +484,24 @@ impl ContainerConfig { /// - Datanode (wait-for-namenodes) fn init_container( &self, - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, + common: &RoleGroupCommon, container_log_config: &ContainerLogConfig, - resolved: &ResolvedRoleGroup, namenode_podrefs: &[HdfsPodRef], ) -> Result { - let role = &resolved.role.node_role(); + let cluster = common.cluster; let mut cb = new_container_builder(self.container_name()); cb.image_from_product_image(&cluster.image) .command(Self::command()) - .args(self.args( - cluster, - cluster_info, - role, - container_log_config, - namenode_podrefs, - )?) - .add_env_vars(self.env(cluster, role, resolved, None)?) - .add_volume_mounts(self.volume_mounts(cluster, &resolved.volume_claim_templates)) + .args(self.args(common, container_log_config, namenode_podrefs)?) + .add_env_vars(self.env(common, None)?) + .add_volume_mounts(self.volume_mounts(cluster, &common.volume_claim_templates)) .context(AddVolumeMountSnafu)?; // We use the main app container resources here in contrast to several operators (which use // hardcoded resources) due to the different code structure. // Going forward this should be replaced by calculating init container resources in the pod builder. - if let Some(resources) = self.resources(&resolved.resources) { + if let Some(resources) = self.resources(&common.resources) { cb.resources(resources); } @@ -627,12 +569,13 @@ impl ContainerConfig { /// Returns the container command arguments. fn args( &self, - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, - role: &HdfsNodeRole, + common: &RoleGroupCommon, container_log_config: &ContainerLogConfig, namenode_podrefs: &[HdfsPodRef], ) -> Result, Error> { + let cluster = common.cluster; + let cluster_info = common.cluster_info; + let role = &common.role; let mut args = String::new(); args.push_str(&self.create_config_directory_cmd()); args.push_str(&self.copy_config_xml_cmd()); @@ -652,9 +595,7 @@ impl ContainerConfig { match self { ContainerConfig::Hdfs { role, .. } => { - args.push_str( - &self.copy_log4j_properties_cmd(HDFS_LOG4J_CONFIG_FILE, container_log_config), - ); + args.push_str(&self.copy_log4j_properties_cmd(container_log_config)); args.push_str(&formatdoc!( r#"\ @@ -681,9 +622,7 @@ impl ContainerConfig { )); } ContainerConfig::Zkfc => { - args.push_str( - &self.copy_log4j_properties_cmd(ZKFC_LOG4J_CONFIG_FILE, container_log_config), - ); + args.push_str(&self.copy_log4j_properties_cmd(container_log_config)); args.push_str(&format!( "{hadoop_home}/bin/hdfs zkfc\n", hadoop_home = Self::HADOOP_HOME @@ -692,10 +631,7 @@ impl ContainerConfig { ContainerConfig::FormatNameNodes => { args.push_str(&bash_capture_shell_helper(self.container_name().as_ref())); - args.push_str(&self.copy_log4j_properties_cmd( - FORMAT_NAMENODES_LOG4J_CONFIG_FILE, - container_log_config, - )); + args.push_str(&self.copy_log4j_properties_cmd(container_log_config)); // First step we check for active namenodes. This step should return an active namenode // for e.g. scaling. It may fail if the active namenode is restarted and the standby // namenode takes over. @@ -763,10 +699,7 @@ impl ContainerConfig { ContainerConfig::FormatZooKeeper => { args.push_str(&bash_capture_shell_helper(self.container_name().as_ref())); - args.push_str(&self.copy_log4j_properties_cmd( - FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, - container_log_config, - )); + args.push_str(&self.copy_log4j_properties_cmd(container_log_config)); args.push_str(&formatdoc!( r###" echo "Attempt to format ZooKeeper ZNode for $POD_NAME ..." @@ -790,10 +723,7 @@ impl ContainerConfig { ContainerConfig::WaitForNameNodes => { args.push_str(&bash_capture_shell_helper(self.container_name().as_ref())); - args.push_str(&self.copy_log4j_properties_cmd( - WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, - container_log_config, - )); + args.push_str(&self.copy_log4j_properties_cmd(container_log_config)); if cluster.has_kerberos_enabled() { args.push_str(&Self::get_kerberos_ticket(cluster, role, cluster_info)?); } @@ -875,11 +805,11 @@ impl ContainerConfig { /// Returns the container env variables. fn env( &self, - cluster: &ValidatedCluster, - role: &HdfsNodeRole, - resolved: &ResolvedRoleGroup, + common: &RoleGroupCommon, resources: Option<&ResourceRequirements>, ) -> Result, Error> { + let cluster = common.cluster; + let role = &common.role; // Maps env var name to env var object. This allows env_overrides to work // as expected (i.e. users can override the env var value). let mut env: BTreeMap = BTreeMap::new(); @@ -908,7 +838,7 @@ impl ContainerConfig { role_opts_name.clone(), EnvVar { name: role_opts_name, - value: Some(self.build_hadoop_opts(cluster, resources, resolved)?), + value: Some(self.build_hadoop_opts(common, resources)?), ..EnvVar::default() }, ); @@ -972,8 +902,7 @@ impl ContainerConfig { ); // Overrides need to come last - let mut env_override_vars: BTreeMap = resolved - .merged + let mut env_override_vars: BTreeMap = common .env_overrides .clone() .into_iter() @@ -1071,20 +1000,11 @@ impl ContainerConfig { /// Return the container volumes. /// - /// `container_log_config` is this container's own, chosen by the caller from - /// [`build::RoleGroupLogging`] or [`RoleSpecificValues`]. `listener_volume` is the role - /// group's ephemeral listener volume, which only the datanode main container has. - fn volumes( - &self, - container_log_config: &ContainerLogConfig, - listener_volume: Option<&Volume>, - object_name: &str, - ) -> Vec { + /// `container_log_config` is this container's own, passed by the role builder adding it. + fn volumes(&self, container_log_config: &ContainerLogConfig, object_name: &str) -> Vec { let mut volumes = vec![]; if let ContainerConfig::Hdfs { .. } = self { - volumes.extend(listener_volume.cloned()); - volumes.push( VolumeBuilder::new(ContainerConfig::STACKABLE_LOG_VOLUME_MOUNT_NAME) .empty_dir(EmptyDirVolumeSource { @@ -1222,11 +1142,7 @@ impl ContainerConfig { /// This will be copied from: /// - Custom: the log dir mount of the custom config map /// - Automatic: the container config mount dir - fn copy_log4j_properties_cmd( - &self, - log4j_config_file: &str, - container_log_config: &ContainerLogConfig, - ) -> String { + fn copy_log4j_properties_cmd(&self, container_log_config: &ContainerLogConfig) -> String { let volume_mount_dirs = self.volume_mount_dirs(); let source_log4j_properties_dir = if let ContainerLogConfig { choice: Some(ContainerLogConfigChoice::Custom(_)), @@ -1240,7 +1156,7 @@ impl ContainerConfig { format!( "cp {log4j_properties_dir}/{file_name} {config_dir}/{LOG4J_PROPERTIES}\n", log4j_properties_dir = source_log4j_properties_dir, - file_name = log4j_config_file, + file_name = log4j_config_file(self), config_dir = volume_mount_dirs.final_config() ) } @@ -1248,10 +1164,10 @@ impl ContainerConfig { /// Build HADOOP_{*node}_OPTS for each namenode, datanodes and journalnodes. fn build_hadoop_opts( &self, - cluster: &ValidatedCluster, + common: &RoleGroupCommon, resources: Option<&ResourceRequirements>, - resolved: &ResolvedRoleGroup, ) -> Result { + let cluster = common.cluster; match self { ContainerConfig::Hdfs { role, metrics_port, .. @@ -1260,7 +1176,7 @@ impl ContainerConfig { let config_dir = volume_mount_dirs.final_config(); construct_role_specific_jvm_args( role, - &resolved.merged.jvm_argument_overrides, + &common.jvm_argument_overrides, cluster.has_kerberos_enabled(), resources, config_dir, diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 6f625274..3a898ac9 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -23,12 +23,18 @@ use stackable_operator::{ }, }, }; +use strum::IntoEnumIterator; use crate::{ controller::{ CONTROLLER_NAME, KubernetesResources, OPERATOR_NAME, PRODUCT_NAME, Prepared, ValidatedCluster, - build::resource::rbac::{build_role_binding, build_service_account}, + build::{ + resource::rbac::{build_role_binding, build_service_account}, + role_group::{ + DataNodeRoleGroupBuilder, JournalNodeRoleGroupBuilder, NameNodeRoleGroupBuilder, + }, + }, }, crd::{ HdfsNodeRole, HdfsPodRef, @@ -57,9 +63,8 @@ pub mod jvm; pub mod kerberos; pub mod opa; pub mod properties; -pub mod resolve; pub mod resource; -pub mod role_group_builder; +pub mod role_group; #[derive(Snafu, Debug)] pub enum Error { @@ -107,14 +112,23 @@ pub enum Error { role: HdfsNodeRole, role_group: RoleGroupName, }, -} -pub(crate) use resolve::{ - ResolvedRoleGroup, RoleGroupLogging, RoleGroupResolver, RoleSpecificValues, -}; -pub(crate) use role_group_builder::RoleGroupBuilder; + #[snafu(display("failed to add the listener volume for role {role} role group {role_group}", role = role.as_ref()))] + AddListenerVolume { + source: stackable_operator::builder::pod::Error, + role: HdfsNodeRole, + role_group: RoleGroupName, + }, + + #[snafu(display("failed to build the containers of role {role} role group {role_group}", role = role.as_ref()))] + Container { + source: container::Error, + role: HdfsNodeRole, + role_group: RoleGroupName, + }, +} -/// The resources of every role, accumulated one role at a time by [`build_role`]. +/// The resources of every role, accumulated one role group at a time by [`build`]. #[derive(Default)] struct RoleGroupResources { services: Vec, @@ -125,38 +139,6 @@ struct RoleGroupResources { pod_disruption_budgets: Vec, } -/// Builds every resource of every role group of one role, plus that role's PDB, appending them to -/// `rg_resources`. -fn build_role( - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, - role_group_configs: &BTreeMap< - RoleGroupName, - RoleGroupConfig, - >, - rg_resources: &mut RoleGroupResources, -) -> Result<(), Error> { - let role = &C::ROLE; - - for (role_group_name, rg_config) in role_group_configs { - let builder = RoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; - - rg_resources.services.extend(builder.build_services()?); - rg_resources.config_maps.push(builder.build_config_map()?); - rg_resources - .stateful_sets - .entry(C::ROLE) - .or_default() - .push(builder.build_stateful_set()?); - } - - if let Some(pdb) = resource::pdb::build_pdb(cluster, role) { - rg_resources.pod_disruption_budgets.push(pdb); - } - - Ok(()) -} - /// Builds every Kubernetes resource for the given validated cluster. /// /// Does not need a Kubernetes client: every external reference is already dereferenced and @@ -164,6 +146,11 @@ fn build_role( /// `cluster_info` carries static cluster information resolved at operator startup (e.g. the /// cluster domain used to build Kerberos principals), not a live client. /// +/// Each of the three loops hands its role group's typed config to that role's builder, which is +/// where everything specific to the role lives. The loops are free to be reordered: the +/// StatefulSets are keyed by role, and the apply step does not depend on the order of the other +/// three collections. +/// /// The resources are returned as flat collections. `stateful_sets` comes out in [`HdfsNodeRole`] /// order, which the apply step depends on; that is structural, from a [`BTreeMap`] flattened in /// key order, not from the order the roles are built in. @@ -176,26 +163,50 @@ pub fn build( ) -> Result, Error> { let mut built = RoleGroupResources::default(); - // These three calls are free to be reordered: the StatefulSets are keyed by role, and the - // apply step does not depend on the order of the other three collections. - build_role( - cluster, - cluster_info, - &cluster.journalnode_role_group_configs, - &mut built, - )?; - build_role( - cluster, - cluster_info, - &cluster.namenode_role_group_configs, - &mut built, - )?; - build_role( - cluster, - cluster_info, - &cluster.datanode_role_group_configs, - &mut built, - )?; + for (role_group_name, rg_config) in &cluster.journalnode_role_group_configs { + let builder = + JournalNodeRoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; + + built.services.extend(builder.build_services()?); + built.config_maps.push(builder.build_config_map()?); + built + .stateful_sets + .entry(HdfsNodeRole::Journal) + .or_default() + .push(builder.build_statefulset()?); + } + + for (role_group_name, rg_config) in &cluster.namenode_role_group_configs { + let builder = + NameNodeRoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; + + built.services.extend(builder.build_services()?); + built.config_maps.push(builder.build_config_map()?); + built + .stateful_sets + .entry(HdfsNodeRole::Name) + .or_default() + .push(builder.build_statefulset()?); + } + + for (role_group_name, rg_config) in &cluster.datanode_role_group_configs { + let builder = + DataNodeRoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; + + built.services.extend(builder.build_services()?); + built.config_maps.push(builder.build_config_map()?); + built + .stateful_sets + .entry(HdfsNodeRole::Data) + .or_default() + .push(builder.build_statefulset()?); + } + + for role in HdfsNodeRole::iter() { + if let Some(pdb) = resource::pdb::build_pdb(cluster, &role) { + built.pod_disruption_budgets.push(pdb); + } + } let RoleGroupResources { services, diff --git a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs index f59e9b4f..bbb087f4 100644 --- a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs @@ -2,6 +2,7 @@ //! `*.log4j.properties` configs and the (static) Vector agent config (`vector.yaml`). use stackable_operator::{ + builder::configmap::ConfigMapBuilder, memory::{BinaryMultiple, MemoryQuantity}, product_logging::{ self, @@ -10,12 +11,9 @@ use stackable_operator::{ v2::product_logging::framework::STACKABLE_LOG_DIR, }; -use crate::controller::build::{ - RoleGroupLogging, RoleSpecificValues, - container::{ - FORMAT_NAMENODES_CONTAINER_NAME, FORMAT_ZOOKEEPER_CONTAINER_NAME, - WAIT_FOR_NAMENODES_CONTAINER_NAME, ZKFC_CONTAINER_NAME, - }, +use crate::controller::build::container::{ + ContainerConfig, FORMAT_NAMENODES_CONTAINER_NAME, FORMAT_ZOOKEEPER_CONTAINER_NAME, + WAIT_FOR_NAMENODES_CONTAINER_NAME, ZKFC_CONTAINER_NAME, }; // We have a maximum of 4 continuous logging files for Namenodes. Datanodes and Journalnodes @@ -59,6 +57,13 @@ const FORMAT_NAMENODES_LOG_FILE: &str = "format-namenodes.log4j.xml"; const FORMAT_ZOOKEEPER_LOG_FILE: &str = "format-zookeeper.log4j.xml"; const WAIT_FOR_NAMENODES_LOG_FILE: &str = "wait-for-namenodes.log4j.xml"; +/// The main `hdfs` container of every role logs into this directory, whatever the container +/// itself is named (`namenode`, `datanode`, `journalnode`). +/// +/// Vector parses the `container` label out of the log path (see the `files_log4j` source in +/// `vector.yaml`), so this name reaches the aggregated logs. +const HDFS_LOG_DIR_NAME: &str = "hdfs"; + /// The vendored Vector agent configuration (`vector.yaml`). /// /// It is static: per-rolegroup values (namespace, cluster, role, role group, log/data dirs and the @@ -72,102 +77,95 @@ pub fn vector_config_file_content() -> String { VECTOR_CONFIG.to_owned() } -/// Renders the `*.log4j.properties` files for every container of this role group that uses the -/// operator's automatic logging configuration. +/// Everything about how one container logs: which file holds its `log4j.properties` in the role +/// group `ConfigMap`, which directory it logs into, which file it writes and how large that file +/// may grow. /// -/// Returns `(filename, rendered content)` pairs; containers using a custom log ConfigMap are -/// skipped, so the result is empty when none use automatic logging. -pub fn build_log4j_configs( - logging: &RoleGroupLogging, - role: &RoleSpecificValues, -) -> Vec<(&'static str, String)> { - let mut configs = Vec::new(); - - add_log4j_config_if_automatic( - &mut configs, - &logging.hdfs, - HDFS_LOG4J_CONFIG_FILE, - "hdfs", - HDFS_LOG_FILE, - MAX_HDFS_LOG_FILE_SIZE, - ); +/// One match, so a container's four log4j facts sit together and cannot drift apart. +struct Log4jSpec { + config_file: &'static str, + log_dir_name: &'static str, + log_file: &'static str, + max_log_file_size: MemoryQuantity, +} - // Exhaustive, so a role's containers and their log4j configs cannot drift apart. - match role { - RoleSpecificValues::Journal => {} - RoleSpecificValues::Name { - zkfc, - format_namenodes, - format_zookeeper, - } => { - add_log4j_config_if_automatic( - &mut configs, - zkfc, - ZKFC_LOG4J_CONFIG_FILE, - ZKFC_CONTAINER_NAME.as_ref(), - ZKFC_LOG_FILE, - MAX_ZKFC_LOG_FILE_SIZE, - ); - add_log4j_config_if_automatic( - &mut configs, - format_namenodes, - FORMAT_NAMENODES_LOG4J_CONFIG_FILE, - FORMAT_NAMENODES_CONTAINER_NAME.as_ref(), - FORMAT_NAMENODES_LOG_FILE, - MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, - ); - add_log4j_config_if_automatic( - &mut configs, - format_zookeeper, - FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, - FORMAT_ZOOKEEPER_CONTAINER_NAME.as_ref(), - FORMAT_ZOOKEEPER_LOG_FILE, - MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, - ); - } - RoleSpecificValues::Data { - wait_for_namenodes, .. - } => { - add_log4j_config_if_automatic( - &mut configs, - wait_for_namenodes, - WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, - WAIT_FOR_NAMENODES_CONTAINER_NAME.as_ref(), - WAIT_FOR_NAMENODES_LOG_FILE, - MAX_WAIT_NAMENODES_LOG_FILE_SIZE, - ); - } +fn log4j_spec(container: &ContainerConfig) -> Log4jSpec { + match container { + ContainerConfig::Hdfs { .. } => Log4jSpec { + config_file: HDFS_LOG4J_CONFIG_FILE, + log_dir_name: HDFS_LOG_DIR_NAME, + log_file: HDFS_LOG_FILE, + max_log_file_size: MAX_HDFS_LOG_FILE_SIZE, + }, + ContainerConfig::Zkfc => Log4jSpec { + config_file: ZKFC_LOG4J_CONFIG_FILE, + log_dir_name: ZKFC_CONTAINER_NAME.as_ref(), + log_file: ZKFC_LOG_FILE, + max_log_file_size: MAX_ZKFC_LOG_FILE_SIZE, + }, + ContainerConfig::FormatNameNodes => Log4jSpec { + config_file: FORMAT_NAMENODES_LOG4J_CONFIG_FILE, + log_dir_name: FORMAT_NAMENODES_CONTAINER_NAME.as_ref(), + log_file: FORMAT_NAMENODES_LOG_FILE, + max_log_file_size: MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, + }, + ContainerConfig::FormatZooKeeper => Log4jSpec { + config_file: FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, + log_dir_name: FORMAT_ZOOKEEPER_CONTAINER_NAME.as_ref(), + log_file: FORMAT_ZOOKEEPER_LOG_FILE, + max_log_file_size: MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, + }, + ContainerConfig::WaitForNameNodes => Log4jSpec { + config_file: WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, + log_dir_name: WAIT_FOR_NAMENODES_CONTAINER_NAME.as_ref(), + log_file: WAIT_FOR_NAMENODES_LOG_FILE, + max_log_file_size: MAX_WAIT_NAMENODES_LOG_FILE_SIZE, + }, } +} - configs +/// The `ConfigMap` key holding the given container's `log4j.properties`. +/// +/// The container copies the file from there into its config directory on startup, so the key the +/// `ConfigMap` is written with and the name the container copies must agree. +pub(crate) fn log4j_config_file(container: &ContainerConfig) -> &'static str { + log4j_spec(container).config_file } -fn add_log4j_config_if_automatic( - configs: &mut Vec<(&'static str, String)>, - log_config: &ContainerLogConfig, - log_config_file: &'static str, - log_dir_name: &str, - log_file: &str, - max_log_file_size: MemoryQuantity, +/// Renders the given container's `log4j.properties` into the role group `ConfigMap`, if that +/// container uses the operator's automatic logging configuration. +/// +/// A container using a custom log `ConfigMap` mounts its own and is skipped here. +pub(crate) fn add_log4j_config( + builder: &mut ConfigMapBuilder, + container: &ContainerConfig, + container_log_config: &ContainerLogConfig, ) { - if let ContainerLogConfig { + let ContainerLogConfig { choice: Some(ContainerLogConfigChoice::Automatic(log_config)), - } = log_config - { - configs.push(( - log_config_file, - product_logging::framework::create_log4j_config( - &format!("{STACKABLE_LOG_DIR}/{log_dir_name}"), - log_file, - max_log_file_size - .scale_to(BinaryMultiple::Mebi) - .floor() - .value as u32, - CONSOLE_CONVERSION_PATTERN, - log_config, + } = container_log_config + else { + return; + }; + + let spec = log4j_spec(container); + + builder.add_data( + spec.config_file, + product_logging::framework::create_log4j_config( + &format!( + "{STACKABLE_LOG_DIR}/{log_dir_name}", + log_dir_name = spec.log_dir_name ), - )); - } + spec.log_file, + spec.max_log_file_size + .scale_to(BinaryMultiple::Mebi) + .floor() + .value as u32, + CONSOLE_CONVERSION_PATTERN, + log_config, + ), + ); } #[cfg(test)] diff --git a/rust/operator-binary/src/controller/build/resolve.rs b/rust/operator-binary/src/controller/build/resolve.rs deleted file mode 100644 index 0a2d0b19..00000000 --- a/rust/operator-binary/src/controller/build/resolve.rs +++ /dev/null @@ -1,317 +0,0 @@ -//! Resolving one role group into everything the shared builders need. -//! -//! One [`RoleGroupResolver`] impl per role config type, so a role's resolution is written once. -//! [`RoleGroupResolver::resolve`] takes the whole [`RoleGroupConfig`] and returns a -//! [`ResolvedRoleGroup`] that no longer mentions the config type, so the builders take one -//! non-generic argument and read the role back out of it. - -use std::fmt::Display; - -use snafu::ResultExt; -use stackable_operator::{ - k8s_openapi::api::core::v1::{ - PersistentVolumeClaim, PodTemplateSpec, ResourceRequirements, Volume, - }, - kvp::Labels, - product_logging::spec::{ContainerLogConfig, Logging}, - v2::{ - builder::pod::container::EnvVarSet, - jvm_argument_overrides::JvmArgumentOverrides, - role_utils::{JavaCommonConfig, RoleGroupConfig}, - types::operator::RoleGroupName, - }, -}; - -use super::{Error, ListenerVolumeSnafu, VolumeClaimTemplatesSnafu, container::ContainerConfig}; -use crate::crd::{ - CommonNodeConfig, DataNodeConfig, DataNodeContainer, HdfsNodeRole, JournalNodeConfig, - JournalNodeContainer, NameNodeConfig, NameNodeContainer, - storage::DataNodeStorageConfigInnerType, v1alpha1, -}; - -/// The role group's merged values that the builders use verbatim: its replica count and the -/// override sets. Nothing here depends on the role, which is why it is carried alongside the -/// resolved values rather than among them. -/// -/// `cli_overrides` is deliberately absent: it is merged during validation but no builder reads it. -pub struct MergedRoleGroupConfig { - pub replicas: Option, - pub config_overrides: v1alpha1::HdfsConfigOverrides, - pub env_overrides: EnvVarSet, - pub pod_overrides: PodTemplateSpec, - pub jvm_argument_overrides: JvmArgumentOverrides, -} - -impl MergedRoleGroupConfig { - fn of( - rg_config: &RoleGroupConfig, - ) -> Self { - Self { - replicas: rg_config.replicas, - config_overrides: rg_config.config_overrides.clone(), - env_overrides: rg_config.env_overrides.clone(), - pod_overrides: rg_config.pod_overrides.clone(), - jvm_argument_overrides: rg_config - .product_specific_common_config - .jvm_argument_overrides - .clone(), - } - } -} - -/// The log config of the two containers every role has. Containers only one role runs carry theirs -/// in [`RoleSpecificValues`], which is the single place the role is decided. -#[derive(Debug)] -pub struct RoleGroupLogging { - /// The main `hdfs` container, which every role has. - pub hdfs: ContainerLogConfig, - /// The Vector sidecar; `None` when the Vector agent is disabled for this role group. - pub vector: Option, -} - -/// The values the shared builders cannot derive themselves, resolved by -/// [`RoleGroupResolver::resolve`], which knows the role. -/// -/// The builders take this and nothing else about the role group, so there is no second argument to -/// pair with the wrong one. The role comes from [`RoleSpecificValues::node_role`]. -pub struct ResolvedRoleGroup { - /// The selector labels of the role group's pods, also used as the `StatefulSet` selector and - /// on its listener volume. - /// - /// We must use the selector labels and not the recommended labels for the listener volumes. - /// This is because the recommended set contains a "managed-by" label. That label triggers the - /// cluster resources to "manage" listeners, which is wrong and leads to errors. The listeners - /// are managed by the listener-operator. - pub selector_labels: Labels, - /// The role group's merged config that is common to every role. - pub common: CommonNodeConfig, - /// The resource requirements of the role group's main and init containers; the ZKFC sidecar - /// has fixed requirements of its own and ignores this. - pub resources: ResourceRequirements, - /// The `StatefulSet`'s persistent volume claim templates. - pub volume_claim_templates: Vec, - /// The values that exist for this role only. - pub role: RoleSpecificValues, - /// The log config of each of the role group's containers. - pub logging: RoleGroupLogging, - /// The role group's replica count and overrides, carried through unchanged. - pub merged: MergedRoleGroupConfig, -} - -/// Everything that exists for one role only: the containers that role runs, their log configs, and -/// its storage and listener arrangements. -/// -/// An enum rather than `Option` fields, so consumers are exhaustive and three silent failures do -/// not compile: a datanode without its storage drops `dfs.datanode.data.dir` and sends its blocks -/// to container-local storage; a namenode with a pod-level listener volume collides with the -/// identically named claim template and is rejected at apply time; a container without its log -/// config falls back to Hadoop's built-in logging, uncollected by Vector. -pub enum RoleSpecificValues { - /// Journalnodes run no role-specific container, have no listener and no role-specific - /// storage configuration. - Journal, - /// Namenodes run the `zkfc` side container and the `format-namenodes` and `format-zookeeper` - /// init containers. They get their listener from a volume claim template in - /// [`ResolvedRoleGroup::volume_claim_templates`], for stable per-pod identity, so they have - /// no pod-level listener volume. - Name { - zkfc: ContainerLogConfig, - format_namenodes: ContainerLogConfig, - format_zookeeper: ContainerLogConfig, - }, - /// Datanodes run the `wait-for-namenodes` init container. They need no stable per-pod - /// identity, so their listener is an ephemeral pod volume, and they are the only role that - /// configures `dfs.datanode.data.dir`. - Data { - listener_volume: Volume, - storage: DataNodeStorageConfigInnerType, - wait_for_namenodes: ContainerLogConfig, - }, -} - -impl RoleSpecificValues { - /// The role these values belong to. - pub fn node_role(&self) -> HdfsNodeRole { - match self { - Self::Journal => HdfsNodeRole::Journal, - Self::Name { .. } => HdfsNodeRole::Name, - Self::Data { .. } => HdfsNodeRole::Data, - } - } - - /// The role group's ephemeral listener volume; only datanodes have one. - pub fn listener_volume(&self) -> Option<&Volume> { - match self { - Self::Data { - listener_volume, .. - } => Some(listener_volume), - Self::Journal | Self::Name { .. } => None, - } - } - - /// The datanode data volume configuration, which drives `dfs.datanode.data.dir`; `None` for - /// the other roles. - pub fn datanode_storage(&self) -> Option<&DataNodeStorageConfigInnerType> { - match self { - Self::Data { storage, .. } => Some(storage), - Self::Journal | Self::Name { .. } => None, - } - } -} - -/// The log config of the two containers every role has: the main `hdfs` container, and the Vector -/// sidecar, which is `None` when the Vector agent is disabled for the role group. -/// -/// Each role names these containers with its own enum, so this is generic over that enum rather -/// than repeated once per role. -fn common_container_logging( - logging: &Logging, - hdfs: T, - vector: T, -) -> (ContainerLogConfig, Option) -where - T: Clone + Display + Ord, -{ - ( - logging.for_container(&hdfs).into_owned(), - logging - .enable_vector_agent - .then(|| logging.for_container(&vector).into_owned()), - ) -} - -/// How to resolve one role group's role-specific values, implemented once per role config type. -/// -/// The trait supplies the role and the single role-dependent step, which is what lets -/// [`build_role`](super::build_role) be written once. The shared builders read [`Self::ROLE`] -/// instead of taking a role parameter a caller could pair with the wrong config. -pub(crate) trait RoleGroupResolver: Sized { - /// The role whose config this is. - const ROLE: HdfsNodeRole; - - /// Resolves everything the shared builders cannot derive themselves. Takes the selector - /// labels because two of the three roles need them to build their listener. - fn resolve( - rg_config: &RoleGroupConfig, - role_group_name: &RoleGroupName, - selector_labels: Labels, - ) -> Result; -} - -impl RoleGroupResolver for JournalNodeConfig { - const ROLE: HdfsNodeRole = HdfsNodeRole::Journal; - - fn resolve( - rg_config: &RoleGroupConfig, - _role_group_name: &RoleGroupName, - selector_labels: Labels, - ) -> Result { - let config = &rg_config.config; - let (hdfs, vector) = common_container_logging( - &config.logging, - JournalNodeContainer::Hdfs, - JournalNodeContainer::Vector, - ); - - Ok(ResolvedRoleGroup { - selector_labels, - common: config.common.clone(), - resources: config.resources.clone().into(), - volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), - role: RoleSpecificValues::Journal, - logging: RoleGroupLogging { hdfs, vector }, - merged: MergedRoleGroupConfig::of(rg_config), - }) - } -} - -impl RoleGroupResolver for NameNodeConfig { - const ROLE: HdfsNodeRole = HdfsNodeRole::Name; - - fn resolve( - rg_config: &RoleGroupConfig, - role_group_name: &RoleGroupName, - selector_labels: Labels, - ) -> Result { - let config = &rg_config.config; - // Namenodes get their listener from a persistent volume claim template, for stable - // per-pod identity, rather than from an ephemeral volume. - let volume_claim_templates = - ContainerConfig::namenode_volume_claim_templates(config, &selector_labels).context( - VolumeClaimTemplatesSnafu { - role: Self::ROLE, - role_group: role_group_name.clone(), - }, - )?; - - let (hdfs, vector) = common_container_logging( - &config.logging, - NameNodeContainer::Hdfs, - NameNodeContainer::Vector, - ); - - Ok(ResolvedRoleGroup { - selector_labels, - common: config.common.clone(), - resources: config.resources.clone().into(), - volume_claim_templates, - role: RoleSpecificValues::Name { - zkfc: config - .logging - .for_container(&NameNodeContainer::Zkfc) - .into_owned(), - format_namenodes: config - .logging - .for_container(&NameNodeContainer::FormatNameNodes) - .into_owned(), - format_zookeeper: config - .logging - .for_container(&NameNodeContainer::FormatZooKeeper) - .into_owned(), - }, - logging: RoleGroupLogging { hdfs, vector }, - merged: MergedRoleGroupConfig::of(rg_config), - }) - } -} - -impl RoleGroupResolver for DataNodeConfig { - const ROLE: HdfsNodeRole = HdfsNodeRole::Data; - - fn resolve( - rg_config: &RoleGroupConfig, - role_group_name: &RoleGroupName, - selector_labels: Labels, - ) -> Result { - let config = &rg_config.config; - // Datanodes use an ephemeral listener volume, since they need no stable per-pod identity. - let listener_volume = ContainerConfig::datanode_listener_volume(config, &selector_labels) - .context(ListenerVolumeSnafu { - role: Self::ROLE, - role_group: role_group_name.clone(), - })?; - - let (hdfs, vector) = common_container_logging( - &config.logging, - DataNodeContainer::Hdfs, - DataNodeContainer::Vector, - ); - - Ok(ResolvedRoleGroup { - selector_labels, - common: config.common.clone(), - resources: config.resources.clone().into(), - volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), - role: RoleSpecificValues::Data { - listener_volume, - storage: config.resources.storage.clone(), - wait_for_namenodes: config - .logging - .for_container(&DataNodeContainer::WaitForNameNodes) - .into_owned(), - }, - logging: RoleGroupLogging { hdfs, vector }, - merged: MergedRoleGroupConfig::of(rg_config), - }) - } -} diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index ef9ee8ea..22ae122e 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -1,4 +1,7 @@ //! Build the per-rolegroup `ConfigMap` for the HdfsCluster. +//! +//! [`common_config_map`] writes the files every role group gets, the role builder adds the log4j +//! config of each container its role runs, and [`finish_config_map`] assembles the result. use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -6,12 +9,17 @@ use stackable_operator::{ product_logging::framework::VECTOR_CONFIG_FILE, v2::config_file_writer::PropertiesWriterError, }; -use crate::controller::build::{ - self, RoleGroupBuilder, - properties::{ - ConfigFileName, core_site, hadoop_policy, hdfs_site, product_logging, security_properties, - ssl_client, ssl_server, +use crate::{ + controller::build::{ + self, + container::ContainerConfig, + properties::{ + ConfigFileName, core_site, hadoop_policy, hdfs_site, product_logging, + security_properties, ssl_client, ssl_server, + }, + role_group::RoleGroupCommon, }, + crd::storage::DataNodeStorageConfigInnerType, }; #[derive(Snafu, Debug)] @@ -32,40 +40,40 @@ pub enum Error { type Result = std::result::Result; -/// Builds the [`ConfigMap`] of one role group. +/// The files every role group's `ConfigMap` gets: the Hadoop XML configs, the JVM security +/// properties, the main `hdfs` container's `log4j.properties` and, when the Vector agent is +/// enabled, the static Vector config. /// -/// Everything about the role group comes from `resolved`, the role and the merged overrides -/// included, so there is nothing here to pair with the wrong role group. The datanode storage -/// configuration comes from `resolved` for the same reason: taking it independently would let a -/// caller pass a datanode without its storage, which silently drops `dfs.datanode.data.dir`. -pub(crate) fn build_rolegroup_config_map(builder: &RoleGroupBuilder) -> Result { - let RoleGroupBuilder { - cluster, - cluster_info, - role_group_name, - resolved, - } = builder; - let role = builder.role(); +/// `datanode_storage` drives `dfs.datanode.data.dir` and is `Some` only for datanodes; the other +/// two roles do not configure it. +pub(crate) fn common_config_map( + common: &RoleGroupCommon, + datanode_storage: Option, +) -> Result { + let cluster = common.cluster; + let cluster_info = common.cluster_info; + let role = &common.role; + let role_group_name = &common.role_group_name; tracing::info!( "Setting up ConfigMap for role {role} role group {role_group_name}", role = role.as_ref() ); - let metadata = build::rolegroup_metadata(cluster, &role, role_group_name); + let metadata = build::rolegroup_metadata(cluster, role, role_group_name); - let config_overrides = &resolved.merged.config_overrides; + let config_overrides = &common.config_overrides; let cluster_config = &cluster.cluster_config; let hdfs_site_xml = hdfs_site::build( cluster, cluster_info, - resolved.role.datanode_storage().cloned(), + datanode_storage, config_overrides.hdfs_site_xml.clone(), ); let core_site_xml = core_site::build( cluster, - role, + *role, cluster_info, config_overrides.core_site_xml.clone(), ); @@ -96,20 +104,30 @@ pub(crate) fn build_rolegroup_config_map(builder: &RoleGroupBuilder) -> Result Result { builder.build().with_context(|_| AssembleSnafu { - role: role.to_string(), - role_group: role_group_name.to_string(), + role: common.role.to_string(), + role_group: common.role_group_name.to_string(), }) } diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index ebb2147d..ede36bb7 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -1,4 +1,8 @@ //! Builds the rolegroup [`StatefulSet`] for an HDFS role group. +//! +//! [`common_pod_builder`] opens the pod, the role builder adds the containers its role runs, and +//! [`finish_statefulset`] closes the pod and wraps it in the `StatefulSet`. Both halves need +//! nothing but [`RoleGroupCommon`]. use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -12,55 +16,35 @@ use stackable_operator::{ }; use crate::controller::build::{ - self, RoleGroupBuilder, - container::{self, ContainerConfig}, + self, graceful_shutdown::{self, add_graceful_shutdown_config}, + role_group::RoleGroupCommon, }; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to create container and volume configuration"))] - FailedToCreateContainerAndVolumeConfiguration { source: container::Error }, - #[snafu(display("failed to configure graceful shutdown"))] GracefulShutdown { source: graceful_shutdown::Error }, } -/// Builds the [`StatefulSet`] of one role group. +/// Opens the role group's pod: everything about it that does not depend on which containers the +/// role runs. /// -/// Everything about the role group comes from `resolved`, the role and the merged overrides -/// included, so there is nothing here to pair with the wrong role group. -pub(crate) fn build_rolegroup_statefulset( - builder: &RoleGroupBuilder, -) -> Result { - let RoleGroupBuilder { - cluster: validated, - role_group_name, - resolved, - .. - } = builder; - let role = &builder.role(); - - tracing::info!( - "Setting up StatefulSet for role {role} role group {role_group_name}", - role = role.as_ref() - ); - - let image = &validated.image; - - // PodBuilder for StatefulSet Pod template. +/// Infallible: every value it sets is already resolved on [`RoleGroupCommon`]. +pub(crate) fn common_pod_builder(common: &RoleGroupCommon) -> PodBuilder { let mut pb = PodBuilder::new(); let pb_metadata = ObjectMeta { - labels: Some(resolved.selector_labels.clone().into()), + labels: Some(common.selector_labels.clone().into()), ..ObjectMeta::default() }; pb.metadata(pb_metadata) - .image_pull_secrets_from_product_image(image) - .affinity(&resolved.common.affinity) + .image_pull_secrets_from_product_image(&common.cluster.image) + .affinity(&common.common.affinity) .service_account_name( - validated + common + .cluster .cluster_resource_names() .service_account_name() .to_string(), @@ -71,32 +55,46 @@ pub(crate) fn build_rolegroup_statefulset( .build(), ); - // Adds all containers and volumes to the pod builder. - ContainerConfig::add_containers_and_volumes(&mut pb, builder) - .context(FailedToCreateContainerAndVolumeConfigurationSnafu)?; + pb +} + +/// Closes the role group's pod and wraps it in its [`StatefulSet`], once the role builder has +/// added the containers its role runs. +pub(crate) fn finish_statefulset( + mut pb: PodBuilder, + common: &RoleGroupCommon, +) -> Result { + let cluster = common.cluster; + let role = &common.role; + let role_group_name = &common.role_group_name; + + tracing::info!( + "Setting up StatefulSet for role {role} role group {role_group_name}", + role = role.as_ref() + ); - add_graceful_shutdown_config(&resolved.common, &mut pb).context(GracefulShutdownSnafu)?; + add_graceful_shutdown_config(&common.common, &mut pb).context(GracefulShutdownSnafu)?; // The `podOverrides` were already merged (role <- role group) during validation // by the local-`framework` `with_validated_config`. let mut pod_template = pb.build_template(); - pod_template.merge_from(resolved.merged.pod_overrides.clone()); + pod_template.merge_from(common.pod_overrides.clone()); let statefulset_spec = StatefulSetSpec { pod_management_policy: Some("OrderedReady".to_string()), - replicas: resolved.merged.replicas.map(i32::from), + replicas: common.replicas.map(i32::from), selector: LabelSelector { - match_labels: Some(resolved.selector_labels.clone().into()), + match_labels: Some(common.selector_labels.clone().into()), ..LabelSelector::default() }, service_name: Some( - validated + cluster .governing_service_name(role, role_group_name) .to_string(), ), template: pod_template, - volume_claim_templates: Some(resolved.volume_claim_templates.clone()), + volume_claim_templates: Some(common.volume_claim_templates.clone()), ..StatefulSetSpec::default() }; @@ -104,7 +102,7 @@ pub(crate) fn build_rolegroup_statefulset( // This is due to problems that might appear when restarting pods during the initial formatting of namenodes. // See: https://github.com/stackabletech/hdfs-operator/issues/750 (disable restart-controller) // https://github.com/stackabletech/issues/issues/816 (enable restart-controller) - let metadata = build::rolegroup_metadata(validated, role, role_group_name); + let metadata = build::rolegroup_metadata(cluster, role, role_group_name); Ok(StatefulSet { metadata: metadata.build(), diff --git a/rust/operator-binary/src/controller/build/role_group/datanode.rs b/rust/operator-binary/src/controller/build/role_group/datanode.rs new file mode 100644 index 00000000..534abf29 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/datanode.rs @@ -0,0 +1,151 @@ +//! Building the Kubernetes resources of one datanode role group. + +use snafu::ResultExt; +use stackable_operator::{ + builder::pod::PodBuilder, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Service, Volume}, + }, + product_logging::spec::ContainerLogConfig, + utils::cluster_info::KubernetesClusterInfo, + v2::types::operator::RoleGroupName, +}; + +use super::{RoleGroupCommon, common_container_logging}; +use crate::{ + controller::{ + DataNodeRoleGroupConfig, ValidatedCluster, + build::{ + self, AddListenerVolumeSnafu, Error, ListenerVolumeSnafu, RoleGroupSelectorLabelsSnafu, + container, container::ContainerConfig, properties::product_logging, resource, + }, + }, + crd::{DataNodeContainer, HdfsNodeRole, storage::DataNodeStorageConfigInnerType}, +}; + +const ROLE: HdfsNodeRole = HdfsNodeRole::Data; + +/// One datanode role group: everything every role group has, plus the ephemeral listener volume, +/// the data volume configuration and the one init container only datanodes run. +pub(crate) struct DataNodeRoleGroupBuilder<'a> { + common: RoleGroupCommon<'a>, + /// Datanodes need no stable per-pod identity, so their listener is an ephemeral pod volume + /// rather than a persistent volume claim template as the namenodes' is. + listener_volume: Volume, + /// The data volume configuration, which drives `dfs.datanode.data.dir`. Datanodes are the + /// only role that configures it; losing it is silent, because the datanodes then fall back + /// to Hadoop's default directory, which is container-local. + storage: DataNodeStorageConfigInnerType, + wait_for_namenodes_logging: ContainerLogConfig, +} + +impl<'a> DataNodeRoleGroupBuilder<'a> { + pub(crate) fn new( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &DataNodeRoleGroupConfig, + ) -> Result { + let config = &rg_config.config; + + let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + + let listener_volume = ContainerConfig::datanode_listener_volume(config, &selector_labels) + .context(ListenerVolumeSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + + let (hdfs_logging, vector_logging) = common_container_logging( + &config.logging, + DataNodeContainer::Hdfs, + DataNodeContainer::Vector, + ); + + Ok(Self { + common: RoleGroupCommon { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + selector_labels, + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), + hdfs_logging, + vector_logging, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + }, + listener_volume, + storage: config.resources.storage.clone(), + wait_for_namenodes_logging: config + .logging + .for_container(&DataNodeContainer::WaitForNameNodes) + .into_owned(), + }) + } + + pub(crate) fn build_services(&self) -> Result, Error> { + self.common.build_services() + } + + pub(crate) fn build_statefulset(&self) -> Result { + let mut pb = resource::statefulset::common_pod_builder(&self.common); + + // Added before the containers because the pod's `volumes` are an ordered list: putting + // the listener volume anywhere else changes the pod template of every datanode + // StatefulSet already running, which rolls its pods for no reason. + pb.add_volume(self.listener_volume.clone()) + .context(AddListenerVolumeSnafu { + role: ROLE, + role_group: self.common.role_group_name.clone(), + })?; + + self.add_containers(&mut pb) + .map_err(|source| self.common.container_error(source))?; + + resource::statefulset::finish_statefulset(pb, &self.common) + .map_err(|source| self.common.stateful_set_error(source)) + } + + pub(crate) fn build_config_map(&self) -> Result { + let mut builder = + resource::config_map::common_config_map(&self.common, Some(self.storage.clone())) + .map_err(|source| self.common.config_map_error(source))?; + + product_logging::add_log4j_config( + &mut builder, + &ContainerConfig::WaitForNameNodes, + &self.wait_for_namenodes_logging, + ); + + resource::config_map::finish_config_map(builder, &self.common) + .map_err(|source| self.common.config_map_error(source)) + } + + /// The containers a datanode role group runs: the `hdfs` main container and the Vector + /// sidecar that every role has, then the init container that waits for the namenodes. + fn add_containers(&self, pb: &mut PodBuilder) -> Result<(), container::Error> { + ContainerConfig::add_hdfs_container_and_common_volumes(pb, &self.common)?; + + ContainerConfig::WaitForNameNodes.add_as_init_container( + pb, + &self.common, + &self.wait_for_namenodes_logging, + )?; + + Ok(()) + } +} diff --git a/rust/operator-binary/src/controller/build/role_group/journalnode.rs b/rust/operator-binary/src/controller/build/role_group/journalnode.rs new file mode 100644 index 00000000..27080084 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/journalnode.rs @@ -0,0 +1,110 @@ +//! Building the Kubernetes resources of one journalnode role group. + +use snafu::ResultExt; +use stackable_operator::{ + builder::pod::PodBuilder, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Service}, + }, + utils::cluster_info::KubernetesClusterInfo, + v2::types::operator::RoleGroupName, +}; + +use super::{RoleGroupCommon, common_container_logging}; +use crate::{ + controller::{ + JournalNodeRoleGroupConfig, ValidatedCluster, + build::{ + self, Error, RoleGroupSelectorLabelsSnafu, container, container::ContainerConfig, + resource, + }, + }, + crd::{HdfsNodeRole, JournalNodeContainer}, +}; + +const ROLE: HdfsNodeRole = HdfsNodeRole::Journal; + +/// One journalnode role group. +/// +/// Journalnodes run no containers beyond the `hdfs` main container and the Vector sidecar that +/// every role has, are only used internally by the namenodes so have no listener, and do not +/// configure `dfs.datanode.data.dir`. That is why this struct holds nothing but the common +/// values. +pub(crate) struct JournalNodeRoleGroupBuilder<'a> { + common: RoleGroupCommon<'a>, +} + +impl<'a> JournalNodeRoleGroupBuilder<'a> { + pub(crate) fn new( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &JournalNodeRoleGroupConfig, + ) -> Result { + let config = &rg_config.config; + + let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + + let (hdfs_logging, vector_logging) = common_container_logging( + &config.logging, + JournalNodeContainer::Hdfs, + JournalNodeContainer::Vector, + ); + + Ok(Self { + common: RoleGroupCommon { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + selector_labels, + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), + hdfs_logging, + vector_logging, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + }, + }) + } + + pub(crate) fn build_services(&self) -> Result, Error> { + self.common.build_services() + } + + pub(crate) fn build_statefulset(&self) -> Result { + let mut pb = resource::statefulset::common_pod_builder(&self.common); + + self.add_containers(&mut pb) + .map_err(|source| self.common.container_error(source))?; + + resource::statefulset::finish_statefulset(pb, &self.common) + .map_err(|source| self.common.stateful_set_error(source)) + } + + pub(crate) fn build_config_map(&self) -> Result { + let builder = resource::config_map::common_config_map(&self.common, None) + .map_err(|source| self.common.config_map_error(source))?; + + resource::config_map::finish_config_map(builder, &self.common) + .map_err(|source| self.common.config_map_error(source)) + } + + /// The containers a journalnode role group runs: the `hdfs` main container and the Vector + /// sidecar that every role has, and nothing else. + fn add_containers(&self, pb: &mut PodBuilder) -> Result<(), container::Error> { + ContainerConfig::add_hdfs_container_and_common_volumes(pb, &self.common) + } +} diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs new file mode 100644 index 00000000..585ceacf --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -0,0 +1,163 @@ +//! Building the Kubernetes resources of one role group. +//! +//! There is one builder per role: [`NameNodeRoleGroupBuilder`], [`DataNodeRoleGroupBuilder`] and +//! [`JournalNodeRoleGroupBuilder`]. Each carries a [`RoleGroupCommon`], which holds everything +//! every role group has, and its own role's extras as plain fields. Each lists in its +//! `add_containers` the containers that role runs. +//! +//! A role's extras live on that role's builder only, so a role that has no listener volume has +//! no such field. +//! +//! The work every role shares lives in the helpers the three builders call, in +//! [`super::container`], [`resource::statefulset`] and [`resource::config_map`]. + +mod datanode; +mod journalnode; +mod namenode; + +use std::fmt::Display; + +pub(crate) use datanode::DataNodeRoleGroupBuilder; +pub(crate) use journalnode::JournalNodeRoleGroupBuilder; +pub(crate) use namenode::NameNodeRoleGroupBuilder; +use snafu::ResultExt; +use stackable_operator::{ + k8s_openapi::api::core::v1::{ + PersistentVolumeClaim, PodTemplateSpec, ResourceRequirements, Service, + }, + kvp::Labels, + product_logging::spec::{ContainerLogConfig, Logging}, + utils::cluster_info::KubernetesClusterInfo, + v2::{ + builder::pod::container::EnvVarSet, jvm_argument_overrides::JvmArgumentOverrides, + types::operator::RoleGroupName, + }, +}; + +use super::{Error, ServiceSnafu, container, resource}; +use crate::{ + controller::ValidatedCluster, + crd::{CommonNodeConfig, HdfsNodeRole, v1alpha1}, +}; + +/// Everything every role group has, whatever its role. +/// +/// The three role builders each carry one of these alongside their own role's extras. Every +/// field here is present for every role group, so no `Option` here stands for "this belongs to a +/// different role". +pub(crate) struct RoleGroupCommon<'a> { + pub(crate) cluster: &'a ValidatedCluster, + pub(crate) cluster_info: &'a KubernetesClusterInfo, + /// The role this role group belongs to. The shared helpers match on it where a role + /// genuinely differs, such as the container name and the ports. + pub(crate) role: HdfsNodeRole, + pub(crate) role_group_name: RoleGroupName, + /// The selector labels of the role group's pods, also used as the `StatefulSet` selector and + /// on the listener volumes. + /// + /// We must use the selector labels and not the recommended labels for the listener volumes. + /// This is because the recommended set contains a "managed-by" label. That label triggers the + /// cluster resources to "manage" listeners, which is wrong and leads to errors. The listeners + /// are managed by the listener-operator. + pub(crate) selector_labels: Labels, + /// The role group's merged config that is common to every role. + pub(crate) common: CommonNodeConfig, + /// The resource requirements of the role group's main and init containers; the ZKFC sidecar + /// has fixed requirements of its own and ignores this. + pub(crate) resources: ResourceRequirements, + /// The `StatefulSet`'s persistent volume claim templates. For namenodes these include the + /// listener claim template. + pub(crate) volume_claim_templates: Vec, + /// The log config of the main `hdfs` container, which every role runs. + pub(crate) hdfs_logging: ContainerLogConfig, + /// The log config of the Vector sidecar; `None` when the Vector agent is disabled for this + /// role group. + pub(crate) vector_logging: Option, + /// The role group's replica count; `None` when unset, which counts as one replica. + pub(crate) replicas: Option, + pub(crate) config_overrides: v1alpha1::HdfsConfigOverrides, + pub(crate) env_overrides: EnvVarSet, + pub(crate) pod_overrides: PodTemplateSpec, + pub(crate) jvm_argument_overrides: JvmArgumentOverrides, +} + +impl RoleGroupCommon<'_> { + /// The name the role group's owned objects share. + pub(crate) fn object_name(&self) -> String { + self.cluster + .role_group_resource_names(&self.role, &self.role_group_name) + .qualified_role_group_name() + .to_string() + } + + /// The headless and metrics `Service`s. + pub(crate) fn build_services(&self) -> Result, Error> { + let context = || ServiceSnafu { + role: self.role, + role_group: self.role_group_name.clone(), + }; + + Ok(vec![ + resource::service::rolegroup_headless_service( + self.cluster, + &self.role, + &self.role_group_name, + ) + .with_context(|_| context())?, + resource::service::rolegroup_metrics_service( + self.cluster, + &self.role, + &self.role_group_name, + ) + .with_context(|_| context())?, + ]) + } + + /// Wraps a container assembly failure with this role group's identity. + pub(crate) fn container_error(&self, source: container::Error) -> Error { + Error::Container { + source, + role: self.role, + role_group: self.role_group_name.clone(), + } + } + + /// Wraps a `StatefulSet` assembly failure with this role group's identity. + pub(crate) fn stateful_set_error(&self, source: resource::statefulset::Error) -> Error { + Error::StatefulSet { + source, + role: self.role, + role_group: self.role_group_name.clone(), + } + } + + /// Wraps a `ConfigMap` assembly failure with this role group's identity. + pub(crate) fn config_map_error(&self, source: resource::config_map::Error) -> Error { + Error::ConfigMap { + source, + role: self.role, + role_group: self.role_group_name.clone(), + } + } +} + +/// The log configs of the two containers every role runs: the main `hdfs` container, and the +/// Vector sidecar, which is `None` when the Vector agent is disabled for the role group. +/// +/// Each role names these containers with its own enum, so this is generic over that enum rather +/// than repeated once per role. +fn common_container_logging( + logging: &Logging, + hdfs: T, + vector: T, +) -> (ContainerLogConfig, Option) +where + T: Clone + Display + Ord, +{ + ( + logging.for_container(&hdfs).into_owned(), + logging + .enable_vector_agent + .then(|| logging.for_container(&vector).into_owned()), + ) +} diff --git a/rust/operator-binary/src/controller/build/role_group/namenode.rs b/rust/operator-binary/src/controller/build/role_group/namenode.rs new file mode 100644 index 00000000..c3e18da6 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/namenode.rs @@ -0,0 +1,162 @@ +//! Building the Kubernetes resources of one namenode role group. + +use snafu::ResultExt; +use stackable_operator::{ + builder::pod::PodBuilder, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Service}, + }, + product_logging::spec::ContainerLogConfig, + utils::cluster_info::KubernetesClusterInfo, + v2::types::operator::RoleGroupName, +}; + +use super::{RoleGroupCommon, common_container_logging}; +use crate::{ + controller::{ + NameNodeRoleGroupConfig, ValidatedCluster, + build::{ + self, Error, RoleGroupSelectorLabelsSnafu, VolumeClaimTemplatesSnafu, container, + container::ContainerConfig, properties::product_logging, resource, + }, + }, + crd::{HdfsNodeRole, NameNodeContainer}, +}; + +const ROLE: HdfsNodeRole = HdfsNodeRole::Name; + +/// One namenode role group: everything every role group has, plus the three containers only +/// namenodes run. +/// +/// Namenodes get their listener from a persistent volume claim template, for stable per-pod +/// identity, so it is already among [`RoleGroupCommon::volume_claim_templates`] and there is no +/// pod-level listener volume here. A pod volume and a claim template of the same name would be +/// rejected at apply time, which is why only one of the two ever exists for a role. +pub(crate) struct NameNodeRoleGroupBuilder<'a> { + common: RoleGroupCommon<'a>, + zkfc_logging: ContainerLogConfig, + format_namenodes_logging: ContainerLogConfig, + format_zookeeper_logging: ContainerLogConfig, +} + +impl<'a> NameNodeRoleGroupBuilder<'a> { + pub(crate) fn new( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &NameNodeRoleGroupConfig, + ) -> Result { + let config = &rg_config.config; + + let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + + let volume_claim_templates = + ContainerConfig::namenode_volume_claim_templates(config, &selector_labels).context( + VolumeClaimTemplatesSnafu { + role: ROLE, + role_group: role_group_name.clone(), + }, + )?; + + let (hdfs_logging, vector_logging) = common_container_logging( + &config.logging, + NameNodeContainer::Hdfs, + NameNodeContainer::Vector, + ); + + Ok(Self { + common: RoleGroupCommon { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + selector_labels, + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates, + hdfs_logging, + vector_logging, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + }, + zkfc_logging: config + .logging + .for_container(&NameNodeContainer::Zkfc) + .into_owned(), + format_namenodes_logging: config + .logging + .for_container(&NameNodeContainer::FormatNameNodes) + .into_owned(), + format_zookeeper_logging: config + .logging + .for_container(&NameNodeContainer::FormatZooKeeper) + .into_owned(), + }) + } + + pub(crate) fn build_services(&self) -> Result, Error> { + self.common.build_services() + } + + pub(crate) fn build_statefulset(&self) -> Result { + let mut pb = resource::statefulset::common_pod_builder(&self.common); + + self.add_containers(&mut pb) + .map_err(|source| self.common.container_error(source))?; + + resource::statefulset::finish_statefulset(pb, &self.common) + .map_err(|source| self.common.stateful_set_error(source)) + } + + pub(crate) fn build_config_map(&self) -> Result { + let mut builder = resource::config_map::common_config_map(&self.common, None) + .map_err(|source| self.common.config_map_error(source))?; + + product_logging::add_log4j_config(&mut builder, &ContainerConfig::Zkfc, &self.zkfc_logging); + product_logging::add_log4j_config( + &mut builder, + &ContainerConfig::FormatNameNodes, + &self.format_namenodes_logging, + ); + product_logging::add_log4j_config( + &mut builder, + &ContainerConfig::FormatZooKeeper, + &self.format_zookeeper_logging, + ); + + resource::config_map::finish_config_map(builder, &self.common) + .map_err(|source| self.common.config_map_error(source)) + } + + /// The containers a namenode role group runs: the `hdfs` main container and the Vector + /// sidecar that every role has, then the ZooKeeper fail-over controller side container and + /// the two init containers that format the namenodes and ZooKeeper. + fn add_containers(&self, pb: &mut PodBuilder) -> Result<(), container::Error> { + ContainerConfig::add_hdfs_container_and_common_volumes(pb, &self.common)?; + + ContainerConfig::Zkfc.add_as_side_container(pb, &self.common, &self.zkfc_logging)?; + ContainerConfig::FormatNameNodes.add_as_init_container( + pb, + &self.common, + &self.format_namenodes_logging, + )?; + ContainerConfig::FormatZooKeeper.add_as_init_container( + pb, + &self.common, + &self.format_zookeeper_logging, + )?; + + Ok(()) + } +} diff --git a/rust/operator-binary/src/controller/build/role_group_builder.rs b/rust/operator-binary/src/controller/build/role_group_builder.rs deleted file mode 100644 index 33c097c5..00000000 --- a/rust/operator-binary/src/controller/build/role_group_builder.rs +++ /dev/null @@ -1,104 +0,0 @@ -//! Building the Kubernetes resources of one role group. -//! -//! [`RoleGroupBuilder::new`] resolves the role group once; every builder below reads that one -//! object, so none of them takes the role, the config or the resolved values as separate -//! arguments and there is nothing to pair with the wrong role group. - -use snafu::ResultExt; -use stackable_operator::{ - k8s_openapi::api::{ - apps::v1::StatefulSet, - core::v1::{ConfigMap, Service}, - }, - utils::cluster_info::KubernetesClusterInfo, - v2::{ - role_utils::{JavaCommonConfig, RoleGroupConfig}, - types::operator::RoleGroupName, - }, -}; - -use super::{ - ConfigMapSnafu, Error, RoleGroupSelectorLabelsSnafu, ServiceSnafu, StatefulSetSnafu, - resolve::{ResolvedRoleGroup, RoleGroupResolver}, - resource, rolegroup_selector_labels, -}; -use crate::{ - controller::ValidatedCluster, - crd::{HdfsNodeRole, v1alpha1}, -}; - -/// One role group's resolved values plus the context every builder needs. -pub(crate) struct RoleGroupBuilder<'a> { - pub(crate) cluster: &'a ValidatedCluster, - pub(crate) cluster_info: &'a KubernetesClusterInfo, - pub(crate) role_group_name: RoleGroupName, - pub(crate) resolved: ResolvedRoleGroup, -} - -impl<'a> RoleGroupBuilder<'a> { - /// Resolves one role group. `C` is the role group's config type and appears here only: the - /// builder it returns names no config type, so nothing below this point is generic. - pub(crate) fn new( - cluster: &'a ValidatedCluster, - cluster_info: &'a KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rg_config: &RoleGroupConfig, - ) -> Result { - let selector_labels = rolegroup_selector_labels(cluster, &C::ROLE, role_group_name) - .context(RoleGroupSelectorLabelsSnafu { - role: C::ROLE, - role_group: role_group_name.clone(), - })?; - let resolved = C::resolve(rg_config, role_group_name, selector_labels)?; - - Ok(Self { - cluster, - cluster_info, - role_group_name: role_group_name.clone(), - resolved, - }) - } - - /// The role this role group belongs to, read back out of the resolved values. - pub(crate) fn role(&self) -> HdfsNodeRole { - self.resolved.role.node_role() - } - - /// The headless and metrics Services. Role-agnostic: neither reads the role config. - pub(crate) fn build_services(&self) -> Result, Error> { - let role = self.role(); - let context = || ServiceSnafu { - role, - role_group: self.role_group_name.clone(), - }; - - Ok(vec![ - resource::service::rolegroup_headless_service( - self.cluster, - &role, - &self.role_group_name, - ) - .with_context(|_| context())?, - resource::service::rolegroup_metrics_service( - self.cluster, - &role, - &self.role_group_name, - ) - .with_context(|_| context())?, - ]) - } - - pub(crate) fn build_config_map(&self) -> Result { - resource::config_map::build_rolegroup_config_map(self).context(ConfigMapSnafu { - role: self.role(), - role_group: self.role_group_name.clone(), - }) - } - - pub(crate) fn build_stateful_set(&self) -> Result { - resource::statefulset::build_rolegroup_statefulset(self).context(StatefulSetSnafu { - role: self.role(), - role_group: self.role_group_name.clone(), - }) - } -} diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index a4063424..521f64c7 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -159,12 +159,10 @@ mod test { use std::str::FromStr; use stackable_operator::{ - builder::pod::PodBuilder, client::Client, commons::networking::DomainName, kube::{ Client as KubeClient, Config, - api::ObjectMeta, runtime::{ controller::Action, events::{Recorder, Reporter}, @@ -177,7 +175,7 @@ mod test { use super::*; use crate::{ HDFS_FULL_CONTROLLER_NAME, - controller::build::{RoleGroupBuilder, container::ContainerConfig}, + controller::build::role_group::DataNodeRoleGroupBuilder, test_support::{datanode_role_group_config, deserialize_cluster, validate_cluster}, }; @@ -225,19 +223,25 @@ spec: cluster_domain: DomainName::try_from("cluster.local").unwrap(), }; // Built through the production path, so this test cannot drift from what the build step - // actually hands the container builder. - let builder = RoleGroupBuilder::new( + // actually produces. + let builder = DataNodeRoleGroupBuilder::new( &validated_cluster, &cluster_info, &role_group_name, role_group_config, ) - .expect("the datanode role group should resolve"); - - let mut pb = PodBuilder::new(); - pb.metadata(ObjectMeta::default()); - ContainerConfig::add_containers_and_volumes(&mut pb, &builder).unwrap(); - let containers = pb.build().unwrap().spec.unwrap().containers; + .expect("the datanode role group builder should be constructed"); + + let stateful_set = builder + .build_statefulset() + .expect("the datanode StatefulSet should build"); + let containers = stateful_set + .spec + .expect("the StatefulSet has a spec") + .template + .spec + .expect("the Pod template has a spec") + .containers; let env_vars = containers .iter() .find(|c| c.name == role.to_string()) From 7785edeb7725ecd640e064871d0e5d8f4efd84cf Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 22 Sep 2026 14:41:27 +0200 Subject: [PATCH 3/4] resolve role groups up front, then emit their resources --- .../src/controller/build/container.rs | 150 +++++------- .../src/controller/build/mod.rs | 10 +- .../build/properties/product_logging/mod.rs | 20 +- .../controller/build/resource/config_map.rs | 167 ++++++------- .../controller/build/resource/statefulset.rs | 97 ++++---- .../controller/build/role_group/datanode.rs | 196 ++++++--------- .../build/role_group/journalnode.rs | 138 ++++------- .../src/controller/build/role_group/mod.rs | 197 +++++++++++---- .../controller/build/role_group/namenode.rs | 225 +++++++----------- rust/operator-binary/src/hdfs_controller.rs | 4 +- 10 files changed, 573 insertions(+), 631 deletions(-) diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 83718931..fcee0866 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -17,7 +17,6 @@ use stackable_operator::{ builder::{ self, pod::{ - PodBuilder, resources::ResourceRequirementsBuilder, volume::{ ListenerOperatorVolumeSourceBuilder, ListenerOperatorVolumeSourceBuilderError, @@ -73,7 +72,7 @@ use crate::{ MAX_HDFS_LOG_FILE_SIZE, MAX_WAIT_NAMENODES_LOG_FILE_SIZE, MAX_ZKFC_LOG_FILE_SIZE, log4j_config_file, }, - role_group::RoleGroupCommon, + role_group::RoleGroupInputs, }, }, crd::{ @@ -207,29 +206,30 @@ impl ContainerConfig { const ZKFC_CONFIG_VOLUME_MOUNT_NAME: &'static str = "zkfc-config"; const ZKFC_LOG_VOLUME_MOUNT_NAME: &'static str = "zkfc-log-config"; - /// Adds the containers every role runs — the `hdfs` main container and, when enabled, the - /// Vector sidecar — plus the volumes every role group needs, to the pod builder. + /// The containers every role runs — the `hdfs` main container and, when enabled, the Vector + /// sidecar — and the pod volumes they and the role group need. /// - /// The containers only one role runs are added by that role's builder in + /// The containers only one role runs are named by that role's module in /// [`role_group`](crate::controller::build::role_group). - pub(crate) fn add_hdfs_container_and_common_volumes( - pb: &mut PodBuilder, - common: &RoleGroupCommon, - ) -> Result<(), Error> { - let cluster = common.cluster; - let role = &common.role; - let object_name = common.object_name(); - let resource_names = cluster.role_group_resource_names(role, &common.role_group_name); + pub(crate) fn common_containers_and_volumes( + inputs: &RoleGroupInputs, + ) -> Result<(Vec, Vec), Error> { + let cluster = inputs.cluster; + let role = &inputs.role; + let object_name = inputs.object_name(); + let resource_names = cluster.role_group_resource_names(role, &inputs.role_group_name); + + let mut containers = Vec::new(); + let mut volumes = Vec::new(); // HDFS main container let main_container_config = Self::from(*role); - pb.add_volumes(main_container_config.volumes(&common.hdfs_logging, &object_name)) - .context(AddVolumeSnafu)?; - pb.add_container(main_container_config.main_container(common, &common.hdfs_logging)?); + volumes.extend(main_container_config.volumes(&inputs.hdfs_logging, &object_name)); + containers.push(main_container_config.main_container(inputs, &inputs.hdfs_logging)?); // Vector sidecar container. - if let Some(vector_logging) = &common.vector_logging { + if let Some(vector_logging) = &inputs.vector_logging { let vector_aggregator_config_map_name = cluster .cluster_config .logging @@ -255,7 +255,7 @@ impl ContainerConfig { ), }; - pb.add_container(vector_container( + containers.push(vector_container( &VECTOR_CONTAINER_NAME, &cluster.image, &VectorContainerLogConfig { @@ -270,7 +270,7 @@ impl ContainerConfig { } if let Some(authentication_config) = cluster.authentication_config() { - pb.add_volume( + volumes.push( VolumeBuilder::new(&*TLS_STORE_VOLUME_NAME) .ephemeral( SecretOperatorVolumeSourceBuilder::new( @@ -286,7 +286,7 @@ impl ContainerConfig { .with_format(SecretFormat::TlsPkcs12) .with_tls_pkcs12_password(TLS_STORE_PASSWORD) .with_auto_tls_cert_lifetime( - common + inputs .common .requested_secret_lifetime .context(MissingSecretLifetimeSnafu)?, @@ -297,10 +297,9 @@ impl ContainerConfig { })?, ) .build(), - ) - .context(AddVolumeSnafu)?; + ); - pb.add_volume( + volumes.push( VolumeBuilder::new(&*KERBEROS_VOLUME_NAME) .ephemeral( SecretOperatorVolumeSourceBuilder::new( @@ -317,51 +316,34 @@ impl ContainerConfig { })?, ) .build(), - ) - .context(AddVolumeSnafu)?; + ); } - Ok(()) + Ok((containers, volumes)) } - /// Adds this container to the pod as a side container, together with the volumes it needs. + /// This container, and the pod volumes it needs. /// - /// `container_log_config` is this container's own, passed by the role builder adding it. - pub(crate) fn add_as_side_container( + /// `container_log_config` is this container's own, passed by the role module naming it. + /// `init` picks how the container is started, the only difference between the two kinds. + pub(crate) fn build_container( &self, - pb: &mut PodBuilder, - common: &RoleGroupCommon, + inputs: &RoleGroupInputs, container_log_config: &ContainerLogConfig, - ) -> Result<(), Error> { - pb.add_volumes(self.volumes(container_log_config, &common.object_name())) - .context(AddVolumeSnafu)?; - pb.add_container(self.main_container(common, container_log_config)?); - - Ok(()) - } + init: bool, + ) -> Result<(Container, Vec), Error> { + let volumes = self.volumes(container_log_config, &inputs.object_name()); + + let container = if init { + // `format-namenodes` and `wait-for-namenodes` address the namenodes by pod name; + // `format-zookeeper` ignores these. + let namenode_podrefs = build::pod_refs(inputs.cluster, &HdfsNodeRole::Name); + self.init_container(inputs, container_log_config, &namenode_podrefs)? + } else { + self.main_container(inputs, container_log_config)? + }; - /// Adds this container to the pod as an init container, together with the volumes it needs. - /// - /// `container_log_config` is this container's own, passed by the role builder adding it. - pub(crate) fn add_as_init_container( - &self, - pb: &mut PodBuilder, - common: &RoleGroupCommon, - container_log_config: &ContainerLogConfig, - ) -> Result<(), Error> { - // `format-namenodes` and `wait-for-namenodes` address the namenodes by pod name; - // `format-zookeeper` ignores these. - let namenode_podrefs = build::pod_refs(common.cluster, &HdfsNodeRole::Name); - - pb.add_volumes(self.volumes(container_log_config, &common.object_name())) - .context(AddVolumeSnafu)?; - pb.add_init_container(self.init_container( - common, - container_log_config, - &namenode_podrefs, - )?); - - Ok(()) + Ok((container, volumes)) } /// The PVC templates for a namenode role group: one data PVC plus the listener PVC. @@ -439,19 +421,19 @@ impl ContainerConfig { /// - Journalnode main process fn main_container( &self, - common: &RoleGroupCommon, + inputs: &RoleGroupInputs, container_log_config: &ContainerLogConfig, ) -> Result { - let cluster = common.cluster; + let cluster = inputs.cluster; let mut cb = new_container_builder(self.container_name()); - let resources = self.resources(&common.resources); + let resources = self.resources(&inputs.resources); cb.image_from_product_image(&cluster.image) .command(Self::command()) - .args(self.args(common, container_log_config, &[])?) - .add_env_vars(self.env(common, resources.as_ref())?) - .add_volume_mounts(self.volume_mounts(cluster, &common.volume_claim_templates)) + .args(self.args(inputs, container_log_config, &[])?) + .add_env_vars(self.env(inputs, resources.as_ref())?) + .add_volume_mounts(self.volume_mounts(cluster, &inputs.volume_claim_templates)) .context(AddVolumeMountSnafu)? .add_container_ports(self.container_ports(cluster)); @@ -484,24 +466,24 @@ impl ContainerConfig { /// - Datanode (wait-for-namenodes) fn init_container( &self, - common: &RoleGroupCommon, + inputs: &RoleGroupInputs, container_log_config: &ContainerLogConfig, namenode_podrefs: &[HdfsPodRef], ) -> Result { - let cluster = common.cluster; + let cluster = inputs.cluster; let mut cb = new_container_builder(self.container_name()); cb.image_from_product_image(&cluster.image) .command(Self::command()) - .args(self.args(common, container_log_config, namenode_podrefs)?) - .add_env_vars(self.env(common, None)?) - .add_volume_mounts(self.volume_mounts(cluster, &common.volume_claim_templates)) + .args(self.args(inputs, container_log_config, namenode_podrefs)?) + .add_env_vars(self.env(inputs, None)?) + .add_volume_mounts(self.volume_mounts(cluster, &inputs.volume_claim_templates)) .context(AddVolumeMountSnafu)?; // We use the main app container resources here in contrast to several operators (which use // hardcoded resources) due to the different code structure. // Going forward this should be replaced by calculating init container resources in the pod builder. - if let Some(resources) = self.resources(&common.resources) { + if let Some(resources) = self.resources(&inputs.resources) { cb.resources(resources); } @@ -569,13 +551,13 @@ impl ContainerConfig { /// Returns the container command arguments. fn args( &self, - common: &RoleGroupCommon, + inputs: &RoleGroupInputs, container_log_config: &ContainerLogConfig, namenode_podrefs: &[HdfsPodRef], ) -> Result, Error> { - let cluster = common.cluster; - let cluster_info = common.cluster_info; - let role = &common.role; + let cluster = inputs.cluster; + let cluster_info = inputs.cluster_info; + let role = &inputs.role; let mut args = String::new(); args.push_str(&self.create_config_directory_cmd()); args.push_str(&self.copy_config_xml_cmd()); @@ -805,11 +787,11 @@ impl ContainerConfig { /// Returns the container env variables. fn env( &self, - common: &RoleGroupCommon, + inputs: &RoleGroupInputs, resources: Option<&ResourceRequirements>, ) -> Result, Error> { - let cluster = common.cluster; - let role = &common.role; + let cluster = inputs.cluster; + let role = &inputs.role; // Maps env var name to env var object. This allows env_overrides to work // as expected (i.e. users can override the env var value). let mut env: BTreeMap = BTreeMap::new(); @@ -838,7 +820,7 @@ impl ContainerConfig { role_opts_name.clone(), EnvVar { name: role_opts_name, - value: Some(self.build_hadoop_opts(common, resources)?), + value: Some(self.build_hadoop_opts(inputs, resources)?), ..EnvVar::default() }, ); @@ -902,7 +884,7 @@ impl ContainerConfig { ); // Overrides need to come last - let mut env_override_vars: BTreeMap = common + let mut env_override_vars: BTreeMap = inputs .env_overrides .clone() .into_iter() @@ -1164,10 +1146,10 @@ impl ContainerConfig { /// Build HADOOP_{*node}_OPTS for each namenode, datanodes and journalnodes. fn build_hadoop_opts( &self, - common: &RoleGroupCommon, + inputs: &RoleGroupInputs, resources: Option<&ResourceRequirements>, ) -> Result { - let cluster = common.cluster; + let cluster = inputs.cluster; match self { ContainerConfig::Hdfs { role, metrics_port, .. @@ -1176,7 +1158,7 @@ impl ContainerConfig { let config_dir = volume_mount_dirs.final_config(); construct_role_specific_jvm_args( role, - &common.jvm_argument_overrides, + &inputs.jvm_argument_overrides, cluster.has_kerberos_enabled(), resources, config_dir, diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 3a898ac9..ead6841a 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -32,7 +32,7 @@ use crate::{ build::{ resource::rbac::{build_role_binding, build_service_account}, role_group::{ - DataNodeRoleGroupBuilder, JournalNodeRoleGroupBuilder, NameNodeRoleGroupBuilder, + build_datanode_role_group, build_journalnode_role_group, build_namenode_role_group, }, }, }, @@ -165,7 +165,7 @@ pub fn build( for (role_group_name, rg_config) in &cluster.journalnode_role_group_configs { let builder = - JournalNodeRoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; + build_journalnode_role_group(cluster, cluster_info, role_group_name, rg_config)?; built.services.extend(builder.build_services()?); built.config_maps.push(builder.build_config_map()?); @@ -177,8 +177,7 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.namenode_role_group_configs { - let builder = - NameNodeRoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; + let builder = build_namenode_role_group(cluster, cluster_info, role_group_name, rg_config)?; built.services.extend(builder.build_services()?); built.config_maps.push(builder.build_config_map()?); @@ -190,8 +189,7 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.datanode_role_group_configs { - let builder = - DataNodeRoleGroupBuilder::new(cluster, cluster_info, role_group_name, rg_config)?; + let builder = build_datanode_role_group(cluster, cluster_info, role_group_name, rg_config)?; built.services.extend(builder.build_services()?); built.config_maps.push(builder.build_config_map()?); diff --git a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs index bbb087f4..e20a7046 100644 --- a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs @@ -2,7 +2,6 @@ //! `*.log4j.properties` configs and the (static) Vector agent config (`vector.yaml`). use stackable_operator::{ - builder::configmap::ConfigMapBuilder, memory::{BinaryMultiple, MemoryQuantity}, product_logging::{ self, @@ -132,26 +131,25 @@ pub(crate) fn log4j_config_file(container: &ContainerConfig) -> &'static str { log4j_spec(container).config_file } -/// Renders the given container's `log4j.properties` into the role group `ConfigMap`, if that -/// container uses the operator's automatic logging configuration. +/// The given container's rendered `log4j.properties` and the `ConfigMap` key to store it under, +/// if that container uses the operator's automatic logging configuration. /// -/// A container using a custom log `ConfigMap` mounts its own and is skipped here. -pub(crate) fn add_log4j_config( - builder: &mut ConfigMapBuilder, +/// `None` for a container using a custom log `ConfigMap`: it mounts its own. +pub(crate) fn log4j_config( container: &ContainerConfig, container_log_config: &ContainerLogConfig, -) { +) -> Option<(String, String)> { let ContainerLogConfig { choice: Some(ContainerLogConfigChoice::Automatic(log_config)), } = container_log_config else { - return; + return None; }; let spec = log4j_spec(container); - builder.add_data( - spec.config_file, + Some(( + spec.config_file.to_owned(), product_logging::framework::create_log4j_config( &format!( "{STACKABLE_LOG_DIR}/{log_dir_name}", @@ -165,7 +163,7 @@ pub(crate) fn add_log4j_config( CONSOLE_CONVERSION_PATTERN, log_config, ), - ); + )) } #[cfg(test)] diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index 22ae122e..dd3bf6de 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -1,7 +1,8 @@ //! Build the per-rolegroup `ConfigMap` for the HdfsCluster. //! -//! [`common_config_map`] writes the files every role group gets, the role builder adds the log4j -//! config of each container its role runs, and [`finish_config_map`] assembles the result. +//! [`common_config_map_data`] renders the files every role group gets, during the gather phase. +//! [`build_config_map`] then wraps whatever the role group ended up with, its own containers' +//! `log4j.properties` included, in the `ConfigMap` object. use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -9,17 +10,14 @@ use stackable_operator::{ product_logging::framework::VECTOR_CONFIG_FILE, v2::config_file_writer::PropertiesWriterError, }; -use crate::{ - controller::build::{ - self, - container::ContainerConfig, - properties::{ - ConfigFileName, core_site, hadoop_policy, hdfs_site, product_logging, - security_properties, ssl_client, ssl_server, - }, - role_group::RoleGroupCommon, +use crate::controller::build::{ + self, + container::ContainerConfig, + properties::{ + ConfigFileName, core_site, hadoop_policy, hdfs_site, product_logging, security_properties, + ssl_client, ssl_server, }, - crd::storage::DataNodeStorageConfigInnerType, + role_group::{RoleGroupBuilder, RoleGroupInputs}, }; #[derive(Snafu, Debug)] @@ -40,94 +38,97 @@ pub enum Error { type Result = std::result::Result; -/// The files every role group's `ConfigMap` gets: the Hadoop XML configs, the JVM security +/// The `ConfigMap` entries every role group gets: the Hadoop XML configs, the JVM security /// properties, the main `hdfs` container's `log4j.properties` and, when the Vector agent is /// enabled, the static Vector config. -/// -/// `datanode_storage` drives `dfs.datanode.data.dir` and is `Some` only for datanodes; the other -/// two roles do not configure it. -pub(crate) fn common_config_map( - common: &RoleGroupCommon, - datanode_storage: Option, -) -> Result { - let cluster = common.cluster; - let cluster_info = common.cluster_info; - let role = &common.role; - let role_group_name = &common.role_group_name; - - tracing::info!( - "Setting up ConfigMap for role {role} role group {role_group_name}", - role = role.as_ref() - ); - - let metadata = build::rolegroup_metadata(cluster, role, role_group_name); - - let config_overrides = &common.config_overrides; +pub(crate) fn common_config_map_data(inputs: &RoleGroupInputs) -> Result> { + let cluster = inputs.cluster; + let cluster_info = inputs.cluster_info; + let role = &inputs.role; + let role_group_name = &inputs.role_group_name; + let config_overrides = &inputs.config_overrides; let cluster_config = &cluster.cluster_config; - let hdfs_site_xml = hdfs_site::build( - cluster, - cluster_info, - datanode_storage, - config_overrides.hdfs_site_xml.clone(), - ); - let core_site_xml = core_site::build( - cluster, - *role, - cluster_info, - config_overrides.core_site_xml.clone(), - ); - let hadoop_policy_xml = hadoop_policy::build(config_overrides.hadoop_policy_xml.clone()); - let ssl_server_xml = ssl_server::build( - cluster_config.authentication.is_some(), - config_overrides.ssl_server_xml.clone(), - ); - let ssl_client_xml = ssl_client::build( - cluster_config.authentication.is_some(), - config_overrides.ssl_client_xml.clone(), - ); - - let mut builder = ConfigMapBuilder::new(); - builder - .metadata(metadata.build()) - .add_data(ConfigFileName::CoreSite.to_string(), core_site_xml) - .add_data(ConfigFileName::HdfsSite.to_string(), hdfs_site_xml) - .add_data(ConfigFileName::HadoopPolicy.to_string(), hadoop_policy_xml) - .add_data(ConfigFileName::SslServer.to_string(), ssl_server_xml) - .add_data(ConfigFileName::SslClient.to_string(), ssl_client_xml) - .add_data( + let mut data = vec![ + ( + ConfigFileName::CoreSite.to_string(), + core_site::build( + cluster, + *role, + cluster_info, + config_overrides.core_site_xml.clone(), + ), + ), + ( + ConfigFileName::HdfsSite.to_string(), + hdfs_site::build( + cluster, + cluster_info, + inputs.datanode_storage.clone(), + config_overrides.hdfs_site_xml.clone(), + ), + ), + ( + ConfigFileName::HadoopPolicy.to_string(), + hadoop_policy::build(config_overrides.hadoop_policy_xml.clone()), + ), + ( + ConfigFileName::SslServer.to_string(), + ssl_server::build( + cluster_config.authentication.is_some(), + config_overrides.ssl_server_xml.clone(), + ), + ), + ( + ConfigFileName::SslClient.to_string(), + ssl_client::build( + cluster_config.authentication.is_some(), + config_overrides.ssl_client_xml.clone(), + ), + ), + ( ConfigFileName::Security.to_string(), security_properties::build(config_overrides.security_properties.clone()).with_context( |_| JvmSecurityPropertiesSnafu { rolegroup: role_group_name.to_string(), }, )?, - ); + ), + ]; - product_logging::add_log4j_config( - &mut builder, + data.extend(product_logging::log4j_config( &ContainerConfig::from(*role), - &common.hdfs_logging, - ); + &inputs.hdfs_logging, + )); - if common.vector_logging.is_some() { - builder.add_data( - VECTOR_CONFIG_FILE, + if inputs.vector_logging.is_some() { + data.push(( + VECTOR_CONFIG_FILE.to_owned(), product_logging::vector_config_file_content(), - ); + )); } - Ok(builder) + Ok(data) } -/// Assembles the role group's `ConfigMap`, once the role builder has added the `log4j.properties` -/// of each container its role runs. -pub(crate) fn finish_config_map( - builder: ConfigMapBuilder, - common: &RoleGroupCommon, -) -> Result { - builder.build().with_context(|_| AssembleSnafu { - role: common.role.to_string(), - role_group: common.role_group_name.to_string(), +/// The role group's `ConfigMap`, from the entries gathered for it. +pub(crate) fn build_config_map(builder: &RoleGroupBuilder) -> Result { + let role = &builder.role; + let role_group_name = &builder.role_group_name; + + tracing::info!( + "Setting up ConfigMap for role {role} role group {role_group_name}", + role = role.as_ref() + ); + + let mut config_map = ConfigMapBuilder::new(); + config_map.metadata(build::rolegroup_metadata(builder.cluster, role, role_group_name).build()); + for (file_name, content) in &builder.config_map_data { + config_map.add_data(file_name, content); + } + + config_map.build().with_context(|_| AssembleSnafu { + role: role.to_string(), + role_group: role_group_name.to_string(), }) } diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index ede36bb7..26efd1cd 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -1,8 +1,7 @@ //! Builds the rolegroup [`StatefulSet`] for an HDFS role group. //! -//! [`common_pod_builder`] opens the pod, the role builder adds the containers its role runs, and -//! [`finish_statefulset`] closes the pod and wraps it in the `StatefulSet`. Both halves need -//! nothing but [`RoleGroupCommon`]. +//! Everything this needs is already resolved on the [`RoleGroupBuilder`], so it reads straight +//! through: open the pod, add the containers and volumes gathered for the role group, close it. use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -17,74 +16,76 @@ use stackable_operator::{ use crate::controller::build::{ self, + container::{self}, graceful_shutdown::{self, add_graceful_shutdown_config}, - role_group::RoleGroupCommon, + role_group::RoleGroupBuilder, }; #[derive(Snafu, Debug)] pub enum Error { + #[snafu(display("failed to add a volume to the Pod"))] + AddVolume { source: container::Error }, + #[snafu(display("failed to configure graceful shutdown"))] GracefulShutdown { source: graceful_shutdown::Error }, -} - -/// Opens the role group's pod: everything about it that does not depend on which containers the -/// role runs. -/// -/// Infallible: every value it sets is already resolved on [`RoleGroupCommon`]. -pub(crate) fn common_pod_builder(common: &RoleGroupCommon) -> PodBuilder { - let mut pb = PodBuilder::new(); - - let pb_metadata = ObjectMeta { - labels: Some(common.selector_labels.clone().into()), - ..ObjectMeta::default() - }; - - pb.metadata(pb_metadata) - .image_pull_secrets_from_product_image(&common.cluster.image) - .affinity(&common.common.affinity) - .service_account_name( - common - .cluster - .cluster_resource_names() - .service_account_name() - .to_string(), - ) - .security_context( - PodSecurityContextBuilder::with_stackable_defaults() - .fs_group(1000) - .build(), - ); - pb + #[snafu(display("failed to add a volume to the Pod"))] + AddPodVolume { + source: stackable_operator::builder::pod::Error, + }, } -/// Closes the role group's pod and wraps it in its [`StatefulSet`], once the role builder has -/// added the containers its role runs. -pub(crate) fn finish_statefulset( - mut pb: PodBuilder, - common: &RoleGroupCommon, -) -> Result { - let cluster = common.cluster; - let role = &common.role; - let role_group_name = &common.role_group_name; +/// The role group's [`StatefulSet`]. +pub(crate) fn build_statefulset(builder: &RoleGroupBuilder) -> Result { + let cluster = builder.cluster; + let role = &builder.role; + let role_group_name = &builder.role_group_name; tracing::info!( "Setting up StatefulSet for role {role} role group {role_group_name}", role = role.as_ref() ); - add_graceful_shutdown_config(&common.common, &mut pb).context(GracefulShutdownSnafu)?; + let mut pb = PodBuilder::new(); + pb.metadata(ObjectMeta { + labels: Some(builder.selector_labels.clone().into()), + ..ObjectMeta::default() + }) + .image_pull_secrets_from_product_image(&cluster.image) + .affinity(&builder.common.affinity) + .service_account_name( + cluster + .cluster_resource_names() + .service_account_name() + .to_string(), + ) + .security_context( + PodSecurityContextBuilder::with_stackable_defaults() + .fs_group(1000) + .build(), + ); + + pb.add_volumes(builder.pod_volumes.clone()) + .context(AddPodVolumeSnafu)?; + for container in &builder.containers { + pb.add_container(container.clone()); + } + for init_container in &builder.init_containers { + pb.add_init_container(init_container.clone()); + } + + add_graceful_shutdown_config(&builder.common, &mut pb).context(GracefulShutdownSnafu)?; // The `podOverrides` were already merged (role <- role group) during validation // by the local-`framework` `with_validated_config`. let mut pod_template = pb.build_template(); - pod_template.merge_from(common.pod_overrides.clone()); + pod_template.merge_from(builder.pod_overrides.clone()); let statefulset_spec = StatefulSetSpec { pod_management_policy: Some("OrderedReady".to_string()), - replicas: common.replicas.map(i32::from), + replicas: builder.replicas.map(i32::from), selector: LabelSelector { - match_labels: Some(common.selector_labels.clone().into()), + match_labels: Some(builder.selector_labels.clone().into()), ..LabelSelector::default() }, service_name: Some( @@ -94,7 +95,7 @@ pub(crate) fn finish_statefulset( ), template: pod_template, - volume_claim_templates: Some(common.volume_claim_templates.clone()), + volume_claim_templates: Some(builder.volume_claim_templates.clone()), ..StatefulSetSpec::default() }; diff --git a/rust/operator-binary/src/controller/build/role_group/datanode.rs b/rust/operator-binary/src/controller/build/role_group/datanode.rs index 534abf29..4a35065c 100644 --- a/rust/operator-binary/src/controller/build/role_group/datanode.rs +++ b/rust/operator-binary/src/controller/build/role_group/datanode.rs @@ -1,151 +1,93 @@ -//! Building the Kubernetes resources of one datanode role group. +//! Gathering one datanode role group. use snafu::ResultExt; use stackable_operator::{ - builder::pod::PodBuilder, - k8s_openapi::api::{ - apps::v1::StatefulSet, - core::v1::{ConfigMap, Service, Volume}, - }, - product_logging::spec::ContainerLogConfig, - utils::cluster_info::KubernetesClusterInfo, - v2::types::operator::RoleGroupName, + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; -use super::{RoleGroupCommon, common_container_logging}; +use super::{ExtraContainer, RoleGroupBuilder, RoleGroupInputs, common_container_logging}; use crate::{ controller::{ DataNodeRoleGroupConfig, ValidatedCluster, build::{ - self, AddListenerVolumeSnafu, Error, ListenerVolumeSnafu, RoleGroupSelectorLabelsSnafu, - container, container::ContainerConfig, properties::product_logging, resource, + self, Error, ListenerVolumeSnafu, RoleGroupSelectorLabelsSnafu, + container::ContainerConfig, }, }, - crd::{DataNodeContainer, HdfsNodeRole, storage::DataNodeStorageConfigInnerType}, + crd::{DataNodeContainer, HdfsNodeRole}, }; const ROLE: HdfsNodeRole = HdfsNodeRole::Data; -/// One datanode role group: everything every role group has, plus the ephemeral listener volume, -/// the data volume configuration and the one init container only datanodes run. -pub(crate) struct DataNodeRoleGroupBuilder<'a> { - common: RoleGroupCommon<'a>, - /// Datanodes need no stable per-pod identity, so their listener is an ephemeral pod volume - /// rather than a persistent volume claim template as the namenodes' is. - listener_volume: Volume, - /// The data volume configuration, which drives `dfs.datanode.data.dir`. Datanodes are the - /// only role that configures it; losing it is silent, because the datanodes then fall back - /// to Hadoop's default directory, which is container-local. - storage: DataNodeStorageConfigInnerType, - wait_for_namenodes_logging: ContainerLogConfig, -} - -impl<'a> DataNodeRoleGroupBuilder<'a> { - pub(crate) fn new( - cluster: &'a ValidatedCluster, - cluster_info: &'a KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rg_config: &DataNodeRoleGroupConfig, - ) -> Result { - let config = &rg_config.config; - - let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) - .context(RoleGroupSelectorLabelsSnafu { - role: ROLE, - role_group: role_group_name.clone(), - })?; - - let listener_volume = ContainerConfig::datanode_listener_volume(config, &selector_labels) - .context(ListenerVolumeSnafu { +/// Gathers one datanode role group. +/// +/// Beyond the `hdfs` main container and the Vector sidecar every role runs, datanodes run one +/// init container that waits for the namenodes. +/// +/// Datanodes need no stable per-pod identity, so their listener is an ephemeral pod volume rather +/// than a claim template as the namenodes' is. They are also the only role that configures +/// `dfs.datanode.data.dir`; losing that is silent, because the datanodes then fall back to +/// Hadoop's default directory, which is container-local. +pub(crate) fn build<'a>( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &DataNodeRoleGroupConfig, +) -> Result, Error> { + let config = &rg_config.config; + + let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { role: ROLE, role_group: role_group_name.clone(), })?; - let (hdfs_logging, vector_logging) = common_container_logging( - &config.logging, - DataNodeContainer::Hdfs, - DataNodeContainer::Vector, - ); + let listener_volume = ContainerConfig::datanode_listener_volume(config, &selector_labels) + .context(ListenerVolumeSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; - Ok(Self { - common: RoleGroupCommon { - cluster, - cluster_info, - role: ROLE, - role_group_name: role_group_name.clone(), - selector_labels, - common: config.common.clone(), - resources: config.resources.clone().into(), - volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), - hdfs_logging, - vector_logging, - replicas: rg_config.replicas, - config_overrides: rg_config.config_overrides.clone(), - env_overrides: rg_config.env_overrides.clone(), - pod_overrides: rg_config.pod_overrides.clone(), - jvm_argument_overrides: rg_config - .product_specific_common_config - .jvm_argument_overrides - .clone(), - }, - listener_volume, - storage: config.resources.storage.clone(), - wait_for_namenodes_logging: config + let (hdfs_logging, vector_logging) = common_container_logging( + &config.logging, + DataNodeContainer::Hdfs, + DataNodeContainer::Vector, + ); + + let inputs = RoleGroupInputs { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + selector_labels, + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), + // First, because the pod's `volumes` are an ordered list: moving the listener volume + // changes the pod template of every datanode StatefulSet already running, which rolls + // its pods for no reason. + extra_pod_volumes: vec![listener_volume], + hdfs_logging, + vector_logging, + datanode_storage: Some(config.resources.storage.clone()), + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + }; + + RoleGroupBuilder::new( + inputs, + vec![ExtraContainer::init( + ContainerConfig::WaitForNameNodes, + config .logging .for_container(&DataNodeContainer::WaitForNameNodes) .into_owned(), - }) - } - - pub(crate) fn build_services(&self) -> Result, Error> { - self.common.build_services() - } - - pub(crate) fn build_statefulset(&self) -> Result { - let mut pb = resource::statefulset::common_pod_builder(&self.common); - - // Added before the containers because the pod's `volumes` are an ordered list: putting - // the listener volume anywhere else changes the pod template of every datanode - // StatefulSet already running, which rolls its pods for no reason. - pb.add_volume(self.listener_volume.clone()) - .context(AddListenerVolumeSnafu { - role: ROLE, - role_group: self.common.role_group_name.clone(), - })?; - - self.add_containers(&mut pb) - .map_err(|source| self.common.container_error(source))?; - - resource::statefulset::finish_statefulset(pb, &self.common) - .map_err(|source| self.common.stateful_set_error(source)) - } - - pub(crate) fn build_config_map(&self) -> Result { - let mut builder = - resource::config_map::common_config_map(&self.common, Some(self.storage.clone())) - .map_err(|source| self.common.config_map_error(source))?; - - product_logging::add_log4j_config( - &mut builder, - &ContainerConfig::WaitForNameNodes, - &self.wait_for_namenodes_logging, - ); - - resource::config_map::finish_config_map(builder, &self.common) - .map_err(|source| self.common.config_map_error(source)) - } - - /// The containers a datanode role group runs: the `hdfs` main container and the Vector - /// sidecar that every role has, then the init container that waits for the namenodes. - fn add_containers(&self, pb: &mut PodBuilder) -> Result<(), container::Error> { - ContainerConfig::add_hdfs_container_and_common_volumes(pb, &self.common)?; - - ContainerConfig::WaitForNameNodes.add_as_init_container( - pb, - &self.common, - &self.wait_for_namenodes_logging, - )?; - - Ok(()) - } + )], + ) } diff --git a/rust/operator-binary/src/controller/build/role_group/journalnode.rs b/rust/operator-binary/src/controller/build/role_group/journalnode.rs index 27080084..f765b452 100644 --- a/rust/operator-binary/src/controller/build/role_group/journalnode.rs +++ b/rust/operator-binary/src/controller/build/role_group/journalnode.rs @@ -1,110 +1,68 @@ -//! Building the Kubernetes resources of one journalnode role group. +//! Gathering one journalnode role group. use snafu::ResultExt; use stackable_operator::{ - builder::pod::PodBuilder, - k8s_openapi::api::{ - apps::v1::StatefulSet, - core::v1::{ConfigMap, Service}, - }, - utils::cluster_info::KubernetesClusterInfo, - v2::types::operator::RoleGroupName, + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; -use super::{RoleGroupCommon, common_container_logging}; +use super::{RoleGroupBuilder, RoleGroupInputs, common_container_logging}; use crate::{ controller::{ JournalNodeRoleGroupConfig, ValidatedCluster, - build::{ - self, Error, RoleGroupSelectorLabelsSnafu, container, container::ContainerConfig, - resource, - }, + build::{self, Error, RoleGroupSelectorLabelsSnafu, container::ContainerConfig}, }, crd::{HdfsNodeRole, JournalNodeContainer}, }; const ROLE: HdfsNodeRole = HdfsNodeRole::Journal; -/// One journalnode role group. +/// Gathers one journalnode role group. /// -/// Journalnodes run no containers beyond the `hdfs` main container and the Vector sidecar that -/// every role has, are only used internally by the namenodes so have no listener, and do not -/// configure `dfs.datanode.data.dir`. That is why this struct holds nothing but the common -/// values. -pub(crate) struct JournalNodeRoleGroupBuilder<'a> { - common: RoleGroupCommon<'a>, -} - -impl<'a> JournalNodeRoleGroupBuilder<'a> { - pub(crate) fn new( - cluster: &'a ValidatedCluster, - cluster_info: &'a KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rg_config: &JournalNodeRoleGroupConfig, - ) -> Result { - let config = &rg_config.config; - - let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) - .context(RoleGroupSelectorLabelsSnafu { - role: ROLE, - role_group: role_group_name.clone(), - })?; - - let (hdfs_logging, vector_logging) = common_container_logging( - &config.logging, - JournalNodeContainer::Hdfs, - JournalNodeContainer::Vector, - ); - - Ok(Self { - common: RoleGroupCommon { - cluster, - cluster_info, - role: ROLE, - role_group_name: role_group_name.clone(), - selector_labels, - common: config.common.clone(), - resources: config.resources.clone().into(), - volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), - hdfs_logging, - vector_logging, - replicas: rg_config.replicas, - config_overrides: rg_config.config_overrides.clone(), - env_overrides: rg_config.env_overrides.clone(), - pod_overrides: rg_config.pod_overrides.clone(), - jvm_argument_overrides: rg_config - .product_specific_common_config - .jvm_argument_overrides - .clone(), - }, - }) - } - - pub(crate) fn build_services(&self) -> Result, Error> { - self.common.build_services() - } - - pub(crate) fn build_statefulset(&self) -> Result { - let mut pb = resource::statefulset::common_pod_builder(&self.common); - - self.add_containers(&mut pb) - .map_err(|source| self.common.container_error(source))?; +/// Journalnodes run nothing beyond the `hdfs` main container and the Vector sidecar every role +/// runs, hence the empty container list. They are used only internally by the namenodes, so they +/// have no listener, and they do not configure `dfs.datanode.data.dir`. +pub(crate) fn build<'a>( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &JournalNodeRoleGroupConfig, +) -> Result, Error> { + let config = &rg_config.config; - resource::statefulset::finish_statefulset(pb, &self.common) - .map_err(|source| self.common.stateful_set_error(source)) - } + let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; - pub(crate) fn build_config_map(&self) -> Result { - let builder = resource::config_map::common_config_map(&self.common, None) - .map_err(|source| self.common.config_map_error(source))?; + let (hdfs_logging, vector_logging) = common_container_logging( + &config.logging, + JournalNodeContainer::Hdfs, + JournalNodeContainer::Vector, + ); - resource::config_map::finish_config_map(builder, &self.common) - .map_err(|source| self.common.config_map_error(source)) - } + let inputs = RoleGroupInputs { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + selector_labels, + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), + extra_pod_volumes: Vec::new(), + hdfs_logging, + vector_logging, + datanode_storage: None, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + }; - /// The containers a journalnode role group runs: the `hdfs` main container and the Vector - /// sidecar that every role has, and nothing else. - fn add_containers(&self, pb: &mut PodBuilder) -> Result<(), container::Error> { - ContainerConfig::add_hdfs_container_and_common_volumes(pb, &self.common) - } + RoleGroupBuilder::new(inputs, Vec::new()) } diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index 585ceacf..ca15b43c 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -1,15 +1,13 @@ //! Building the Kubernetes resources of one role group. //! -//! There is one builder per role: [`NameNodeRoleGroupBuilder`], [`DataNodeRoleGroupBuilder`] and -//! [`JournalNodeRoleGroupBuilder`]. Each carries a [`RoleGroupCommon`], which holds everything -//! every role group has, and its own role's extras as plain fields. Each lists in its -//! `add_containers` the containers that role runs. +//! Two phases, so neither has to reach into the other. First each role's module ([`namenode`], +//! [`datanode`], [`journalnode`]) gathers its [`RoleGroupInputs`] and names the containers only +//! that role runs; [`RoleGroupBuilder::new`] turns those into finished containers, pod volumes +//! and rendered `ConfigMap` entries. Then [`RoleGroupBuilder`]'s `build_*` methods emit the +//! Kubernetes objects, reading straight through fields that are already resolved. //! -//! A role's extras live on that role's builder only, so a role that has no listener volume has -//! no such field. -//! -//! The work every role shares lives in the helpers the three builders call, in -//! [`super::container`], [`resource::statefulset`] and [`resource::config_map`]. +//! The emit phase is identical for every role, so it lives here once. Everything that differs +//! between roles happens in the three role modules' `build` functions. mod datanode; mod journalnode; @@ -17,13 +15,17 @@ mod namenode; use std::fmt::Display; -pub(crate) use datanode::DataNodeRoleGroupBuilder; -pub(crate) use journalnode::JournalNodeRoleGroupBuilder; -pub(crate) use namenode::NameNodeRoleGroupBuilder; +pub(crate) use datanode::build as build_datanode_role_group; +pub(crate) use journalnode::build as build_journalnode_role_group; +pub(crate) use namenode::build as build_namenode_role_group; use snafu::ResultExt; use stackable_operator::{ - k8s_openapi::api::core::v1::{ - PersistentVolumeClaim, PodTemplateSpec, ResourceRequirements, Service, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ + ConfigMap, Container, PersistentVolumeClaim, PodTemplateSpec, ResourceRequirements, + Service, Volume, + }, }, kvp::Labels, product_logging::spec::{ContainerLogConfig, Logging}, @@ -34,22 +36,49 @@ use stackable_operator::{ }, }; -use super::{Error, ServiceSnafu, container, resource}; +use super::{ + Error, ServiceSnafu, container::ContainerConfig, properties::product_logging, resource, +}; use crate::{ controller::ValidatedCluster, - crd::{CommonNodeConfig, HdfsNodeRole, v1alpha1}, + crd::{CommonNodeConfig, HdfsNodeRole, storage::DataNodeStorageConfigInnerType, v1alpha1}, }; -/// Everything every role group has, whatever its role. +/// A container only one role runs, named by that role's module together with the log config that +/// belongs to it. +pub(crate) struct ExtraContainer { + config: ContainerConfig, + logging: ContainerLogConfig, + init: bool, +} + +impl ExtraContainer { + /// A container running alongside the main one for the pod's whole lifetime. + pub(crate) fn side(config: ContainerConfig, logging: ContainerLogConfig) -> Self { + Self { + config, + logging, + init: false, + } + } + + /// A container running to completion before the main one starts. + pub(crate) fn init(config: ContainerConfig, logging: ContainerLogConfig) -> Self { + Self { + config, + logging, + init: true, + } + } +} + +/// One role group's validated configuration, as its role's module reads it off the CRD. /// -/// The three role builders each carry one of these alongside their own role's extras. Every -/// field here is present for every role group, so no `Option` here stands for "this belongs to a -/// different role". -pub(crate) struct RoleGroupCommon<'a> { +/// [`RoleGroupBuilder::new`] consumes this: every field is either resolved into the builder's +/// finished containers and `ConfigMap` entries, or carried over to the emit phase. +pub(crate) struct RoleGroupInputs<'a> { pub(crate) cluster: &'a ValidatedCluster, pub(crate) cluster_info: &'a KubernetesClusterInfo, - /// The role this role group belongs to. The shared helpers match on it where a role - /// genuinely differs, such as the container name and the ports. pub(crate) role: HdfsNodeRole, pub(crate) role_group_name: RoleGroupName, /// The selector labels of the role group's pods, also used as the `StatefulSet` selector and @@ -68,11 +97,17 @@ pub(crate) struct RoleGroupCommon<'a> { /// The `StatefulSet`'s persistent volume claim templates. For namenodes these include the /// listener claim template. pub(crate) volume_claim_templates: Vec, + /// Pod-level volumes beyond those the containers bring with them. Only datanodes have one, + /// their ephemeral listener volume. + pub(crate) extra_pod_volumes: Vec, /// The log config of the main `hdfs` container, which every role runs. pub(crate) hdfs_logging: ContainerLogConfig, /// The log config of the Vector sidecar; `None` when the Vector agent is disabled for this /// role group. pub(crate) vector_logging: Option, + /// The data volume configuration behind `dfs.datanode.data.dir`; `Some` only for datanodes, + /// the one role that configures it. + pub(crate) datanode_storage: Option, /// The role group's replica count; `None` when unset, which counts as one replica. pub(crate) replicas: Option, pub(crate) config_overrides: v1alpha1::HdfsConfigOverrides, @@ -81,7 +116,7 @@ pub(crate) struct RoleGroupCommon<'a> { pub(crate) jvm_argument_overrides: JvmArgumentOverrides, } -impl RoleGroupCommon<'_> { +impl RoleGroupInputs<'_> { /// The name the role group's owned objects share. pub(crate) fn object_name(&self) -> String { self.cluster @@ -89,6 +124,97 @@ impl RoleGroupCommon<'_> { .qualified_role_group_name() .to_string() } +} + +/// Everything one role group's Kubernetes objects are built from, already resolved. +/// +/// These fields are outputs rather than configuration: the containers are built and the +/// `ConfigMap` entries rendered. The `build_*` methods therefore read straight through, and +/// nothing about which role this is reaches them. +pub(crate) struct RoleGroupBuilder<'a> { + pub(crate) cluster: &'a ValidatedCluster, + pub(crate) role: HdfsNodeRole, + pub(crate) role_group_name: RoleGroupName, + pub(crate) selector_labels: Labels, + /// Carried for the pod's affinity and its graceful shutdown timeout. + pub(crate) common: CommonNodeConfig, + pub(crate) replicas: Option, + pub(crate) pod_overrides: PodTemplateSpec, + pub(crate) volume_claim_templates: Vec, + /// The main `hdfs` container, the Vector sidecar when it is enabled, and the role's own side + /// containers, in the order the role named them. + pub(crate) containers: Vec, + /// The role's init containers, in the order the role named them. + pub(crate) init_containers: Vec, + /// Every pod-level volume the containers and the role need. + pub(crate) pod_volumes: Vec, + /// The rendered `ConfigMap` entries, as (file name, content). + pub(crate) config_map_data: Vec<(String, String)>, +} + +impl<'a> RoleGroupBuilder<'a> { + /// Resolves one role group: builds the containers every role runs plus the `extra_containers` + /// this role named, renders its `ConfigMap` entries and collects its pod volumes. + pub(crate) fn new( + inputs: RoleGroupInputs<'a>, + extra_containers: Vec, + ) -> Result { + let role = inputs.role; + let role_group_name = inputs.role_group_name.clone(); + + let container_error = |source| Error::Container { + source, + role, + role_group: role_group_name.clone(), + }; + + // The role's own volumes come first: the pod's `volumes` are an ordered list, and + // reordering them changes the pod template of every StatefulSet already running. + let mut pod_volumes = inputs.extra_pod_volumes.clone(); + let (mut containers, container_volumes) = + ContainerConfig::common_containers_and_volumes(&inputs).map_err(container_error)?; + pod_volumes.extend(container_volumes); + + let mut config_map_data = + resource::config_map::common_config_map_data(&inputs).map_err(|source| { + Error::ConfigMap { + source, + role, + role_group: role_group_name.clone(), + } + })?; + + let mut init_containers = Vec::new(); + for extra in &extra_containers { + let (container, volumes) = extra + .config + .build_container(&inputs, &extra.logging, extra.init) + .map_err(container_error)?; + + if extra.init { + init_containers.push(container); + } else { + containers.push(container); + } + pod_volumes.extend(volumes); + config_map_data.extend(product_logging::log4j_config(&extra.config, &extra.logging)); + } + + Ok(Self { + cluster: inputs.cluster, + role, + role_group_name: inputs.role_group_name, + selector_labels: inputs.selector_labels, + common: inputs.common, + replicas: inputs.replicas, + pod_overrides: inputs.pod_overrides, + volume_claim_templates: inputs.volume_claim_templates, + containers, + init_containers, + pod_volumes, + config_map_data, + }) + } /// The headless and metrics `Service`s. pub(crate) fn build_services(&self) -> Result, Error> { @@ -113,31 +239,20 @@ impl RoleGroupCommon<'_> { ]) } - /// Wraps a container assembly failure with this role group's identity. - pub(crate) fn container_error(&self, source: container::Error) -> Error { - Error::Container { - source, - role: self.role, - role_group: self.role_group_name.clone(), - } - } - - /// Wraps a `StatefulSet` assembly failure with this role group's identity. - pub(crate) fn stateful_set_error(&self, source: resource::statefulset::Error) -> Error { - Error::StatefulSet { + pub(crate) fn build_config_map(&self) -> Result { + resource::config_map::build_config_map(self).map_err(|source| Error::ConfigMap { source, role: self.role, role_group: self.role_group_name.clone(), - } + }) } - /// Wraps a `ConfigMap` assembly failure with this role group's identity. - pub(crate) fn config_map_error(&self, source: resource::config_map::Error) -> Error { - Error::ConfigMap { + pub(crate) fn build_statefulset(&self) -> Result { + resource::statefulset::build_statefulset(self).map_err(|source| Error::StatefulSet { source, role: self.role, role_group: self.role_group_name.clone(), - } + }) } } diff --git a/rust/operator-binary/src/controller/build/role_group/namenode.rs b/rust/operator-binary/src/controller/build/role_group/namenode.rs index c3e18da6..6685ca31 100644 --- a/rust/operator-binary/src/controller/build/role_group/namenode.rs +++ b/rust/operator-binary/src/controller/build/role_group/namenode.rs @@ -1,24 +1,17 @@ -//! Building the Kubernetes resources of one namenode role group. +//! Gathering one namenode role group. use snafu::ResultExt; use stackable_operator::{ - builder::pod::PodBuilder, - k8s_openapi::api::{ - apps::v1::StatefulSet, - core::v1::{ConfigMap, Service}, - }, - product_logging::spec::ContainerLogConfig, - utils::cluster_info::KubernetesClusterInfo, - v2::types::operator::RoleGroupName, + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; -use super::{RoleGroupCommon, common_container_logging}; +use super::{ExtraContainer, RoleGroupBuilder, RoleGroupInputs, common_container_logging}; use crate::{ controller::{ NameNodeRoleGroupConfig, ValidatedCluster, build::{ - self, Error, RoleGroupSelectorLabelsSnafu, VolumeClaimTemplatesSnafu, container, - container::ContainerConfig, properties::product_logging, resource, + self, Error, RoleGroupSelectorLabelsSnafu, VolumeClaimTemplatesSnafu, + container::ContainerConfig, }, }, crd::{HdfsNodeRole, NameNodeContainer}, @@ -26,137 +19,91 @@ use crate::{ const ROLE: HdfsNodeRole = HdfsNodeRole::Name; -/// One namenode role group: everything every role group has, plus the three containers only -/// namenodes run. +/// Gathers one namenode role group. /// -/// Namenodes get their listener from a persistent volume claim template, for stable per-pod -/// identity, so it is already among [`RoleGroupCommon::volume_claim_templates`] and there is no -/// pod-level listener volume here. A pod volume and a claim template of the same name would be -/// rejected at apply time, which is why only one of the two ever exists for a role. -pub(crate) struct NameNodeRoleGroupBuilder<'a> { - common: RoleGroupCommon<'a>, - zkfc_logging: ContainerLogConfig, - format_namenodes_logging: ContainerLogConfig, - format_zookeeper_logging: ContainerLogConfig, -} - -impl<'a> NameNodeRoleGroupBuilder<'a> { - pub(crate) fn new( - cluster: &'a ValidatedCluster, - cluster_info: &'a KubernetesClusterInfo, - role_group_name: &RoleGroupName, - rg_config: &NameNodeRoleGroupConfig, - ) -> Result { - let config = &rg_config.config; - - let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) - .context(RoleGroupSelectorLabelsSnafu { +/// Beyond the `hdfs` main container and the Vector sidecar every role runs, namenodes run the +/// ZooKeeper fail-over controller alongside them and two init containers that format the +/// namenodes and ZooKeeper. +/// +/// Their listener comes from a persistent volume claim template, for stable per-pod identity, so +/// it is among the volume claim templates rather than a pod-level volume. A pod volume and a +/// claim template of the same name would be rejected at apply time, which is why a role has only +/// ever one of the two. +pub(crate) fn build<'a>( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &NameNodeRoleGroupConfig, +) -> Result, Error> { + let config = &rg_config.config; + + let selector_labels = build::rolegroup_selector_labels(cluster, &ROLE, role_group_name) + .context(RoleGroupSelectorLabelsSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + + let volume_claim_templates = + ContainerConfig::namenode_volume_claim_templates(config, &selector_labels).context( + VolumeClaimTemplatesSnafu { role: ROLE, role_group: role_group_name.clone(), - })?; - - let volume_claim_templates = - ContainerConfig::namenode_volume_claim_templates(config, &selector_labels).context( - VolumeClaimTemplatesSnafu { - role: ROLE, - role_group: role_group_name.clone(), - }, - )?; - - let (hdfs_logging, vector_logging) = common_container_logging( - &config.logging, - NameNodeContainer::Hdfs, - NameNodeContainer::Vector, - ); - - Ok(Self { - common: RoleGroupCommon { - cluster, - cluster_info, - role: ROLE, - role_group_name: role_group_name.clone(), - selector_labels, - common: config.common.clone(), - resources: config.resources.clone().into(), - volume_claim_templates, - hdfs_logging, - vector_logging, - replicas: rg_config.replicas, - config_overrides: rg_config.config_overrides.clone(), - env_overrides: rg_config.env_overrides.clone(), - pod_overrides: rg_config.pod_overrides.clone(), - jvm_argument_overrides: rg_config - .product_specific_common_config - .jvm_argument_overrides - .clone(), }, - zkfc_logging: config - .logging - .for_container(&NameNodeContainer::Zkfc) - .into_owned(), - format_namenodes_logging: config - .logging - .for_container(&NameNodeContainer::FormatNameNodes) - .into_owned(), - format_zookeeper_logging: config - .logging - .for_container(&NameNodeContainer::FormatZooKeeper) - .into_owned(), - }) - } - - pub(crate) fn build_services(&self) -> Result, Error> { - self.common.build_services() - } - - pub(crate) fn build_statefulset(&self) -> Result { - let mut pb = resource::statefulset::common_pod_builder(&self.common); - - self.add_containers(&mut pb) - .map_err(|source| self.common.container_error(source))?; - - resource::statefulset::finish_statefulset(pb, &self.common) - .map_err(|source| self.common.stateful_set_error(source)) - } - - pub(crate) fn build_config_map(&self) -> Result { - let mut builder = resource::config_map::common_config_map(&self.common, None) - .map_err(|source| self.common.config_map_error(source))?; - - product_logging::add_log4j_config(&mut builder, &ContainerConfig::Zkfc, &self.zkfc_logging); - product_logging::add_log4j_config( - &mut builder, - &ContainerConfig::FormatNameNodes, - &self.format_namenodes_logging, - ); - product_logging::add_log4j_config( - &mut builder, - &ContainerConfig::FormatZooKeeper, - &self.format_zookeeper_logging, - ); - - resource::config_map::finish_config_map(builder, &self.common) - .map_err(|source| self.common.config_map_error(source)) - } - - /// The containers a namenode role group runs: the `hdfs` main container and the Vector - /// sidecar that every role has, then the ZooKeeper fail-over controller side container and - /// the two init containers that format the namenodes and ZooKeeper. - fn add_containers(&self, pb: &mut PodBuilder) -> Result<(), container::Error> { - ContainerConfig::add_hdfs_container_and_common_volumes(pb, &self.common)?; - - ContainerConfig::Zkfc.add_as_side_container(pb, &self.common, &self.zkfc_logging)?; - ContainerConfig::FormatNameNodes.add_as_init_container( - pb, - &self.common, - &self.format_namenodes_logging, - )?; - ContainerConfig::FormatZooKeeper.add_as_init_container( - pb, - &self.common, - &self.format_zookeeper_logging, )?; - Ok(()) - } + let (hdfs_logging, vector_logging) = common_container_logging( + &config.logging, + NameNodeContainer::Hdfs, + NameNodeContainer::Vector, + ); + + let inputs = RoleGroupInputs { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + selector_labels, + common: config.common.clone(), + resources: config.resources.clone().into(), + volume_claim_templates, + extra_pod_volumes: Vec::new(), + hdfs_logging, + vector_logging, + datanode_storage: None, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + }; + + RoleGroupBuilder::new( + inputs, + vec![ + ExtraContainer::side( + ContainerConfig::Zkfc, + config + .logging + .for_container(&NameNodeContainer::Zkfc) + .into_owned(), + ), + ExtraContainer::init( + ContainerConfig::FormatNameNodes, + config + .logging + .for_container(&NameNodeContainer::FormatNameNodes) + .into_owned(), + ), + ExtraContainer::init( + ContainerConfig::FormatZooKeeper, + config + .logging + .for_container(&NameNodeContainer::FormatZooKeeper) + .into_owned(), + ), + ], + ) } diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index 521f64c7..10d406ed 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -175,7 +175,7 @@ mod test { use super::*; use crate::{ HDFS_FULL_CONTROLLER_NAME, - controller::build::role_group::DataNodeRoleGroupBuilder, + controller::build::role_group::build_datanode_role_group, test_support::{datanode_role_group_config, deserialize_cluster, validate_cluster}, }; @@ -224,7 +224,7 @@ spec: }; // Built through the production path, so this test cannot drift from what the build step // actually produces. - let builder = DataNodeRoleGroupBuilder::new( + let builder = build_datanode_role_group( &validated_cluster, &cluster_info, &role_group_name, From 167657025146090fd069b3c35f736aa05522c44c Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 22 Sep 2026 16:20:29 +0200 Subject: [PATCH 4/4] keep only the role on ContainerConfig::Hdfs --- .../src/controller/build/container.rs | 89 ++++++------------- .../src/controller/build/mod.rs | 15 +++- 2 files changed, 38 insertions(+), 66 deletions(-) diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index fcee0866..50556065 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -52,10 +52,7 @@ use stackable_operator::{ STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, VectorContainerLogConfig, vector_container, }, - types::{ - common::Port, - kubernetes::{ConfigMapName, ContainerName, VolumeName}, - }, + types::kubernetes::{ConfigMapName, ContainerName, VolumeName}, }, }; use strum::{Display, EnumDiscriminants, IntoStaticStr}; @@ -78,14 +75,12 @@ use crate::{ crd::{ DataNodeConfig, HdfsNodeRole, HdfsPodRef, JournalNodeConfig, NameNodeConfig, UpgradeState, constants::{ - DATANODE_ROOT_DATA_DIR_PREFIX, DEFAULT_DATA_NODE_METRICS_PORT, - DEFAULT_JOURNAL_NODE_METRICS_PORT, DEFAULT_NAME_NODE_METRICS_PORT, LISTENER_VOLUME_DIR, - LISTENER_VOLUME_NAME, LIVENESS_PROBE_FAILURE_THRESHOLD, - LIVENESS_PROBE_INITIAL_DELAY_SECONDS, LIVENESS_PROBE_PERIOD_SECONDS, LOG4J_PROPERTIES, - NAMENODE_ROOT_DATA_DIR, READINESS_PROBE_FAILURE_THRESHOLD, - READINESS_PROBE_INITIAL_DELAY_SECONDS, READINESS_PROBE_PERIOD_SECONDS, - SERVICE_PORT_NAME_HTTP, SERVICE_PORT_NAME_HTTPS, SERVICE_PORT_NAME_IPC, - SERVICE_PORT_NAME_RPC, STACKABLE_ROOT_DATA_DIR, + DATANODE_ROOT_DATA_DIR_PREFIX, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, + LIVENESS_PROBE_FAILURE_THRESHOLD, LIVENESS_PROBE_INITIAL_DELAY_SECONDS, + LIVENESS_PROBE_PERIOD_SECONDS, LOG4J_PROPERTIES, NAMENODE_ROOT_DATA_DIR, + READINESS_PROBE_FAILURE_THRESHOLD, READINESS_PROBE_INITIAL_DELAY_SECONDS, + READINESS_PROBE_PERIOD_SECONDS, SERVICE_PORT_NAME_HTTP, SERVICE_PORT_NAME_HTTPS, + STACKABLE_ROOT_DATA_DIR, }, storage::DataNodeStorageConfig, }, @@ -168,17 +163,11 @@ pub enum Error { /// the HDFS cluster. #[derive(Display)] pub enum ContainerConfig { + /// The main container of a role, named after the role it runs. Hdfs { - /// HDFS role (name-, data-, journal-node) which determines the container name. + /// The HDFS role this is the main container of. It determines the container name, the + /// port the readiness probe checks and the JMX Exporter port. role: HdfsNodeRole, - /// Port name of the IPC/RPC port, used for the readiness probe. - ipc_port_name: &'static str, - /// Port name of the web UI HTTP port, used for the liveness probe. - web_ui_http_port_name: &'static str, - /// Port name of the web UI HTTPS port, used for the liveness probe. - web_ui_https_port_name: &'static str, - /// The JMX Exporter metrics port. - metrics_port: Port, }, /// The ZooKeeper fail-over controller side container of the namenodes. Zkfc, @@ -493,7 +482,7 @@ impl ContainerConfig { /// Return the typed container name. fn container_name(&self) -> &'static ContainerName { match self { - ContainerConfig::Hdfs { role, .. } => match role { + ContainerConfig::Hdfs { role } => match role { HdfsNodeRole::Name => &NAMENODE_CONTAINER_NAME, HdfsNodeRole::Data => &DATANODE_CONTAINER_NAME, HdfsNodeRole::Journal => &JOURNALNODE_CONTAINER_NAME, @@ -576,7 +565,7 @@ impl ContainerConfig { }; match self { - ContainerConfig::Hdfs { role, .. } => { + ContainerConfig::Hdfs { role } => { args.push_str(&self.copy_log4j_properties_cmd(container_log_config)); args.push_str(&formatdoc!( @@ -814,7 +803,7 @@ impl ContainerConfig { // When the users tries to start a cli tool the port is already taken by the hdfs services, // so we don't want to stuff all the config into HADOOP_OPTS, but rather into the specialized env variables // See https://github.com/stackabletech/hdfs-operator/issues/138 for details - if let ContainerConfig::Hdfs { role, .. } = self { + if let ContainerConfig::Hdfs { role } = self { let role_opts_name = role.hadoop_opts_env_var_for_role().to_string(); env.insert( role_opts_name.clone(), @@ -929,24 +918,20 @@ impl ContainerConfig { initial_delay_seconds: i32, failure_threshold: i32, ) -> Option { - let ContainerConfig::Hdfs { - web_ui_http_port_name, - web_ui_https_port_name, - .. - } = self - else { + let ContainerConfig::Hdfs { .. } = self else { return None; }; + // Every role serves its web UI under the same two port names. let port = if cluster.has_https_enabled() { - web_ui_https_port_name + SERVICE_PORT_NAME_HTTPS } else { - web_ui_http_port_name + SERVICE_PORT_NAME_HTTP }; Some(Probe { // Use tcp_socket instead of http_get so that the probe is independent of the authentication settings. - tcp_socket: Some(Self::tcp_socket_action_for_port(*port)), + tcp_socket: Some(Self::tcp_socket_action_for_port(port)), period_seconds: Some(period_seconds), initial_delay_seconds: Some(initial_delay_seconds), failure_threshold: Some(failure_threshold), @@ -962,8 +947,8 @@ impl ContainerConfig { failure_threshold: i32, ) -> Option { match self { - ContainerConfig::Hdfs { ipc_port_name, .. } => Some(Probe { - tcp_socket: Some(Self::tcp_socket_action_for_port(*ipc_port_name)), + ContainerConfig::Hdfs { role } => Some(Probe { + tcp_socket: Some(Self::tcp_socket_action_for_port(build::ipc_port_name(role))), period_seconds: Some(period_seconds), initial_delay_seconds: Some(initial_delay_seconds), failure_threshold: Some(failure_threshold), @@ -1061,7 +1046,7 @@ impl ContainerConfig { .build(), ); } - ContainerConfig::Hdfs { role, .. } => { + ContainerConfig::Hdfs { role } => { // JournalNode doesn't use listeners, since it's only used internally by the namenodes if let HdfsNodeRole::Name | HdfsNodeRole::Data = role { volume_mounts.push( @@ -1151,9 +1136,7 @@ impl ContainerConfig { ) -> Result { let cluster = inputs.cluster; match self { - ContainerConfig::Hdfs { - role, metrics_port, .. - } => { + ContainerConfig::Hdfs { role } => { let volume_mount_dirs = self.volume_mount_dirs(); let config_dir = volume_mount_dirs.final_config(); construct_role_specific_jvm_args( @@ -1162,7 +1145,7 @@ impl ContainerConfig { cluster.has_kerberos_enabled(), resources, config_dir, - metrics_port.clone(), + build::jmx_metrics_port(role), ) .with_context(|_| ConstructJvmArgumentsSnafu { role: role.to_string(), @@ -1175,7 +1158,7 @@ impl ContainerConfig { /// Container ports for the main containers namenode, datanode and journalnode. fn container_ports(&self, cluster: &ValidatedCluster) -> Vec { match self { - ContainerConfig::Hdfs { role, .. } => { + ContainerConfig::Hdfs { role } => { // data ports build::hdfs_main_container_ports(cluster, role) .into_iter() @@ -1281,29 +1264,7 @@ impl ContainerConfig { impl From for ContainerConfig { fn from(role: HdfsNodeRole) -> Self { - match role { - HdfsNodeRole::Name => Self::Hdfs { - role, - ipc_port_name: SERVICE_PORT_NAME_RPC, - web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, - web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, - metrics_port: DEFAULT_NAME_NODE_METRICS_PORT, - }, - HdfsNodeRole::Data => Self::Hdfs { - role, - ipc_port_name: SERVICE_PORT_NAME_IPC, - web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, - web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, - metrics_port: DEFAULT_DATA_NODE_METRICS_PORT, - }, - HdfsNodeRole::Journal => Self::Hdfs { - role, - ipc_port_name: SERVICE_PORT_NAME_RPC, - web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, - web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, - metrics_port: DEFAULT_JOURNAL_NODE_METRICS_PORT, - }, - } + Self::Hdfs { role } } } diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index ead6841a..485d1446 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -413,8 +413,19 @@ pub(crate) fn native_metrics_port(cluster: &ValidatedCluster, role: &HdfsNodeRol } } -/// The deprecated JMX exporter metrics port for the given `role`. -fn jmx_metrics_port(role: &HdfsNodeRole) -> Port { +/// The name of the port the given `role` serves IPC/RPC on, which its readiness probe checks. +/// +/// The datanodes call theirs `ipc`, the other two `rpc`; the same names [`role_data_ports`] +/// exposes them under. +pub(crate) fn ipc_port_name(role: &HdfsNodeRole) -> &'static str { + match role { + HdfsNodeRole::Name | HdfsNodeRole::Journal => SERVICE_PORT_NAME_RPC, + HdfsNodeRole::Data => SERVICE_PORT_NAME_IPC, + } +} + +/// The deprecated JMX Exporter metrics port for the given `role`. +pub(crate) fn jmx_metrics_port(role: &HdfsNodeRole) -> Port { match role { HdfsNodeRole::Name => DEFAULT_NAME_NODE_METRICS_PORT, HdfsNodeRole::Data => DEFAULT_DATA_NODE_METRICS_PORT,