From fca965d9a5fd4d5184d3ae3ece952ba39b1598ae Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 12 Oct 2023 11:59:15 +0200 Subject: [PATCH 1/8] reafctor!: Change PodBuilder::termination_grace_period to take Duration struct --- src/builder/pod/mod.rs | 19 ++++++++++++++----- src/error.rs | 5 ++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index d366c8220..c1b08a39a 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -12,6 +12,7 @@ use crate::commons::resources::{ ComputeResource, ResourceRequirementsExt, ResourceRequirementsType, LIMIT_REQUEST_RATIO_CPU, LIMIT_REQUEST_RATIO_MEMORY, }; +use crate::duration::Duration; use crate::error::{Error, OperatorResult}; use super::{ListenerOperatorVolumeSourceBuilder, ListenerReference, VolumeBuilder}; @@ -452,12 +453,19 @@ impl PodBuilder { self } - pub fn termination_grace_period_seconds( + pub fn termination_grace_period( &mut self, - termination_grace_period_seconds: i64, - ) -> &mut Self { + termination_grace_period: Duration, + ) -> OperatorResult<&mut Self> { + let termination_grace_period_seconds = termination_grace_period + .as_secs() + .try_into() + .map_err(|_| Error::DurationTooLong { + duration: termination_grace_period, + })?; + self.termination_grace_period_seconds = Some(termination_grace_period_seconds); - self + Ok(self) } /// Consumes the Builder and returns a constructed [`Pod`] @@ -637,7 +645,8 @@ mod tests { .with_config_map("configmap") .build(), ) - .termination_grace_period_seconds(42) + .termination_grace_period(Duration::from_secs(42)) + .unwrap() .build() .unwrap(); diff --git a/src/error.rs b/src/error.rs index de802a3b3..5b8e672f8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,4 @@ -use crate::product_config_utils; +use crate::{duration::Duration, product_config_utils}; use std::path::PathBuf; #[derive(Debug, thiserror::Error)] @@ -111,6 +111,9 @@ pub enum Error { container_name: String, violation: String, }, + + #[error("The duration {duration:?} is too long to process")] + DurationTooLong { duration: Duration }, } pub type OperatorResult = std::result::Result; From 6e94371a802989fd57c0a19117dfcb2b114e9cac Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 12 Oct 2023 12:01:43 +0200 Subject: [PATCH 2/8] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d087bff42..e9f65d7ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,10 @@ All notable changes to this project will be documented in this file. ### Changed - Convert the format of the Vector configuration from TOML to YAML ([#670]). +- BREAKING: Change `PodBuilder::termination_grace_period` to take `Duration` struct ([#672]). [#670]: https://github.com/stackabletech/operator-rs/pull/670 +[#672]: https://github.com/stackabletech/operator-rs/pull/672 ## [0.54.0] - 2023-10-10 From 47368f6e5f3ca6db9d9b3f4ca720471797412eb1 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 12 Oct 2023 12:09:44 +0200 Subject: [PATCH 3/8] take by reference --- src/builder/pod/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index c1b08a39a..a21ccd735 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -455,13 +455,13 @@ impl PodBuilder { pub fn termination_grace_period( &mut self, - termination_grace_period: Duration, + termination_grace_period: &Duration, ) -> OperatorResult<&mut Self> { let termination_grace_period_seconds = termination_grace_period .as_secs() .try_into() .map_err(|_| Error::DurationTooLong { - duration: termination_grace_period, + duration: *termination_grace_period, })?; self.termination_grace_period_seconds = Some(termination_grace_period_seconds); @@ -645,7 +645,7 @@ mod tests { .with_config_map("configmap") .build(), ) - .termination_grace_period(Duration::from_secs(42)) + .termination_grace_period(&Duration::from_secs(42)) .unwrap() .build() .unwrap(); From 1b638799c60183ef874379d4f4823fdeecac4ceb Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 12 Oct 2023 12:23:23 +0200 Subject: [PATCH 4/8] Update CHANGELOG.md Co-authored-by: Natalie --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9f65d7ec..db5b3cd96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Changed - Convert the format of the Vector configuration from TOML to YAML ([#670]). -- BREAKING: Change `PodBuilder::termination_grace_period` to take `Duration` struct ([#672]). +- BREAKING: Rename `PodBuilder::termination_grace_period_seconds` to `termination_grace_period`, and change it to take `Duration` struct ([#672]). [#670]: https://github.com/stackabletech/operator-rs/pull/670 [#672]: https://github.com/stackabletech/operator-rs/pull/672 From d36fe79f94c86949176d7b2f4f4193b9f4673058 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 12 Oct 2023 13:54:27 +0200 Subject: [PATCH 5/8] rework error handling --- src/builder/pod/mod.rs | 37 ++++++++++++++++++++++++++++++++----- src/error.rs | 5 +---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index a21ccd735..93856a74f 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -4,6 +4,7 @@ pub mod security; pub mod volume; use std::collections::BTreeMap; +use std::num::TryFromIntError; use crate::builder::meta::ObjectMetaBuilder; use crate::commons::affinity::StackableAffinity; @@ -13,7 +14,7 @@ use crate::commons::resources::{ LIMIT_REQUEST_RATIO_MEMORY, }; use crate::duration::Duration; -use crate::error::{Error, OperatorResult}; +use crate::error::{self, OperatorResult}; use super::{ListenerOperatorVolumeSourceBuilder, ListenerReference, VolumeBuilder}; use k8s_openapi::{ @@ -26,8 +27,18 @@ use k8s_openapi::{ }; use tracing::warn; +#[derive(Debug, thiserror::Error, PartialEq)] +pub enum Error { + #[error("The duration {duration} is too long to put it into the Pods terminationGracePeriodSeconds. The maximum duration is {}", Duration::from_secs(i64::MAX as u64))] + DurationTooLong { + source: TryFromIntError, + duration: Duration, + }, +} +pub type Result = std::result::Result; + /// A builder to build [`Pod`] or [`PodTemplateSpec`] objects. -#[derive(Clone, Default)] +#[derive(Clone, Debug, Default, PartialEq)] pub struct PodBuilder { containers: Vec, host_network: Option, @@ -456,11 +467,12 @@ impl PodBuilder { pub fn termination_grace_period( &mut self, termination_grace_period: &Duration, - ) -> OperatorResult<&mut Self> { + ) -> Result<&mut Self> { let termination_grace_period_seconds = termination_grace_period .as_secs() .try_into() - .map_err(|_| Error::DurationTooLong { + .map_err(|err| Error::DurationTooLong { + source: err, duration: *termination_grace_period, })?; @@ -472,7 +484,7 @@ impl PodBuilder { pub fn build(&self) -> OperatorResult { Ok(Pod { metadata: match self.metadata { - None => return Err(Error::MissingObjectKey { key: "metadata" }), + None => return Err(error::Error::MissingObjectKey { key: "metadata" }), Some(ref metadata) => metadata.clone(), }, spec: Some(self.build_spec()), @@ -700,4 +712,19 @@ mod tests { .unwrap(); assert_eq!(pod.spec.unwrap().restart_policy.unwrap(), "Always"); } + + #[test] + fn test_pod_builder_too_long_termination_grace_period() { + let too_long_duration = Duration::from_secs(i64::MAX as u64 + 1); + let mut pod_builder = PodBuilder::new(); + + let result = pod_builder.termination_grace_period(&too_long_duration); + assert!(matches!( + result, + Err(Error::DurationTooLong { + source: TryFromIntError { .. }, + duration, + }) if duration == too_long_duration + )); + } } diff --git a/src/error.rs b/src/error.rs index 5b8e672f8..de802a3b3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,4 @@ -use crate::{duration::Duration, product_config_utils}; +use crate::product_config_utils; use std::path::PathBuf; #[derive(Debug, thiserror::Error)] @@ -111,9 +111,6 @@ pub enum Error { container_name: String, violation: String, }, - - #[error("The duration {duration:?} is too long to process")] - DurationTooLong { duration: Duration }, } pub type OperatorResult = std::result::Result; From c477c2e34e0bd13ff7a918b41530032ec580617d Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 13 Oct 2023 08:21:41 +0200 Subject: [PATCH 6/8] Update src/builder/pod/mod.rs Co-authored-by: Natalie --- src/builder/pod/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 93856a74f..34341006c 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -27,7 +27,7 @@ use k8s_openapi::{ }; use tracing::warn; -#[derive(Debug, thiserror::Error, PartialEq)] +#[derive(Debug, thiserror::Error)] pub enum Error { #[error("The duration {duration} is too long to put it into the Pods terminationGracePeriodSeconds. The maximum duration is {}", Duration::from_secs(i64::MAX as u64))] DurationTooLong { From ddb7e0dc7edca5f0eb3fa34880b9380fd8098a70 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 13 Oct 2023 08:22:49 +0200 Subject: [PATCH 7/8] Update src/builder/pod/mod.rs Co-authored-by: Natalie --- src/builder/pod/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 34341006c..8c3e3f314 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -29,7 +29,7 @@ use tracing::warn; #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("The duration {duration} is too long to put it into the Pods terminationGracePeriodSeconds. The maximum duration is {}", Duration::from_secs(i64::MAX as u64))] + #[error("termination grace period is too long (got {duration}, maximum allowed is {max})", max = Duration::from_secs(i64::MAX as u64))] DurationTooLong { source: TryFromIntError, duration: Duration, From f0c2539e820cae201f3ec495da35f7d9b55cdfce Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 13 Oct 2023 08:23:37 +0200 Subject: [PATCH 8/8] rename error to TerminationGracePeriodTooLong --- src/builder/pod/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 8c3e3f314..d112bd811 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -30,7 +30,7 @@ use tracing::warn; #[derive(Debug, thiserror::Error)] pub enum Error { #[error("termination grace period is too long (got {duration}, maximum allowed is {max})", max = Duration::from_secs(i64::MAX as u64))] - DurationTooLong { + TerminationGracePeriodTooLong { source: TryFromIntError, duration: Duration, }, @@ -471,7 +471,7 @@ impl PodBuilder { let termination_grace_period_seconds = termination_grace_period .as_secs() .try_into() - .map_err(|err| Error::DurationTooLong { + .map_err(|err| Error::TerminationGracePeriodTooLong { source: err, duration: *termination_grace_period, })?; @@ -721,7 +721,7 @@ mod tests { let result = pod_builder.termination_grace_period(&too_long_duration); assert!(matches!( result, - Err(Error::DurationTooLong { + Err(Error::TerminationGracePeriodTooLong { source: TryFromIntError { .. }, duration, }) if duration == too_long_duration