fix: drop pvc claims if not needed anymore

CHANGELOG.md

@@ -15,10 +15,12 @@ All notable changes to this project will be documented in this file.
   the CSI node driver DaemonSet previously ignored in favour of a hardcoded name.
   `serviceAccount.create=false` now requires `serviceAccount.name`; it used to fall back to the
   namespace default ServiceAccount, which lacks the operator ClusterRole ([#736]).
+- `CreateVolume` no longer returns gRPC codes that make external-provisioner retry indefinitely ([#743]).
 
 [#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
 
 ## [26.7.0] - 2026-07-21
 

rust/operator-binary/src/csi_server/controller.rs

@@ -6,9 +6,10 @@ use stackable_operator::{
     k8s_openapi::api::core::v1::{PersistentVolumeClaim, Pod},
     kube::runtime::reflector::ObjectRef,
 };
-use tonic::{Request, Response, Status};
+use tonic::{Code, Request, Response, Status};
 use uuid::Uuid;
 
+use super::log_if_endpoint_error;
 use crate::{
     backend::{
         self, InternalSecretVolumeSelectorParams, SecretBackendError, SecretVolumeSelector,
@@ -70,23 +71,64 @@ enum CreateVolumeError {
     NoMatchingNode,
 }
 
+/// Rewrites the gRPC codes that `external-provisioner` treats as "the operation may still be
+/// running, call CreateVolume again later" into a terminal code.
+///
+/// Such a code makes the provisioner park the PVC in an in-memory map that is only ever cleared
+/// on success or on a terminal code.
+///
+/// [`Controller::create_volume`] never leaves anything running in the background. It creates no
+/// state per volume, so none of these codes may escape it.
+/// The backends still use them for [`Controller::node_publish_volume`],
+/// where they are correct, and so must be rewritten here rather than at their source.
+fn make_terminal(code: Code) -> Code {
+    match code {
+        Code::Unavailable | Code::Aborted | Code::Cancelled | Code::DeadlineExceeded => {
+            Code::Internal
+        }
+        code => code,
+    }
+}
+
 impl From<CreateVolumeError> for Status {
     fn from(err: CreateVolumeError) -> Self {
         let full_msg = error_full_message(&err);
         // Convert to an appropriate tonic::Status representation and include full error message
-        match err {
-            CreateVolumeError::InvalidParams { .. } => Status::invalid_argument(full_msg),
-            CreateVolumeError::FindPvc { .. } => Status::unavailable(full_msg),
-            CreateVolumeError::ResolveOwnerPod { .. } => Status::failed_precondition(full_msg),
-            CreateVolumeError::GetPod { .. } => Status::unavailable(full_msg),
-            CreateVolumeError::ParsePod { .. } => Status::failed_precondition(full_msg),
-            CreateVolumeError::InvalidSecretSelector { .. } => {
-                Status::failed_precondition(full_msg)
-            }
-            CreateVolumeError::InitBackend { source } => Status::new(source.grpc_code(), full_msg),
-            CreateVolumeError::FindNodes { source } => Status::new(source.grpc_code(), full_msg),
-            CreateVolumeError::NoMatchingNode => Status::unavailable(full_msg),
+        let raw_code = match err {
+            CreateVolumeError::InvalidParams { .. } => Code::InvalidArgument,
+            CreateVolumeError::FindPvc { .. } => Code::FailedPrecondition,
+            CreateVolumeError::ResolveOwnerPod { .. } => Code::FailedPrecondition,
+            CreateVolumeError::GetPod { .. } => Code::FailedPrecondition,
+            CreateVolumeError::ParsePod { .. } => Code::FailedPrecondition,
+            CreateVolumeError::InvalidSecretSelector { .. } => Code::FailedPrecondition,
+            CreateVolumeError::InitBackend { source } => source.grpc_code(),
+            CreateVolumeError::FindNodes { source } => source.grpc_code(),
+            // CSI defines ResourceExhausted as "unable to provision in accessible_topology", which
+            // describes this case, but external-provisioner turns it into ProvisioningReschedule
+            // whenever the volume has a selected node, so it strips the node annotation and the
+            // scheduler picks another one. No node ever satisfies the scopes here, so the retry is
+            // futile.
+            // Additionally that path is not rate limited: the claim is forgotten rather than
+            // requeued, leaving the scheduler to re-trigger it immediately.
+            //
+            // ResourceExhausted would only stay dormant today because the operator has no `update` on
+            // persistentvolumeclaims and the annotation delete therefore fails (see roles.yaml).
+            // FailedPrecondition does not depend on that.
+            CreateVolumeError::NoMatchingNode => Code::FailedPrecondition,
+        };
+
+        // Applied to every variant, including the codes inherited from the backends (which are
+        // only known at runtime), so that no future variant can reintroduce the retry loop.
+        let code = make_terminal(raw_code);
+        if code != raw_code {
+            tracing::debug!(
+                grpc.code.original = ?raw_code,
+                grpc.code.returned = ?code,
+                "Mapped CreateVolume error code:"
+            );
         }
+
+        Status::new(code, full_msg)
     }
 }
 
@@ -184,52 +226,59 @@ impl Controller for SecretProvisionerController {
         request: Request<csi::v1::CreateVolumeRequest>,
     ) -> Result<Response<csi::v1::CreateVolumeResponse>, Status> {
         use create_volume_error::*;
-        let request = request.into_inner();
-        let params = CreateVolumeParams::deserialize(request.parameters.into_deserializer())
-            .context(InvalidParamsSnafu)?;
-        let (pvc_selector, selector) = self.get_pvc_secret_selector(&params).await?;
-
-        let pod = self
-            .client
-            .get::<Pod>(&selector.pod, &selector.namespace)
-            .await
-            .context(GetPodSnafu)?;
-        let pod_info = SchedulingPodInfo::from_pod(&self.client, &pod, &selector.scope)
-            .await
-            .context(ParsePodSnafu)?;
-
-        let backend = backend::dynamic::from_selector(&self.client, &selector)
-            .await
-            .context(create_volume_error::InitBackendSnafu)?;
-        let accessible_topology = match backend
-            .get_qualified_node_names(&selector, pod_info)
-            .await
-            .context(create_volume_error::FindNodesSnafu)?
-        {
-            // No node constraints apply to this volume, so allow any topology
-            None => Vec::new(),
-            // No nodes match the constraints on this volume, so fail
-            Some(nodes) if nodes.is_empty() => {
-                return Err(create_volume_error::NoMatchingNodeSnafu.build().into());
+        log_if_endpoint_error(
+            "failed to create volume",
+            async move {
+                let request = request.into_inner();
+                let params =
+                    CreateVolumeParams::deserialize(request.parameters.into_deserializer())
+                        .context(InvalidParamsSnafu)?;
+                let (pvc_selector, selector) = self.get_pvc_secret_selector(&params).await?;
+
+                let pod = self
+                    .client
+                    .get::<Pod>(&selector.pod, &selector.namespace)
+                    .await
+                    .context(GetPodSnafu)?;
+                let pod_info = SchedulingPodInfo::from_pod(&self.client, &pod, &selector.scope)
+                    .await
+                    .context(ParsePodSnafu)?;
+
+                let backend = backend::dynamic::from_selector(&self.client, &selector)
+                    .await
+                    .context(create_volume_error::InitBackendSnafu)?;
+                let accessible_topology = match backend
+                    .get_qualified_node_names(&selector, pod_info)
+                    .await
+                    .context(create_volume_error::FindNodesSnafu)?
+                {
+                    // No node constraints apply to this volume, so allow any topology
+                    None => Vec::new(),
+                    // No nodes match the constraints on this volume, so fail
+                    Some(nodes) if nodes.is_empty() => {
+                        return Err(create_volume_error::NoMatchingNodeSnafu.build().into());
+                    }
+                    // Matching nodes were found, only allow scheduling to them
+                    Some(nodes) => nodes
+                        .into_iter()
+                        .map(|node| Topology {
+                            segments: [(TOPOLOGY_NODE.to_string(), node)].into(),
+                        })
+                        .collect(),
+                };
+                Ok(Response::new(CreateVolumeResponse {
+                    volume: Some(Volume {
+                        // We don't care about the volume ID ourselves, but generate something unique
+                        // in case anyone else relies on it for some kind of deduplication
+                        volume_id: Uuid::new_v4().to_string(),
+                        accessible_topology,
+                        volume_context: pvc_selector.into_iter().collect(),
+                        ..Volume::default()
+                    }),
+                }))
             }
-            // Matching nodes were found, only allow scheduling to them
-            Some(nodes) => nodes
-                .into_iter()
-                .map(|node| Topology {
-                    segments: [(TOPOLOGY_NODE.to_string(), node)].into(),
-                })
-                .collect(),
-        };
-        Ok(Response::new(CreateVolumeResponse {
-            volume: Some(Volume {
-                // We don't care about the volume ID ourselves, but generate something unique
-                // in case anyone else relies on it for some kind of deduplication
-                volume_id: Uuid::new_v4().to_string(),
-                accessible_topology,
-                volume_context: pvc_selector.into_iter().collect(),
-                ..Volume::default()
-            }),
-        }))
+            .await,
+        )
     }
 
     async fn delete_volume(
@@ -325,3 +374,44 @@ struct CreateVolumeParams {
     #[serde(rename = "csi.storage.k8s.io/pvc/namespace")]
     pvc_namespace: String,
 }
+
+#[cfg(test)]
+mod tests {
+    use tonic::Code;
+
+    use super::make_terminal;
+
+    /// Every code `external-provisioner` reads as "may still be running in the background".
+    const RETRIED_FOREVER: [Code; 4] = [
+        Code::Unavailable,
+        Code::Aborted,
+        Code::Cancelled,
+        Code::DeadlineExceeded,
+    ];
+
+    /// Every variant of [`CreateVolumeError`] funnels through [`make_terminal`], including the
+    /// codes inherited from the backends, so covering the whole code space here covers every
+    /// error `CreateVolume` can produce.
+    #[test]
+    fn create_volume_never_returns_a_retried_forever_code() {
+        for raw in 0..=16 {
+            let code = Code::from_i32(raw);
+            assert!(
+                !RETRIED_FOREVER.contains(&make_terminal(code)),
+                "{code:?} must not be returned by CreateVolume, it makes external-provisioner \
+                 retry the PVC forever (issue #722)"
+            );
+        }
+    }
+
+    /// The rewrite must only touch the codes that cause the retry loop.
+    #[test]
+    fn make_terminal_leaves_other_codes_alone() {
+        for raw in 0..=16 {
+            let code = Code::from_i32(raw);
+            if !RETRIED_FOREVER.contains(&code) {
+                assert_eq!(make_terminal(code), code);
+            }
+        }
+    }
+}

rust/operator-binary/src/csi_server/mod.rs

@@ -1,3 +1,17 @@
 pub mod controller;
 pub mod identity;
 pub mod node;
+
+/// Logs the error returned by a CSI endpoint, if any.
+///
+/// CSI errors are otherwise only visible to whoever called us (the Kubelet, or the
+/// external-provisioner sidecar), never in our own logs.
+fn log_if_endpoint_error<T, E: std::error::Error + 'static>(
+    error_msg: &str,
+    res: Result<T, E>,
+) -> Result<T, E> {
+    if let Err(err) = &res {
+        tracing::warn!(error = err as &dyn std::error::Error, "{error_msg}");
+    }
+    res
+}

rust/operator-binary/src/csi_server/node.rs

@@ -20,7 +20,7 @@ use tokio::{
 };
 use tonic::{Request, Response, Status};
 
-use super::controller::TOPOLOGY_NODE;
+use super::{controller::TOPOLOGY_NODE, log_if_endpoint_error};
 use crate::{
     backend::{
         self, SecretBackendError, SecretContents, SecretVolumeSelector,
@@ -496,13 +496,3 @@ impl Node for SecretProvisionerNode {
         }))
     }
 }
-
-fn log_if_endpoint_error<T, E: std::error::Error + 'static>(
-    error_msg: &str,
-    res: Result<T, E>,
-) -> Result<T, E> {
-    if let Err(err) = &res {
-        tracing::warn!(error = err as &dyn std::error::Error, "{error_msg}");
-    }
-    res
-}