From 4e70b635b668955aaeddb2e6d4c244ee35680bde Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 18 Sep 2023 14:22:38 +0200 Subject: [PATCH] feat: Add Pdb struct and PdbBuilder --- src/builder/mod.rs | 1 + src/builder/pdb.rs | 225 ++++++++++++++++++++++++++++++++++++ src/commons/mod.rs | 1 + src/commons/pdb.rs | 29 +++++ src/product_config_utils.rs | 21 ++++ src/role_utils.rs | 3 + 6 files changed, 280 insertions(+) create mode 100644 src/builder/pdb.rs create mode 100644 src/commons/pdb.rs 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..865c312eb --- /dev/null +++ b/src/builder/pdb.rs @@ -0,0 +1,225 @@ +use crate::{builder::ObjectMetaBuilder, labels::role_selector_labels}; +use k8s_openapi::{ + api::policy::v1::{PodDisruptionBudget, PodDisruptionBudgetSpec}, + apimachinery::pkg::{ + apis::meta::v1::{LabelSelector, ObjectMeta}, + util::intstr::IntOrString, + }, +}; +use kube::{Resource, ResourceExt}; + +#[derive(Debug, Default)] +pub struct PdbBuilder { + metadata: ObjectMeta, + selector: LabelSelector, + /// We intentionally only support fixed numbers, so percentage, see ADR 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. + max_unavailable: Option, + /// We intentionally only support fixed numbers, so percentage, see ADR 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. + min_available: Option, + /// Tracks wether either [`max_unavailable`] or [`min_available`] is set + _constraints: Constraints, +} + +impl PdbBuilder<(), (), ()> { + pub fn new() -> Self { + PdbBuilder::default() + } + + pub fn new_for_role( + owner: &T, + app_name: &str, + role: &str, + ) -> PdbBuilder { + let metadata = ObjectMetaBuilder::new() + .namespace_opt(owner.namespace()) + .name(format!("{}-{}", owner.name_any(), role)) + .build(); + let role_selector_labels = role_selector_labels(owner, app_name, role); + PdbBuilder { + metadata, + selector: LabelSelector { + match_expressions: None, + match_labels: Some(role_selector_labels), + }, + ..PdbBuilder::default() + } + } + + pub fn metadata(self, metadata: impl Into) -> PdbBuilder { + PdbBuilder { + metadata: metadata.into(), + selector: self.selector, + max_unavailable: self.max_unavailable, + min_available: self.min_available, + _constraints: self._constraints, + } + } +} + +impl PdbBuilder { + pub fn selector(self, selector: LabelSelector) -> PdbBuilder { + PdbBuilder { + metadata: self.metadata, + selector, + max_unavailable: self.max_unavailable, + min_available: self.min_available, + _constraints: self._constraints, + } + } +} + +impl PdbBuilder { + pub fn max_unavailable( + self, + max_unavailable: u16, + ) -> PdbBuilder { + PdbBuilder { + metadata: self.metadata, + selector: self.selector, + max_unavailable: Some(max_unavailable), + min_available: self.min_available, + _constraints: true, // Some dummy value to set Constraints to something other than () + } + } + + pub fn min_available(self, min_available: u16) -> PdbBuilder { + PdbBuilder { + metadata: self.metadata, + selector: self.selector, + max_unavailable: self.max_unavailable, + min_available: Some(min_available), + _constraints: true, // Some dummy value to set Constraints to something other than () + } + } +} + +impl PdbBuilder { + pub fn build(self) -> PodDisruptionBudget { + PodDisruptionBudget { + metadata: self.metadata, + spec: Some(PodDisruptionBudgetSpec { + max_unavailable: self.max_unavailable.map(i32::from).map(IntOrString::Int), + min_available: self.min_available.map(i32::from).map(IntOrString::Int), + selector: Some(self.selector), + /// As this is beta as of 1.27 we can not use it yet, + /// so this builder 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; + + use super::PdbBuilder; + + #[test] + pub fn test_normal_build() { + let pdb = PdbBuilder::new() + .metadata( + ObjectMetaBuilder::new() + .namespace("default") + .name("trino") + .build(), + ) + .selector(LabelSelector { + match_expressions: None, + match_labels: Some(BTreeMap::from([("foo".to_string(), "bar".to_string())])), + }) + .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 + spec: {} + ", + ) + .unwrap(); + let app_name = "trino"; + let role = "worker"; + let pdb = PdbBuilder::new_for_role(&trino, app_name, role) + .max_unavailable(2) + .build(); + + assert_eq!( + pdb, + PodDisruptionBudget { + metadata: ObjectMeta { + name: Some("simple-trino-worker".to_string()), + namespace: Some("default".to_string()), + ..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/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..4f1debcbe --- /dev/null +++ b/src/commons/pdb.rs @@ -0,0 +1,29 @@ +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct Pdb { + /// 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. + pub max_unavailable: Option, +} + +fn default_pdb_enabled() -> bool { + true +} + +impl Default for Pdb { + fn default() -> Self { + Self { + enabled: true, + max_unavailable: None, + } + } +} diff --git a/src/product_config_utils.rs b/src/product_config_utils.rs index 7105c4d1f..586c0b0ab 100644 --- a/src/product_config_utils.rs +++ b/src/product_config_utils.rs @@ -619,6 +619,7 @@ mod tests { build_env_override(ROLE_ENV_OVERRIDE), build_cli_override(ROLE_CLI_OVERRIDE), ), + pdb: Default::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), ), + pdb: Default::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -650,6 +652,7 @@ mod tests { None, None, ), + pdb: Default::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -667,6 +670,7 @@ mod tests { None, None, ), + pdb: Default::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), ), + pdb: Default::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), ), + pdb: Default::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: CommonConfiguration::default(), @@ -714,6 +720,7 @@ mod tests { None, None, ), + pdb: Default::default(), role_groups: collection! {role_group => RoleGroup { replicas: Some(1), config: build_common_config( @@ -732,6 +739,7 @@ mod tests { None, None, ), + pdb: Default::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), ), + pdb: Default::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), ), + pdb: Default::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(), + pdb: Default::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(), + pdb: Default::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), ), + pdb: Default::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), ), + pdb: Default::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(), + pdb: Default::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(), + pdb: Default::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"), ), + pdb: Default::default(), role_groups: collection! {role_group.to_string() => RoleGroup { replicas: Some(1), config: build_common_config( @@ -1098,6 +1115,7 @@ mod tests { None, None, ), + pdb: Default::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, ), + pdb: Default::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(), + pdb: Default::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(), + pdb: Default::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..420b69212 100644 --- a/src/role_utils.rs +++ b/src/role_utils.rs @@ -86,6 +86,7 @@ use std::{ }; use crate::{ + commons::pdb::Pdb, config::{ fragment::{self, FromFragment}, merge::Merge, @@ -172,6 +173,7 @@ fn config_schema_default() -> serde_json::Value { pub struct Role { #[serde(flatten)] pub config: CommonConfiguration, + pub pdb: Pdb, pub role_groups: HashMap>, } @@ -191,6 +193,7 @@ impl Role { cli_overrides: self.config.cli_overrides, pod_overrides: self.config.pod_overrides, }, + pdb: Default::default(), role_groups: self .role_groups .into_iter()