fix: reduce RoleGroupRef usages

Author: maltesander

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

@@ -48,13 +48,13 @@ use stackable_operator::{
             ContainerLogConfigChoice, CustomContainerLogConfig,
         },
     },
-    role_utils::RoleGroupRef,
     utils::{COMMON_BASH_TRAP_FUNCTIONS, cluster_info::KubernetesClusterInfo},
     v2::{
         builder::pod::container::new_container_builder,
         types::{
             common::Port,
             kubernetes::{ContainerName, VolumeName},
+            operator::RoleGroupName,
         },
     },
 };
@@ -90,7 +90,6 @@ use crate::{
             SERVICE_PORT_NAME_RPC, STACKABLE_ROOT_DATA_DIR,
         },
         storage::DataNodeStorageConfig,
-        v1alpha1,
     },
 };
 
@@ -212,15 +211,16 @@ impl ContainerConfig {
         cluster: &ValidatedCluster,
         cluster_info: &KubernetesClusterInfo,
         role: &HdfsNodeRole,
-        rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
+        role_group_name: &RoleGroupName,
         rolegroup_config: &ValidatedRoleGroupConfig,
         labels: &Labels,
     ) -> Result<(), Error> {
         let namenode_podrefs = build::pod_refs(cluster, &HdfsNodeRole::Name);
 
         // HDFS main container
         let main_container_config = Self::from(*role);
-        let object_name = rolegroup_ref.object_name();
+        let resource_names = cluster.resource_names(role, role_group_name);
+        let object_name = resource_names.qualified_role_group_name().to_string();
         let merged_config = &rolegroup_config.config;
 
         pb.add_volumes(main_container_config.volumes(merged_config, &object_name, labels)?)
@@ -277,7 +277,7 @@ impl ContainerConfig {
                         .with_pod_scope()
                         .with_node_scope()
                         // To scrape metrics behind TLS endpoint (without FQDN)
-                        .with_service_scope(rolegroup_ref.rolegroup_metrics_service_name())
+                        .with_service_scope(resource_names.metrics_service_name().to_string())
                         .with_format(SecretFormat::TlsPkcs12)
                         .with_tls_pkcs12_password(TLS_STORE_PASSWORD)
                         .with_auto_tls_cert_lifetime(

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

@@ -98,7 +98,7 @@ mod tests {
     use crate::{
         controller::build::container::ContainerConfig,
         crd::constants::DEFAULT_NAME_NODE_METRICS_PORT,
-        test_support::{deserialize_and_validate_cluster, role_group_config},
+        test_support::{deserialize_and_validate_cluster, role_group_config, role_group_name},
     };
 
     #[test]
@@ -197,7 +197,8 @@ mod tests {
         let role = HdfsNodeRole::Name;
 
         let validated_cluster = deserialize_and_validate_cluster(hdfs_cluster);
-        let role_group_config = role_group_config(&validated_cluster, &role, "default");
+        let role_group_config =
+            role_group_config(&validated_cluster, &role, &role_group_name("default"));
 
         let resources = ContainerConfig::from(role).resources(&role_group_config.config);
 

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

@@ -1,10 +1,12 @@
-use std::{collections::HashMap, ops::Deref};
+use std::collections::HashMap;
 
 use stackable_operator::{
     builder::meta::ObjectMetaBuilder,
     kvp::{LabelError, Labels},
-    role_utils::RoleGroupRef,
-    v2::{builder::meta::ownerreference_from_resource, types::common::Port},
+    v2::{
+        builder::meta::ownerreference_from_resource,
+        types::{common::Port, operator::RoleGroupName},
+    },
 };
 
 use crate::{
@@ -27,7 +29,6 @@ use crate::{
             SERVICE_PORT_NAME_IPC, SERVICE_PORT_NAME_JMX_METRICS, SERVICE_PORT_NAME_METRICS,
             SERVICE_PORT_NAME_RPC,
         },
-        v1alpha1,
     },
     hdfs_controller::RESOURCE_MANAGER_HDFS_CONTROLLER,
 };
@@ -60,7 +61,10 @@ pub(crate) fn pod_refs(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Vec<H
         .into_iter()
         .flatten()
         .flat_map(|(role_group_name, role_group)| {
-            let object_name = format!("{}-{role}-{role_group_name}", cluster.name);
+            let object_name = cluster
+                .resource_names(role, role_group_name)
+                .qualified_role_group_name()
+                .to_string();
             let ports = ports.clone();
             (0..role_group.replicas.unwrap_or(1)).map(move |i| HdfsPodRef {
                 namespace: cluster.namespace.to_string(),
@@ -82,19 +86,25 @@ pub(crate) fn pod_refs(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Vec<H
 /// the owner reference nor the recommended labels can fail to build.
 pub(crate) fn rolegroup_metadata(
     cluster: &ValidatedCluster,
-    rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
+    role: &HdfsNodeRole,
+    role_group_name: &RoleGroupName,
 ) -> ObjectMetaBuilder {
+    let role_name = role.to_string();
     let mut metadata = ObjectMetaBuilder::new();
     metadata
         .name_and_namespace(cluster)
-        .name(rolegroup_ref.object_name())
+        .name(
+            cluster
+                .resource_names(role, role_group_name)
+                .qualified_role_group_name(),
+        )
         .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
         .with_recommended_labels(&build_recommended_labels(
             cluster,
             RESOURCE_MANAGER_HDFS_CONTROLLER,
             &cluster.image.app_version_label_value,
-            &rolegroup_ref.role,
-            &rolegroup_ref.role_group,
+            &role_name,
+            role_group_name.as_ref(),
         ))
         .expect(
             "the recommended labels are valid because the ValidatedCluster uses \
@@ -104,19 +114,17 @@ pub(crate) fn rolegroup_metadata(
 }
 
 /// The rolegroup selector labels (also used as `Service`/`StatefulSet` selectors) for
-/// the given `rolegroup_ref`.
+/// the given role group.
 pub(crate) fn rolegroup_selector_labels(
     cluster: &ValidatedCluster,
-    rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
+    role: &HdfsNodeRole,
+    role_group_name: &RoleGroupName,
 ) -> Result<Labels, LabelError> {
-    let mut group_labels = Labels::role_group_selector(
-        cluster,
-        APP_NAME,
-        &rolegroup_ref.role,
-        &rolegroup_ref.role_group,
-    )?;
-    group_labels.parse_insert(("role", rolegroup_ref.role.deref()))?;
-    group_labels.parse_insert(("group", rolegroup_ref.role_group.deref()))?;
+    let role_name = role.to_string();
+    let mut group_labels =
+        Labels::role_group_selector(cluster, APP_NAME, &role_name, role_group_name.as_ref())?;
+    group_labels.parse_insert(("role", &role_name))?;
+    group_labels.parse_insert(("group", role_group_name.as_ref()))?;
 
     Ok(group_labels)
 }

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

@@ -334,11 +334,15 @@ mod tests {
     use crate::{
         controller::build::properties::test_support::{cluster_info, validated_cluster},
         crd::HdfsNodeRole,
-        test_support::anynode_config,
+        test_support::{anynode_config, role_group_name},
     };
 
     fn namenode_merged_config(validated_cluster: &ValidatedCluster) -> &AnyNodeConfig {
-        anynode_config(validated_cluster, &HdfsNodeRole::Name, "default")
+        anynode_config(
+            validated_cluster,
+            &HdfsNodeRole::Name,
+            &role_group_name("default"),
+        )
     }
 
     #[test]

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

@@ -3,15 +3,20 @@
 use std::{borrow::Cow, fmt::Display};
 
 use stackable_operator::{
+    kube::runtime::reflector::ObjectRef,
     memory::{BinaryMultiple, MemoryQuantity},
     product_logging::{
         self,
         spec::{ContainerLogConfig, ContainerLogConfigChoice},
     },
     role_utils::RoleGroupRef,
+    v2::types::operator::RoleGroupName,
 };
 
-use crate::crd::{AnyNodeConfig, DataNodeContainer, NameNodeContainer, v1alpha1};
+use crate::{
+    controller::ValidatedCluster,
+    crd::{AnyNodeConfig, DataNodeContainer, HdfsNodeRole, NameNodeContainer},
+};
 
 pub const STACKABLE_LOG_DIR: &str = "/stackable/log";
 // We have a maximum of 4 continuous logging files for Namenodes. Datanodes and Journalnodes
@@ -150,7 +155,9 @@ fn add_log4j_config_if_automatic(
 ///
 /// Returns `None` when the Vector agent is disabled for this role group.
 pub fn build_vector_config(
-    rolegroup: &RoleGroupRef<v1alpha1::HdfsCluster>,
+    cluster: &ValidatedCluster,
+    role: &HdfsNodeRole,
+    role_group_name: &RoleGroupName,
     merged_config: &AnyNodeConfig,
 ) -> Option<String> {
     if !merged_config.vector_logging_enabled() {
@@ -167,8 +174,19 @@ pub fn build_vector_config(
         None
     };
 
+    // TODO: The framework's `create_vector_config` still requires a `RoleGroupRef`. We build one
+    // over the `ValidatedCluster` (not the raw cluster) purely to satisfy this API; it only reads
+    // the cluster name/namespace and the role/role-group strings, so the output is unchanged.
+    // Hive ships a static `vector.yaml` instead and avoids `RoleGroupRef` entirely - we should
+    // follow once a static config is available for HDFS, which would drop this last usage.
+    let rolegroup = RoleGroupRef {
+        cluster: ObjectRef::<ValidatedCluster>::from_obj(cluster),
+        role: role.to_string(),
+        role_group: role_group_name.to_string(),
+    };
+
     Some(product_logging::framework::create_vector_config(
-        rolegroup,
+        &rolegroup,
         vector_log_config,
     ))
 }

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

@@ -1,12 +1,12 @@
 //! Build the per-rolegroup `ConfigMap` for the HdfsCluster.
 
-use std::str::FromStr;
-
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
-    builder::configmap::ConfigMapBuilder, k8s_openapi::api::core::v1::ConfigMap,
-    product_logging::framework::VECTOR_CONFIG_FILE, role_utils::RoleGroupRef,
-    utils::cluster_info::KubernetesClusterInfo, v2::config_file_writer::PropertiesWriterError,
+    builder::configmap::ConfigMapBuilder,
+    k8s_openapi::api::core::v1::ConfigMap,
+    product_logging::framework::VECTOR_CONFIG_FILE,
+    utils::cluster_info::KubernetesClusterInfo,
+    v2::{config_file_writer::PropertiesWriterError, types::operator::RoleGroupName},
 };
 
 use crate::{
@@ -20,17 +20,11 @@ use crate::{
             },
         },
     },
-    crd::{HdfsNodeRole, v1alpha1},
+    crd::HdfsNodeRole,
 };
 
 #[derive(Snafu, Debug)]
 pub enum Error {
-    #[snafu(display("could not parse HDFS role [{role}]"))]
-    UnidentifiedHdfsRole {
-        source: strum::ParseError,
-        role: String,
-    },
-
     #[snafu(display("the validated cluster has no role group {role_group:?} for role {role:?}"))]
     MissingRoleGroup { role: String, role_group: String },
 
@@ -53,24 +47,20 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 pub fn build_rolegroup_config_map(
     cluster: &ValidatedCluster,
     cluster_info: &KubernetesClusterInfo,
-    rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
+    role: &HdfsNodeRole,
+    role_group_name: &RoleGroupName,
 ) -> Result<ConfigMap> {
-    tracing::info!("Setting up ConfigMap for {:?}", rolegroup_ref);
+    tracing::info!("Setting up ConfigMap for role {role} role group {role_group_name}");
 
-    let metadata = build::rolegroup_metadata(cluster, rolegroup_ref);
+    let metadata = build::rolegroup_metadata(cluster, role, role_group_name);
 
-    let role = HdfsNodeRole::from_str(&rolegroup_ref.role).with_context(|_| {
-        UnidentifiedHdfsRoleSnafu {
-            role: rolegroup_ref.role.clone(),
-        }
-    })?;
     let rolegroup_config = cluster
         .role_groups
-        .get(&role)
-        .and_then(|role_groups| role_groups.get(&rolegroup_ref.role_group))
+        .get(role)
+        .and_then(|role_groups| role_groups.get(role_group_name))
         .with_context(|| MissingRoleGroupSnafu {
-            role: rolegroup_ref.role.clone(),
-            role_group: rolegroup_ref.role_group.clone(),
+            role: role.to_string(),
+            role_group: role_group_name.to_string(),
         })?;
     let merged_config = &rolegroup_config.config;
     let config_overrides = &rolegroup_config.config_overrides;
@@ -84,7 +74,7 @@ pub fn build_rolegroup_config_map(
     );
     let core_site_xml = core_site::build(
         cluster,
-        role,
+        *role,
         cluster_info,
         config_overrides.core_site_xml.clone(),
     );
@@ -110,20 +100,22 @@ pub fn build_rolegroup_config_map(
             ConfigFileName::Security.to_string(),
             security_properties::build(config_overrides.security_properties.clone()).with_context(
                 |_| JvmSecurityPropertiesSnafu {
-                    rolegroup: rolegroup_ref.role_group.clone(),
+                    rolegroup: role_group_name.to_string(),
                 },
             )?,
         );
 
     for (log_config_file, log4j_config) in logging::build_log4j_configs(merged_config) {
         builder.add_data(log_config_file, log4j_config);
     }
-    if let Some(vector_config) = logging::build_vector_config(rolegroup_ref, merged_config) {
+    if let Some(vector_config) =
+        logging::build_vector_config(cluster, role, role_group_name, merged_config)
+    {
         builder.add_data(VECTOR_CONFIG_FILE, vector_config);
     }
 
     builder.build().with_context(|_| AssembleSnafu {
-        role: rolegroup_ref.role.clone(),
-        role_group: rolegroup_ref.role_group.clone(),
+        role: role.to_string(),
+        role_group: role_group_name.to_string(),
     })
 }