fix: cleanup, move constants, fix comments

Author: maltesander

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

@@ -4,12 +4,10 @@
 
 use std::collections::BTreeMap;
 
-use stackable_operator::v2::{
-    config_file_writer::to_hadoop_xml, config_overrides::KeyValueConfigOverrides,
-};
+use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
 
 use crate::{
-    controller::build::properties::resolved_overrides,
+    controller::build::properties::build_xml_config,
     crd::{
         AnyServiceConfig, HBASE_CLUSTER_DISTRIBUTED, HBASE_MASTER_PORT, HBASE_MASTER_UI_PORT,
         HBASE_REGIONSERVER_PORT, HBASE_REGIONSERVER_UI_PORT, HBASE_ROOTDIR, HbaseRole,
@@ -110,9 +108,7 @@ pub fn build(
     };
 
     // configOverride come last
-    config.extend(resolved_overrides(overrides));
-
-    to_hadoop_xml(config.iter())
+    build_xml_config(config, overrides)
 }
 
 #[cfg(test)]

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

@@ -5,7 +5,9 @@
 
 use std::collections::BTreeMap;
 
-use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
+use stackable_operator::v2::{
+    config_file_writer::to_hadoop_xml, config_overrides::KeyValueConfigOverrides,
+};
 
 pub mod hbase_env;
 pub mod hbase_site;
@@ -31,6 +33,16 @@ fn resolved_overrides(
     defined_entries(overrides.overrides)
 }
 
+/// Render an XML config file from base `settings` merged with user `overrides`
+/// (overrides applied last, so users win), serialized to the Hadoop-XML on-wire format.
+fn build_xml_config(
+    mut config: BTreeMap<String, String>,
+    overrides: KeyValueConfigOverrides,
+) -> String {
+    config.extend(resolved_overrides(overrides));
+    to_hadoop_xml(config.iter())
+}
+
 /// The names of the HBase config files assembled into the rolegroup `ConfigMap`.
 #[derive(Clone, Copy, Debug, strum::Display)]
 pub enum ConfigFileName {

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

@@ -1,18 +1,13 @@
 //! Builds the `ssl-client.xml` config file: kerberos/TLS client settings + overrides.
 use std::collections::BTreeMap;
 
-use stackable_operator::v2::{
-    config_file_writer::to_hadoop_xml, config_overrides::KeyValueConfigOverrides,
-};
+use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
 
-use crate::controller::build::properties::resolved_overrides;
+use crate::controller::build::properties::build_xml_config;
 
 /// Renders `ssl-client.xml`.
 pub fn build(settings: BTreeMap<String, String>, overrides: KeyValueConfigOverrides) -> String {
-    let mut config: BTreeMap<String, String> = BTreeMap::new();
-    config.extend(settings);
-    config.extend(resolved_overrides(overrides));
-    to_hadoop_xml(config.iter())
+    build_xml_config(settings, overrides)
 }
 
 #[cfg(test)]

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

@@ -1,18 +1,13 @@
 //! Builds the `ssl-server.xml` config file: kerberos/TLS server settings + overrides.
 use std::collections::BTreeMap;
 
-use stackable_operator::v2::{
-    config_file_writer::to_hadoop_xml, config_overrides::KeyValueConfigOverrides,
-};
+use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
 
-use crate::controller::build::properties::resolved_overrides;
+use crate::controller::build::properties::build_xml_config;
 
 /// Renders `ssl-server.xml`.
 pub fn build(settings: BTreeMap<String, String>, overrides: KeyValueConfigOverrides) -> String {
-    let mut config: BTreeMap<String, String> = BTreeMap::new();
-    config.extend(settings);
-    config.extend(resolved_overrides(overrides));
-    to_hadoop_xml(config.iter())
+    build_xml_config(settings, overrides)
 }
 
 #[cfg(test)]

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

@@ -72,8 +72,7 @@ pub fn validate_cluster(
     for hbase_role in HbaseRole::iter() {
         let role_group_names = role_group_names(hbase, &hbase_role);
 
-        // masters and region servers are required (preserves the old build_role_properties check);
-        // rest servers are optional.
+        // masters and region servers are required; rest servers are optional.
         if role_group_names.is_empty() {
             match hbase_role {
                 HbaseRole::Master | HbaseRole::RegionServer => {

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

@@ -47,6 +47,7 @@ pub mod security;
 
 pub const APP_NAME: &str = "hbase";
 pub const FIELD_MANAGER: &str = "hbase-operator";
+pub const OPERATOR_NAME: &str = "hbase.stackable.com";
 
 // This constant is hard coded in hbase-entrypoint.sh
 // You need to change it there too.
@@ -107,12 +108,6 @@ pub type RestServerRoleType =
 
 #[derive(Snafu, Debug)]
 pub enum Error {
-    #[snafu(display("the role [{role}] is invalid and does not exist in HBase"))]
-    InvalidRole {
-        source: strum::ParseError,
-        role: String,
-    },
-
     #[snafu(display("the HBase role [{role}] is missing from spec"))]
     MissingHbaseRole { role: String },
 
@@ -128,11 +123,6 @@ pub enum Error {
     #[snafu(display("role-group not found by name"))]
     RoleGroupNotFound,
 
-    #[snafu(display("failed to build listener volume source"))]
-    ListenerVolumeSource {
-        source: ListenerOperatorVolumeSourceBuilderError,
-    },
-
     #[snafu(display("failed to build listener volume"))]
     BuildListenerVolume {
         source: ListenerOperatorVolumeSourceBuilderError,
@@ -441,11 +431,6 @@ impl v1alpha1::HbaseCluster {
         }
     }
 
-    /// The name of the role-level load-balanced Kubernetes `Service`
-    pub fn server_role_service_name(&self) -> Option<String> {
-        self.metadata.name.clone()
-    }
-
     /// Metadata about a server rolegroup
     pub fn server_rolegroup_ref(
         &self,
@@ -544,70 +529,6 @@ impl HbaseRole {
     const DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration =
         Duration::from_minutes_unchecked(5);
 
-    pub fn default_config(
-        &self,
-        cluster_name: &str,
-        hdfs_discovery_cm_name: &str,
-    ) -> HbaseConfigFragment {
-        let resources = match &self {
-            HbaseRole::Master => ResourcesFragment {
-                cpu: CpuLimitsFragment {
-                    min: Some(Quantity("250m".to_owned())),
-                    max: Some(Quantity("1".to_owned())),
-                },
-                memory: MemoryLimitsFragment {
-                    limit: Some(Quantity("1Gi".to_owned())),
-                    runtime_limits: NoRuntimeLimitsFragment {},
-                },
-                storage: HbaseStorageConfigFragment {},
-            },
-            HbaseRole::RegionServer => ResourcesFragment {
-                cpu: CpuLimitsFragment {
-                    min: Some(Quantity("250m".to_owned())),
-                    max: Some(Quantity("1".to_owned())),
-                },
-                memory: MemoryLimitsFragment {
-                    limit: Some(Quantity("1Gi".to_owned())),
-                    runtime_limits: NoRuntimeLimitsFragment {},
-                },
-                storage: HbaseStorageConfigFragment {},
-            },
-            HbaseRole::RestServer => ResourcesFragment {
-                cpu: CpuLimitsFragment {
-                    min: Some(Quantity("100m".to_owned())),
-                    max: Some(Quantity("400m".to_owned())),
-                },
-                memory: MemoryLimitsFragment {
-                    limit: Some(Quantity("512Mi".to_owned())),
-                    runtime_limits: NoRuntimeLimitsFragment {},
-                },
-                storage: HbaseStorageConfigFragment {},
-            },
-        };
-
-        let graceful_shutdown_timeout = match &self {
-            HbaseRole::Master => Self::DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT,
-            HbaseRole::RegionServer => Self::DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT,
-            HbaseRole::RestServer => Self::DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT,
-        };
-
-        let requested_secret_lifetime = match &self {
-            HbaseRole::Master => Self::DEFAULT_MASTER_SECRET_LIFETIME,
-            HbaseRole::RegionServer => Self::DEFAULT_REGION_SECRET_LIFETIME,
-            HbaseRole::RestServer => Self::DEFAULT_REST_SECRET_LIFETIME,
-        };
-
-        HbaseConfigFragment {
-            hbase_rootdir: Some(default_hbase_rootdir()),
-            resources,
-            logging: product_logging::spec::default_logging(),
-            affinity: get_affinity(cluster_name, self, hdfs_discovery_cm_name),
-            graceful_shutdown_timeout: Some(graceful_shutdown_timeout),
-            requested_secret_lifetime: Some(requested_secret_lifetime),
-            listener_class: Some(DEFAULT_LISTENER_CLASS.to_string()),
-        }
-    }
-
     /// Returns the name of the role as it is needed by the `bin/hbase {cli_role_name} start` command.
     pub fn cli_role_name(&self) -> String {
         match self {

rust/operator-binary/src/hbase_controller.rs

@@ -63,14 +63,13 @@ use stackable_operator::{
 use strum::{EnumDiscriminants, IntoStaticStr};
 
 use crate::{
-    OPERATOR_NAME,
     controller::build::{
         discovery::build_discovery_config_map,
         properties::logging::{MAX_HBASE_LOG_FILES_SIZE, STACKABLE_LOG_DIR},
     },
     crd::{
         APP_NAME, AnyServiceConfig, CONFIG_DIR_NAME, Container, HbaseClusterStatus, HbaseRole,
-        LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, merged_env, v1alpha1,
+        LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, OPERATOR_NAME, merged_env, v1alpha1,
     },
     kerberos::{self, add_kerberos_pod_config},
     operations::{graceful_shutdown::add_graceful_shutdown_config, pdb::add_pdbs},
@@ -585,7 +584,7 @@ fn build_rolegroup_service(
 }
 
 /// The rolegroup metrics [`Service`] is a service that exposes metrics and a prometheus scraping label.
-pub fn build_rolegroup_metrics_service(
+fn build_rolegroup_metrics_service(
     hbase: &v1alpha1::HbaseCluster,
     hbase_role: &HbaseRole,
     rolegroup: &RoleGroupRef<v1alpha1::HbaseCluster>,