WIP: storage, ipv6: CDI upload on IPv6 single-stack clusters#3871
Open
azhivovk wants to merge 2 commits intoRedHatQE:mainfrom
Open
WIP: storage, ipv6: CDI upload on IPv6 single-stack clusters#3871azhivovk wants to merge 2 commits intoRedHatQE:mainfrom
azhivovk wants to merge 2 commits intoRedHatQE:mainfrom
Conversation
Signed-off-by: Asia Khromov <azhivovk@redhat.com>
Contributor
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds IPv6 single-stack cluster detection to the test configuration via a runtime check of the cluster's serviceNetwork, storing the result in py_config. It also introduces a new fixture for obtaining CDI upload proxy URLs in IPv6 single-stack environments, complete with port-forwarding setup and teardown. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
Contributor
Author
|
/wip |
openshift-virtualization-qe-bot-5
requested review from
duyanyan,
rnetser and
vsibirsk
openshift-virtualization-qe-bot-5
requested review from
jpeimer and
kshvaika
openshift-virtualization-qe-bot-3
changed the title
openshift-virtualization-qe-bot-5
requested review from
dalia-frank and
dshchedr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/storage/cdi_upload/test_upload_virtctl.py`:
- Around line 8-9: The test imports the time module but the reviewer notes
time.sleep should be removed when switching to a subprocess.Popen readiness
check; remove the unused import and replace any time.sleep usage in
tests/storage/cdi_upload/test_upload_virtctl.py with a proper readiness loop
that polls the subprocess started with subprocess.Popen (or update the fixture
that starts the process) until the expected output/port is ready, ensuring no
dead time.sleep and removing the now-unused import time.
- Around line 58-81: The fixture cdi_uploadproxy_url is treated as a generator
(contains yield) so replace the early return with "yield None" to avoid pytest
generator termination; replace the time.sleep(2) with a retry/wait loop (e.g.,
timeout_sampler or similar polling) that checks the port-forward readiness;
start the port-forward using subprocess.Popen with an args list (no shell=True)
in cdi_uploadproxy_url so you capture the process handle, store that handle (or
PID) and on teardown kill/terminate that specific process instead of running a
broad "pkill -f 'oc port-forward'".
Comment on lines
8
to
9
| import subprocess | ||
| import time |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LOW: import time can be removed if you adopt the subprocess.Popen approach.
If the time.sleep is replaced with a proper readiness check or kept only temporarily, the time import would become dead code once removed. Keep this in sync with the fixture fix.
🤖 Prompt for AI Agents
In `@tests/storage/cdi_upload/test_upload_virtctl.py` around lines 8 - 9, The test
imports the time module but the reviewer notes time.sleep should be removed when
switching to a subprocess.Popen readiness check; remove the unused import and
replace any time.sleep usage in tests/storage/cdi_upload/test_upload_virtctl.py
with a proper readiness loop that polls the subprocess started with
subprocess.Popen (or update the fixture that starts the process) until the
expected output/port is ready, ensuring no dead time.sleep and removing the
now-unused import time.
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Add port-forward fixture to enable virtctl image upload on IPv6 single-stack clusters, where the upload proxy route resolves to unreachable IPv4 addresses. Signed-off-by: Asia Khromov <azhivovk@redhat.com>
azhivovk
force-pushed
the
add_ipv6_proxy_virtctl_img_upload_pvc
branch
from
0c4baa5 to
0cdb1db
Compare
Contributor
Author
|
Change: use popen |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot-3
removed
the
changes-requested-coderabbitai[bot]
label
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:failed
|
D/S test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add port-forward fixture to enable virtctl image upload
on IPv6 single-stack clusters, where the upload proxy route
resolves to unreachable IPv4 addresses.
Summary by CodeRabbit
Release Notes