diff --git a/CHANGELOG.md b/CHANGELOG.md index ceae4a987..5924e2f10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- Add `Pdb` struct and `PdbBuilder` ([#653]). + +[#653]: https://github.com/stackabletech/operator-rs/pull/653 + ## [0.50.0] - 2023-09-18 - Add `Duration` capable of parsing human-readable duration formats ([#647]). diff --git a/src/builder/mod.rs b/src/builder/mod.rs index 40d72fe7e..1602fbdf7 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -6,6 +6,7 @@ pub mod configmap; pub mod event; pub mod meta; +pub mod pdb; pub mod pod; #[deprecated(since = "0.15.0", note = "Please use `builder::configmap::*` instead.")] diff --git a/src/builder/pdb.rs b/src/builder/pdb.rs new file mode 100644 index 000000000..e567a96db --- /dev/null +++ b/src/builder/pdb.rs @@ -0,0 +1,320 @@ +use crate::{ + builder::ObjectMetaBuilder, + error::OperatorResult, + labels::{role_selector_labels, APP_MANAGED_BY_LABEL}, + utils::format_full_controller_name, +}; +use k8s_openapi::{ + api::policy::v1::{PodDisruptionBudget, PodDisruptionBudgetSpec}, + apimachinery::pkg::{ + apis::meta::v1::{LabelSelector, ObjectMeta}, + util::intstr::IntOrString, + }, +}; +use kube::{Resource, ResourceExt}; + +/// This builder is used to construct [`PodDisruptionBudget`]s. +/// If you are using this to create [`PodDisruptionBudget`]s according to [ADR 30 on Allowed Pod disruptions][adr], +/// the use of [`PodDisruptionBudgetBuilder::new_with_role`] is recommended. +/// +/// The following attributes on a [`PodDisruptionBudget`] are considered mandatory and must be specified +/// before being able to construct the [`PodDisruptionBudget`]: +/// +/// 1. [`PodDisruptionBudget::metadata`] +/// 2. [`PodDisruptionBudgetSpec::selector`] +/// 3. Either [`PodDisruptionBudgetSpec::min_available`] or [`PodDisruptionBudgetSpec::max_unavailable`] +/// +/// Both [`PodDisruptionBudget::metadata`] and [`PodDisruptionBudgetSpec::selector`] will be set by [`PodDisruptionBudgetBuilder::new_with_role`]. +/// +/// [adr]: https://docs.stackable.tech/home/stable/contributor/adr/adr030-allowed-pod-disruptions +#[derive(Debug, Default)] +pub struct PodDisruptionBudgetBuilder { + metadata: ObjectMeta, + selector: LabelSelector, + /// Tracks wether either `maxUnavailable` or `minAvailable` is set. + constraint: Option, +} + +/// We intentionally only support fixed numbers, no percentage, see ADR 30 on Pod disruptions for details. +/// We use u16, as [`IntOrString`] takes an i32 and we don't want to allow negative numbers. u16 will always fit in i32. +#[derive(Debug)] +pub enum PodDisruptionBudgetConstraint { + MaxUnavailable(u16), + MinAvailable(u16), +} + +impl PodDisruptionBudgetBuilder<(), (), ()> { + pub fn new() -> Self { + PodDisruptionBudgetBuilder::default() + } + + /// This method populates [`PodDisruptionBudget::metadata`] and [`PodDisruptionBudgetSpec::selector`] from the give role + /// (not roleGroup!). + /// + /// The parameters are the same as the fields from [`crate::labels::ObjectLabels`]: + /// * `owner` - Reference to the k8s object owning the created resource, such as `HdfsCluster` or `TrinoCluster`. + /// * `app_name` - The name of the app being managed, such as `hdfs` or `trino`. + /// * `role` - The role that this object belongs to, e.g. `datanode` or `worker`. + /// * `operator_name` - The DNS-style name of the operator managing the object (such as `hdfs.stackable.tech`). + /// * `controller_name` - The name of the controller inside of the operator managing the object (such as `hdfscluster`) + pub fn new_with_role>( + owner: &T, + app_name: &str, + role: &str, + operator_name: &str, + controller_name: &str, + ) -> OperatorResult> { + let role_selector_labels = role_selector_labels(owner, app_name, role); + let metadata = ObjectMetaBuilder::new() + .namespace_opt(owner.namespace()) + .name(format!("{}-{}", owner.name_any(), role)) + .ownerreference_from_resource(owner, None, Some(true))? + .with_labels(role_selector_labels.clone()) + .with_label( + APP_MANAGED_BY_LABEL.to_string(), + format_full_controller_name(operator_name, controller_name), + ) + .build(); + + Ok(PodDisruptionBudgetBuilder { + metadata, + selector: LabelSelector { + match_expressions: None, + match_labels: Some(role_selector_labels), + }, + ..PodDisruptionBudgetBuilder::default() + }) + } + + /// Sets the mandatory [`PodDisruptionBudget::metadata`]. + pub fn new_with_metadata( + self, + metadata: impl Into, + ) -> PodDisruptionBudgetBuilder { + PodDisruptionBudgetBuilder { + metadata: metadata.into(), + ..PodDisruptionBudgetBuilder::default() + } + } +} + +impl PodDisruptionBudgetBuilder { + /// Sets the mandatory [`PodDisruptionBudgetSpec::selector`]. + pub fn with_selector( + self, + selector: LabelSelector, + ) -> PodDisruptionBudgetBuilder { + PodDisruptionBudgetBuilder { + metadata: self.metadata, + selector, + constraint: self.constraint, + } + } +} + +impl PodDisruptionBudgetBuilder { + /// Sets the mandatory [`PodDisruptionBudgetSpec::max_unavailable`]. + /// Mutually exclusive with [`PodDisruptionBudgetBuilder::with_min_available`]. + pub fn with_max_unavailable( + self, + max_unavailable: u16, + ) -> PodDisruptionBudgetBuilder { + PodDisruptionBudgetBuilder { + metadata: self.metadata, + selector: self.selector, + constraint: Some(PodDisruptionBudgetConstraint::MaxUnavailable( + max_unavailable, + )), + } + } + + /// Sets the mandatory [`PodDisruptionBudgetSpec::min_available`]. + /// Mutually exclusive with [`PodDisruptionBudgetBuilder::with_max_unavailable`]. + #[deprecated( + since = "0.51.0", + note = "It is strongly recommended to use [`max_unavailable`]. Please read the ADR on Pod disruptions before using this function." + )] + pub fn with_min_available( + self, + min_available: u16, + ) -> PodDisruptionBudgetBuilder { + PodDisruptionBudgetBuilder { + metadata: self.metadata, + selector: self.selector, + constraint: Some(PodDisruptionBudgetConstraint::MinAvailable(min_available)), + } + } +} + +impl PodDisruptionBudgetBuilder { + /// This function can be called after [`PodDisruptionBudget::metadata`], [`PodDisruptionBudgetSpec::selector`] + /// and either [`PodDisruptionBudgetSpec::min_available`] or [`PodDisruptionBudgetSpec::max_unavailable`] are set. + pub fn build(self) -> PodDisruptionBudget { + let (max_unavailable, min_available) = match self.constraint { + Some(PodDisruptionBudgetConstraint::MaxUnavailable(max_unavailable)) => { + (Some(max_unavailable), None) + } + Some(PodDisruptionBudgetConstraint::MinAvailable(min_unavailable)) => { + (None, Some(min_unavailable)) + } + None => { + unreachable!("Either minUnavailable or maxUnavailable must be set at this point!") + } + }; + PodDisruptionBudget { + metadata: self.metadata, + spec: Some(PodDisruptionBudgetSpec { + max_unavailable: max_unavailable.map(i32::from).map(IntOrString::Int), + min_available: min_available.map(i32::from).map(IntOrString::Int), + selector: Some(self.selector), + // Because this feature is still in beta in k8s version 1.27, the builder currently does not offer this attribute. + unhealthy_pod_eviction_policy: Default::default(), + }), + ..Default::default() + } + } +} + +#[cfg(test)] +mod test { + use std::collections::BTreeMap; + + use k8s_openapi::{ + api::policy::v1::{PodDisruptionBudget, PodDisruptionBudgetSpec}, + apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString}, + }; + use kube::{core::ObjectMeta, CustomResource}; + use schemars::JsonSchema; + use serde::{Deserialize, Serialize}; + + use crate::builder::{ObjectMetaBuilder, OwnerReferenceBuilder}; + + use super::PodDisruptionBudgetBuilder; + + #[test] + pub fn test_normal_build() { + #[allow(deprecated)] + let pdb = PodDisruptionBudgetBuilder::new() + .new_with_metadata( + ObjectMetaBuilder::new() + .namespace("default") + .name("trino") + .build(), + ) + .with_selector(LabelSelector { + match_expressions: None, + match_labels: Some(BTreeMap::from([("foo".to_string(), "bar".to_string())])), + }) + .with_min_available(42) + .build(); + + assert_eq!( + pdb, + PodDisruptionBudget { + metadata: ObjectMeta { + name: Some("trino".to_string()), + namespace: Some("default".to_string()), + ..Default::default() + }, + spec: Some(PodDisruptionBudgetSpec { + min_available: Some(IntOrString::Int(42)), + selector: Some(LabelSelector { + match_expressions: None, + match_labels: Some(BTreeMap::from([( + "foo".to_string(), + "bar".to_string() + )])), + }), + ..Default::default() + }), + ..Default::default() + } + ) + } + + #[test] + pub fn test_build_from_role() { + #[derive( + Clone, CustomResource, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize, + )] + #[kube(group = "test", version = "v1", kind = "TrinoCluster", namespaced)] + pub struct TrinoClusterSpec {} + let trino: TrinoCluster = serde_yaml::from_str( + " + apiVersion: test/v1 + kind: TrinoCluster + metadata: + name: simple-trino + namespace: default + uid: 123 # Needed for the ownerreference + spec: {} + ", + ) + .unwrap(); + let app_name = "trino"; + let role = "worker"; + let operator_name = "trino.stackable.tech"; + let controller_name = "trino-operator-trino-controller"; + let pdb = PodDisruptionBudgetBuilder::new_with_role( + &trino, + app_name, + role, + operator_name, + controller_name, + ) + .unwrap() + .with_max_unavailable(2) + .build(); + + assert_eq!( + pdb, + PodDisruptionBudget { + metadata: ObjectMeta { + name: Some("simple-trino-worker".to_string()), + namespace: Some("default".to_string()), + labels: Some(BTreeMap::from([ + ("app.kubernetes.io/name".to_string(), "trino".to_string()), + ( + "app.kubernetes.io/instance".to_string(), + "simple-trino".to_string() + ), + ( + "app.kubernetes.io/managed-by".to_string(), + "trino.stackable.tech_trino-operator-trino-controller".to_string() + ), + ( + "app.kubernetes.io/component".to_string(), + "worker".to_string() + ) + ])), + owner_references: Some(vec![OwnerReferenceBuilder::new() + .initialize_from_resource(&trino) + .block_owner_deletion_opt(None) + .controller_opt(Some(true)) + .build() + .unwrap()]), + ..Default::default() + }, + spec: Some(PodDisruptionBudgetSpec { + max_unavailable: Some(IntOrString::Int(2)), + selector: Some(LabelSelector { + match_expressions: None, + match_labels: Some(BTreeMap::from([ + ("app.kubernetes.io/name".to_string(), "trino".to_string()), + ( + "app.kubernetes.io/instance".to_string(), + "simple-trino".to_string() + ), + ( + "app.kubernetes.io/component".to_string(), + "worker".to_string() + ) + ])), + }), + ..Default::default() + }), + ..Default::default() + } + ) + } +} diff --git a/src/cluster_resources.rs b/src/cluster_resources.rs index 422e88123..af1001341 100644 --- a/src/cluster_resources.rs +++ b/src/cluster_resources.rs @@ -10,25 +10,24 @@ use crate::{ }, }, error::{Error, OperatorResult}, - k8s_openapi::{ - api::{ - apps::v1::{DaemonSet, DaemonSetSpec, StatefulSet, StatefulSetSpec}, - batch::v1::Job, - core::v1::{ - ConfigMap, ObjectReference, PodSpec, PodTemplateSpec, Secret, Service, - ServiceAccount, - }, - rbac::v1::RoleBinding, - }, - apimachinery::pkg::apis::meta::v1::{LabelSelector, LabelSelectorRequirement}, - NamespaceResourceScope, - }, - kube::{Resource, ResourceExt}, labels::{APP_INSTANCE_LABEL, APP_MANAGED_BY_LABEL, APP_NAME_LABEL}, utils::format_full_controller_name, }; -use kube::core::ErrorResponse; +use k8s_openapi::{ + api::{ + apps::v1::{DaemonSet, DaemonSetSpec, StatefulSet, StatefulSetSpec}, + batch::v1::Job, + core::v1::{ + ConfigMap, ObjectReference, PodSpec, PodTemplateSpec, Secret, Service, ServiceAccount, + }, + policy::v1::PodDisruptionBudget, + rbac::v1::RoleBinding, + }, + apimachinery::pkg::apis::meta::v1::{LabelSelector, LabelSelectorRequirement}, + NamespaceResourceScope, +}; +use kube::{core::ErrorResponse, Resource, ResourceExt}; use serde::{de::DeserializeOwned, Serialize}; use std::{ collections::{BTreeMap, HashSet}, @@ -160,11 +159,13 @@ impl ClusterResourceApplyStrategy { } } +// IMPORTANT: Don't forget to add new Resources to [`delete_orphaned_resources`] as well! impl ClusterResource for ConfigMap {} +impl ClusterResource for Secret {} impl ClusterResource for Service {} impl ClusterResource for ServiceAccount {} impl ClusterResource for RoleBinding {} -impl ClusterResource for Secret {} +impl ClusterResource for PodDisruptionBudget {} impl ClusterResource for Job { fn pod_spec(&self) -> Option<&PodSpec> { @@ -546,16 +547,18 @@ impl ClusterResources { /// # Arguments /// /// * `client` - The client which is used to access Kubernetes + /// pub async fn delete_orphaned_resources(self, client: &Client) -> OperatorResult<()> { tokio::try_join!( self.delete_orphaned_resources_of_kind::(client), self.delete_orphaned_resources_of_kind::(client), self.delete_orphaned_resources_of_kind::(client), + self.delete_orphaned_resources_of_kind::(client), self.delete_orphaned_resources_of_kind::(client), + self.delete_orphaned_resources_of_kind::(client), self.delete_orphaned_resources_of_kind::(client), self.delete_orphaned_resources_of_kind::(client), - self.delete_orphaned_resources_of_kind::(client), - self.delete_orphaned_resources_of_kind::(client), + self.delete_orphaned_resources_of_kind::(client), )?; Ok(()) diff --git a/src/commons/mod.rs b/src/commons/mod.rs index 171d2728d..a77a0915c 100644 --- a/src/commons/mod.rs +++ b/src/commons/mod.rs @@ -5,6 +5,7 @@ pub mod authentication; pub mod cluster_operation; pub mod listener; pub mod opa; +pub mod pdb; pub mod product_image_selection; pub mod rbac; pub mod resources; diff --git a/src/commons/pdb.rs b/src/commons/pdb.rs new file mode 100644 index 000000000..286528a6e --- /dev/null +++ b/src/commons/pdb.rs @@ -0,0 +1,31 @@ +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct PdbConfig { + /// Wether a PodDisruptionBudget should be written out for this role. + /// Disabling this enables you to specify you own - custom - one. + /// Defaults to true. + #[serde(default = "default_pdb_enabled")] + pub enabled: bool, + + /// The number of Pods that are allowed to be down because of voluntary disruptions. + /// If you don't explicitly set this, the operator will use a sane default based + /// upon knowledge about the individual product. + #[serde(default)] + pub max_unavailable: Option, +} + +fn default_pdb_enabled() -> bool { + true +} + +impl Default for PdbConfig { + fn default() -> Self { + Self { + enabled: true, + max_unavailable: None, + } + } +} diff --git a/src/labels.rs b/src/labels.rs index 17d75c6ca..a788a8ff2 100644 --- a/src/labels.rs +++ b/src/labels.rs @@ -29,7 +29,7 @@ pub const APP_ROLE_GROUP_LABEL: &str = concatcp!(APP_KUBERNETES_LABEL_BASE, "rol /// See [`get_recommended_labels`] and [`ObjectMetaBuilder::with_recommended_labels`]. #[derive(Debug, Clone, Copy)] pub struct ObjectLabels<'a, T> { - /// The name of the object that this object is being created on behalf of, such as a `ZookeeperCluster` + /// Reference to the k8s object owning the created resource, such as `HdfsCluster` or `TrinoCluster`. pub owner: &'a T, /// The name of the app being managed, such as `zookeeper` pub app_name: &'a str, diff --git a/src/product_config_utils.rs b/src/product_config_utils.rs index 7105c4d1f..4e014e095 100644 --- a/src/product_config_utils.rs +++ b/src/product_config_utils.rs @@ -490,7 +490,7 @@ mod tests { } use super::*; - use crate::role_utils::{Role, RoleGroup}; + use crate::role_utils::{Role, RoleConfig, RoleGroup}; use k8s_openapi::api::core::v1::PodTemplateSpec; use rstest::*; use std::collections::HashMap; @@ -619,6 +619,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -636,6 +637,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -650,6 +652,7 @@ mod tests { None, None, ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -667,6 +670,7 @@ mod tests { None, None, ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -684,6 +688,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -701,6 +706,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: CommonConfiguration::default(), @@ -714,6 +720,7 @@ mod tests { None, None, ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -732,6 +739,7 @@ mod tests { None, None, ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: CommonConfiguration::default(), @@ -745,6 +753,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -762,6 +771,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -774,6 +784,7 @@ mod tests { }, (false, true, false, true) => Role { config: CommonConfiguration::default(), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -786,6 +797,7 @@ mod tests { }, (false, true, false, false) => Role { config: CommonConfiguration::default(), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -803,6 +815,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -820,6 +833,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: CommonConfiguration::default(), @@ -828,6 +842,7 @@ mod tests { }, (false, false, false, true) => Role { config: CommonConfiguration::default(), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -840,6 +855,7 @@ mod tests { }, (false, false, false, false) => Role { config: CommonConfiguration::default(), + role_config: RoleConfig::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: CommonConfiguration::default(), @@ -1039,6 +1055,7 @@ mod tests { // should override build_cli_override("cli"), ), + role_config: RoleConfig::default(), role_groups: collection! {role_group.to_string() => RoleGroup { replicas: Some(1), config: build_common_config( @@ -1098,6 +1115,7 @@ mod tests { None, None, ), + role_config: RoleConfig::default(), role_groups: collection! {role_group_1.to_string() => RoleGroup { replicas: Some(1), config: build_common_config( @@ -1126,6 +1144,7 @@ mod tests { None, None, ), + role_config: RoleConfig::default(), role_groups: collection! {role_group_1.to_string() => RoleGroup { replicas: Some(1), config: build_common_config( @@ -1187,6 +1206,7 @@ mod tests { let roles: HashMap, Role>)> = collection! { role_1.to_string() => (vec![PropertyNameKind::File(file_name.to_string()), PropertyNameKind::Env], Role { config: CommonConfiguration::default(), + role_config: RoleConfig::default(), role_groups: collection! { role_group_1.to_string() => RoleGroup { replicas: Some(1), @@ -1202,6 +1222,7 @@ mod tests { ), role_2.to_string() => (vec![PropertyNameKind::File(file_name.to_string())], Role { config: CommonConfiguration::default(), + role_config: RoleConfig::default(), role_groups: collection! { role_group_2.to_string() => RoleGroup { replicas: Some(1), diff --git a/src/role_utils.rs b/src/role_utils.rs index e576728df..a90227479 100644 --- a/src/role_utils.rs +++ b/src/role_utils.rs @@ -86,6 +86,7 @@ use std::{ }; use crate::{ + commons::pdb::PdbConfig, config::{ fragment::{self, FromFragment}, merge::Merge, @@ -172,6 +173,10 @@ fn config_schema_default() -> serde_json::Value { pub struct Role { #[serde(flatten)] pub config: CommonConfiguration, + + #[serde(default)] + pub role_config: RoleConfig, + pub role_groups: HashMap>, } @@ -191,6 +196,7 @@ impl Role { cli_overrides: self.config.cli_overrides, pod_overrides: self.config.pod_overrides, }, + role_config: RoleConfig::default(), role_groups: self .role_groups .into_iter() @@ -216,6 +222,13 @@ impl Role { } } +#[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct RoleConfig { + #[serde(default)] + pub pod_disruption_budget: PdbConfig, +} + #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde( rename_all = "camelCase",