refactor: build_pdb returns PDB via infallible v2 builder; add typed name helpers

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -45,7 +45,7 @@ use stackable_operator::{
         role_group_utils::ResourceNames,
         types::{
             kubernetes::{ContainerName, VolumeName},
-            operator::RoleGroupName,
+            operator::{ControllerName, OperatorName, ProductName, RoleGroupName},
         },
     },
 };
@@ -57,7 +57,7 @@ use crate::{
             LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener,
             build_group_listener_pvc, group_listener_name, secret_volume_listener_scope,
         },
-        pdb::add_pdbs,
+        pdb::build_pdb,
         service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
     },
     crd::{
@@ -85,6 +85,22 @@ pub const FULL_CONTROLLER_NAME: &str = concatcp!(DRUID_CONTROLLER_NAME, '.', OPE
 
 pub(super) const CONTAINER_IMAGE_BASE_NAME: &str = "druid";
 
+/// The product name (`druid`) as a type-safe label value.
+pub(crate) fn product_name() -> ProductName {
+    ProductName::from_str(APP_NAME).expect("'druid' is a valid product name")
+}
+
+/// The operator name as a type-safe label value.
+pub(crate) fn operator_name() -> OperatorName {
+    OperatorName::from_str(OPERATOR_NAME).expect("the operator name is a valid label value")
+}
+
+/// The controller name as a type-safe label value.
+pub(crate) fn controller_name() -> ControllerName {
+    ControllerName::from_str(DRUID_CONTROLLER_NAME)
+        .expect("the controller name is a valid label value")
+}
+
 // volume names
 const DRUID_CONFIG_VOLUME_NAME: &str = "config";
 const HDFS_CONFIG_VOLUME_NAME: &str = "hdfs";
@@ -192,9 +208,9 @@ pub enum Error {
         source: stackable_operator::commons::rbac::Error,
     },
 
-    #[snafu(display("failed to create PodDisruptionBudget"))]
-    FailedToCreatePdb {
-        source: crate::controller::build::resource::pdb::Error,
+    #[snafu(display("failed to apply PodDisruptionBudget"))]
+    ApplyPdb {
+        source: stackable_operator::cluster_resources::Error,
     },
 
     #[snafu(display("failed to configure graceful shutdown"))]
@@ -417,15 +433,16 @@ pub async fn reconcile_druid(
 
         let role_config = druid.generic_role_config(druid_role);
 
-        add_pdbs(
+        if let Some(pdb) = build_pdb(
             &role_config.pod_disruption_budget,
-            druid,
+            &validated_cluster,
             druid_role,
-            client,
-            &mut cluster_resources,
-        )
-        .await
-        .context(FailedToCreatePdbSnafu)?;
+        ) {
+            cluster_resources
+                .add(client, pdb)
+                .await
+                .context(ApplyPdbSnafu)?;
+        }
     }
 
     let cluster_operation_cond_builder =

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

@@ -1,37 +1,24 @@
-use snafu::{ResultExt, Snafu};
+use std::str::FromStr;
+
 use stackable_operator::{
-    builder::pdb::PodDisruptionBudgetBuilder, client::Client, cluster_resources::ClusterResources,
-    commons::pdb::PdbConfig, kube::ResourceExt,
+    commons::pdb::PdbConfig,
+    k8s_openapi::api::policy::v1::PodDisruptionBudget,
+    v2::{builder::pdb::pod_disruption_budget_builder_with_role, types::operator::RoleName},
 };
 
 use crate::{
-    controller::DRUID_CONTROLLER_NAME,
-    crd::{APP_NAME, DruidRole, OPERATOR_NAME, v1alpha1},
+    controller::{controller_name, operator_name, product_name, validate::ValidatedCluster},
+    crd::DruidRole,
 };
 
-#[derive(Snafu, Debug)]
-pub enum Error {
-    #[snafu(display("Cannot create PodDisruptionBudget for role [{role}]"))]
-    CreatePdb {
-        source: stackable_operator::builder::pdb::Error,
-        role: String,
-    },
-    #[snafu(display("Cannot apply PodDisruptionBudget [{name}]"))]
-    ApplyPdb {
-        source: stackable_operator::cluster_resources::Error,
-        name: String,
-    },
-}
-
-pub async fn add_pdbs(
+/// Builds the [`PodDisruptionBudget`] for the given `role`, or `None` if PDBs are disabled.
+pub fn build_pdb(
     pdb: &PdbConfig,
-    druid: &v1alpha1::DruidCluster,
+    cluster: &ValidatedCluster,
     role: &DruidRole,
-    client: &Client,
-    cluster_resources: &mut ClusterResources<'_>,
-) -> Result<(), Error> {
+) -> Option<PodDisruptionBudget> {
     if !pdb.enabled {
-        return Ok(());
+        return None;
     }
     let max_unavailable = pdb.max_unavailable.unwrap_or(match role {
         DruidRole::Broker => max_unavailable_brokers(),
@@ -40,25 +27,17 @@ pub async fn add_pdbs(
         DruidRole::MiddleManager => max_unavailable_middle_managers(),
         DruidRole::Router => max_unavailable_routers(),
     });
-    let pdb = PodDisruptionBudgetBuilder::new_with_role(
-        druid,
-        APP_NAME,
-        &role.to_string(),
-        OPERATOR_NAME,
-        DRUID_CONTROLLER_NAME,
+    let pdb = pod_disruption_budget_builder_with_role(
+        cluster,
+        &product_name(),
+        &RoleName::from_str(&role.to_string()).expect("a DruidRole is a valid role name"),
+        &operator_name(),
+        &controller_name(),
     )
-    .with_context(|_| CreatePdbSnafu {
-        role: role.to_string(),
-    })?
     .with_max_unavailable(max_unavailable)
     .build();
-    let pdb_name = pdb.name_any();
-    cluster_resources
-        .add(client, pdb)
-        .await
-        .with_context(|_| ApplyPdbSnafu { name: pdb_name })?;
 
-    Ok(())
+    Some(pdb)
 }
 
 fn max_unavailable_brokers() -> u16 {

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

@@ -14,7 +14,7 @@ use stackable_operator::{
     k8s_openapi::api::core::v1::Volume,
     kube::{Resource, api::ObjectMeta},
     v2::{
-        HasName, HasUid,
+        HasName, HasUid, NameIsValidLabelValue,
         controller_utils::{get_cluster_name, get_namespace, get_uid},
         role_group_utils::ResourceNames,
         types::{
@@ -195,6 +195,12 @@ impl HasUid for ValidatedCluster {
     }
 }
 
+impl NameIsValidLabelValue for ValidatedCluster {
+    fn to_label_value(&self) -> String {
+        self.name.to_label_value()
+    }
+}
+
 /// Validates the cluster spec and the dereferenced inputs.
 pub fn validate(
     druid: &v1alpha1::DruidCluster,

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

@@ -447,11 +447,17 @@ impl v1alpha1::DruidCluster {
         s3_ingestion || s3_storage
     }
 
-    /// Merges and validates all role groups of the given role.
+    /// Merges and validates all role groups of the given role. Invoked from the validate step
+    /// (`controller::validate`).
     ///
     /// All four override categories (config / env / cli / pod) are merged by
     /// [`with_validated_config`] (role group wins over role); the typed per-role config is then
     /// erased to the shared [`ValidatedDruidConfig`] view consumed by the build step.
+    ///
+    /// This lives in `crate::crd` and is a `macro_rules!` rather than a generic function on purpose: the erasure
+    /// reads the per-role configs' private `resources` field and calls their private
+    /// `default_config`, and the five roles wrap *different* `resources` types into different
+    /// [`RoleResource`] variants.
     pub fn merged_role(
         &self,
         role: &DruidRole,