refactor: consolidate error handling

Author: maltesander

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

@@ -2,7 +2,6 @@ use std::sync::Arc;
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
-    builder::{self},
     cluster_resources::ClusterResourceApplyStrategy,
     commons::rbac::build_rbac_resources,
     crd::listener,
@@ -18,16 +17,13 @@ use stackable_operator::{
     },
     logging::controller::ReconcilerError,
     shared::time::Duration,
-    v2::{cluster_resources::cluster_resources_new, config_file_writer::PropertiesWriterError},
+    v2::cluster_resources::cluster_resources_new,
 };
 use strum::{EnumDiscriminants, IntoStaticStr};
 
 use crate::{
     Ctx,
-    crd::{
-        constants::{HISTORY_APP_NAME, JVM_SECURITY_PROPERTIES_FILE},
-        history::v1alpha1,
-    },
+    crd::{constants::HISTORY_APP_NAME, history::v1alpha1},
 };
 
 pub mod build;
@@ -43,14 +39,8 @@ pub enum Error {
         source: stackable_operator::commons::rbac::Error,
     },
 
-    #[snafu(display("missing secret lifetime"))]
-    MissingSecretLifetime,
-
-    #[snafu(display("invalid config map {name}"))]
-    InvalidConfigMap {
-        source: stackable_operator::builder::configmap::Error,
-        name: String,
-    },
+    #[snafu(display("failed to build SparkHistoryServer resources"))]
+    BuildSparkHistoryServer { source: build::Error },
 
     #[snafu(display("failed to apply Kubernetes resource"))]
     ApplyResource {
@@ -78,41 +68,18 @@ pub enum Error {
         source: stackable_operator::cluster_resources::Error,
     },
 
-    #[snafu(display(
-        "History server : failed to serialize [{JVM_SECURITY_PROPERTIES_FILE}] for group {}",
-        rolegroup
-    ))]
-    JvmSecurityProperties {
-        source: PropertiesWriterError,
-        rolegroup: String,
-    },
-
     #[snafu(display("failed to get required Labels"))]
     GetRequiredLabels {
         source:
             stackable_operator::kvp::KeyValuePairError<stackable_operator::kvp::LabelValueError>,
     },
 
-    #[snafu(display("failed to create the log dir volumes specification"))]
-    CreateLogDirVolumesSpec { source: crate::crd::logdir::Error },
-
-    #[snafu(display("failed to add needed volume"))]
-    AddVolume { source: builder::pod::Error },
-
-    #[snafu(display("failed to add needed volumeMount"))]
-    AddVolumeMount {
-        source: builder::pod::container::Error,
-    },
-
     #[snafu(display("SparkHistoryServer object is invalid"))]
     InvalidSparkHistoryServer {
         // boxed because otherwise Clippy warns about a large enum variant
         #[snafu(source(from(error_boundary::InvalidObject, Box::new)))]
         source: Box<error_boundary::InvalidObject>,
     },
-
-    #[snafu(display("failed to serialize Spark default properties"))]
-    InvalidSparkDefaults { source: PropertiesWriterError },
 }
 
 impl ReconcilerError for Error {
@@ -180,7 +147,8 @@ pub async fn reconcile(
     )
     .context(BuildRbacResourcesSnafu)?;
 
-    let resources = build::build(&validated, service_account, role_binding)?;
+    let resources = build::build(&validated, service_account, role_binding)
+        .context(BuildSparkHistoryServerSnafu)?;
 
     // Apply order: ServiceAccount and RoleBinding first, then the ConfigMaps, metrics Services,
     // Listener and PodDisruptionBudget, and finally the StatefulSets (they mount the ConfigMaps

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

@@ -1,41 +1,54 @@
 pub mod resource;
 
+use snafu::{ResultExt, Snafu};
 use stackable_operator::k8s_openapi::api::{core::v1::ServiceAccount, rbac::v1::RoleBinding};
 
 use crate::{
     crd::constants::HISTORY_ROLE_NAME,
     history::controller::{
-        Error, SparkHistoryResources,
+        SparkHistoryResources,
         build::resource::{
-            config_map::build_config_map, listener::build_group_listener, pdb::build_pdb,
-            service::build_rolegroup_metrics_service, statefulset::build_stateful_set,
+            config_map::{self, build_config_map},
+            listener::build_group_listener,
+            pdb::build_pdb,
+            service::build_rolegroup_metrics_service,
+            statefulset::{self, build_stateful_set},
         },
         validate::ValidatedSparkHistoryServer,
     },
 };
 
+#[derive(Snafu, Debug)]
+pub enum Error {
+    #[snafu(display("failed to build ConfigMap"))]
+    BuildConfigMap { source: config_map::Error },
+
+    #[snafu(display("failed to build StatefulSet"))]
+    BuildStatefulSet { source: statefulset::Error },
+}
+
+type Result<T, E = Error> = std::result::Result<T, E>;
+
 /// Builds every Kubernetes resource for the given validated SparkHistoryServer.
 pub fn build(
     validated: &ValidatedSparkHistoryServer,
     service_account: ServiceAccount,
     role_binding: RoleBinding,
-) -> Result<SparkHistoryResources, Error> {
+) -> Result<SparkHistoryResources> {
     let log_dir = &validated.cluster_config.log_dir;
 
     let mut config_maps = vec![];
     let mut metrics_services = vec![];
     let mut stateful_sets = vec![];
 
     for (role_group_name, rg) in &validated.role_groups {
-        config_maps.push(build_config_map(validated, role_group_name, rg)?);
+        config_maps
+            .push(build_config_map(validated, role_group_name, rg).context(BuildConfigMapSnafu)?);
         metrics_services.push(build_rolegroup_metrics_service(validated, role_group_name));
-        stateful_sets.push(build_stateful_set(
-            validated,
-            role_group_name,
-            rg,
-            log_dir,
-            &service_account,
-        )?);
+        stateful_sets.push(
+            build_stateful_set(validated, role_group_name, rg, log_dir, &service_account)
+                .context(BuildStatefulSetSnafu)?,
+        );
     }
 
     let listener = build_group_listener(

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

@@ -1,6 +1,6 @@
 use std::collections::BTreeMap;
 
-use snafu::ResultExt;
+use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
     k8s_openapi::api::core::v1::ConfigMap,
@@ -20,19 +20,40 @@ use crate::{
         history::SparkHistoryServerContainer,
         to_spark_env_sh_string,
     },
-    history::controller::{
-        Error, InvalidConfigMapSnafu, InvalidSparkDefaultsSnafu, JvmSecurityPropertiesSnafu,
-        validate::{self, ValidatedHistoryRoleGroup},
-    },
+    history::controller::validate::{self, ValidatedHistoryRoleGroup},
     product_logging::{self},
 };
 
-#[allow(clippy::result_large_err)]
+#[derive(Snafu, Debug)]
+pub enum Error {
+    #[snafu(display("invalid config map {name}"))]
+    InvalidConfigMap {
+        source: stackable_operator::builder::configmap::Error,
+        name: String,
+    },
+
+    #[snafu(display(
+        "History server : failed to serialize [{JVM_SECURITY_PROPERTIES_FILE}] for group {}",
+        rolegroup
+    ))]
+    JvmSecurityProperties {
+        source: stackable_operator::v2::config_file_writer::PropertiesWriterError,
+        rolegroup: String,
+    },
+
+    #[snafu(display("failed to serialize Spark default properties"))]
+    InvalidSparkDefaults {
+        source: stackable_operator::v2::config_file_writer::PropertiesWriterError,
+    },
+}
+
+type Result<T, E = Error> = std::result::Result<T, E>;
+
 pub(crate) fn build_config_map(
     validated: &validate::ValidatedSparkHistoryServer,
     role_group_name: &RoleGroupName,
     rg: &ValidatedHistoryRoleGroup,
-) -> Result<ConfigMap, Error> {
+) -> Result<ConfigMap> {
     let cm_name = validated
         .resource_names(role_group_name)
         .role_group_config_map()

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

@@ -1,6 +1,6 @@
 use std::str::FromStr;
 
-use snafu::{OptionExt, ResultExt};
+use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
     builder::{
         meta::ObjectMetaBuilder,
@@ -52,22 +52,40 @@ use crate::{
     history::{
         config::jvm::construct_history_jvm_args,
         controller::{
-            AddVolumeMountSnafu, AddVolumeSnafu, CreateLogDirVolumesSpecSnafu, Error,
-            MissingSecretLifetimeSnafu,
             build::resource::listener::group_listener_name,
             validate::{self, ValidatedHistoryRoleGroup},
         },
     },
 };
 
-#[allow(clippy::result_large_err)]
+#[derive(Snafu, Debug)]
+pub enum Error {
+    #[snafu(display("missing secret lifetime"))]
+    MissingSecretLifetime,
+
+    #[snafu(display("failed to create the log dir volumes specification"))]
+    CreateLogDirVolumesSpec { source: crate::crd::logdir::Error },
+
+    #[snafu(display("failed to add needed volume"))]
+    AddVolume {
+        source: stackable_operator::builder::pod::Error,
+    },
+
+    #[snafu(display("failed to add needed volumeMount"))]
+    AddVolumeMount {
+        source: stackable_operator::builder::pod::container::Error,
+    },
+}
+
+type Result<T, E = Error> = std::result::Result<T, E>;
+
 pub(crate) fn build_stateful_set(
     validated: &validate::ValidatedSparkHistoryServer,
     role_group_name: &RoleGroupName,
     rg: &ValidatedHistoryRoleGroup,
     log_dir: &ResolvedLogDir,
     serviceaccount: &ServiceAccount,
-) -> Result<StatefulSet, Error> {
+) -> Result<StatefulSet> {
     let resolved_product_image = &validated.resolved_product_image;
     let resource_names = validated.resource_names(role_group_name);