net, tests, hot-plug: W/A guest-agent deadlock on hot-plugged interfaces - #5584
Conversation
|
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
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
📌 Additional InformationCustom Commands:
For more information, please refer to the project documentation or contact the maintainers. |
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds CNV-77961-gated console fallbacks for hot-plugged interface discovery and IPv4 verification. It moves guest IPv4 parsing into a shared guest helper and updates the IP specification test import. ChangesConsole fallback for hot-plugged interfaces
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (5 passed)
✨ 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 |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
|
/verified Verified by executing the hot-plug tests on 2 iterations - one on a clean code, second with forcing the guest-agent (with #5467) to make sure this W/A works. |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. |
aa8ddc4 to
5e86d58
Compare
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/network/l2_bridge/libl2bridge.py`:
- Around line 230-235: The is_jira_open fallback currently accepts any interface
IPv4 address instead of verifying the requested one. In the branch around
read_guest_interface_ipv4, compare hot_plugged_interface_ip with
IPv4Address(address=ipv4_address), raise IpNotFound on mismatch, and emit the
LOGGER.warning “Verified” message only after the comparison succeeds.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7e0cbb0e-6e53-45b6-9d9d-1e575c7f75be
📒 Files selected for processing (4)
tests/network/l2_bridge/libl2bridge.pytests/network/libs/guest.pytests/network/user_defined_network/ip_specification/libipspec.pytests/network/user_defined_network/ip_specification/test_ip_specification.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
|
change: apply rabbit's validation comment |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/network/l2_bridge/libl2bridge.py`:
- Around line 307-310: Update the warning in hot_plug_interface to state that
console lookup by MAC is being used because Jira issue CNV-77961 is open, rather
than claiming the guest agent failed to report the interface. Preserve the
existing VM, interface, and MAC context.
- Around line 245-249: Update the IpNotFound error message in the static-IP
verification path to add the missing closing parenthesis after the actual
interface IP value, while preserving the existing message content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5c8ae502-10f2-4fc8-a41c-24f4b3421dc3
📒 Files selected for processing (4)
tests/network/l2_bridge/libl2bridge.pytests/network/libs/guest.pytests/network/user_defined_network/ip_specification/libipspec.pytests/network/user_defined_network/ip_specification/test_ip_specification.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
|
Clean rebase detected — no code changes compared to previous head ( |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/verified Verified by running all the affected tests (see previous verification comments). |
CNV-77961 causes guest-agent to stop reporting newly hot-plugged interfaces, failing tests that depend on interface data from VMI status. Work around the bug so tests continue to run until the fix is released. As part of this change, functionality from another package (user_defined_network/ip_specification) was re-used, therefore this package is handled here as well. Signed-off-by: Anat Wax <awax@redhat.com> Assisted-by: Claude <noreply@anthropic.com> Assisted-by: Yossi Segev <ysegev@redhat.com>
|
/verified Verified by running all the affected tests (see previous verification comments). |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git rev-parse HEAD~1)"
echo '=== PR head and changed files ==='
git rev-parse HEAD
git diff --name-status "$base"...HEAD
echo '=== changed declarations ==='
git diff --unified=0 "$base"...HEAD -- tests \
| grep -E '^[+-][[:space:]]*(async[[:space:]]+)?def[[:space:]]+|^[+-][[:space:]]*class[[:space:]]+' \
|| true
echo '=== changed-file diff ==='
git diff --unified=4 "$base"...HEAD -- tests \
| sed -n '1,1000p'
echo '=== outlines of changed source and affected test modules ==='
for file in \
tests/network/l2_bridge/libl2bridge.py \
tests/network/libs/guest.py \
tests/network/user_defined_network/ip_specification/libipspec.py \
tests/network/l2_bridge/test_bridge_nic_hot_plug.py \
tests/network/user_defined_network/ip_specification/test_ip_specification.py; do
if [[ -f "$file" ]]; then
echo "--- $file ---"
ast-grep outline "$file" --items all || true
fi
done
echo '=== callers, fixtures, wrappers, and test definitions ==='
rg -n --type py -C 5 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address|_lookup_hotplugged_iface_via_console)\b|`@pytest`\.(fixture|mark\.(smoke|gating))|^(async )?def test_|^class Test|yield from|autouse\s*=' \
tests || true
echo '=== smoke and gating test modules ==='
rg -l --type py '`@pytest`\.mark\.smoke\b' tests | sort || true
printf '%s\n' '--- gating ---'
rg -l --type py '`@pytest`\.mark\.gating\b' tests | sort || true
echo '=== ancestor conftest references ==='
for file in \
tests/conftest.py \
tests/network/conftest.py \
tests/network/l2_bridge/conftest.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/ip_specification/conftest.py; do
if [[ -f "$file" ]]; then
echo "--- $file ---"
rg -n --type py -C 3 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address)\b|yield from|autouse\s*=' \
"$file" || true
fi
doneLength of output: 2632 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5584"
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
pr_json="$(gh api "repos/$repo/pulls/$pr")"
sha="$(jq -r '.head.sha' <<<"$pr_json")"
echo '=== PR metadata ==='
jq '{head_sha: .head.sha, base_sha: .base.sha, changed_files: .changed_files}' <<<"$pr_json"
echo '=== changed files and changed declarations ==='
gh api --paginate "repos/$repo/pulls/$pr/files?per_page=100" >"$workdir/files.json"
jq -r '.[] | "\(.status)\t\(.filename)"' "$workdir/files.json"
jq -r '.[] | select(.patch != null) | .patch' "$workdir/files.json" \
| grep -E '^[+-][[:space:]]*(async[[:space:]]+)?def[[:space:]]+|^[+-][[:space:]]*class[[:space:]]+' \
|| true
curl --fail --silent --show-error --location \
"https://codeload.github.com/$repo/zip/$sha" \
--output "$workdir/source.zip"
unzip -q "$workdir/source.zip" -d "$workdir"
root="$(find "$workdir" -mindepth 1 -maxdepth 1 -type d | head -n1)"
cd "$root"
echo '=== source outlines ==='
for file in \
tests/network/l2_bridge/libl2bridge.py \
tests/network/libs/guest.py \
tests/network/user_defined_network/ip_specification/libipspec.py \
tests/network/l2_bridge/test_bridge_nic_hot_plug.py \
tests/network/user_defined_network/ip_specification/test_ip_specification.py; do
if [[ -f "$file" ]]; then
echo "--- $file ---"
ast-grep outline "$file" --items all || true
fi
done
echo '=== changed-helper callers, fixtures, wrappers, and tests ==='
rg -n --type py -C 5 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address|_lookup_hotplugged_iface_via_console)\b|`@pytest`\.(fixture|mark\.(smoke|gating))|^(async )?def test_|^class Test|yield from|autouse\s*=' \
tests || true
echo '=== smoke and gating test modules ==='
rg -l --type py '`@pytest`\.mark\.smoke\b' tests | sort || true
printf '%s\n' '--- gating ---'
rg -l --type py '`@pytest`\.mark\.gating\b' tests | sort || true
echo '=== ancestor conftest references ==='
for file in \
tests/conftest.py \
tests/network/conftest.py \
tests/network/l2_bridge/conftest.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/ip_specification/conftest.py; do
if [[ -f "$file" ]]; then
echo "--- $file ---"
rg -n --type py -C 3 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address)\b|yield from|autouse\s*=' \
"$file" || true
fi
doneLength of output: 203066 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5584"
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
pr_json="$(gh api "repos/$repo/pulls/$pr")"
sha="$(jq -r '.head.sha' <<<"$pr_json")"
gh api --paginate "repos/$repo/pulls/$pr/files?per_page=100" >"$workdir/files.json"
path="$(jq -r '.[0].filename' "$workdir/files.json")"
patch="$(jq -r '.[0].patch // empty' "$workdir/files.json")"
if [[ -z "$patch" ]]; then
echo "The first changed file has no patch." >&2
exit 1
fi
line="$(
awk '
/^@@/ {
h = $0
sub(/^@@ -[0-9]+(,[0-9]+)? \+/, "", h)
sub(/ .*/, "", h)
current = h + 0
next
}
/^\+/ && !/^\+\+\+/ { print current; exit }
/^ / || /^-/ { current++ }
' <<<"$patch"
)"
if [[ -z "$line" ]]; then
echo "The first changed file has no added line." >&2
exit 1
fi
cat >"$workdir/comment.md" <<'EOF'
## Test Execution Plan
**Run smoke tests: False** — the smoke-marker search returned concrete paths. Neither direct-caller module is smoke-marked. The ancestor `conftest.py` files do not import or call `hot_plug_interface()`, `set_secondary_static_ip_address()`, `hot_plug_interface_and_set_address()`, or `read_guest_interface_ipv4()`.
**Run gating tests: False** — the gating-marker search returned concrete paths. Neither direct-caller module is gating-marked.
**Affected tests to run**
- `tests/network/l2_bridge/test_bridge_nic_hot_plug.py` — its fixtures directly call `hot_plug_interface()`, `set_secondary_static_ip_address()`, and `hot_plug_interface_and_set_address()`. This covers the normal guest-agent status path and the CNV-77961 console MAC/IP fallback.
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py::TestVMWithExplicitIPAddressSpecification::test_vm_is_started_with_successful_connectivity` — directly calls `read_guest_interface_ipv4()`.
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py::TestVMWithExplicitIPAddressSpecification::test_ip_address_is_preserved_over_power_cycle` — directly calls `read_guest_interface_ipv4()` after restart.
**Real tests (cluster required)**
Error path — force the guest-agent failure condition from `#5467`:
```bash
uv run pytest --verbose --jira -rs -s -o log_cli=true \
--junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log \
--leftovers-collector --data-collector --tb=native \
--skip-deprecated-api-test --cluster-sanity-skip-storage-check \
-m 'not sriov' tests/network/l2_bridge/test_bridge_nic_hot_plug.pyExpected: hot-plug discovery and static-IP validation use console MAC/IP lookup when VMI interface status is absent. Happy path — use normal guest-agent reporting: uv run pytest --verbose --jira -rs -s -o log_cli=true \
--junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log \
--leftovers-collector --data-collector --tb=native \
--skip-deprecated-api-test --cluster-sanity-skip-storage-check \
-m 'not sriov' \
tests/network/l2_bridge/test_bridge_nic_hot_plug.py \
tests/network/user_defined_network/ip_specification/test_ip_specification.pyExpected: VMI status verification remains active, and the hot-plug and explicit-IP workflows pass. payload="$( for attempt in 1 2; do exit 1 You are interacting with an AI system. |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git rev-parse HEAD~1)"
echo '=== current head and changed files ==='
git rev-parse HEAD
git diff --name-status "$base"...HEAD
echo '=== changed declarations ==='
git diff --unified=0 "$base"...HEAD -- tests \
| grep -E '^[+-][[:space:]]*(async[[:space:]]+)?(def|class)[[:space:]]+' || true
echo '=== changed helper references in tests and conftests ==='
rg -n --type py -C 4 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address|_lookup_hotplugged_iface_via_console)\b' \
tests || true
echo '=== affected test fixtures, tests, and markers ==='
rg -n --type py -C 5 \
'`@pytest`\.(fixture|mark\.(smoke|gating))|^(async )?def test_|^class Test|yield from|hot_plug_interface|set_secondary_static_ip_address|read_guest_interface_ipv4' \
tests/network/l2_bridge/test_bridge_nic_hot_plug.py \
tests/network/user_defined_network/ip_specification/test_ip_specification.py || true
echo '=== smoke and gating marked files ==='
rg -l --type py '`@pytest`\.mark\.smoke\b' tests | sort || true
printf '%s\n' '--- gating ---'
rg -l --type py '`@pytest`\.mark\.gating\b' tests | sort || true
echo '=== ancestor conftest references ==='
for file in \
tests/conftest.py \
tests/network/conftest.py \
tests/network/l2_bridge/conftest.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/ip_specification/conftest.py; do
if [[ -f "$file" ]]; then
echo "--- $file ---"
rg -n --type py -C 3 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address)\b|yield from|autouse\s*=' \
"$file" || true
fi
doneLength of output: 2321 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5584"
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
pr_json="$(gh api "repos/$repo/pulls/$pr")"
sha="$(jq -r '.head.sha' <<<"$pr_json")"
echo '=== PR metadata ==='
jq '{head_sha: .head.sha, base_sha: .base.sha, changed_files: .changed_files}' <<<"$pr_json"
echo '=== changed files and declarations ==='
gh api --paginate "repos/$repo/pulls/$pr/files?per_page=100" >"$workdir/files.json"
jq -r '.[] | "\(.status)\t\(.filename)"' "$workdir/files.json"
jq -r '.[] | select(.patch != null) | .patch' "$workdir/files.json" \
| grep -E '^[+-][[:space:]]*(async[[:space:]]+)?(def|class)[[:space:]]+' || true
curl --fail --silent --show-error --location \
"https://codeload.github.com/$repo/zip/$sha" \
--output "$workdir/source.zip"
unzip -q "$workdir/source.zip" -d "$workdir"
root="$(find "$workdir" -mindepth 1 -maxdepth 1 -type d | head -n1)"
cd "$root"
echo '=== changed-helper callers, fixtures, test definitions, and markers ==='
rg -n --type py -C 5 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address|_lookup_hotplugged_iface_via_console)\b|`@pytest`\.(fixture|mark\.(smoke|gating))|^(async )?def test_|^class Test|yield from|autouse\s*=' \
tests || true
echo '=== smoke and gating test files ==='
rg -l --type py '`@pytest`\.mark\.smoke\b' tests | sort || true
printf '%s\n' '--- gating ---'
rg -l --type py '`@pytest`\.mark\.gating\b' tests | sort || true
echo '=== ancestor conftest references ==='
for file in \
tests/conftest.py \
tests/network/conftest.py \
tests/network/l2_bridge/conftest.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/ip_specification/conftest.py; do
if [[ -f "$file" ]]; then
echo "--- $file ---"
rg -n --type py -C 3 \
'\b(read_guest_interface_ipv4|hot_plug_interface|set_secondary_static_ip_address|hot_plug_interface_and_set_address)\b|yield from|autouse\s*=' \
"$file" || true
fi
doneLength of output: 202594 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5584"
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
pr_json="$(gh api "repos/$repo/pulls/$pr")"
sha="$(jq -r '.head.sha' <<<"$pr_json")"
gh api --paginate "repos/$repo/pulls/$pr/files?per_page=100" >"$workdir/files.json"
path="$(jq -r '.[0].filename' "$workdir/files.json")"
patch="$(jq -r '.[0].patch // empty' "$workdir/files.json")"
test -n "$patch"
line="$(
awk '
/^@@/ {
header = $0
sub(/^@@ -[0-9]+(,[0-9]+)? \+/, "", header)
sub(/ .*/, "", header)
current = header + 0
next
}
/^\+/ && !/^\+\+\+/ { print current; exit }
/^ / || /^-/ { current++ }
' <<<"$patch"
)"
test -n "$line"
cat >"$workdir/comment.md" <<'EOF'
## Test Execution Plan
**Run smoke tests: False** — the smoke-marker search returned concrete paths. Neither direct-caller module is smoke-marked. The ancestor `conftest.py` files do not import or call `hot_plug_interface()`, `set_secondary_static_ip_address()`, `hot_plug_interface_and_set_address()`, or `read_guest_interface_ipv4()`.
**Run gating tests: False** — the gating-marker search returned concrete paths. Neither direct-caller module is gating-marked.
**Affected tests to run**
- `tests/network/l2_bridge/test_bridge_nic_hot_plug.py` — its fixtures directly call `hot_plug_interface()`, `set_secondary_static_ip_address()`, and `hot_plug_interface_and_set_address()`. These calls cover the normal guest-agent status path and the CNV-77961 console MAC/IP fallback.
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py::TestVMWithExplicitIPAddressSpecification::test_vm_is_started_with_successful_connectivity` — directly calls `read_guest_interface_ipv4()`.
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py::TestVMWithExplicitIPAddressSpecification::test_ip_address_is_preserved_over_power_cycle` — directly calls `read_guest_interface_ipv4()` after VM restart.
**Real tests (cluster required)**
Error path — force the guest-agent failure condition from `#5467`:
```bash
uv run pytest --verbose --jira -rs -s -o log_cli=true \
--junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log \
--leftovers-collector --data-collector --tb=native \
--skip-deprecated-api-test --cluster-sanity-skip-storage-check \
-m 'not sriov' tests/network/l2_bridge/test_bridge_nic_hot_plug.pyExpected: hot-plug discovery and static-IP validation use console MAC/IP lookup when VMI interface status is absent. Happy path — use normal guest-agent reporting: uv run pytest --verbose --jira -rs -s -o log_cli=true \
--junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log \
--leftovers-collector --data-collector --tb=native \
--skip-deprecated-api-test --cluster-sanity-skip-storage-check \
-m 'not sriov' \
tests/network/l2_bridge/test_bridge_nic_hot_plug.py \
tests/network/user_defined_network/ip_specification/test_ip_specification.pyExpected: VMI-status verification remains active, and the hot-plug and explicit-IP workflows pass. payload="$( for attempt in 1 2; do exit 1 You are interacting with an AI system. |
|
/approve |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
|
/cherry-pick cnv-4.22 cnv-4.21 cnv-4.20 cnv-4.19 cnv-4.18 |
|
Manual cherry-pick is needed |
|
Manual cherry-pick is needed |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR net, tests, hot-plug: W/A guest-agent deadlock on hot-plugged interfaces into cnv-4.18: #5965 @yossisegev Manual verification is required — please review the changes and test before merging. |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR net, tests, hot-plug: W/A guest-agent deadlock on hot-plugged interfaces into cnv-4.20: #5966 @yossisegev Manual verification is required — please review the changes and test before merging. |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR net, tests, hot-plug: W/A guest-agent deadlock on hot-plugged interfaces into cnv-4.19: #5967 @yossisegev Manual verification is required — please review the changes and test before merging. |
12 participants
What this PR does / why we need it:
CNV-77961 causes guest-agent to stop reporting newly hot-plugged
interfaces, failing tests that depend on interface data from VMI status.
This changes works around the partial VMI status report by validating
the interface from the guest console, so tests continue to run until the
issue is fixed.
This flow is valid because it reflects user's behavior - users (and their
tools) don't necessarily inspect the openshift resources (VMI status) for
checking guest components, but rather might do that directly via the
guest.
As part of this change, functionality from another package
(user_defined_network/ip_specification) was re-used, therefore this
package is handled here as well, including a race fix by explicitly checking
the interfaces are available.
Originally composed by: Anat Wax awax@redhat.com
Assisted-by: Claude noreply@anthropic.com
Assisted-by: Yossi Segev ysegev@redhat.com
Which issue(s) this PR fixes:
CNV-77961
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-88390
Summary by CodeRabbit
Bug Fixes
Refactor