verify that idled PVCs are deleted#1249
Conversation
WalkthroughAdds PVC creation and a wait-for-PVC-deletion step to an existing e2e test and introduces a helper method to poll for PVC deletion in test support. Tests now create a PVC and Pod alongside a DataVolume and wait for PVC deletion during teardown. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/e2e/parallel/user_workloads_test.go`:
- Around line 114-117: The test is waiting for a PVC deletion using
WaitUntilDataVolumeDeleted with dataVolumeRes and the wrong PVC name; change the
wait to target the core PVC resource and correct the name: use
memberAwait.Client (typed client) to wait for deletion of the
PersistentVolumeClaim named "test-idler-pvc-pvc" (or construct the name via
createPvcWithPod's name + "-pvc"), or if you prefer dynamic client use the PVC
GVR {Group:"", Version:"v1", Resource:"persistentvolumeclaims"}; replace the
WaitUntilDataVolumeDeleted call with a proper wait-for-deletion against the PVC
using memberAwait.Client or the correct GVR so the test observes actual PVC
removal.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/e2e/parallel/user_workloads_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (1)
test/e2e/parallel/user_workloads_test.go (1)
177-179: LGTM with note on naming convention.The PVC and Pod creation logic is correct. Note that the PVC will be named
"test-idler-pvc-pvc"(with-pvcsuffix appended bycreatePvcWithPod), which needs to match the name used in the deletion wait at line 116.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@testsupport/wait/member.go`:
- Line 1591: The current unconditional t.Logf("failed waiting for PVC '%s' to be
deleted in namespace '%s': %q", name, namespace, pvc) prints a failure even on
success; update the wait logic to only log that failure message inside the
error/failure branch (where the error variable is non-nil) and otherwise log a
success/info message (e.g., "succeeded waiting for PVC ... to be deleted") or
omit logging on success; ensure you reference the same variables pvc, name,
namespace and the t.Logf call so the fix replaces the unconditional log with
conditional logging based on the wait result.
♻️ Duplicate comments (1)
test/e2e/parallel/user_workloads_test.go (1)
115-117: Bug: PVC name mismatch - test will timeout waiting for non-existent resource.The PVC is created with name
"test-idler-pvc" + "-pvc"="test-idler-pvc-pvc"at line 178 (viacreatePvcWithPodwhich appends-pvcto the name at line 423), but the wait expects"test-idler-pvc".🐛 Proposed fix
// Wait for the PVC to be deleted - err = memberAwait.WaitUntilPVCDeleted(t, "test-idler-pvc", idler.Name) + err = memberAwait.WaitUntilPVCDeleted(t, "test-idler-pvc-pvc", idler.Name) require.NoError(t, err)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/e2e/parallel/user_workloads_test.gotestsupport/wait/member.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🔇 Additional comments (1)
test/e2e/parallel/user_workloads_test.go (1)
177-179: LGTM!The PVC and Pod creation logic is correct. The nil owner creates a standalone PVC (not owned by a DataVolume), and the pod count is correctly incremented.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, fbm3307, MatousJobanek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
MatousJobanek
merged commit 8d62db8
into
codeready-toolchain:master
3 participants
paired with codeready-toolchain/member-operator#722
SANDBOX-1573
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.