refactor: use RoleGroupName, use v2 infallible labels, ResourceNames and reduce usage of RoleGroupRef.

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -5,20 +5,15 @@ use stackable_operator::{
     builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
     k8s_openapi::api::core::v1::ConfigMap,
     product_logging::framework::VECTOR_CONFIG_FILE,
-    role_utils::RoleGroupRef,
     v2::{
         builder::meta::ownerreference_from_resource,
         config_file_writer::{PropertiesWriterError, to_hadoop_xml, to_java_properties_string},
     },
 };
 
-use crate::{
-    controller::{
-        HiveRoleGroupConfig, ValidatedCluster,
-        build::properties::{ConfigFileName, core_site, hive_site, logging, security_properties},
-        build_recommended_labels,
-    },
-    crd::v1alpha1,
+use crate::controller::{
+    HiveRoleGroupConfig, RoleGroupName, ValidatedCluster,
+    build::properties::{ConfigFileName, core_site, hive_site, logging, security_properties},
 };
 
 #[derive(Debug, Snafu)]
@@ -29,26 +24,25 @@ pub enum Error {
     #[snafu(display("failed to serialize {}", ConfigFileName::Security))]
     WriteSecurityProperties { source: PropertiesWriterError },
 
-    #[snafu(display("failed to build metadata"))]
-    MetadataBuild {
-        source: stackable_operator::builder::meta::Error,
-    },
-
-    #[snafu(display("failed to assemble ConfigMap for {rolegroup}"))]
+    #[snafu(display("failed to assemble ConfigMap for role group {role_group}"))]
     Assemble {
         source: stackable_operator::builder::configmap::Error,
-        rolegroup: RoleGroupRef<v1alpha1::HiveCluster>,
+        role_group: RoleGroupName,
     },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 /// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the
 /// administrator.
+///
+/// `vector_config` is the Vector agent config (`vector.yaml`) built by the caller (where a
+/// `RoleGroupRef` is available); it is `None` when the Vector agent is disabled.
 pub fn build_metastore_rolegroup_config_map(
     cluster: &ValidatedCluster,
-    rolegroup: &RoleGroupRef<v1alpha1::HiveCluster>,
+    role_group_name: &RoleGroupName,
     rg: &HiveRoleGroupConfig,
+    vector_config: Option<String>,
 ) -> Result<ConfigMap> {
     // hive-site.xml
     let hive_site_overrides = rg.config_overrides.hive_site_xml.overrides.clone();
@@ -69,15 +63,14 @@ pub fn build_metastore_rolegroup_config_map(
         .metadata(
             ObjectMetaBuilder::new()
                 .name_and_namespace(cluster)
-                .name(rolegroup.object_name())
+                .name(
+                    cluster
+                        .resource_names(role_group_name)
+                        .role_group_config_map()
+                        .to_string(),
+                )
                 .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
-                .with_recommended_labels(&build_recommended_labels(
-                    cluster,
-                    &cluster.image.app_version_label_value,
-                    &rolegroup.role,
-                    &rolegroup.role_group,
-                ))
-                .context(MetadataBuildSnafu)?
+                .with_labels(cluster.recommended_labels(role_group_name))
                 .build(),
         )
         .add_data(
@@ -101,11 +94,11 @@ pub fn build_metastore_rolegroup_config_map(
     if let Some(log4j2_properties) = logging::build_log4j2(&rg.config.logging) {
         cm_builder.add_data(ConfigFileName::Log4j2.to_string(), log4j2_properties);
     }
-    if let Some(vector_config) = logging::build_vector_config(rolegroup, &rg.config.logging) {
+    if let Some(vector_config) = vector_config {
         cm_builder.add_data(VECTOR_CONFIG_FILE, vector_config);
     }
 
     cm_builder.build().with_context(|_| AssembleSnafu {
-        rolegroup: rolegroup.clone(),
+        role_group: role_group_name.clone(),
     })
 }

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

@@ -1,3 +1,5 @@
+use std::str::FromStr;
+
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
@@ -8,7 +10,7 @@ use stackable_operator::{
 };
 
 use crate::{
-    controller::{ValidatedCluster, build_recommended_labels},
+    controller::{RoleGroupName, ValidatedCluster},
     crd::{HiveRole, v1alpha1},
     listener::build_listener_connection_string,
 };
@@ -21,10 +23,6 @@ pub enum Error {
         obj_ref: ObjectRef<v1alpha1::HiveCluster>,
     },
 
-    #[snafu(display("failed to build Metadata"))]
-    MetadataBuild {
-        source: stackable_operator::builder::meta::Error,
-    },
     #[snafu(display("failed to configure listener discovery configmap"))]
     ListenerConfiguration { source: crate::listener::Error },
 }
@@ -66,13 +64,14 @@ fn build_discovery_configmap(
         ObjectMetaBuilder::new()
             .name_and_namespace(cluster)
             .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
-            .with_recommended_labels(&build_recommended_labels(
-                cluster,
-                &cluster.image.app_version_label_value,
-                &hive_role.to_string(),
-                "discovery",
-            ))
-            .context(MetadataBuildSnafu)?
+            // Discovery is a role-level object; "discovery" is used as a placeholder role-group
+            // name for the recommended labels.
+            .with_labels(
+                cluster.recommended_labels(
+                    &RoleGroupName::from_str("discovery")
+                        .expect("'discovery' is a valid role group name"),
+                ),
+            )
             .build(),
     );
 

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

@@ -91,7 +91,7 @@ mod tests {
         validated
             .role_group_configs
             .get(&HiveRole::MetaStore)
-            .and_then(|groups| groups.get("default"))
+            .and_then(|groups| groups.get(&"default".parse().expect("valid role group name")))
             .expect("metastore default role group should exist")
             .clone()
     }

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

@@ -51,16 +51,22 @@ pub enum Error {
         source: stackable_operator::v2::controller_utils::Error,
     },
 
+    #[snafu(display("invalid role group name {role_group}"))]
+    ParseRoleGroupName {
+        source: stackable_operator::v2::macros::attributed_string_type::Error,
+        role_group: String,
+    },
+
     #[snafu(display("failed to validate the config for role group {role_group}"))]
     ValidateConfig {
         source: fragment::ValidationError,
-        role_group: String,
+        role_group: RoleGroupName,
     },
 
     #[snafu(display("invalid environment variable override name in role group {role_group}"))]
     ParseEnvVarName {
         source: container::Error,
-        role_group: String,
+        role_group: RoleGroupName,
     },
 
     #[snafu(display("invalid metadata database connection"))]
@@ -111,8 +117,12 @@ pub fn validate_cluster(
 
     let mut groups: BTreeMap<RoleGroupName, HiveRoleGroupConfig> = BTreeMap::new();
     for (rg_name, rg) in &role.role_groups {
-        let validated_rg = validate_role_group_config(rg_name, rg, role, &default_config)?;
-        groups.insert(rg_name.clone(), validated_rg);
+        let role_group_name =
+            RoleGroupName::from_str(rg_name).with_context(|_| ParseRoleGroupNameSnafu {
+                role_group: rg_name.clone(),
+            })?;
+        let validated_rg = validate_role_group_config(&role_group_name, rg, role, &default_config)?;
+        groups.insert(role_group_name, validated_rg);
     }
 
     let mut role_group_configs = BTreeMap::new();
@@ -171,7 +181,7 @@ pub fn validate_cluster(
 /// (`HashMap`) are converted into an [`EnvVarSet`] here so invalid names fail validation
 /// early (the opensearch-operator pattern).
 fn validate_role_group_config(
-    role_group_name: &str,
+    role_group_name: &RoleGroupName,
     role_group: &crate::crd::HiveRoleGroupType,
     role: &crate::crd::HiveRoleType,
     default_config: &crate::crd::MetaStoreConfigFragment,
@@ -184,14 +194,14 @@ fn validate_role_group_config(
         v1alpha1::HiveConfigOverrides,
     >(role_group, role, default_config)
     .with_context(|_| ValidateConfigSnafu {
-        role_group: role_group_name.to_owned(),
+        role_group: role_group_name.clone(),
     })?;
 
     let mut env_overrides = EnvVarSet::new();
     for (env_var_name, env_var_value) in merged.config.env_overrides {
         env_overrides = env_overrides.with_value(
             &EnvVarName::from_str(&env_var_name).with_context(|_| ParseEnvVarNameSnafu {
-                role_group: role_group_name.to_owned(),
+                role_group: role_group_name.clone(),
             })?,
             env_var_value,
         );

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

@@ -60,7 +60,7 @@ mod tests {
         let merged_config = validated
             .role_group_configs
             .get(&role)
-            .and_then(|groups| groups.get("default"))
+            .and_then(|groups| groups.get(&"default".parse().expect("valid role group name")))
             .expect("role group should exist")
             .config
             .clone();

rust/operator-binary/src/crd/affinity.rs

@@ -1,9 +1,7 @@
-use std::{collections::BTreeMap, str::FromStr};
-
 use databases::MetadataDatabaseConnection;
 use security::AuthenticationConfig;
 use serde::{Deserialize, Serialize};
-use snafu::{OptionExt, ResultExt, Snafu};
+use snafu::Snafu;
 use stackable_operator::{
     commons::{
         affinity::StackableAffinity,
@@ -22,17 +20,16 @@ use stackable_operator::{
     crd::s3,
     deep_merger::ObjectOverrides,
     k8s_openapi::apimachinery::pkg::api::resource::Quantity,
-    kube::{CustomResource, ResourceExt, runtime::reflector::ObjectRef},
+    kube::{CustomResource, runtime::reflector::ObjectRef},
     product_logging::{self, spec::Logging},
     role_utils::{GenericRoleConfig, Role, RoleGroup, RoleGroupRef},
     schemars::{self, JsonSchema},
     shared::time::Duration,
     status::condition::{ClusterCondition, HasStatusCondition},
-    utils::cluster_info::KubernetesClusterInfo,
     v2::{config_overrides::KeyValueConfigOverrides, role_utils::JavaCommonConfig},
     versioned::versioned,
 };
-use strum::{Display, EnumIter, EnumString, IntoEnumIterator};
+use strum::{Display, EnumIter, EnumString};
 use v1alpha1::HiveMetastoreRoleConfig;
 
 use crate::{crd::affinity::get_affinity, listener::metastore_default_listener_class};
@@ -83,16 +80,6 @@ pub enum Error {
 
     #[snafu(display("the role {role} is not defined"))]
     CannotRetrieveHiveRole { role: String },
-
-    #[snafu(display("the role group {role_group} is not defined"))]
-    CannotRetrieveHiveRoleGroup { role_group: String },
-
-    #[snafu(display("unknown role {role}. Should be one of {roles:?}"))]
-    UnknownHiveRole {
-        source: strum::ParseError,
-        role: String,
-        roles: Vec<String>,
-    },
 }
 
 #[versioned(
@@ -221,65 +208,6 @@ impl v1alpha1::HiveCluster {
         }
     }
 
-    /// List all pods expected to form the cluster
-    ///
-    /// We try to predict the pods here rather than looking at the current cluster state in order to
-    /// avoid instance churn.
-    pub fn pods(&self) -> Result<impl Iterator<Item = PodRef> + '_, NoNamespaceError> {
-        let ns = self.metadata.namespace.clone().context(NoNamespaceSnafu)?;
-        Ok(self
-            .spec
-            .metastore
-            .iter()
-            .flat_map(|role| &role.role_groups)
-            // Order rolegroups consistently, to avoid spurious downstream rewrites
-            .collect::<BTreeMap<_, _>>()
-            .into_iter()
-            .flat_map(move |(rolegroup_name, rolegroup)| {
-                let rolegroup_ref = self.metastore_rolegroup_ref(rolegroup_name);
-                let ns = ns.clone();
-                (0..rolegroup.replicas.unwrap_or(0)).map(move |i| PodRef {
-                    namespace: ns.clone(),
-                    role_group_service_name: rolegroup_ref.object_name(),
-                    pod_name: format!("{}-{}", rolegroup_ref.object_name(), i),
-                })
-            }))
-    }
-
-    pub fn role(&self, role_variant: &HiveRole) -> Result<&HiveRoleType, Error> {
-        match role_variant {
-            HiveRole::MetaStore => self.spec.metastore.as_ref(),
-        }
-        .with_context(|| CannotRetrieveHiveRoleSnafu {
-            role: role_variant.to_string(),
-        })
-    }
-
-    /// The name of the role-listener provided for a specific role.
-    /// returns a name `<cluster>-<role>`
-    pub fn role_listener_name(&self, hive_role: &HiveRole) -> String {
-        format!("{name}-{role}", name = self.name_any(), role = hive_role)
-    }
-
-    pub fn rolegroup(
-        &self,
-        rolegroup_ref: &RoleGroupRef<Self>,
-    ) -> Result<HiveRoleGroupType, Error> {
-        let role_variant =
-            HiveRole::from_str(&rolegroup_ref.role).with_context(|_| UnknownHiveRoleSnafu {
-                role: rolegroup_ref.role.to_owned(),
-                roles: HiveRole::roles(),
-            })?;
-
-        let role = self.role(&role_variant)?;
-        role.role_groups
-            .get(&rolegroup_ref.role_group)
-            .with_context(|| CannotRetrieveHiveRoleGroupSnafu {
-                role_group: rolegroup_ref.role_group.to_owned(),
-            })
-            .cloned()
-    }
-
     pub fn role_config(&self, role: &HiveRole) -> Option<&HiveMetastoreRoleConfig> {
         match role {
             HiveRole::MetaStore => self.spec.metastore.as_ref().map(|m| &m.role_config),
@@ -326,27 +254,6 @@ pub enum HiveRole {
 }
 
 impl HiveRole {
-    /// Metadata about a rolegroup
-    pub fn rolegroup_ref(
-        &self,
-        hive: &v1alpha1::HiveCluster,
-        group_name: impl Into<String>,
-    ) -> RoleGroupRef<v1alpha1::HiveCluster> {
-        RoleGroupRef {
-            cluster: ObjectRef::from_obj(hive),
-            role: self.to_string(),
-            role_group: group_name.into(),
-        }
-    }
-
-    pub fn roles() -> Vec<String> {
-        let mut roles = vec![];
-        for role in Self::iter() {
-            roles.push(role.to_string())
-        }
-        roles
-    }
-
     /// A Kerberos principal has three parts, with the form username/fully.qualified.domain.name@YOUR-REALM.COM.
     /// We only have one role and will use "hive" everywhere (which e.g. differs from the current hdfs implementation).
     pub fn kerberos_service_name(&self) -> &'static str {
@@ -466,30 +373,6 @@ pub struct HiveClusterStatus {
     pub conditions: Vec<ClusterCondition>,
 }
 
-#[derive(Debug, Snafu)]
-#[snafu(display("object has no namespace associated"))]
-pub struct NoNamespaceError;
-
-/// Reference to a single `Pod` that is a component of a [`HiveCluster`]
-/// Used for service discovery.
-pub struct PodRef {
-    pub namespace: String,
-    pub role_group_service_name: String,
-    pub pod_name: String,
-}
-
-impl PodRef {
-    pub fn fqdn(&self, cluster_info: &KubernetesClusterInfo) -> String {
-        format!(
-            "{pod_name}.{service_name}.{namespace}.svc.{cluster_domain}",
-            pod_name = self.pod_name,
-            service_name = self.role_group_service_name,
-            namespace = self.namespace,
-            cluster_domain = cluster_info.cluster_domain
-        )
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use stackable_operator::versioned::test_utils::RoundtripTestData;