reconcile: wait till PVC is in bound phase

[AndrewChubatiuk]

fixes #1970

Summary by cubic

Wait for PVCs to reach the Bound phase before proceeding in reconcile and during StatefulSet PVC expansion. This removes race conditions where pods start while PVCs are still Pending or terminating.

• Bug Fixes
  • Added waitForPVCBound with 1s polling and a 5s timeout; respects PVC generation and errors on unexpected phases or termination.
  • Wrapped PVC create/update in a conflict-retry flow and skipped updates for PVCs with a non-zero DeletionTimestamp.
  • Invoked the wait after PVC reconcile and in updateSTSPVC to ensure expansions complete before continuing.

^{Written for commit 83d01f0. Summary will update on new commits.}

[vrutkovs]

Lets add unit tests for this function

[vrutkovs]

Not sure why we need generation here at all - and why would PVC generation match the deployment?

[cubic-dev-ai]

4 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/controller/operator/factory/reconcile/reconcile.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/reconcile.go:27">
P1: The new PVC wait uses a hardcoded 5s timeout, so PVC binding can fail much earlier than the operator's configured readiness deadlines.</violation>
</file>

<file name="internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go:124">
P1: `waitForPVCBound` is called with the StatefulSet name instead of the PVC name, so it polls a non-existent PVC and times out.</violation>
</file>

<file name="internal/controller/operator/factory/reconcile/pvc.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/pvc.go:51">
P1: Terminating PVCs no longer short-circuit successfully; this unconditional wait turns the previous skip path into a reconcile error.</violation>

<violation number="2" location="internal/controller/operator/factory/reconcile/pvc.go:51">
P1: Waiting for PVCs to reach `Bound` here can block valid `WaitForFirstConsumer` claims before their Deployment is created.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: The new PVC wait uses a hardcoded 5s timeout, so PVC binding can fail much earlier than the operator's configured readiness deadlines.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/reconcile/reconcile.go, line 27:

<comment>The new PVC wait uses a hardcoded 5s timeout, so PVC binding can fail much earlier than the operator's configured readiness deadlines.</comment>

<file context>
@@ -23,6 +23,8 @@ import (
 
 var (
+	pvcWaitBoundIntervalCheck = 1 * time.Second
+	pvcWaitReadyTimeout       = 5 * time.Second
 	podWaitReadyIntervalCheck = 50 * time.Millisecond
 	appWaitReadyDeadline      = 5 * time.Second
</file context>

[cubic-dev-ai]

P1: Waiting for PVCs to reach Bound here can block valid WaitForFirstConsumer claims before their Deployment is created.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/reconcile/pvc.go, line 51:

<comment>Waiting for PVCs to reach `Bound` here can block valid `WaitForFirstConsumer` claims before their Deployment is created.</comment>

<file context>
@@ -20,24 +21,58 @@ import (
+	case !existingObj.CreationTimestamp.IsZero():
+		generation = existingObj.Generation
 	}
+	return waitForPVCBound(ctx, rclient, nsn, generation)
+}
 
</file context>