From e5414dddfa45c38ecbdef0e3b3493c7cdef59e9a Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 8 Sep 2025 08:39:56 +0200 Subject: [PATCH 1/4] fix: Invalid termination_grace_period default in ProbeBuilder --- crates/stackable-operator/CHANGELOG.md | 6 +- .../src/builder/pod/probe.rs | 73 ++++++++++++++----- 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 89c9924d7..15cef97fa 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -11,9 +11,13 @@ All notable changes to this project will be documented in this file. ### Removed - BREAKING: Remove the Merge implementation for PodTemplateSpec ([#1087]). - It was broken because the instance was overriden by the given defaults. + It was broken because the instance was overridden by the given defaults. This function is not used by the Stackable operators. +### Fixed + +- Don't default the `termination_grace_period` of the `ProbeBuilder` to 0, as this is an invalid value ([#XXXX]). + [#1085]: https://github.com/stackabletech/operator-rs/pull/1085 [#1087]: https://github.com/stackabletech/operator-rs/pull/1087 diff --git a/crates/stackable-operator/src/builder/pod/probe.rs b/crates/stackable-operator/src/builder/pod/probe.rs index 7c647e523..c66edf251 100644 --- a/crates/stackable-operator/src/builder/pod/probe.rs +++ b/crates/stackable-operator/src/builder/pod/probe.rs @@ -64,7 +64,7 @@ pub struct ProbeBuilder { failure_threshold: i32, timeout: Duration, initial_delay: Duration, - termination_grace_period: Duration, + termination_grace_period: Option, } /// Available probes @@ -145,7 +145,7 @@ impl ProbeBuilder<(), ()> { failure_threshold: 1, timeout: Duration::from_secs(1), initial_delay: Duration::from_secs(0), - termination_grace_period: Duration::from_secs(0), + termination_grace_period: None, } } } @@ -200,24 +200,48 @@ impl ProbeBuilder { Ok(self.with_success_threshold(success_threshold.ceil() as i32)) } - /// How often the probe must fail before being considered failed. + /// After a probe fails `failureThreshold` times in a row, Kubernetes considers that the + /// overall check has failed: the container is not ready/healthy/live. + /// + /// Minimum value is 1 second. For the case of a startup or liveness probe, if at least + /// `failureThreshold`` probes have failed, Kubernetes treats the container as unhealthy and + /// triggers a restart for that specific container. The kubelet honors the setting of + /// `terminationGracePeriodSeconds`` for that container. For a failed readiness probe, the + /// kubelet continues running the container that failed checks, and also continues to run more + /// probes; because the check failed, the kubelet sets the `Ready` condition on the Pod to + /// `false`. pub fn with_failure_threshold(mut self, failure_threshold: i32) -> Self { self.failure_threshold = failure_threshold; self } + /// Number of seconds after which the probe times out. + /// + /// Minimum value is 1 second. pub fn with_timeout(mut self, timeout: Duration) -> Self { self.timeout = timeout; self } + /// Number of seconds after the container has started before startup, liveness or readiness + /// probes are initiated. + /// + /// If a startup probe is defined, liveness and readiness probe delays do not begin until the + /// startup probe has succeeded. If the value of periodSeconds is greater than + /// `initialDelaySeconds` then the `initialDelaySeconds` will be ignored. pub fn with_initial_delay(mut self, initial_delay: Duration) -> Self { self.initial_delay = initial_delay; self } + /// Configure a grace period for the kubelet to wait between triggering a shut down of the + /// failed container, and then forcing the container runtime to stop that container. + /// + /// The default (if this function is not called) is to inherit the Pod-level value for + /// `terminationGracePeriodSeconds`` (30 seconds if not specified), and the minimum value is + /// 1 second. See probe-level `terminationGracePeriodSeconds`` for more detail. pub fn with_termination_grace_period(mut self, termination_grace_period: Duration) -> Self { - self.termination_grace_period = termination_grace_period; + self.termination_grace_period = Some(termination_grace_period); self } @@ -263,14 +287,17 @@ impl ProbeBuilder { )?), success_threshold: Some(self.success_threshold), tcp_socket: None, - termination_grace_period_seconds: Some( - self.termination_grace_period.as_secs().try_into().context( - DurationTooLongSnafu { - field: "terminationGracePeriod", - duration: self.termination_grace_period, - }, - )?, - ), + termination_grace_period_seconds: match self.termination_grace_period { + Some(termination_grace_period) => { + Some(termination_grace_period.as_secs().try_into().context( + DurationTooLongSnafu { + field: "terminationGracePeriod", + duration: termination_grace_period, + }, + )?) + } + None => None, + }, timeout_seconds: Some(self.timeout.as_secs().try_into().context( DurationTooLongSnafu { field: "timeout", @@ -302,13 +329,23 @@ mod tests { .expect("Valid inputs must produce a Probe"); assert_eq!( - probe.http_get, - Some(HTTPGetAction { - port: IntOrString::Int(8080), - ..Default::default() - }) + probe, + Probe { + exec: None, + failure_threshold: Some(1), + grpc: None, + http_get: Some(HTTPGetAction { + port: IntOrString::Int(8080), + ..Default::default() + }), + initial_delay_seconds: Some(0), + period_seconds: Some(10), + success_threshold: Some(1), + tcp_socket: None, + termination_grace_period_seconds: None, + timeout_seconds: Some(1), + } ); - assert_eq!(probe.period_seconds, Some(10)); } #[test] From b0d5e44659ff4cd8c2dd1bc5b06d3a03fa01c6d0 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 8 Sep 2025 08:44:11 +0200 Subject: [PATCH 2/4] changelog --- crates/stackable-operator/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 15cef97fa..903e4f7a1 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -16,10 +16,11 @@ All notable changes to this project will be documented in this file. ### Fixed -- Don't default the `termination_grace_period` of the `ProbeBuilder` to 0, as this is an invalid value ([#XXXX]). +- Don't default the `termination_grace_period` of the `ProbeBuilder` to 0, as this is an invalid value ([#1090]). [#1085]: https://github.com/stackabletech/operator-rs/pull/1085 [#1087]: https://github.com/stackabletech/operator-rs/pull/1087 +[#1090]: https://github.com/stackabletech/operator-rs/pull/1090 ## [0.96.0] - 2025-08-25 From c8666aed8b8ef4050cc4dcbb6c25795cdb782daa Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 8 Sep 2025 08:50:59 +0200 Subject: [PATCH 3/4] Remove space --- crates/stackable-operator/src/builder/pod/probe.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/builder/pod/probe.rs b/crates/stackable-operator/src/builder/pod/probe.rs index c66edf251..76044664c 100644 --- a/crates/stackable-operator/src/builder/pod/probe.rs +++ b/crates/stackable-operator/src/builder/pod/probe.rs @@ -200,7 +200,7 @@ impl ProbeBuilder { Ok(self.with_success_threshold(success_threshold.ceil() as i32)) } - /// After a probe fails `failureThreshold` times in a row, Kubernetes considers that the + /// After a probe fails `failureThreshold` times in a row, Kubernetes considers that the /// overall check has failed: the container is not ready/healthy/live. /// /// Minimum value is 1 second. For the case of a startup or liveness probe, if at least From eee3fafa65f0794a02dd95a1a1910939f109479f Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 8 Sep 2025 09:29:10 +0200 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Malte Sander --- crates/stackable-operator/src/builder/pod/probe.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/stackable-operator/src/builder/pod/probe.rs b/crates/stackable-operator/src/builder/pod/probe.rs index 76044664c..6bb4ebd08 100644 --- a/crates/stackable-operator/src/builder/pod/probe.rs +++ b/crates/stackable-operator/src/builder/pod/probe.rs @@ -204,9 +204,9 @@ impl ProbeBuilder { /// overall check has failed: the container is not ready/healthy/live. /// /// Minimum value is 1 second. For the case of a startup or liveness probe, if at least - /// `failureThreshold`` probes have failed, Kubernetes treats the container as unhealthy and + /// `failureThreshold` probes have failed, Kubernetes treats the container as unhealthy and /// triggers a restart for that specific container. The kubelet honors the setting of - /// `terminationGracePeriodSeconds`` for that container. For a failed readiness probe, the + /// `terminationGracePeriodSeconds` for that container. For a failed readiness probe, the /// kubelet continues running the container that failed checks, and also continues to run more /// probes; because the check failed, the kubelet sets the `Ready` condition on the Pod to /// `false`. @@ -238,8 +238,8 @@ impl ProbeBuilder { /// failed container, and then forcing the container runtime to stop that container. /// /// The default (if this function is not called) is to inherit the Pod-level value for - /// `terminationGracePeriodSeconds`` (30 seconds if not specified), and the minimum value is - /// 1 second. See probe-level `terminationGracePeriodSeconds`` for more detail. + /// `terminationGracePeriodSeconds` (30 seconds if not specified), and the minimum value is + /// 1 second. See probe-level `terminationGracePeriodSeconds` for more detail. pub fn with_termination_grace_period(mut self, termination_grace_period: Duration) -> Self { self.termination_grace_period = Some(termination_grace_period); self