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()