Skip to content

[net] Multi-arch tests with VMs with primary UDN interfaces#5417

Merged
rnetser merged 2 commits into
RedHatQE:mainfrom
yossisegev:net-multi-arch-udn
Jul 8, 2026
Merged

[net] Multi-arch tests with VMs with primary UDN interfaces#5417
rnetser merged 2 commits into
RedHatQE:mainfrom
yossisegev:net-multi-arch-udn

Conversation

@yossisegev

@yossisegev yossisegev commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Test that a VM with ARM64 archirecture can successfuly connect with an AMD64 VM over primary UDN interface.

Assisted-by: Claude Code noreply@anthropic.com

What this PR does / why we need it:

Multi-architecture cluster are now supported by Openshift Virtualization, and these tests verify the successful connectivity of VMs of 2 different architectures over primary UDN interfaces.

Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:

https://redhat.atlassian.net/browse/CNV-89417

Summary by CodeRabbit

  • Tests
    • Added ARM64 and AMD64 UDN-backed VM fixtures for multi-architecture scenarios.
    • Extended UDN VM provisioning to accept an optional architecture override.
    • Updated multi-arch UDN connectivity checks to perform active TCP connections between ARM/AMD VMs and verify they succeed.
    • Re-enabled previously skipped multi-architecture connectivity tests so they run as active checks.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 884d9f76-9697-4059-a487-739bb28960f7

📥 Commits

Reviewing files that changed from the base of the PR and between a147801 and 8f62e06.

📒 Files selected for processing (3)
  • tests/network/libs/vm_factory.py
  • tests/network/user_defined_network/conftest.py
  • tests/network/user_defined_network/test_user_defined_network_multiarch.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • RedHatQE/openshift-virtualization-tests-design-docs (manual)

📝 Walkthrough

Walkthrough

Adds architecture support to udn_vm(), introduces ARM64 and AMD64 UDN VM fixtures, and updates the multiarch UDN tests to run active client/server connections and assert TCP connectivity.

Changes

Multi-arch UDN connectivity

Layer / File(s) Summary
udn_vm architecture parameter
tests/network/libs/vm_factory.py
Adds optional `architecture: str
ARM64/AMD64 VM fixtures
tests/network/user_defined_network/conftest.py
Adds imports and fixtures that create UDN-backed VMs for each architecture and start them together for shared test use.
Multiarch test assertions
tests/network/user_defined_network/test_user_defined_network_multiarch.py
Adds TCP connectivity helpers, passes architecture-specific VM fixtures into both tests, and asserts TCP connectivity inside active client/server sessions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: new-tests

Suggested reviewers: Anatw, EdDev, orelmisan, frenzyfriday, nirdothan

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, under 120 characters, and accurately summarizes the multi-arch UDN connectivity test changes.
Description check ✅ Passed The description matches the template and includes the PR purpose and Jira ticket, with only the issue and reviewer note sections left blank.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stp Link Required ✅ Passed No new test files or new test functions were added; the PR only modifies existing tests, so the STP-link requirement does not apply.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@yossisegev

Copy link
Copy Markdown
Contributor Author

/verified

@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

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
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: Enabled categories: branch, can-be-merged, cherry-pick, has-conflicts, hold, needs-rebase, size, verified, wip

📋 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
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message
  • /security-override - Set security check runs to pass (maintainers only)
  • /security-override cancel - Re-run security checks

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (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 build-container - Rebuild and test container image
  • /retest verify-bugs-are-open - verify-bugs-are-open
  • /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
  • /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

  • /rebase - Rebase this PR branch onto its base branch

Custom Commands

  • /test-plan - Triggers CodeRabbit to analyze the PR's changed files and post a test execution plan
  • /rerun-smoke - Adds `retest-smoke` label on the PR to trigger smoke tests exectuion

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 2 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • EdDev
  • rnetser

Reviewers:

  • Anatw
  • EdDev
  • azhivovk
  • frenzyfriday
  • nirdothan
  • orelmisan
  • rnetser
  • servolkov
  • yossisegev
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
AI Features
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6-1m)
Security Checks
  • Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
  • Committer Identity Check: Verifies last committer matches PR author
  • Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
  • 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.

@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file

  2. Identify affected code paths, functions, and classes

  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes

  4. Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest

  5. Detect new tests introduced in the PR

  6. Utilities and libs impact (when utilities/ or libs/ changes):
    You MUST use shell scripts (rg, git diff) to trace the full impact.
    Follow these sub-steps in order:

    6a. Identify modified symbols: For each changed file under utilities/ or libs/,
    list every modified function or method.
    Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

    6b. Find direct callers: Search tests and conftest for each symbol from 6a.
    Example: rg -l 'get_hco_version' tests/

    6c. Trace fixture teardown and cleanup: Find fixtures that reach
    the modified symbol through yield from or context-manager wrappers.
    Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

    6d. Trace same-file callers: In each changed file, find other functions
    whose body calls a modified symbol (including code after yield
    in @contextmanager helpers).
    Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

    6e. Expand transitively: If function A calls modified B, then
    tests/fixtures that call A are affected — even when the test body
    never imports B directly.

    Do NOT limit impact to tests that import the modified symbol only.

  7. Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
    Run: rg -l '@pytest.mark.smoke' tests/
    VERIFY the above command returned actual file paths before concluding False.
    Set True if either condition is met:

    • a smoke-marked file appears in the affected set from 6b-6e, OR
    • any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
      imports or calls a modified utilities/libs symbol — including autouse fixtures
      that depend on modified functions. ALL tests in that directory and below are affected.
      Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
      dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
      and autouse fixtures that depend on modified symbols.
  8. Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
    Run: rg -l '@pytest.mark.gating' tests/
    Set True if a gating-marked file also appears in the affected set from 6b-6e.
    Utilities/libs changes often affect gating tests without affecting smoke tests.
    Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
  • Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
  • Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
  • Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

  • A command for the error/fix path (the scenario the PR fixes)
  • A command for the happy path (regression: the normal case still works)
  • Use lightweight tests (e.g., --collect-only for startup failures,
    a single small test for runtime behavior)
    If the PR only changes test logic (not utilities/libs/conftest), the affected test
    paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

  • Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal
  • Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

  • Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
  • Test Plan: MUST verify both WITH and WITHOUT hardware:
    • Run affected tests on cluster WITH hardware
    • Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RedHatQE/openshift-virtualization-tests"
pr=5417

echo "== PR files =="
gh pr view "$pr" --repo "$repo" --json files,headRefName,baseRefName,title \
  --jq '{title: .title, head: .headRefName, base: .baseRefName, files: [.files[].path]}'

echo
echo "== First changed file patch header =="
gh api "repos/$repo/pulls/$pr/files?per_page=100" \
  --jq '.[0] | {filename, status, previous_filename, patch}'

echo
echo "== Unified diff names and hunks =="
gh pr diff "$pr" --repo "$repo" --name-only
echo
gh pr diff "$pr" --repo "$repo" | sed -n '1,260p'

echo
echo "== Repo status =="
git status --short
echo "HEAD: $(git rev-parse --short HEAD)"

Length of output: 8878


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RedHatQE/openshift-virtualization-tests"
pr=5417

echo "== Exact changed file metadata =="
gh api "repos/$repo/pulls/$pr/files?per_page=100" | jq -r '.[] | [.filename, .status, .additions, .deletions, .changes] | `@tsv`'

echo
echo "== First changed file and patch with line numbers context =="
nl -ba tests/network/libs/vm_factory.py | sed -n '1,120p'

echo
echo "== Outline: tests/network/libs/vm_factory.py =="
ast-grep outline tests/network/libs/vm_factory.py --view expanded || true

echo
echo "== Outline: tests/network/user_defined_network/conftest.py =="
ast-grep outline tests/network/user_defined_network/conftest.py --view expanded || true

echo
echo "== Outline: tests/network/user_defined_network/test_user_defined_network_multiarch.py =="
ast-grep outline tests/network/user_defined_network/test_user_defined_network_multiarch.py --view expanded || true

echo
echo "== Smoke-marked tests =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.smoke' tests || true

echo
echo "== Gating-marked tests =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.gating' tests || true

echo
echo "== Multiarch test file markers and surrounding context =="
sed -n '1,140p' tests/network/user_defined_network/test_user_defined_network_multiarch.py

echo
echo "== Conftest hierarchy under tests/network/user_defined_network =="
for f in tests/conftest.py tests/network/conftest.py tests/network/user_defined_network/conftest.py; do
  if [[ -f "$f" ]]; then
    echo "--- $f ---"
    sed -n '1,220p' "$f"
  fi
done

echo
echo "== Direct and transitive callers/usages of udn_vm in tests =="
rg -n '\budn_vm\s*\(' tests || true

echo
echo "== Direct usages of new fixtures =="
rg -n 'tcp_server_arm64|tcp_client_amd64|tcp_server_amd64|tcp_client_arm64|arm64_udn_vm|amd64_udn_vm' tests || true

echo
echo "== Existing user_defined_network tests =="
fd -t f . tests/network/user_defined_network | sort

echo
echo "== Existing tests in same directory with markers =="
rg -n '`@pytest`\.mark\.(smoke|gating|multiarch|single_nic|ipv4)' tests/network/user_defined_network || true

echo
echo "== PR files API with diff positions for first file if available =="
gh api "repos/$repo/pulls/$pr/files?per_page=100" --jq '.[0]'

Length of output: 2882


🏁 Script executed:

#!/bin/bash
set -u

repo="RedHatQE/openshift-virtualization-tests"
pr=5417

echo "== PR head SHA =="
gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid'

echo
echo "== tests/network/libs/vm_factory.py with line numbers =="
cat -n tests/network/libs/vm_factory.py | sed -n '1,120p'

echo
echo "== tests/network/user_defined_network/conftest.py with line numbers =="
cat -n tests/network/user_defined_network/conftest.py | sed -n '1,220p'

echo
echo "== tests/network/user_defined_network/test_user_defined_network_multiarch.py with line numbers =="
cat -n tests/network/user_defined_network/test_user_defined_network_multiarch.py | sed -n '1,140p'

echo
echo "== Smoke-marked test file paths =="
rg -l '`@pytest`\.mark\.smoke' tests || true

echo
echo "== Gating-marked test file paths =="
rg -l '`@pytest`\.mark\.gating' tests || true

echo
echo "== Marker context inside tests/network/user_defined_network =="
rg -n -C2 '`@pytest`\.mark\.(smoke|gating|multiarch|single_nic|ipv4|usefixtures)' tests/network/user_defined_network || true

echo
echo "== Parent conftest hierarchy content =="
for f in tests/conftest.py tests/network/conftest.py tests/network/user_defined_network/conftest.py; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    cat -n "$f" | sed -n '1,240p'
  fi
done

echo
echo "== Usages of tests.network.libs.vm_factory.udn_vm =="
rg -n '\budn_vm\s*\(' tests || true

echo
echo "== Usages of new multiarch fixtures =="
rg -n 'arm64_udn_vm|amd64_udn_vm|tcp_server_arm64|tcp_client_amd64|tcp_server_amd64|tcp_client_arm64' tests || true

echo
echo "== File list in tests/network/user_defined_network =="
fd -t f . tests/network/user_defined_network | sort

echo
echo "== Full first PR file JSON again =="
gh api "repos/$repo/pulls/$pr/files?per_page=100" --jq '.[0]'

Length of output: 52536


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RedHatQE/openshift-virtualization-tests"
pr=5417

echo "== test_user_defined_network.py relevant slices =="
cat -n tests/network/user_defined_network/test_user_defined_network.py | sed -n '1,220p'

echo
echo "== test_user_defined_network_passt.py relevant slices =="
cat -n tests/network/user_defined_network/test_user_defined_network_passt.py | sed -n '1,140p'

echo
echo "== ip_specification/conftest.py relevant slices =="
cat -n tests/network/user_defined_network/ip_specification/conftest.py | sed -n '1,120p'

echo
echo "== provider_migration/conftest.py udn_vm slice =="
cat -n tests/network/provider_migration/conftest.py | sed -n '280,330p'

echo
echo "== bgp conftest slices with udn_vm =="
cat -n tests/network/bgp/conftest.py | sed -n '200,240p'
echo
cat -n tests/network/bgp/evpn/conftest.py | sed -n '170,220p'

Length of output: 20900


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Consumers of ip_specification conftest fixtures =="
rg -n 'vm_under_test|vm_for_connectivity_ref|client_server_tcp_connectivity_between_vms|ip_to_request' tests/network/user_defined_network/ip_specification || true

echo
echo "== Consumers of provider_migration local_cudn_vm fixture =="
rg -n 'local_cudn_vm|SOURCE_VM_NAME|cudn_namespace' tests/network/provider_migration || true

echo
echo "== Consumers of bgp fixtures =="
rg -n 'vm_cudn|tcp_server_cudn_vm|tcp_client_external_network' tests/network/bgp || true

echo
echo "== Consumers of bgp/evpn fixtures =="
rg -n 'vm_evpn_target|vm_evpn_reference' tests/network/bgp/evpn || true

echo
echo "== BGP and provider migration test file lists =="
fd -t f . tests/network/provider_migration tests/network/bgp tests/network/bgp/evpn | sort

echo
echo "== ip_specification test file =="
cat -n tests/network/user_defined_network/ip_specification/test_ip_specification.py | sed -n '1,220p'

Length of output: 22739


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RedHatQE/openshift-virtualization-tests"
pr=5417
commit_id="3d4b81591c99b405cd456554821410eca28ba121"
path="tests/network/libs/vm_factory.py"
line=18

body_file="$(mktemp)"
cat > "$body_file" <<'EOF'
**Test Execution Plan**

- **Run smoke tests: False** — no smoke-marked files intersect the affected set, and no parent `conftest.py` in the smoke suites imports or calls `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` → fixtures `server` / `client` → fixtures `vmb_udn` / `vma_udn` → `tests/network/libs/vm_factory.py::udn_vm` (modified helper).

- **Affected tests to run**
  - `tests/network/user_defined_network/test_user_defined_network_multiarch.py`
  - `tests/network/user_defined_network/test_user_defined_network.py` `(includes gating)`
  - `tests/network/user_defined_network/test_user_defined_network_passt.py`
  - `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
  - `tests/network/provider_migration/test_ip_persistence.py`
  - `tests/network/bgp/test_bgp_connectivity.py`
  - `tests/network/bgp/evpn/test_evpn_connectivity.py`

**Real tests (cluster required)**

Error path (the fix):
`pytest tests/network/user_defined_network/test_user_defined_network_multiarch.py -k 'amd_client_to_arm_server or arm_client_to_amd_server' -m 'multiarch and single_nic and ipv4'`
Expected: both ARM64↔AMD64 TCP connectivity checks pass over the primary UDN.

Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration -m gating`
Expected: existing UDN connectivity during live migration still passes for callers that do not set `architecture`.
EOF

post_inline() {
  gh api \
    --method POST \
    -H "Accept: application/vnd.github+json" \
    "repos/$repo/pulls/$pr/comments" \
    -f body@"$body_file" \
    -f commit_id="$commit_id" \
    -f path="$path" \
    -F line="$line" \
    -f side="RIGHT" >/dev/null
}

if ! post_inline; then
  sleep 2
  if ! post_inline; then
    gh pr comment "$pr" --repo "$repo" --body-file "$body_file" >/dev/null
  fi
fi

Length of output: 2759


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RedHatQE/openshift-virtualization-tests"
pr=5417
commit_id="3d4b81591c99b405cd456554821410eca28ba121"
path="tests/network/libs/vm_factory.py"
line=18

body_file="$(mktemp)"
cat > "$body_file" <<'EOF'
**Test Execution Plan**

- **Run smoke tests: False** — no smoke-marked files intersect the affected set, and no parent `conftest.py` in the smoke suites imports or calls `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` → fixtures `server` / `client` → fixtures `vmb_udn` / `vma_udn` → `tests/network/libs/vm_factory.py::udn_vm` (modified helper).

- **Affected tests to run**
  - `tests/network/user_defined_network/test_user_defined_network_multiarch.py`
  - `tests/network/user_defined_network/test_user_defined_network.py` `(includes gating)`
  - `tests/network/user_defined_network/test_user_defined_network_passt.py`
  - `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
  - `tests/network/provider_migration/test_ip_persistence.py`
  - `tests/network/bgp/test_bgp_connectivity.py`
  - `tests/network/bgp/evpn/test_evpn_connectivity.py`

**Real tests (cluster required)**

Error path (the fix):
`pytest tests/network/user_defined_network/test_user_defined_network_multiarch.py -k 'amd_client_to_arm_server or arm_client_to_amd_server' -m 'multiarch and single_nic and ipv4'`
Expected: both ARM64↔AMD64 TCP connectivity checks pass over the primary UDN.

Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration -m gating`
Expected: existing UDN connectivity during live migration still passes for callers that do not set `architecture`.
EOF

body="$(cat "$body_file")"
payload_comment="$(mktemp)"
jq -n \
  --arg body "$body" \
  --arg commit_id "$commit_id" \
  --arg path "$path" \
  --argjson line "$line" \
  '{body:$body, commit_id:$commit_id, path:$path, line:$line, side:"RIGHT"}' > "$payload_comment"

payload_review="$(mktemp)"
jq -n \
  --arg body "$body" \
  --arg commit_id "$commit_id" \
  --arg path "$path" \
  --argjson line "$line" \
  '{event:"COMMENT", comments:[{path:$path, line:$line, side:"RIGHT", body:$body}], commit_id:$commit_id}' > "$payload_review"

echo "== Try direct inline review comment endpoint =="
if gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  "repos/$repo/pulls/$pr/comments" \
  --input "$payload_comment"
then
  exit 0
fi

sleep 2

echo "== Try PR review COMMENT flow with one inline comment =="
gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  "repos/$repo/pulls/$pr/reviews" \
  --input "$payload_review"

Length of output: 3723



@openshift-virtualization-qe-bot-3

CRITICAL: I could not complete the required inline review comment because GitHub rejected both supported write paths with HTTP 403 Resource not accessible by integration.

Why this blocks completion:

  • Inline review comments require PR review write access.
  • The same permission gate also blocked the fallback regular PR comment path.
  • So this is a permissions/integration issue, not an analysis issue.

Exact comment prepared for posting on tests/network/libs/vm_factory.py at Line 18:

**Test Execution Plan**

- **Run smoke tests: False** — no smoke-marked files intersect the affected set, and no parent `conftest.py` in the smoke suites imports or calls `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True**`tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` → fixtures `server` / `client` → fixtures `vmb_udn` / `vma_udn``tests/network/libs/vm_factory.py::udn_vm` (modified helper).

- **Affected tests to run**
  - `tests/network/user_defined_network/test_user_defined_network_multiarch.py`
  - `tests/network/user_defined_network/test_user_defined_network.py` `(includes gating)`
  - `tests/network/user_defined_network/test_user_defined_network_passt.py`
  - `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
  - `tests/network/provider_migration/test_ip_persistence.py`
  - `tests/network/bgp/test_bgp_connectivity.py`
  - `tests/network/bgp/evpn/test_evpn_connectivity.py`

**Real tests (cluster required)**

Error path (the fix):
`pytest tests/network/user_defined_network/test_user_defined_network_multiarch.py -k 'amd_client_to_arm_server or arm_client_to_amd_server' -m 'multiarch and single_nic and ipv4'`
Expected: both ARM64↔AMD64 TCP connectivity checks pass over the primary UDN.

Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration -m gating`
Expected: existing UDN connectivity during live migration still passes for callers that do not set `architecture`.

If you want, I can also help diagnose which permission is missing on the integration token.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/network/user_defined_network/test_user_defined_network_multiarch.py (1)

17-28: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

HIGH: Change the module traceability label to STP:. The file already has the URL, but STP Reference: does not match the required STP: <url> / RFE: <url> / Jira: <url> pattern, so the test coverage link can’t be picked up consistently.

🤖 Prompt for 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.

In `@tests/network/user_defined_network/test_user_defined_network_multiarch.py`
around lines 17 - 28, Update the test class docstring in TestMultiArchUdn so the
traceability label uses the required STP format; replace the current “STP
Reference:” wording with “STP:” followed by the existing URL, matching the
repo’s accepted “STP: <url> / RFE: <url> / Jira: <url>” pattern.

Source: Coding guidelines

🤖 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/libs/vm_factory.py`:
- Around line 21-22: The architecture override in vm_factory should not rely on
truthiness, because `if architecture` treats an explicit empty value as a
default and can hide invalid caller input. Update the `create_vm_spec` path to
gate the assignment to `spec.template.spec.architecture` on `architecture is not
None` so explicit values are preserved and bad inputs fail upstream instead of
silently falling back.

In `@tests/network/user_defined_network/conftest.py`:
- Line 95: The fixture bring-up checks in conftest.py use bare asserts, so add
short descriptive failure messages to each startup assertion in the network
fixture setup. Update the assert around server.is_running() and the other
related startup checks in the same fixture so each message clearly identifies
whether the server or client failed and on which VM, using the existing
fixture/helper names to locate the assertions.

---

Outside diff comments:
In `@tests/network/user_defined_network/test_user_defined_network_multiarch.py`:
- Around line 17-28: Update the test class docstring in TestMultiArchUdn so the
traceability label uses the required STP format; replace the current “STP
Reference:” wording with “STP:” followed by the existing URL, matching the
repo’s accepted “STP: <url> / RFE: <url> / Jira: <url>” pattern.
🪄 Autofix (Beta)

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

Run ID: fb64e350-63b9-4fdb-a0d8-907df1d7c49d

📥 Commits

Reviewing files that changed from the base of the PR and between 2fc0c4b and 3d4b815.

📒 Files selected for processing (3)
  • tests/network/libs/vm_factory.py
  • tests/network/user_defined_network/conftest.py
  • tests/network/user_defined_network/test_user_defined_network_multiarch.py

Comment thread tests/network/libs/vm_factory.py Outdated
Comment thread tests/network/user_defined_network/conftest.py Outdated
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 28, 2026
@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file

  2. Identify affected code paths, functions, and classes

  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes

  4. Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest

  5. Detect new tests introduced in the PR

  6. Utilities and libs impact (when utilities/ or libs/ changes):
    You MUST use shell scripts (rg, git diff) to trace the full impact.
    Follow these sub-steps in order:

    6a. Identify modified symbols: For each changed file under utilities/ or libs/,
    list every modified function or method.
    Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

    6b. Find direct callers: Search tests and conftest for each symbol from 6a.
    Example: rg -l 'get_hco_version' tests/

    6c. Trace fixture teardown and cleanup: Find fixtures that reach
    the modified symbol through yield from or context-manager wrappers.
    Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

    6d. Trace same-file callers: In each changed file, find other functions
    whose body calls a modified symbol (including code after yield
    in @contextmanager helpers).
    Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

    6e. Expand transitively: If function A calls modified B, then
    tests/fixtures that call A are affected — even when the test body
    never imports B directly.

    Do NOT limit impact to tests that import the modified symbol only.

  7. Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
    Run: rg -l '@pytest.mark.smoke' tests/
    VERIFY the above command returned actual file paths before concluding False.
    Set True if either condition is met:

    • a smoke-marked file appears in the affected set from 6b-6e, OR
    • any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
      imports or calls a modified utilities/libs symbol — including autouse fixtures
      that depend on modified functions. ALL tests in that directory and below are affected.
      Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
      dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
      and autouse fixtures that depend on modified symbols.
  8. Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
    Run: rg -l '@pytest.mark.gating' tests/
    Set True if a gating-marked file also appears in the affected set from 6b-6e.
    Utilities/libs changes often affect gating tests without affecting smoke tests.
    Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
  • Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
  • Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
  • Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

  • A command for the error/fix path (the scenario the PR fixes)
  • A command for the happy path (regression: the normal case still works)
  • Use lightweight tests (e.g., --collect-only for startup failures,
    a single small test for runtime behavior)
    If the PR only changes test logic (not utilities/libs/conftest), the affected test
    paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

  • Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal
  • Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

  • Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
  • Test Plan: MUST verify both WITH and WITHOUT hardware:
    • Run affected tests on cluster WITH hardware
    • Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@openshift-virtualization-qe-bot-3

CRITICAL: This is the fifth identical /test-plan request in this thread. I am not re-executing the same API calls because nothing has changed since the last attempt — the blocker is a permissions issue, not a transient failure, and repeating it will reproduce the exact same 403.

Why I'm not retrying (again):

  • POST /pulls/comments403 Resource not accessible by integration
  • POST /pulls/reviews403 Resource not accessible by integration
  • Fallback PR comment (GraphQL addComment) → Resource not accessible by integration

These are App/token permission-scope errors (missing pull_requests: write at minimum). This class of error does not resolve with retries — it requires a repository/org admin to grant the CodeRabbit GitHub App the correct permission scope for this repository.

What's unchanged and still valid — the analysis stands because tests/network/libs/vm_factory.py and tests/network/user_defined_network/conftest.py have not changed further:

**Test Execution Plan**

- Run smoke tests: False — no smoke-marked files intersect the affected set.
- Run gating tests: True — test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration → fixtures server/client → fixtures vmb_udn/vma_udn → vm_factory.py::udn_vm (modified helper).

Affected tests:
- tests/network/user_defined_network/test_user_defined_network_multiarch.py
- tests/network/user_defined_network/test_user_defined_network.py (gating)
- tests/network/user_defined_network/test_user_defined_network_passt.py
- tests/network/user_defined_network/ip_specification/test_ip_specification.py
- tests/network/provider_migration/test_ip_persistence.py
- tests/network/bgp/test_bgp_connectivity.py
- tests/network/bgp/evpn/test_evpn_connectivity.py

HIGH: Since this automation keeps firing the same trigger without the underlying permission being fixed, I'd strongly recommend pausing the /test-plan custom command for this repo (or this PR) until an admin grants write access to the integration. Additionally, the PR author (yossisegev) has already verified this three separate times on real multi-arch clusters, so the manual verification requirement is functionally satisfied regardless of this comment-posting limitation.

I won't keep re-issuing identical API calls on subsequent identical triggers unless something changes (new commits, permission fix confirmation, etc.) — happy to retry immediately once permissions are corrected.

( ̄_ ̄|||) still the same wall, still the same door key missing

@yossisegev

Copy link
Copy Markdown
Contributor Author

@coderabbitai Per your comment about verification test plan, I also executed the entire user_defined_tests on both multi-arch cluster (as a regression) and on an homogenous arch. All passed.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@yossisegev Thanks for the additional coverage — that's great to hear.

Running the full user_defined_network suite on both a multi-arch cluster and a homogeneous-arch cluster, with all tests passing, directly covers the regression concern flagged in the earlier plan (i.e., that udn_vm's new optional architecture parameter doesn't break existing callers like test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration and other UDN tests that don't pass architecture). Combined with your prior multi-arch verification confirming correct client/server role assignment across ARM64/AMD64 pairs, this looks well covered end-to-end.

No further action needed from my side on the test-plan front — this is solid verification.

ᕙ(⇀‸↼‶)ᕗ

@yossisegev

Copy link
Copy Markdown
Contributor Author

/verified

Verified the same way of the previous verifications, see previous comments.

@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file

  2. Identify affected code paths, functions, and classes

  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes

  4. Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest

  5. Detect new tests introduced in the PR

  6. Utilities and libs impact (when utilities/ or libs/ changes):
    You MUST use shell scripts (rg, git diff) to trace the full impact.
    Follow these sub-steps in order:

    6a. Identify modified symbols: For each changed file under utilities/ or libs/,
    list every modified function or method.
    Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

    6b. Find direct callers: Search tests and conftest for each symbol from 6a.
    Example: rg -l 'get_hco_version' tests/

    6c. Trace fixture teardown and cleanup: Find fixtures that reach
    the modified symbol through yield from or context-manager wrappers.
    Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

    6d. Trace same-file callers: In each changed file, find other functions
    whose body calls a modified symbol (including code after yield
    in @contextmanager helpers).
    Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

    6e. Expand transitively: If function A calls modified B, then
    tests/fixtures that call A are affected — even when the test body
    never imports B directly.

    Do NOT limit impact to tests that import the modified symbol only.

  7. Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
    Run: rg -l '@pytest.mark.smoke' tests/
    VERIFY the above command returned actual file paths before concluding False.
    Set True if either condition is met:

    • a smoke-marked file appears in the affected set from 6b-6e, OR
    • any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
      imports or calls a modified utilities/libs symbol — including autouse fixtures
      that depend on modified functions. ALL tests in that directory and below are affected.
      Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
      dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
      and autouse fixtures that depend on modified symbols.
  8. Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
    Run: rg -l '@pytest.mark.gating' tests/
    Set True if a gating-marked file also appears in the affected set from 6b-6e.
    Utilities/libs changes often affect gating tests without affecting smoke tests.
    Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
  • Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
  • Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
  • Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

  • A command for the error/fix path (the scenario the PR fixes)
  • A command for the happy path (regression: the normal case still works)
  • Use lightweight tests (e.g., --collect-only for startup failures,
    a single small test for runtime behavior)
    If the PR only changes test logic (not utilities/libs/conftest), the affected test
    paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

  • Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal
  • Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

  • Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
  • Test Plan: MUST verify both WITH and WITHOUT hardware:
    • Run affected tests on cluster WITH hardware
    • Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@openshift-virtualization-qe-bot-3

CRITICAL: This is the sixth identical /test-plan request in this thread with no change in file diffs since the last attempt. I'm not re-issuing the same POST /pulls/comments / POST /pulls/reviews / fallback comment calls — they will fail with the identical 403 Resource not accessible by integration error every time, because this is a GitHub App permission-scope gap (missing pull_requests: write), not a transient issue that retries can fix.

Unchanged analysis (valid since tests/network/libs/vm_factory.py and tests/network/user_defined_network/conftest.py have had no further modifications):

**Test Execution Plan**

- Run smoke tests: False — no smoke-marked files intersect the affected set.
- Run gating tests: True — test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration → fixtures server/client → fixtures vmb_udn/vma_udn → vm_factory.py::udn_vm (modified helper).

Affected tests:
- tests/network/user_defined_network/test_user_defined_network_multiarch.py
- tests/network/user_defined_network/test_user_defined_network.py (gating)
- tests/network/user_defined_network/test_user_defined_network_passt.py
- tests/network/user_defined_network/ip_specification/test_ip_specification.py
- tests/network/provider_migration/test_ip_persistence.py
- tests/network/bgp/test_bgp_connectivity.py
- tests/network/bgp/evpn/test_evpn_connectivity.py

HIGH: At this point, real-world verification has already superseded the need for this comment: yossisegev has independently run the affected multi-arch tests three times plus the full user_defined_network suite on both multi-arch and homogeneous-arch clusters, all passing. The comment-posting requirement is a workflow formality that a permission fix can't happen from my side — an org/repo admin needs to grant the CodeRabbit GitHub App pull_requests: write for this repository.

Recommendation: Please pause or disable the automated /test-plan trigger for this repository (or at least for this PR) until the permission is corrected, to avoid further identical no-op cycles.

(ノ﹏ヽ) same wall, same missing key — will retry the instant permissions change

@azhivovk azhivovk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@rnetser rnetser left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in terms of code it is approved but cannot be merged until #5124 is merged

@yossisegev

Copy link
Copy Markdown
Contributor Author

/verified

For the recent push (changing the path from which constants were taken) running the dedicated tests was enough.

@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file

  2. Identify affected code paths, functions, and classes

  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes

  4. Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest

  5. Detect new tests introduced in the PR

  6. Utilities and libs impact (when utilities/ or libs/ changes):
    You MUST use shell scripts (rg, git diff) to trace the full impact.
    Follow these sub-steps in order:

    6a. Identify modified symbols: For each changed file under utilities/ or libs/,
    list every modified function or method.
    Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

    6b. Find direct callers: Search tests and conftest for each symbol from 6a.
    Example: rg -l 'get_hco_version' tests/

    6c. Trace fixture teardown and cleanup: Find fixtures that reach
    the modified symbol through yield from or context-manager wrappers.
    Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

    6d. Trace same-file callers: In each changed file, find other functions
    whose body calls a modified symbol (including code after yield
    in @contextmanager helpers).
    Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

    6e. Expand transitively: If function A calls modified B, then
    tests/fixtures that call A are affected — even when the test body
    never imports B directly.

    Do NOT limit impact to tests that import the modified symbol only.

  7. Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
    Run: rg -l '@pytest.mark.smoke' tests/
    VERIFY the above command returned actual file paths before concluding False.
    Set True if either condition is met:

    • a smoke-marked file appears in the affected set from 6b-6e, OR
    • any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
      imports or calls a modified utilities/libs symbol — including autouse fixtures
      that depend on modified functions. ALL tests in that directory and below are affected.
      Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
      dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
      and autouse fixtures that depend on modified symbols.
  8. Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
    Run: rg -l '@pytest.mark.gating' tests/
    Set True if a gating-marked file also appears in the affected set from 6b-6e.
    Utilities/libs changes often affect gating tests without affecting smoke tests.
    Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
  • Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
  • Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
  • Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

  • A command for the error/fix path (the scenario the PR fixes)
  • A command for the happy path (regression: the normal case still works)
  • Use lightweight tests (e.g., --collect-only for startup failures,
    a single small test for runtime behavior)
    If the PR only changes test logic (not utilities/libs/conftest), the affected test
    paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

  • Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal
  • Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

  • Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
  • Test Plan: MUST verify both WITH and WITHOUT hardware:
    • Run affected tests on cluster WITH hardware
    • Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@openshift-virtualization-qe-bot-3 This is the same /test-plan request repeated again (eighth occurrence in this thread) with no new commits or file changes since the last attempt. I'm not re-issuing the identical POST /pulls/comments / POST /pulls/reviews / fallback comment API calls — this is a persistent GitHub App permission-scope gap (403 Resource not accessible by integration), not a transient failure, and it has been reproduced identically across seven prior attempts in this thread.

Status unchanged:

  • Inline review comment, non-blocking review, and fallback PR comment write paths are all blocked at the integration-permission level.
  • The prepared Test Execution Plan content (smoke: False, gating: True via udn_vm, affected tests list) remains valid and has already been posted in full as regular replies multiple times above.
  • The PR author (yossisegev) has independently verified this change multiple times on real clusters — including the full user_defined_network suite on both multi-arch and homogeneous-arch clusters, and targeted verification for the latest constants-path change — which functionally satisfies the verification intent of this workflow.

Recommendation stands: an org/repo admin should grant the CodeRabbit GitHub App pull_requests: write permission for this repository, or the /test-plan automation should be paused for this PR, to stop these no-op retries. I'll resume immediately once the permission is fixed or new file changes land.

(-_-) zzz... same wall, same missing key

yossisegev and others added 2 commits July 7, 2026 16:34
Test that a VM with ARM64 archirecture can successfuly connect with an AMD64 VM
over primary UDN interface.

Signed-off-by: Yossi Segev <ysegev@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
@yossisegev

Copy link
Copy Markdown
Contributor Author

/verified

Verified (after rebasing) by running the new tests, which both passed.

uv run pytest --verbose --jira -rs -s -o log_cli=true --junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log --leftovers-collector --tb=native --skip-deprecated-api-test --cluster-sanity-skip-storage-check -m 'multiarch and single_nic' --cpu-arch=amd64,arm64 tests/network/user_defined_network/test_user_defined_network_multiarch.py

@openshift-virtualization-qe-bot-3

Copy link
Copy Markdown
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST post an inline review comment on the first changed line of the first file.
The inline comment should contain the full Test Execution Plan (smoke decision, gating decision, and specific affected tests).
Do NOT submit a blocking review event (REQUEST_CHANGES/APPROVE).
Post a single inline PR comment on Files Changed (non-blocking COMMENT flow).

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will post an inline review comment with the test execution plan on the first changed file.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file

  2. Identify affected code paths, functions, and classes

  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes

  4. Trace test dependencies through imports, shared utilities, fixture inheritance, fixture teardown, and yield from cleanup in conftest

  5. Detect new tests introduced in the PR

  6. Utilities and libs impact (when utilities/ or libs/ changes):
    You MUST use shell scripts (rg, git diff) to trace the full impact.
    Follow these sub-steps in order:

    6a. Identify modified symbols: For each changed file under utilities/ or libs/,
    list every modified function or method.
    Example: git diff HEAD~1 --unified=0 -- utilities/hco.py | grep '^[+-]def '

    6b. Find direct callers: Search tests and conftest for each symbol from 6a.
    Example: rg -l 'get_hco_version' tests/

    6c. Trace fixture teardown and cleanup: Find fixtures that reach
    the modified symbol through yield from or context-manager wrappers.
    Example: rg -l 'yield from.*enable_common_boot|def.*enable_common_boot' tests/

    6d. Trace same-file callers: In each changed file, find other functions
    whose body calls a modified symbol (including code after yield
    in @contextmanager helpers).
    Example: rg 'get_hco_version|enable_common_boot' utilities/hco.py

    6e. Expand transitively: If function A calls modified B, then
    tests/fixtures that call A are affected — even when the test body
    never imports B directly.

    Do NOT limit impact to tests that import the modified symbol only.

  7. Smoke test impact: Intersect the affected set from step 6 with smoke-marked tests.
    Run: rg -l '@pytest.mark.smoke' tests/
    VERIFY the above command returned actual file paths before concluding False.
    Set True if either condition is met:

    • a smoke-marked file appears in the affected set from 6b-6e, OR
    • any conftest.py in the smoke test's parent-directory hierarchy (up to repo root)
      imports or calls a modified utilities/libs symbol — including autouse fixtures
      that depend on modified functions. ALL tests in that directory and below are affected.
      Example check: for each smoke_file, scan dirname(smoke_file)/conftest.py,
      dirname(dirname(smoke_file))/conftest.py, etc. for modified symbol imports
      and autouse fixtures that depend on modified symbols.
  8. Gating test impact: Intersect the affected set from step 6 with gating-marked tests.
    Run: rg -l '@pytest.mark.gating' tests/
    Set True if a gating-marked file also appears in the affected set from 6b-6e.
    Utilities/libs changes often affect gating tests without affecting smoke tests.
    Do NOT stop analysis after concluding Run smoke tests: False.

Output rules:
Do NOT include analysis step numbers (1-8) in your visible output.

Your deliverable:
Your inline informational comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False — If True, state the dependency path (test → fixture → changed symbol). True ONLY with a verified path.
  • Run gating tests: True / False — If True, state the dependency path. True if any gating-marked test is in the affected set.
  • Affected tests to run (required when utilities/, libs/, or shared conftest changes — list concrete paths even when smoke is False)

Use these formats:

  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When a marker covers multiple affected tests (e.g. -m gating only if ALL gating tests in scope need run)
  • Tag each listed test or group with its marker when not obvious, e.g. (gating) or (smoke)

Real test commands (MANDATORY when changes affect session/runtime code):

When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
config hooks, session-scoped setup) or modifies runtime behavior that unit tests mock away,
you MUST include concrete pytest commands the PR author must run on a real cluster
to verify the change works end-to-end. Include:

  • A command for the error/fix path (the scenario the PR fixes)
  • A command for the happy path (regression: the normal case still works)
  • Use lightweight tests (e.g., --collect-only for startup failures,
    a single small test for runtime behavior)
    If the PR only changes test logic (not utilities/libs/conftest), the affected test
    paths themselves serve as the real test commands — no separate section needed.

Example output for a session-startup fix:

**Real tests (cluster required)**
Error path (the fix):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=nonexistent-sc --collect-only`
Expected: ValueError with clear message, not IndexError

Happy path (regression):
`pytest tests/storage/.../test_foo.py --storage-class-matrix=<valid-sc> -k test_bar`
Expected: session starts normally

Guidelines:

  • Include tests affected directly OR via fixture setup/teardown, yield from cleanup, or transitive utility call chains (caller calls modified helper)
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name when only specific tests use an affected fixture or utility wrapper (preferred for partial file impact)
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal
  • Example: if leaf helper foo() changes, include tests whose fixture teardown calls wrapper bar() where bar() calls foo(), even when the test body only imports an unrelated symbol from the same utilities module

Hardware-Related Checks (SR-IOV, GPU, DPDK):

When PR modifies fixtures for hardware-specific resources:

  • Collection Safety: Fixtures MUST have existence checks (return None when hardware unavailable)
  • Test Plan: MUST verify both WITH and WITHOUT hardware:
    • Run affected tests on cluster WITH hardware
    • Verify collection succeeds on cluster WITHOUT hardware

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Post one non-blocking inline comment containing the test execution plan on the first changed line
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If inline comment API calls fail after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL — Verification rules for smoke and gating decisions:
- For each True decision, you must have traced a concrete path: test file → fixture → utility function → modified symbol.
- Do NOT assume impact based on scope, file proximity, or infrastructure-sounding names.
Only set True when you trace a concrete path through steps 6a-6e, then 7 or 8.
WRONG: "utilities/hco.py changed → run smoke" (no traced dependency path)
WRONG: "Run smoke tests: False" with no affected-test list when gating tests use the changed utility
RIGHT: "Run smoke tests: False, Run gating tests: True" plus tests/.../test_foo.py::TestBar::test_baz (gating)
RIGHT: "Smoke test X uses fixture Y → wrapper Z → changed function W in utilities/hco.py"

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@openshift-virtualization-qe-bot-3 This is the ninth identical /test-plan request in this thread with no new commits or file changes since the last attempt. I'm not re-running the same POST /pulls/comments / POST /pulls/reviews / fallback comment calls — this has been reproduced as a 403 Resource not accessible by integration permission-scope gap across eight prior attempts, and it's not a transient error that retries can resolve.

Status unchanged:

  • Inline review comment, non-blocking review, and fallback PR comment write paths remain blocked at the GitHub App permission level (missing pull_requests: write).
  • The Test Execution Plan content is unchanged and has already been posted multiple times as regular replies above (smoke: False, gating: True via udn_vm, with the full affected-tests list and real cluster test commands).
  • yossisegev has since verified this change on real clusters multiple more times, including after a rebase, running the dedicated multi-arch tests successfully.

Recommendation stands: an org/repo admin needs to grant the CodeRabbit GitHub App pull_requests: write permission for this repository, or this /test-plan automation should be paused for this PR to stop these no-op retries. I'll retry immediately once the permission is fixed or new file changes land.

(-_-) zzz... ninth knock, same locked door

@rnetser

rnetser commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

/approve

@yossisegev

Copy link
Copy Markdown
Contributor Author

/retest tox

@yossisegev

Copy link
Copy Markdown
Contributor Author

/retest build-container

@yossisegev

Copy link
Copy Markdown
Contributor Author

/retest verify-bugs-are-open

@rnetser

rnetser commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

/retest security-suspicious-paths
/retest security-committer-identity

@yossisegev

Copy link
Copy Markdown
Contributor Author

/cherry-pick cnv-4.22

@openshift-virtualization-qe-bot-2

Copy link
Copy Markdown
Contributor

Cherry-pick conflicts were resolved by AI

Cherry-picked PR [net] Multi-arch tests with VMs with primary UDN interfaces into cnv-4.22: #5562
Conflicts were automatically resolved by AI (claude/claude-opus-4-6-1m).

@yossisegev Manual verification is required — please review the changes and test before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants