Skip to content

net, tests, infra: Make localnet_vm affinity configurable via parameter#4638

Open
Anatw wants to merge 1 commit intoRedHatQE:mainfrom
Anatw:refactor_localnet_vm_affinity
Open

net, tests, infra: Make localnet_vm affinity configurable via parameter#4638
Anatw wants to merge 1 commit intoRedHatQE:mainfrom
Anatw:refactor_localnet_vm_affinity

Conversation

@Anatw
Copy link
Copy Markdown
Contributor

@Anatw Anatw commented Apr 28, 2026

Short description:

Make VM affinity configurable instead of hardcoded for localnet tests.

More details:

Previously, all localnet VMs were forced to use anti-affinity scheduling with cross-namespace selector. This prevented customization for scenarios like stuntime tests that need VMs to co-locate initially before migration.

Following the same pattern used for network configuration, callers now specify the affinity constraints they need, or none if unconstrained scheduling is acceptable.

Cross-namespace affinity is now configured automatically, eliminating repetitive setup code.

Affinity structure now matches Kubernetes semantics—resources can have anti-affinity, affinity, both, or neither.

What this PR does / why we need it:
  • Replaces hardcoded scheduling behavior with configurable constraints
  • Enables future scenarios requiring VM co-location (e.g., stuntime measurement during migration)
  • Eliminates code duplication in affinity configuration
Which issue(s) this PR fixes:

Addresses code review feedback from #4568.

Special notes for reviewer:
  • This is part 1 of affinity refactoring - affinity (co-location) support will be added in a separate, stuntime-related PR.
  • All existing test behavior is preserved (VMs still use anti-affinity, just now explicit).
  • Follow-up PR will introduce a label constant to clean up verbose extraction pattern.
jira-ticket:

Summary by CodeRabbit

  • New Features

    • Pod anti-affinity for virtual machines can now be omitted explicitly; helper accepts an optional anti-affinity parameter.
    • Namespace scoping for anti-affinity is now represented explicitly when used.
  • Documentation

    • Added a detailed docstring describing anti-affinity scoping behavior.
  • Tests

    • VM test fixtures updated to apply the optional pod anti-affinity across localnet and IPAM scenarios.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Warning

Rate limit exceeded

@Anatw has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 24 minutes and 42 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1264b3d7-306a-4e0d-9cff-2724412280d4

📥 Commits

Reviewing files that changed from the base of the PR and between af91aa1 and 33e0d06.

📒 Files selected for processing (5)
  • libs/vm/affinity.py
  • libs/vm/spec.py
  • tests/network/localnet/conftest.py
  • tests/network/localnet/ipam/conftest.py
  • tests/network/localnet/liblocalnet.py
📝 Walkthrough

Walkthrough

Pod anti-affinity construction documents namespace-scoping and now sets namespaceSelector to {} when namespaces is None (otherwise None). Affinity.podAntiAffinity is nullable by default. localnet_vm() accepts an optional affinity and tests pass a shared LOCALNET_VM_ANTI_AFFINITY constant into VM fixtures.

Changes

Affinity implementation

Layer / File(s) Summary
Behavior doc & intent
libs/vm/affinity.py
Adds docstring describing Kubernetes namespace-scoping behavior for PodAffinityTerm.
Core construction
libs/vm/affinity.py
new_pod_anti_affinity() now sets namespaceSelector to {} when namespaces is None, otherwise sets it to None; namespaces parameter is still passed through.

Spec dataclass

Layer / File(s) Summary
Type/shape
libs/vm/spec.py
Affinity.podAntiAffinity changed from PodAntiAffinity to `PodAntiAffinity

Localnet tests & fixtures

Layer / File(s) Summary
Constants / imports
tests/network/localnet/liblocalnet.py
Adds LOCALNET_VM_ANTI_AFFINITY constant (from new_pod_anti_affinity(...)) and imports Affinity from libs.vm.spec.
API surface
tests/network/localnet/liblocalnet.py
localnet_vm(...) signature updated to accept `affinity: Affinity
Wiring / runtime
tests/network/localnet/liblocalnet.py
Stop applying a hardcoded anti-affinity unconditionally; set vmi_spec.affinity = affinity only when provided.
Fixtures updated
tests/network/localnet/conftest.py, tests/network/localnet/ipam/conftest.py
Multiple VM fixtures import LOCALNET_VM_ANTI_AFFINITY and pass it as affinity=... to localnet_vm(...) (including OVS-bridge and jumbo-frame fixtures).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and concisely summarizes the main change: making localnet_vm affinity configurable, which aligns with the primary objective across all modified files.
Description check ✅ Passed Description comprehensively follows the template with all sections filled: short description, detailed rationale, PR objectives, related issue reference, reviewer notes, and implementation context provided.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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
Review rate limit: 0/1 reviews remaining, refill in 24 minutes and 42 seconds.

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

@openshift-virtualization-qe-bot
Copy link
Copy Markdown

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

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

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
  • dshchedr
  • myakove
  • rnetser
  • vsibirsk

Reviewers:

  • Anatw
  • EdDev
  • RoniKishner
  • azhivovk
  • dshchedr
  • frenzyfriday
  • nirdothan
  • orelmisan
  • rnetser
  • servolkov
  • vsibirsk
  • yossisegev
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
AI Features
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])

💡 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.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 28, 2026
@EdDev
Copy link
Copy Markdown
Contributor

EdDev commented Apr 30, 2026

@coderabbitai the commit message does not comply with docs/AI_CONTRIBUTION_POLICY.md , please check why it was not detected.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the AI contribution policy
cat docs/AI_CONTRIBUTION_POLICY.md

echo "---COMMITS---"
# Get commit messages for this PR
gh pr view 4638 --json commits --jq '.commits[] | {oid: .oid, message: .messageHeadline, body: .messageBody}'

echo "---CODERABBIT CONFIG---"
cat .coderabbit.yaml

Length of output: 4695


🏁 Script executed:

#!/bin/bash
echo "---AGENTS.md content---"
cat AGENTS.md

echo ""
echo "---Check if AI_CONTRIBUTION_POLICY.md exists on main---"
git show main:docs/AI_CONTRIBUTION_POLICY.md 2>/dev/null || echo "NOT FOUND on main"

echo ""
echo "---Search for AI_CONTRIBUTION_POLICY anywhere in repo---"
fd -i "AI_CONTRIBUTION_POLICY"

echo ""
echo "---Search for commit message policy in AGENTS.md---"
grep -i -n "commit\|ai.*contrib\|co-author\|co_author\|signed" AGENTS.md | head -40

Length of output: 7457


@EdDev: I've investigated the root causes. Here's what I found:


The Violation

The commit uses the wrong trailer keyword:

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>   ← actual
Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>      ← required by policy

The policy on main (docs/AI_CONTRIBUTION_POLICY.md) explicitly mandates the Assisted-by: trailer format. Co-Authored-By: is a GitHub convention for human co-authors, not the project's AI disclosure trailer.


Why It Wasn't Detected — Two Root Causes

1. AGENTS.md is absent from the PR branch

The .coderabbit.yaml code_guidelines section is configured to load rules from AGENTS.md:

code_guidelines:
  enabled: true
  filePatterns:
    - "AGENTS.md"

However, AGENTS.md does not exist in this PR's branch, so CodeRabbit loaded no code guidelines at all.

2. No commit-message validation rule in reviews.instructions

The reviews.instructions block in .coderabbit.yaml only covers PR template section validation. There is no rule instructing CodeRabbit to check commit messages for the Assisted-by: trailer format or compliance with docs/AI_CONTRIBUTION_POLICY.md.


Recommended Fixes

To prevent this class of miss going forward, two changes to .coderabbit.yaml are needed:

a) Add a commit message validation rule to reviews.instructions:

reviews:
  instructions: |
    ## Commit Message Validation
    For every commit in this PR, check the commit message trailers:
    - If the commit was AI-assisted, the commit MUST include an `Assisted-by:` trailer
      in the format: `Assisted-by: <Model Name> <noreply@anthropic.com>` (or equivalent provider).
    - `Co-Authored-By:` with an AI model address is NOT compliant — flag it as HIGH severity
      and ask the author to replace it with `Assisted-by:`.
    - All commits MUST include a `Signed-off-by:` trailer.
    ...

b) Ensure AGENTS.md (or an equivalent policy file) exists on every branch that receives reviews, OR change filePatterns to also include docs/AI_CONTRIBUTION_POLICY.md so the policy is always available to CodeRabbit's guidelines engine.


🧠 Learnings used
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:55.037Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers minimal pre-merge checks in CodeRabbit configuration: only docstrings enforcement (80% threshold) is needed, not title or description checks.

Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 4118
File: utilities/database.py:0-0
Timestamp: 2026-03-17T01:32:13.674Z
Learning: In RedHatQE/openshift-virtualization-tests, maintainer dshchedr prefers CodeRabbit to post targeted inline comments on the Files Changed tab at each applicable location rather than aggregating multiple issues into a single PR discussion thread reply.

Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 4118
File: utilities/database.py:0-0
Timestamp: 2026-03-17T01:32:13.674Z
Learning: In RedHatQE/openshift-virtualization-tests, maintainer dshchedr prefers CodeRabbit to post targeted inline comments on the Files changed tab for each applicable location rather than aggregating multiple issues into a single discussion thread.

Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 4000
File: conftest.py:856-858
Timestamp: 2026-02-27T11:25:14.486Z
Learning: In the openshift-virtualization-tests repository, the AI-analysis related code sections in conftest.py (specifically the code that calls setup_ai_analysis() and enrich_junit_xml()) are vendored from https://github.com/myk-org/jenkins-job-insight/blob/main/examples/pytest-junitxml and should be kept as-is to maintain consistency with upstream for easier maintenance and updates. Do not flag style issues in these vendored sections.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR `#2469`) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3371
File: scripts/tests_analyzer/compare_coderabbit_decisions.py:1-3
Timestamp: 2026-01-12T18:39:22.849Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep tool attribution comments (like "Generated using Claude cli") in production code files. These references should not be flagged for removal.

Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 3571
File: tests/storage/storage_migration/utils.py:158-167
Timestamp: 2026-01-25T13:18:26.819Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers to avoid nitpicky style changes (e.g., removing `.keys()` from dict membership checks) because verifying every change is expensive. Be cautious about suggesting low-impact stylistic improvements that require verification overhead.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 4328
File: tests/network/flat_overlay/test_multi_network_policy.py:32-32
Timestamp: 2026-04-14T16:15:36.741Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, when reviewing any PR with "Quarantine" in the title or a `quarantine` label, always verify compliance with docs/QUARANTINE_GUIDELINES.md:
- Category 1 (Product Bug): must use `pytest.mark.jira("CNV-XXXXX", run=False)` — the `pytest_jira` plugin conditionally skips the test when the Jira issue is open. Do NOT suggest replacing this with `xfail`.
- Category 2 (Automation Issue): must use `pytest.mark.xfail(run=False, reason=...)` — pytest itself handles the skip.
Flag any quarantine PR that uses the wrong category marker, is missing a Jira ticket reference, or uses `run=False` in the wrong context. Raising these compliance questions is always appropriate for quarantine PRs, even if the final resolution confirms the marker is correct.

Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests PR: 4417
File: tests/network/localnet/migration_stuntime/conftest.py:26-58
Timestamp: 2026-04-20T14:11:01.712Z
Learning: In `tests/network/localnet/migration_stuntime/conftest.py`, setting `pod_anti_affinity=False` on the server VM (`localnet_stuntime_server_vm`) is intentional and not a no-op error. The stuntime scenario requires both VMs to co-locate on the same node at startup (the client uses `node_selector` to match the server's node), so anti-affinity is explicitly disabled on both fixtures for symmetry and to make the scheduling contract clear to future maintainers. Do not flag this as a redundant or no-op argument.

Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR `#2199` depends on PR `#2139` which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.

Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR `#2199` depends on PR `#2139` which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.

Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests PR: 4417
File: tests/network/localnet/migration_stuntime/conftest.py:100-102
Timestamp: 2026-04-16T07:39:23.678Z
Learning: In `tests/network/localnet/migration_stuntime/conftest.py`, calling `client_vm.update_template_node_selector(node_selector=None)` after `run_vms(...)` is intentional and correct. Clearing the node selector on a running VM's template does NOT affect the currently running VMI/pod; it only applies to the next pod lifecycle event. In the stuntime fixture pattern, the client VM is initially pinned to the server's node (so they co-locate on startup), then the template node selector is cleared so that the post-migration target pod can land on any node. This works on any KubeVirt cluster regardless of vmRolloutStrategy (LiveUpdate or staged). Do not flag this pattern as requiring LiveUpdate in future reviews.

Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 4147
File: tests/network/upgrade/test_upgrade_network.py:166-177
Timestamp: 2026-03-19T10:37:02.008Z
Learning: In the openshift-virtualization-tests repository, unused fixture parameters in test methods (e.g., `bridge_on_one_node` in `tests/network/upgrade/test_upgrade_network.py`) that exist purely for pytest dependency ordering should not be flagged for removal, and no follow-up issues need to be opened for them. This is an intentional pattern, consistent with the analogous convention for fixture definitions in this codebase.

Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.

Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.

@Anatw
Copy link
Copy Markdown
Contributor Author

Anatw commented May 3, 2026

Changes - according to CR comments:

  • Updated new_pod_anti_affinity() docstring to explain Kubernetes behavior and cross-namespace matching
  • Added LOCALNET_VM_ANTI_AFFINITY constant in liblocalnet.py to remove code duplication and improve readability.
    Replaced code duplication with the new LOCALNET_VM_ANTI_AFFINITY constant (in localnet/conftest.py and localnet/ipam/conftest.py).

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/network/localnet/liblocalnet.py`:
- Around line 143-144: The current assignment vmi_spec.affinity = affinity
attaches the shared mutable LOCALNET_VM_ANTI_AFFINITY instance by reference
(used by localnet fixtures), causing mutations to leak across VMs; fix it by
making a deep copy of the incoming affinity before attaching it to vmi_spec
(e.g., use copy.deepcopy) so vmi_spec.affinity gets an independent nested
dataclass tree; update the code around the affinity parameter handling in
liblocalnet.py where vmi_spec.affinity is set and ensure copy is
imported/available.
🪄 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: a305a2f5-b2ce-4aab-85de-5a51aec6160d

📥 Commits

Reviewing files that changed from the base of the PR and between 66a5b77 and af91aa1.

📒 Files selected for processing (5)
  • libs/vm/affinity.py
  • libs/vm/spec.py
  • tests/network/localnet/conftest.py
  • tests/network/localnet/ipam/conftest.py
  • tests/network/localnet/liblocalnet.py

Comment thread tests/network/localnet/liblocalnet.py Outdated
Previously, localnet_vm always applied pod anti-affinity with cross-namespace
selector. This prevented customization for scenarios like stuntime tests that
need VMs to co-locate initially.

Following the network_data pattern, callers now pass the Affinity structure
they need, or None for no affinity constraints.

The new_pod_anti_affinity helper now sets namespaceSelector={} automatically,
eliminating the need for manual configuration after construction.

podAntiAffinity is optional fields in the Affinity class to match
Kubernetes semantics.

Signed-off-by: Anat Wax <awax@redhat.com>
Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
@Anatw Anatw force-pushed the refactor_localnet_vm_affinity branch from af91aa1 to 33e0d06 Compare May 3, 2026 11:35
@Anatw
Copy link
Copy Markdown
Contributor Author

Anatw commented May 3, 2026

Change - according to CodeRabbit review:
Inside localnet_vm(), deep copy the affinity parameter to prevents multiple VMs from sharing the same mutable object, which would cause mutations in one VM to leak into others.

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.

9 participants