Skip to content

feat(MTV-596): copy-offload physical RDM disk migration test - #509

Merged
myakove merged 3 commits into
RedHatQE:mainfrom
prabinovRedhat:feat/MTV-596-compatibility-mode-physical-rdm
May 21, 2026
Merged

myakove merged 3 commits into
RedHatQE:mainfrom
prabinovRedhat:feat/MTV-596-compatibility-mode-physical-rdm

Conversation

@prabinovRedhat

@prabinovRedhat prabinovRedhat commented May 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds copy-offload automation for compatibility-mode physical RDM disks (MTV-596) and aligns MTV-562 with shared RDM configuration validation.

  • New test_copyoffload_rdm_physical_disk_migration plan config (rdm_type: physical → physicalMode + independent_persistent in VMware provider)
  • New TestCopyoffloadRdmPhysicalDiskMigration with full type annotations and standard copy-offload flow (XCOPY + VM checks)
  • New rdm_config class fixture validating copyoffload.rdm_lun_uuid (used by MTV-562 and MTV-596)
  • MTV-562: remove pytest.fail in storagemap step; use rdm_config fixture instead

Verification

Jenkins: both TestCopyoffloadRdmVirtualDiskMigration (MTV-562) and TestCopyoffloadRdmPhysicalDiskMigration (MTV-596) passed.

Requirements

  • copyoffload.rdm_lun_uuid in provider config
  • VMFS datastore_id for RDM mapping (same as existing RDM tests)

Summary by CodeRabbit

  • Tests
    • Added end-to-end tests for RDM physical-disk copy-offload, including XCOPY verification and migrated-disk validation.
    • Added a centralized fixture to validate RDM configuration before class tests run.
    • Expanded existing copy-offload test classes to include SSH key setup and updated test configurations to enable the new physical-disk scenario.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 20, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

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

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ 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: d990584d-0380-4323-9995-2f3a157f7272

📥 Commits

Reviewing files that changed from the base of the PR and between 3fbfae5 and 3cd7fa0.

📒 Files selected for processing (3)
  • tests/copyoffload/conftest.py
  • tests/copyoffload/test_copyoffload_migration.py
  • tests/tests_config/config.py

Walkthrough

Adds an RDM validation fixture, refactors the RDM virtual-disk test to use it, introduces a full RDM physical-disk migration test class (MTV-596), updates several tests to include the SSH-key fixture, and adds test config for the new physical-disk case.

Changes

RDM Copy-Offload Test Infrastructure and Physical Disk Migration Tests

Layer / File(s) Summary
RDM configuration validation fixture
tests/copyoffload/conftest.py
New rdm_config class-scoped fixture validates presence of copyoffload.rdm_lun_uuid in source provider data, raising ValueError if absent and logging on success.
Existing RDM virtual disk test refactor
tests/copyoffload/test_copyoffload_migration.py
TestCopyoffloadRdmVirtualDiskMigration now injects rdm_config via @pytest.mark.usefixtures; removed in-test pytest.fail guard for rdm_lun_uuid.
RDM physical disk migration test class and methods
tests/copyoffload/test_copyoffload_migration.py
New TestCopyoffloadRdmPhysicalDiskMigration (MTV-596) implementing storage map creation with XCOPY config, network map creation, plan creation with VM enrichment, migration execution, XCOPY verification, and post-migration VM/disk validation.
Test configuration for RDM physical disk migration
tests/tests_config/config.py
New test_copyoffload_rdm_physical_disk_migration config entry defining VM xcopy-template-test with guest agent + cloning and add_disks using rdm_type: "physical", copyoffload enabled, warm migration disabled.
Add copyoffload_ssh_key fixture to copyoffload tests
tests/copyoffload/test_copyoffload_migration.py
Multiple copyoffload test classes updated to include copyoffload_ssh_key in their class-level @pytest.mark.usefixtures lists.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • myakove
  • krcmarik
  • solenoci
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the primary change: adding a new test for copy-offload physical RDM disk migration (MTV-596), which aligns perfectly with the changeset additions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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

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 and usage tips.

@redhat-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: Disabled for this repository
  • 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: All label categories are enabled (default configuration)

📋 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)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and 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 conventional-title - Validate commit message format
  • /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. Status Checks: All required status checks must pass
  3. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  4. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • solenoci

Reviewers:

  • krcmarik
  • myakove
  • solenoci
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge
AI Features
  • Conventional Title: Mode: fix (claude/claude-opus-4-6[1m])
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])
  • Test Oracle: Triggers: approved (cursor/gpt-5.4-xhigh-fast); /test-oracle can be used anytime

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

@prabinovRedhat

Copy link
Copy Markdown
Collaborator Author

please review - @tshefi

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/copyoffload/test_copyoffload_migration.py`:
- Around line 1423-1429: The pytest class decorators removed the side-effect
fixture copyoffload_ssh_key causing ESXi SSH key setup to be skipped when
copyoffload.esxi_clone_method=ssh; re-add "copyoffload_ssh_key" to the
pytest.mark.usefixtures list for the affected RDM test classes (the decorators
shown with vmware_cloud_init_ready, multus_network_name, copyoffload_config,
rdm_config, cleanup_migrated_vms) so the fixture runs before tests and restores
ESXi SSH key setup for the RDM flows.
🪄 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: 784e033e-8a78-4f8a-a141-10e3884fef19

📥 Commits

Reviewing files that changed from the base of the PR and between cbe35dd and 38da2f4.

📒 Files selected for processing (3)
  • tests/copyoffload/conftest.py
  • tests/copyoffload/test_copyoffload_migration.py
  • tests/tests_config/config.py

prabinovRedhat and others added 3 commits May 20, 2026 15:51
Add test_copyoffload_rdm_physical_disk_migration and
TestCopyoffloadRdmPhysicalDiskMigration for compatibility-mode physical
RDM (independent persistent). Add rdm_config fixture for rdm_lun_uuid validation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Align MTV-562 with MTV-596 by validating copyoffload.rdm_lun_uuid in the
rdm_config fixture instead of pytest.fail in test_create_storagemap.

Co-authored-by: Cursor <cursoragent@cursor.com>
Register copyoffload_ssh_key on every copy-offload class so SSH cloning
can be enabled via esxi_clone_method in providers.json without per-test
code changes. On VIB (default) the fixture no-ops.

Co-authored-by: Cursor <cursoragent@cursor.com>
@prabinovRedhat
prabinovRedhat force-pushed the feat/MTV-596-compatibility-mode-physical-rdm branch from 3fbfae5 to 3cd7fa0 Compare May 20, 2026 12:53
@redhat-qe-bot

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (3fbfae5).
The following labels were preserved: commented-prabinovRedhat, commented-coderabbitai[bot], lgtm-coderabbitai[bot].

@tshefi

tshefi commented May 20, 2026

Copy link
Copy Markdown
Collaborator

/lgtm

@prabinovRedhat

Copy link
Copy Markdown
Collaborator Author

/verified

@myakove

myakove commented May 21, 2026

Copy link
Copy Markdown
Collaborator

/lgtm
/approve

@myakove
myakove enabled auto-merge (squash) May 21, 2026 07:39
@myakove
myakove merged commit af83d48 into RedHatQE:main May 21, 2026
7 checks passed
@redhat-qe-bot

Copy link
Copy Markdown

New container for ghcr.io/redhatqe/mtv-api-tests:latest published

@prabinovRedhat

Copy link
Copy Markdown
Collaborator Author

/cherry-pick v2.11 v2.10

@rh-bot-1

Copy link
Copy Markdown

Cherry-picked PR feat(MTV-596): copy-offload physical RDM disk migration test into v2.10: #510

@rh-bot-1

Copy link
Copy Markdown

Cherry-picked PR feat(MTV-596): copy-offload physical RDM disk migration test into v2.11: #511

redhat-qe-bot pushed a commit that referenced this pull request May 25, 2026
…511)

* feat(MTV-596): copy-offload physical RDM disk migration test

Add test_copyoffload_rdm_physical_disk_migration and
TestCopyoffloadRdmPhysicalDiskMigration for compatibility-mode physical
RDM (independent persistent). Add rdm_config fixture for rdm_lun_uuid validation.



* refactor(MTV-562): use rdm_config fixture for rdm_lun_uuid validation

Align MTV-562 with MTV-596 by validating copyoffload.rdm_lun_uuid in the
rdm_config fixture instead of pytest.fail in test_create_storagemap.



* fix(copyoffload): add copyoffload_ssh_key fixture to all test classes

Register copyoffload_ssh_key on every copy-offload class so SSH cloning
can be enabled via esxi_clone_method in providers.json without per-test
code changes. On VIB (default) the fixture no-ops.



---------

Co-authored-by: Polina Rabinovich <58551443+prabinovRedhat@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
redhat-qe-bot1 pushed a commit that referenced this pull request May 25, 2026
…510)

* feat(MTV-596): copy-offload physical RDM disk migration test

Add test_copyoffload_rdm_physical_disk_migration and
TestCopyoffloadRdmPhysicalDiskMigration for compatibility-mode physical
RDM (independent persistent). Add rdm_config fixture for rdm_lun_uuid validation.



* refactor(MTV-562): use rdm_config fixture for rdm_lun_uuid validation

Align MTV-562 with MTV-596 by validating copyoffload.rdm_lun_uuid in the
rdm_config fixture instead of pytest.fail in test_create_storagemap.



* fix(copyoffload): add copyoffload_ssh_key fixture to all test classes

Register copyoffload_ssh_key on every copy-offload class so SSH cloning
can be enabled via esxi_clone_method in providers.json without per-test
code changes. On VIB (default) the fixture no-ops.



---------

Co-authored-by: Polina Rabinovich <58551443+prabinovRedhat@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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.

7 participants