fix: make PVC Bound timeout configurable in wait_for_dv_success

[Ahmad-Hafe]

Short description:

some PVC take more than 1 Min to bound, increase time for 2Min
addin the ability to Increase the PVC Bound wait timeout from 1 minute to X minutes to handle delayed binding in specific scenarios.

More details:

Some PVCs take slightly longer than 1 minute to reach the Bound phase, especially when the scratch PVC is restarted. In such cases, the main PVC may temporarily report the following condition:

cdi.kubevirt.io/storage.condition.running.message: scratch space required and none found
cdi.kubevirt.io/storage.condition.running.reason: Scratch space required

This delay can cause test flakes, particularly in T2 test cases. To address this, the wait_for_dv_success method now accepts a configurable pvc_wait_for_bound_timeout argument (defaulting to 1 minute), allowing tests to extend this timeout as needed.

What this PR does / why we need it:

Introduces a configurable PVC Bound wait timeout (pvc_wait_for_bound_timeout) to avoid flakiness.

Fixes some T2 test failures caused by PVCs taking slightly longer to bind.

Which issue(s) this PR fixes:

https://issues.redhat.com/browse/CNV-66509

Special notes for reviewer:

https://redhat-internal.slack.com/archives/C01B610HL81/p1755006925664399

Bug:

Summary by CodeRabbit

• Refactor
  • Made the post-completion storage Bound wait configurable via a new optional timeout setting, improving reliability and flexibility when storage provisioning is slow.
  • Updated examples/tests to use the new configurable timeout.

[coderabbitai]

Caution

Review failed

The head commit changed during the review from 3f59fa9 to 2214fcc.

Walkthrough

Added a new optional parameter pvc_wait_for_bound_timeout to DataVolume.wait_for_dv_success (default TIMEOUT_1MINUTE); docstring and implementation updated so the PVC Bound wait uses this parameter; example/test call updated to use the new parameter. No other control-flow or import changes.

Changes

Cohort / File(s)	Change Summary
DataVolume API change ocp_resources/datavolume.py	Added pvc_wait_for_bound_timeout parameter to wait_for_dv_success (default TIMEOUT_1MINUTE); updated docstring; changed implementation to use the new timeout for waiting the PVC to reach Bound; example/test usage updated to pass pvc_wait_for_bound_timeout to wait_for_status.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[redhat-qe-bot]

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

• Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
• Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
• Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
• Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
• Branch Labeling: Branch-specific labels are applied to track the target branch
• Auto-verification: Auto-verified users have their PRs automatically marked as verified

📋 Available Commands

PR Status Management

• /wip - Mark PR as work in progress (adds WIP: prefix to title)
• /wip cancel - Remove work in progress status
• /hold - Block PR merging (approvers only)
• /hold cancel - Unblock PR merging
• /verified - Mark PR as verified
• /verified cancel - Remove verification status

Review & Approval

• /lgtm - Approve changes (looks good to me)
• /approve - Approve PR (approvers only)
• /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
• /assign-reviewers - Assign reviewers based on OWNERS file
• /assign-reviewer @username - Assign specific reviewer
• /check-can-merge - Check if PR meets merge requirements

Testing & Validation

• /retest tox - Run Python test suite with tox
• /retest python-module-install - Test Python package installation
• /retest conventional-title - Validate commit message format
• /retest all - Run all available tests

Container Operations

• /build-and-push-container - Build and push container image (tagged with PR number)
  • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

• /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
  • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

• /<label-name> - Add a label to the PR
• /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

1. Approval: /approve from at least one approver
2. LGTM Count: Minimum 0 /lgtm from reviewers
3. Status Checks: All required status checks must pass
4. No Blockers: No WIP, hold, or conflict labels
5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

• myakove
• rnetser

Reviewers:

• dbasunag
• myakove
• rnetser

Available Labels

• hold
• verified
• wip
• lgtm
• approve
• automerge

💡 Tips

• WIP Status: Use /wip when your PR is not ready for review
• Verification: The verified label is automatically removed on each new commit
• Cherry-picking: Cherry-pick labels are processed when the PR is merged
• Container Builds: Container images are automatically tagged with the PR number
• Permission Levels: Some commands require approver permissions
• Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

[Ahmad-Hafe]

/verified

[Ahmad-Hafe]

/retest conventional-title

[Ahmad-Hafe]

/verified

[Ahmad-Hafe]

/wip

[coderabbitai]

Actionable comments posted: 0

🔭 Outside diff range comments (1)

ocp_resources/datavolume.py (1)

269-278: Preserve backward compatibility by making pvc_wait_for_bound_timeout keyword-only

Our search found no internal calls to wait_for_dv_success with three or more positional arguments. However, to avoid breaking any external callers that rely on positional arguments, move the new parameter after *stop_status_func_args:

 def wait_for_dv_success(
     self,
     timeout=TIMEOUT_10MINUTES,
     failure_timeout=TIMEOUT_2MINUTES,
-    pvc_wait_for_bound_timeout=TIMEOUT_1MINUTE,
     dv_garbage_collection_enabled=False,
     stop_status_func=None,
-    *stop_status_func_args,
+    *stop_status_func_args,
+    pvc_wait_for_bound_timeout=TIMEOUT_1MINUTE,
     **stop_status_func_kwargs,
 ):

🧹 Nitpick comments (2)

ocp_resources/datavolume.py (2)

282-287: Clarify units/defaults in docstring for the new argument and existing ones.

Explicitly state units and that pvc_wait_for_bound_timeout is keyword-only. This reduces misuse and improves readability.

Apply this docstring tweak:

-            timeout (int):  Time to wait for the DataVolume to succeed.
-            failure_timeout (int): Time to wait for the DataVolume to have not Pending/None status
-            pvc_wait_for_bound_timeout (int): Time to wait for the PVC to reach 'Bound' status.
-            dv_garbage_collection_enabled (bool, default: False): if True, expect that DV will disappear after success
+            timeout (int, seconds): Time to wait for the DataVolume to succeed.
+            failure_timeout (int, seconds): Time to wait for the DataVolume to have not Pending/None status.
+            dv_garbage_collection_enabled (bool, default: False): If True, expect that DV will disappear after success.
+            pvc_wait_for_bound_timeout (int, seconds, keyword-only): Time to wait for the PVC to reach 'Bound' status (default: 60s).

---

298-301: Docstring return type is incorrect; function returns None or raises.

wait_for_status returns None and raises TimeoutExpiredError on failure. The docstring claiming a bool return is misleading.

Apply this docstring correction:

-        Returns:
-            bool: True if DataVolume succeeded.
+        Returns:
+            None: Waits until success. Raises TimeoutExpiredError on timeout/failure.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f68e6e and 9b6c14e.

📒 Files selected for processing (1)

• ocp_resources/datavolume.py (3 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

ocp_resources/datavolume.py (2)

ocp_resources/resource.py (4)

• wait_for_status (832-880)
• status (937-947)
• Status (288-305)
• Status (317-320)

ocp_resources/persistent_volume_claim.py (2)

• PersistentVolumeClaim (7-115)
• Status (14-15)

🔇 Additional comments (1)

ocp_resources/datavolume.py (1)

329-331: LGTM: PVC Bound wait now respects the configurable timeout.

Using pvc_wait_for_bound_timeout here properly decouples DV success timeout from PVC Bound timeout.

[Ahmad-Hafe]

/verified

[Ahmad-Hafe]

/lgtm

[dalia-frank]

/lgtm

[rnetser]

/approve
/lgtm
/verified