implement review feedback

Author: razvan

rust/crd/src/lib.rs

@@ -87,7 +87,9 @@ const DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration =
 const DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(5);
 
 // Auto TLS certificate lifetime
-pub const DEFAULT_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
+const DEFAULT_MASTER_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
+const DEFAULT_REGION_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
+const DEFAULT_REST_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
 
 #[derive(Snafu, Debug)]
 pub enum Error {
@@ -307,9 +309,15 @@ impl HbaseRole {
         };
 
         let graceful_shutdown_timeout = match &self {
-            HbaseRole::Master => DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT,
-            HbaseRole::RegionServer => DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT,
-            HbaseRole::RestServer => DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT,
+            HbaseRole::Master => Some(DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT),
+            HbaseRole::RegionServer => Some(DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT),
+            HbaseRole::RestServer => Some(DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT),
+        };
+
+        let requested_secret_lifetime = match &self {
+            HbaseRole::Master => Some(DEFAULT_MASTER_SECRET_LIFETIME),
+            HbaseRole::RegionServer => Some(DEFAULT_REGION_SECRET_LIFETIME),
+            HbaseRole::RestServer => Some(DEFAULT_REST_SECRET_LIFETIME),
         };
 
         HbaseConfigFragment {
@@ -318,8 +326,8 @@ impl HbaseRole {
             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(DEFAULT_SECRET_LIFETIME),
+            graceful_shutdown_timeout,
+            requested_secret_lifetime,
         }
     }
 

rust/operator-binary/src/hbase_controller.rs

@@ -67,10 +67,10 @@ use strum::{EnumDiscriminants, IntoStaticStr, ParseError};
 
 use stackable_hbase_crd::{
     merged_env, Container, HbaseCluster, HbaseClusterStatus, HbaseConfig, HbaseConfigFragment,
-    HbaseRole, APP_NAME, CONFIG_DIR_NAME, DEFAULT_SECRET_LIFETIME, HBASE_ENV_SH, HBASE_HEAPSIZE,
-    HBASE_MANAGES_ZK, HBASE_MASTER_OPTS, HBASE_REGIONSERVER_OPTS, HBASE_REST_OPTS,
-    HBASE_REST_PORT_NAME_HTTP, HBASE_REST_PORT_NAME_HTTPS, HBASE_SITE_XML, JVM_HEAP_FACTOR,
-    JVM_SECURITY_PROPERTIES_FILE, METRICS_PORT, SSL_CLIENT_XML, SSL_SERVER_XML,
+    HbaseRole, APP_NAME, CONFIG_DIR_NAME, HBASE_ENV_SH, HBASE_HEAPSIZE, HBASE_MANAGES_ZK,
+    HBASE_MASTER_OPTS, HBASE_REGIONSERVER_OPTS, HBASE_REST_OPTS, HBASE_REST_PORT_NAME_HTTP,
+    HBASE_REST_PORT_NAME_HTTPS, HBASE_SITE_XML, JVM_HEAP_FACTOR, JVM_SECURITY_PROPERTIES_FILE,
+    METRICS_PORT, SSL_CLIENT_XML, SSL_SERVER_XML,
 };
 
 use crate::product_logging::STACKABLE_LOG_DIR;
@@ -113,6 +113,9 @@ pub struct Ctx {
 #[strum_discriminants(derive(IntoStaticStr))]
 #[allow(clippy::enum_variant_names)]
 pub enum Error {
+    #[snafu(display("missing secret lifetime"))]
+    MissingSecretLifetime,
+
     #[snafu(display("object defines no version"))]
     ObjectHasNoVersion,
 
@@ -777,7 +780,7 @@ fn build_rolegroup_statefulset(
     hbase_role: &HbaseRole,
     rolegroup_ref: &RoleGroupRef<HbaseCluster>,
     rolegroup_config: &HashMap<PropertyNameKind, BTreeMap<String, String>>,
-    config: &HbaseConfig,
+    merged_config: &HbaseConfig,
     resolved_product_image: &ResolvedProductImage,
     service_account: &ServiceAccount,
 ) -> Result<StatefulSet> {
@@ -899,7 +902,7 @@ fn build_rolegroup_statefulset(
         .add_volume_mount("log", STACKABLE_LOG_DIR)
         .context(AddVolumeMountSnafu)?
         .add_container_ports(ports)
-        .resources(config.resources.clone().into())
+        .resources(merged_config.resources.clone().into())
         .startup_probe(startup_probe)
         .liveness_probe(liveness_probe)
         .readiness_probe(readiness_probe);
@@ -919,7 +922,7 @@ fn build_rolegroup_statefulset(
     pod_builder
         .metadata(pb_metadata)
         .image_pull_secrets_from_product_image(resolved_product_image)
-        .affinity(&config.affinity)
+        .affinity(&merged_config.affinity)
         .add_volume(stackable_operator::k8s_openapi::api::core::v1::Volume {
             name: "hbase-config".to_string(),
             config_map: Some(ConfigMapVolumeSource {
@@ -959,7 +962,7 @@ fn build_rolegroup_statefulset(
             Some(ContainerLogConfigChoice::Custom(CustomContainerLogConfig {
                 custom: ConfigMapLogConfig { config_map },
             })),
-    }) = config.logging.containers.get(&Container::Hbase)
+    }) = merged_config.logging.containers.get(&Container::Hbase)
     {
         pod_builder
             .add_volume(Volume {
@@ -984,29 +987,29 @@ fn build_rolegroup_statefulset(
             .context(AddVolumeSnafu)?;
     }
 
-    add_graceful_shutdown_config(config, &mut pod_builder).context(GracefulShutdownSnafu)?;
+    add_graceful_shutdown_config(merged_config, &mut pod_builder).context(GracefulShutdownSnafu)?;
     if hbase.has_kerberos_enabled() {
         add_kerberos_pod_config(
             hbase,
             hbase_role,
             &mut hbase_container,
             &mut pod_builder,
-            config
+            merged_config
                 .requested_secret_lifetime
-                .unwrap_or(DEFAULT_SECRET_LIFETIME),
+                .context(MissingSecretLifetimeSnafu)?,
         )
         .context(AddKerberosConfigSnafu)?;
     }
     pod_builder.add_container(hbase_container.build());
 
     // Vector sidecar shall be the last container in the list
-    if config.logging.enable_vector_agent {
+    if merged_config.logging.enable_vector_agent {
         pod_builder.add_container(
             product_logging::framework::vector_container(
                 resolved_product_image,
                 "hbase-config",
                 "log",
-                config.logging.containers.get(&Container::Vector),
+                merged_config.logging.containers.get(&Container::Vector),
                 ResourceRequirementsBuilder::new()
                     .with_cpu_request("250m")
                     .with_cpu_limit("500m")