diff --git a/CHANGELOG.md b/CHANGELOG.md index 12117d31..ee350d24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,12 +21,18 @@ All notable changes to this project will be documented in this file. so a Pod kept the certificate it was given at startup and eventually ran on an expired one indefinitely, even though cert-manager had long since renewed it in the Secret. The restart is scheduled halfway between cert-manager's own `status.renewalTime` and the expiry ([#752]). +- Skip the `PodListeners`, `Listener` and `ListenerClass` lookups when the served Pod is being + deleted, and issue the certificate without listener addresses. During namespace deletion these + objects could be garbage-collected while `NodePublishVolume` was still waiting for them, blocking + the Pod's termination indefinitely (holding `pvc-protection`). A terminating Pod no longer needs + listener-addressed certificates ([#755]). [#730]: https://github.com/stackabletech/secret-operator/pull/730 [#735]: https://github.com/stackabletech/secret-operator/pull/735 [#736]: https://github.com/stackabletech/secret-operator/pull/736 [#743]: https://github.com/stackabletech/secret-operator/pull/743 [#752]: https://github.com/stackabletech/secret-operator/pull/752 +[#755]: https://github.com/stackabletech/secret-operator/pull/755 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/backend/mod.rs b/rust/operator-binary/src/backend/mod.rs index f668fa54..1331e27c 100644 --- a/rust/operator-binary/src/backend/mod.rs +++ b/rust/operator-binary/src/backend/mod.rs @@ -285,6 +285,10 @@ impl SecretVolumeSelector { pod_listeners: listener_addresses.source.clone(), })? .to_vec(), + // The listener addresses are deliberately not fetched for a terminating Pod (see + // `PodInfo::from_pod`), which no longer needs listener-addressed certificates. + // Contribute no addresses for this scope instead of failing. + None if pod_info.is_being_deleted => Vec::new(), None => return ListenerAddressesNotFetchedSnafu.fail(), }, }) @@ -444,4 +448,75 @@ mod tests { ) .unwrap(); } + + fn listener_scoped_selector() -> SecretVolumeSelector { + let mut map = required_fields_map(); + map.insert( + "secrets.stackable.tech/scope".to_owned(), + "listener-volume=my-listener".to_owned(), + ); + SecretVolumeSelector::deserialize::>( + map.into_deserializer(), + ) + .unwrap() + } + + fn pod_info_without_listener_addresses(is_being_deleted: bool) -> pod_info::PodInfo { + pod_info::PodInfo { + pod_ips: Vec::new(), + pod_name: "my-pod".to_owned(), + service_name: None, + namespace: "my-namespace".to_owned(), + node_name: "my-node".to_owned(), + node_ips: Vec::new(), + // No addresses were fetched, mirroring the terminating-Pod path in `PodInfo::from_pod`. + listener_addresses: None, + kubernetes_cluster_domain: + stackable_operator::commons::networking::DomainName::try_from("cluster.local") + .unwrap(), + scheduling: SchedulingPodInfo { + namespace: "my-namespace".to_owned(), + volume_listener_names: HashMap::from([( + "my-listener".to_owned(), + "my-listener".to_owned(), + )]), + has_node_scope: false, + }, + is_being_deleted, + } + } + + /// A terminating Pod whose listener addresses were deliberately not fetched contributes no + /// addresses for a listener scope instead of failing, so publishing (and thus terminating) the + /// Pod is not blocked. See . + #[test] + fn scope_addresses_for_listener_scope_of_terminating_pod_is_empty() { + let selector = listener_scoped_selector(); + let pod_info = pod_info_without_listener_addresses(true); + let scope = SecretScope::ListenerVolume { + name: "my-listener".to_owned(), + }; + + let addresses = selector.scope_addresses(&pod_info, &scope).unwrap(); + + assert!(addresses.is_empty()); + } + + /// A running Pod that is missing listener addresses is still an error: we must not silently + /// issue a certificate without the listener SANs it is supposed to carry. + #[test] + fn scope_addresses_for_listener_scope_of_running_pod_without_addresses_errors() { + let selector = listener_scoped_selector(); + let pod_info = pod_info_without_listener_addresses(false); + let scope = SecretScope::ListenerVolume { + name: "my-listener".to_owned(), + }; + + let err = selector.scope_addresses(&pod_info, &scope).unwrap_err(); + + assert!(matches!( + err, + ScopeAddressesError::ListenerAddressesNotFetched + )); + } } diff --git a/rust/operator-binary/src/backend/pod_info.rs b/rust/operator-binary/src/backend/pod_info.rs index 2c1e44d2..67e8fdac 100644 --- a/rust/operator-binary/src/backend/pod_info.rs +++ b/rust/operator-binary/src/backend/pod_info.rs @@ -112,6 +112,14 @@ pub struct PodInfo { pub listener_addresses: Option, pub kubernetes_cluster_domain: DomainName, pub scheduling: SchedulingPodInfo, + + /// Whether the served Pod is being deleted (has a `.metadata.deletionTimestamp`). + /// + /// A terminating Pod's [`PodListeners`](listener::v1alpha1::PodListeners) object may already + /// have been garbage-collected (e.g. during namespace deletion), so we skip fetching it and + /// issue without listener addresses instead of blocking termination on an object that will + /// never reappear. + pub is_being_deleted: bool, } impl PodInfo { @@ -134,12 +142,26 @@ impl PodInfo { .with_context(|_| GetNodeSnafu { node: ObjectRef::new(&node_name), })?; - let scheduling = SchedulingPodInfo::from_pod(client, &pod, scopes).await?; - let listener_addresses = if !scheduling.volume_listener_names.is_empty() { - Some(ListenerAddresses::fetch_for_pod(client, &pod, &scheduling, scopes).await?) - } else { + let is_being_deleted = pod.metadata.deletion_timestamp.is_some(); + let scheduling = + SchedulingPodInfo::from_pod(client, &pod, scopes, is_being_deleted).await?; + let listener_addresses = if scheduling.volume_listener_names.is_empty() { // We don't care about the listener addresses if there is no listener scope, so we can save the API call None + } else if is_being_deleted { + // The Pod is terminating. Its PodListeners object may already have been + // garbage-collected (e.g. during namespace deletion), and a Pod that is going away no + // longer needs listener-addressed certificates. Skip the lookup so that publishing the + // volume (and thus deleting the Pod) is not blocked waiting for an object that will + // never reappear. See https://github.com/stackabletech/secret-operator/issues/720 + tracing::warn!( + pod.name = %pod_name, + pod.namespace = %namespace, + "Pod is being deleted, skipping PodListeners lookup and issuing secret without listener addresses" + ); + None + } else { + Some(ListenerAddresses::fetch_for_pod(client, &pod, &scheduling, scopes).await?) }; Ok(Self { // This will generally be empty, since Kubernetes assigns pod IPs *after* CSI plugins are successful @@ -175,6 +197,7 @@ impl PodInfo { listener_addresses, kubernetes_cluster_domain: client.kubernetes_cluster_info.cluster_domain.clone(), scheduling, + is_being_deleted, }) } } @@ -216,6 +239,10 @@ impl SchedulingPodInfo { client: &stackable_operator::client::Client, pod: &Pod, scopes: &[SecretScope], + // Whether the Pod is being deleted (see [`PodInfo::is_being_deleted`]). A terminating Pod's + // `Listener` objects may already have been garbage-collected, so we avoid the lookups that + // would otherwise fail and block termination. + is_being_deleted: bool, ) -> Result { use from_pod_error::*; let pod_name = pod.metadata.name.clone().context(NoPodNameSnafu)?; @@ -280,12 +307,18 @@ impl SchedulingPodInfo { }) .collect::>(); let has_node_scope = scopes.contains(&SecretScope::Node) - || trystream_any(futures::stream::iter(volume_listener_pvcs).then( - |(listener_volume, _, pvc)| { - listener_pvc_is_node_scoped(client, &namespace, listener_volume, pvc) - }, - )) - .await?; + // Determining whether a listener scope is node-equivalent requires fetching the + // `Listener` and `ListenerClass`. For a terminating Pod those may already have been + // garbage-collected (e.g. during namespace deletion), and the Pod no longer needs an + // accurate node scope, so fall back to the statically-declared scopes instead of + // blocking termination on a lookup that will never succeed. + || (!is_being_deleted + && trystream_any(futures::stream::iter(volume_listener_pvcs).then( + |(listener_volume, _, pvc)| { + listener_pvc_is_node_scoped(client, &namespace, listener_volume, pvc) + }, + )) + .await?); Ok(SchedulingPodInfo { volume_listener_names, has_node_scope, diff --git a/rust/operator-binary/src/csi_server/controller.rs b/rust/operator-binary/src/csi_server/controller.rs index 6e7ace45..5f7cf059 100644 --- a/rust/operator-binary/src/csi_server/controller.rs +++ b/rust/operator-binary/src/csi_server/controller.rs @@ -240,9 +240,15 @@ impl Controller for SecretProvisionerController { .get::(&selector.pod, &selector.namespace) .await .context(GetPodSnafu)?; - let pod_info = SchedulingPodInfo::from_pod(&self.client, &pod, &selector.scope) - .await - .context(ParsePodSnafu)?; + let is_being_deleted = pod.metadata.deletion_timestamp.is_some(); + let pod_info = SchedulingPodInfo::from_pod( + &self.client, + &pod, + &selector.scope, + is_being_deleted, + ) + .await + .context(ParsePodSnafu)?; let backend = backend::dynamic::from_selector(&self.client, &selector) .await