refactor: switch to generic rolegroupconfig

Author: maltesander

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

@@ -1,11 +1,8 @@
 use snafu::{OptionExt, ResultExt, Snafu};
-use stackable_operator::{
-    memory::{BinaryMultiple, MemoryQuantity},
-    v2::jvm_argument_overrides::JvmArgumentOverrides,
-};
+use stackable_operator::memory::{BinaryMultiple, MemoryQuantity};
 
 use crate::{
-    controller::build::kerberos::KRB5_CONFIG_PATH,
+    controller::{HbaseRoleGroupConfig, ValidatedCluster, build::kerberos::KRB5_CONFIG_PATH},
     crd::{AnyServiceConfig, CONFIG_DIR_NAME, JVM_SECURITY_PROPERTIES_FILE},
 };
 
@@ -44,23 +41,24 @@ pub fn construct_global_jvm_args(kerberos_enabled: bool) -> String {
 /// JVM arguments that are specifically for the role (server), so will *not* be used e.g. by CLI tools.
 /// Heap settings are excluded, as they go into `HBASE_HEAPSIZE`.
 ///
-/// `merged_jvm_argument_overrides` is the role <- role-group merged [`JvmArgumentOverrides`]
-/// produced by
-/// [`with_validated_config`](stackable_operator::v2::role_utils::with_validated_config). The
-/// operator-generated arguments below form the base that the user overrides are applied on top of.
+/// The role <- role-group merged `jvmArgumentOverrides` (from `rg.product_specific_common_config`)
+/// are applied on top of the operator-generated base arguments below.
 pub fn construct_role_specific_non_heap_jvm_args(
-    kerberos_enabled: bool,
-    merged_jvm_argument_overrides: &JvmArgumentOverrides,
+    cluster: &ValidatedCluster,
+    rg: &HbaseRoleGroupConfig,
 ) -> String {
     let mut operator_generated = vec![format!(
         "-Djava.security.properties={CONFIG_DIR_NAME}/{JVM_SECURITY_PROPERTIES_FILE}"
     )];
 
-    if kerberos_enabled {
+    if cluster.has_kerberos_enabled() {
         operator_generated.push(format!("-Djava.security.krb5.conf={KRB5_CONFIG_PATH}"));
     }
 
-    let mut jvm_args = merged_jvm_argument_overrides.apply_to(operator_generated);
+    let mut jvm_args = rg
+        .product_specific_common_config
+        .jvm_argument_overrides
+        .apply_to(operator_generated);
     jvm_args.retain(|arg| !is_heap_jvm_argument(arg));
 
     jvm_args.join(" ")
@@ -136,10 +134,8 @@ mod tests {
             construct_hbase_heapsize_env(&region_server.config.config).unwrap();
 
         assert_eq!(global_jvm_args, "");
-        // `non_heap_jvm_args` is the output of `construct_role_specific_non_heap_jvm_args`,
-        // pre-resolved during validation.
         assert_eq!(
-            region_server.non_heap_jvm_args,
+            construct_role_specific_non_heap_jvm_args(&validated_cluster, region_server),
             "-Djava.security.properties=/stackable/conf/security.properties"
         );
         assert_eq!(hbase_heapsize_env, "819m");
@@ -202,7 +198,7 @@ mod tests {
             "-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf"
         );
         assert_eq!(
-            region_server.non_heap_jvm_args,
+            construct_role_specific_non_heap_jvm_args(&validated_cluster, region_server),
             "-Djava.security.properties=/stackable/conf/security.properties \
             -Djava.security.krb5.conf=/stackable/kerberos/krb5.conf \
             -Dhttps.proxyHost=proxy.my.corp \

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

@@ -11,9 +11,12 @@ use stackable_operator::{
 use crate::{
     controller::{
         ValidatedCluster,
-        build::properties::{
-            ConfigFileName, hbase_env, hbase_site, logging, security_properties, ssl_client,
-            ssl_server,
+        build::{
+            jvm::construct_role_specific_non_heap_jvm_args,
+            properties::{
+                ConfigFileName, hbase_env, hbase_site, logging, security_properties, ssl_client,
+                ssl_server,
+            },
         },
     },
     crd::HbaseRole,
@@ -81,7 +84,7 @@ pub fn build_rolegroup_config_map(
         merged_config,
         role,
         cluster_config.kerberos_enabled,
-        rg.non_heap_jvm_args.clone(),
+        construct_role_specific_non_heap_jvm_args(cluster, rg),
         overrides.hbase_env_sh.clone(),
     )
     .context(BuildHbaseEnvSnafu)?;

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

@@ -36,7 +36,7 @@ use stackable_operator::{
 
 use crate::{
     controller::{
-        ValidatedCluster, ValidatedRoleGroupConfig,
+        HbaseRoleGroupConfig, ValidatedCluster,
         build::{
             graceful_shutdown::{self, add_graceful_shutdown_config},
             kerberos::{self, add_kerberos_pod_config},
@@ -99,7 +99,7 @@ pub fn build_rolegroup_statefulset(
     cluster: &ValidatedCluster,
     hbase_role: &HbaseRole,
     role_group_name: &RoleGroupName,
-    validated_rg_config: &ValidatedRoleGroupConfig,
+    validated_rg_config: &HbaseRoleGroupConfig,
     service_account: &ServiceAccount,
 ) -> Result<StatefulSet> {
     let resolved_product_image = &cluster.image;

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

@@ -10,12 +10,12 @@ pub use stackable_operator::v2::types::operator::RoleGroupName;
 use stackable_operator::{
     builder::meta::ObjectMetaBuilder,
     commons::product_image_selection::ResolvedProductImage,
-    k8s_openapi::{api::core::v1::PodTemplateSpec, apimachinery::pkg::apis::meta::v1::ObjectMeta},
+    k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta,
     kube::Resource,
     kvp::Labels,
     v2::{
         HasName, HasUid, NameIsValidLabelValue,
-        builder::{meta::ownerreference_from_resource, pod::container::EnvVarSet},
+        builder::meta::ownerreference_from_resource,
         kvp::label::{recommended_labels, role_group_selector},
         role_group_utils::ResourceNames,
         types::{
@@ -71,7 +71,7 @@ pub struct ValidatedCluster {
     /// value.
     pub product_version: ProductVersion,
     pub cluster_config: ValidatedClusterConfig,
-    pub role_group_configs: BTreeMap<HbaseRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
+    pub role_group_configs: BTreeMap<HbaseRole, BTreeMap<RoleGroupName, HbaseRoleGroupConfig>>,
     pub role_configs: BTreeMap<HbaseRole, ValidatedRoleConfig>,
 }
 
@@ -83,7 +83,7 @@ impl ValidatedCluster {
         uid: Uid,
         image: ResolvedProductImage,
         cluster_config: ValidatedClusterConfig,
-        role_group_configs: BTreeMap<HbaseRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
+        role_group_configs: BTreeMap<HbaseRole, BTreeMap<RoleGroupName, HbaseRoleGroupConfig>>,
         role_configs: BTreeMap<HbaseRole, ValidatedRoleConfig>,
     ) -> Self {
         // `app_version_label_value` is constructed to be a valid label value, so it is also a
@@ -283,15 +283,13 @@ pub struct ValidatedHbaseConfig {
     pub logging: validate::ValidatedLogging,
 }
 
-#[derive(Clone, Debug)]
-pub struct ValidatedRoleGroupConfig {
-    /// The desired number of replicas (defaulted to 1 during validation).
-    pub replicas: u16,
-    pub config: ValidatedHbaseConfig,
-    pub config_overrides: v1alpha1::HbaseConfigOverrides,
-    pub env_overrides: EnvVarSet,
-    /// Merged (role <- role group) pod template overrides.
-    pub pod_overrides: PodTemplateSpec,
-    /// Pre-resolved role-specific non-heap JVM args (operator-generated + role/role-group overrides).
-    pub non_heap_jvm_args: String,
-}
+/// Per-rolegroup configuration: a v2
+/// [`RoleGroupConfig`](stackable_operator::v2::role_utils::RoleGroupConfig) over the validated
+/// HBase config. The merged (role <- role group) `jvmArgumentOverrides` are available via
+/// `product_specific_common_config` and applied at build time by
+/// [`construct_role_specific_non_heap_jvm_args`](build::jvm::construct_role_specific_non_heap_jvm_args).
+pub type HbaseRoleGroupConfig = stackable_operator::v2::role_utils::RoleGroupConfig<
+    ValidatedHbaseConfig,
+    stackable_operator::v2::role_utils::JavaCommonConfig,
+    v1alpha1::HbaseConfigOverrides,
+>;

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

@@ -26,14 +26,11 @@ use strum::IntoEnumIterator;
 
 use crate::{
     controller::{
-        ValidatedCluster, ValidatedClusterConfig, ValidatedHbaseConfig, ValidatedRoleConfig,
-        ValidatedRoleGroupConfig,
-        build::{
-            jvm::construct_role_specific_non_heap_jvm_args,
-            kerberos::{
-                kerberos_config_properties, kerberos_discovery_config_properties,
-                kerberos_ssl_client_settings, kerberos_ssl_server_settings,
-            },
+        HbaseRoleGroupConfig, ValidatedCluster, ValidatedClusterConfig, ValidatedHbaseConfig,
+        ValidatedRoleConfig,
+        build::kerberos::{
+            kerberos_config_properties, kerberos_discovery_config_properties,
+            kerberos_ssl_client_settings, kerberos_ssl_server_settings,
         },
         dereference::DereferencedObjects,
     },
@@ -156,7 +153,6 @@ pub fn validate_cluster(
     for hbase_role in HbaseRole::iter() {
         let group_configs = match hbase_role {
             HbaseRole::Master => validate_role_group_configs(
-                hbase,
                 hbase.spec.masters.as_ref(),
                 HbaseConfigFragment::default_config(
                     &hbase_role,
@@ -167,7 +163,6 @@ pub fn validate_cluster(
                 &vector_aggregator_config_map_name,
             )?,
             HbaseRole::RegionServer => validate_role_group_configs(
-                hbase,
                 hbase.spec.region_servers.as_ref(),
                 RegionServerConfigFragment::default_config(
                     &hbase_role,
@@ -178,7 +173,6 @@ pub fn validate_cluster(
                 &vector_aggregator_config_map_name,
             )?,
             HbaseRole::RestServer => validate_role_group_configs(
-                hbase,
                 hbase.spec.rest_servers.as_ref(),
                 HbaseConfigFragment::default_config(
                     &hbase_role,
@@ -275,20 +269,18 @@ pub fn validate_cluster(
 /// `jvmArgumentOverrides` (role group wins) into a single merged
 /// [`RoleGroup`](stackable_operator::role_utils::RoleGroup). The per-role validated config
 /// is wrapped into [`AnyServiceConfig`] via `wrap`; the merged `envOverrides` are converted
-/// into an [`EnvVarSet`] (validating each name eagerly), and the role-specific non-heap JVM
-/// args are pre-resolved from the merged `jvmArgumentOverrides` so the build step stays a
-/// pure function of [`ValidatedCluster`].
+/// into an [`EnvVarSet`] (validating each name eagerly). The merged `jvmArgumentOverrides` are
+/// kept in `product_specific_common_config` and applied at build time.
 ///
 /// Returns an empty map if the role is not configured.
 fn validate_role_group_configs<Config, ValidatedConfig>(
-    hbase: &v1alpha1::HbaseCluster,
     role: Option<
         &Role<Config, v1alpha1::HbaseConfigOverrides, GenericRoleConfig, JavaCommonConfig>,
     >,
     default_config: Config,
     wrap: fn(ValidatedConfig) -> AnyServiceConfig,
     vector_aggregator_config_map_name: &Option<ConfigMapName>,
-) -> Result<BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>, Error>
+) -> Result<BTreeMap<RoleGroupName, HbaseRoleGroupConfig>, Error>
 where
     Config: Clone + Merge,
     ValidatedConfig: FromFragment<Fragment = Config>,
@@ -318,16 +310,11 @@ where
                 config,
                 config_overrides,
                 env_overrides,
-                cli_overrides: _,
+                cli_overrides,
                 pod_overrides,
                 product_specific_common_config,
             } = validated.config;
 
-            let non_heap_jvm_args = construct_role_specific_non_heap_jvm_args(
-                hbase.has_kerberos_enabled(),
-                &product_specific_common_config.jvm_argument_overrides,
-            );
-
             // Convert the merged env-override HashMap into an EnvVarSet, validating each name
             // eagerly. Keys are unique (HashMap), so insertion order is irrelevant.
             let mut env_overrides_set = EnvVarSet::new();
@@ -345,13 +332,14 @@ where
             // time. The build step then consumes the validated logging instead of the raw config.
             let logging = validate_logging(config.logging(), vector_aggregator_config_map_name)?;
 
-            let validated = ValidatedRoleGroupConfig {
+            let validated = HbaseRoleGroupConfig {
                 replicas: validated.replicas.unwrap_or(1),
                 config: ValidatedHbaseConfig { config, logging },
                 config_overrides,
                 env_overrides: env_overrides_set,
+                cli_overrides,
                 pod_overrides,
-                non_heap_jvm_args,
+                product_specific_common_config,
             };
             Ok((role_group_name.clone(), validated))
         })