[krcmarik]

Add verify_rdm_disk_bus_types() to validate that when rdmAsLun=true is set on the migration plan, RDM disks are mapped as LUN devices with SCSI bus (lun.bus: scsi) instead of regular virtio disks.

• Add rdm_as_lun parameter to create_plan_resource()
• Add rdm_as_lun config flag to RDM test configurations
• Call verify_rdm_disk_bus_types in both virtual and physical RDM copy-offload test classes

Summary by CodeRabbit

• New Features

  • Added an optional configuration to map RDM disks as LUN devices using SCSI buses instead of virtio.
  • Added validation to confirm migrated RDM disks use the expected device types and bus assignments.

• Tests

  • Expanded virtual and physical RDM migration coverage to verify LUN mapping after migration.

[coderabbitai]

Warning

Review limit reached

@krcmarik, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 196565ee-4361-45b1-bf4a-6b3586cd2737

📥 Commits

Reviewing files that changed from the base of the PR and between 1e38146 and 8ba8e2b.

📒 Files selected for processing (3)

• AGENTS.md
• tests/copyoffload/test_copyoffload_migration.py
• utilities/post_migration.py

Walkthrough

The change adds the optional rdm_as_lun plan setting. RDM copy-offload tests enable it, pass it to plan creation, and verify that migrated LUN disks use SCSI buses while regular disks use virtio buses.

Changes

RDM LUN mapping

Layer / File(s)	Summary
Plan configuration and test setup utilities/mtv_migration.py, tests/tests_config/config.py, AGENTS.md	create_plan_resource accepts and forwards rdm_as_lun. Virtual and physical RDM configurations enable the option. The setting behavior is documented.
Migrated disk bus verification utilities/post_migration.py	verify_rdm_disk_bus_types inspects migrated VMs and validates SCSI buses for LUN disks and virtio buses for regular disks.
Virtual and physical RDM test integration tests/copyoffload/test_copyoffload_migration.py	Both RDM migration scenarios pass rdm_as_lun and run post-migration disk bus validation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RDMTest
  participant PlanResource
  participant VirtualMachine
  RDMTest->>PlanResource: Create plan
  PlanResource->>VirtualMachine: Migrate RDM disks
  RDMTest->>VirtualMachine: Verify disk buses
  VirtualMachine-->>RDMTest: Return bus validation

Loading

Possibly related PRs

• RedHatQE/mtv-api-tests#509: Modifies RDM copy-offload migration tests and configurations.

Suggested reviewers: prabinovredhat, tshefi

🚥 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 clearly identifies the RDM disk bus verification added for the rdmAsLun migration configuration.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[redhat-qe-bot]

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 (PR author or approvers)
• /hold cancel - Unblock PR merging (PR author or approvers)
• /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)
• /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
• /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

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

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:

• krcmarik
• 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

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.

[qodo-code-review]

PR Summary by Qodo

feat(MTV-689): Add RDM disk bus type verification for rdmAsLun

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add verify_rdm_disk_bus_types() helper to validate RDM disks mapped as LUN devices with SCSI bus
 when rdmAsLun=true.
• Add rdm_as_lun parameter to create_plan_resource() to set the plan flag conditionally.
• Enable rdm_as_lun in virtual and physical RDM copy-offload test configs and call the new
 verification in both test classes.
• Document the new rdm_as_lun test parameter in AGENTS.md.

Diagram

graph TD
  Config["tests_config: rdm_as_lun"] --> PlanFn["create_plan_resource()"] --> PlanCR[(Plan CR)]
  TestClass["Copy-offload RDM Tests"] --> PlanFn
  TestClass --> Verify["verify_rdm_disk_bus_types()"] --> VM[(Migrated VM CR)]
  VM --> Check{"lun.bus == scsi?"}
  Check -->|pass| Pass([Assertion Pass])
  Check -->|fail| Fail([AssertionError])

  subgraph Legend
    direction LR
    _cfg["Config/Input"] ~~~ _proc(["Process/Function"]) ~~~ _db[(Resource)] ~~~ _dec{"Decision"}
  end

Loading

High-Level Assessment

The PR follows the existing pattern of verification helpers in post_migration.py (e.g., verify_vm_disk_count) and reuses create_plan_resource's optional-kwargs convention, which is consistent with the codebase's established test-utility architecture. No meaningfully different approach warrants consideration for this scoped test-verification addition.

Files changed (5) +81 / -2

Enhancement (3) +78 / -2

test_copyoffload_migration.pyWire rdm_as_lun and verification into RDM copy-offload tests +9/-1

Wire rdm_as_lun and verification into RDM copy-offload tests

• Imports verify_rdm_disk_bus_types, passes rdm_as_lun from prepared_plan into create_plan_resource, and invokes verify_rdm_disk_bus_types after VM checks in both virtual and physical RDM test classes.

tests/copyoffload/test_copyoffload_migration.py

mtv_migration.pyAdd rdm_as_lun parameter to create_plan_resource +6/-0

Add rdm_as_lun parameter to create_plan_resource

• Adds an optional rdm_as_lun parameter that, when set, populates plan_kwargs to configure RDM-as-LUN mapping on the migration plan CR.

utilities/mtv_migration.py

post_migration.pyAdd verify_rdm_disk_bus_types verification helper +63/-1

Add verify_rdm_disk_bus_types verification helper

• Introduces verify_rdm_disk_bus_types() which fetches the migrated VM CR and asserts that LUN disks use SCSI bus while regular disks use virtio; also adds a TYPE_CHECKING import for OCPProvider and a VirtualMachine import.

utilities/post_migration.py

Documentation (1) +1 / -0

AGENTS.mdDocument rdm_as_lun test parameter +1/-0

Document rdm_as_lun test parameter

• Adds a row describing the new rdm_as_lun test configuration flag used to map RDM disks as LUN devices with SCSI bus.

AGENTS.md

Other (1) +2 / -0

config.pyEnable rdm_as_lun in RDM copy-offload test configs +2/-0

Enable rdm_as_lun in RDM copy-offload test configs

• Sets rdm_as_lun to True for the virtual and physical RDM disk copy-offload migration test configurations.

tests/tests_config/config.py

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@AGENTS.md`:
- Line 905: Update the `rdm_as_lun` table row in `AGENTS.md` to include the
missing trailing pipe, matching the surrounding Markdown table rows and
satisfying MD055.

In `@tests/copyoffload/test_copyoffload_migration.py`:
- Line 1575: Require the rdm_as_lun configuration in both RDM test plan
construction sites by replacing the fallback get access at
tests/copyoffload/test_copyoffload_migration.py lines 1575 and 1755 with direct
indexing. This ensures missing configuration fails immediately during plan
creation rather than defaulting to False.

In `@utilities/post_migration.py`:
- Around line 1651-1653: Justify the Any annotations for the plan and disks
parameters in the surrounding function. Add a concise comment explaining their
heterogeneous schemas, or replace Any with shared structured types that model
the supported test-plan and KubeVirt disk data; apply the same treatment to both
parameters.
- Around line 1665-1667: Update the docstring for the function containing the
disk bus validation to add ValueError to its Raises section, documenting that it
occurs when destination_provider.ocp_resource is unavailable. Preserve the
existing AssertionError documentation.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cf315456-206b-45c0-a982-1dee60cd4835

📥 Commits

Reviewing files that changed from the base of the PR and between 321ee60 and 1e38146.

📒 Files selected for processing (5)

• AGENTS.md
• tests/copyoffload/test_copyoffload_migration.py
• tests/tests_config/config.py
• utilities/mtv_migration.py
• utilities/post_migration.py

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Wrong VM lookup name/namespace ✓ Resolved 🐞 Bug ≡ Correctness

Description

verify_rdm_disk_bus_types() queries the destination VirtualMachine using vm_config["name"] and the
passed target_namespace, but destination VMs may be renamed (targetName/DNS-1123 sanitization) and
may live in plan["_vm_target_namespace"]. This can raise NotFound/ensure_exists failures even when
migration succeeded.

Code

utilities/post_migration.py[R1675-1678]

+        migrated_vm = VirtualMachine(
+            client=destination_provider.ocp_resource.client,
+            name=vm_name,
+            namespace=target_namespace,

Relevance

●●● Strong

Team previously fixed wrong VM namespace/name lookups causing false NotFound; likely accept aligning
with plan-resolved target.

PR-#283
PR-#602

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repo already establishes that destination VM names and namespaces can differ from the source
name/plan namespace; the new helper does not follow that established resolution logic, so it can
query a non-existent resource and fail tests incorrectly.

utilities/post_migration.py[1668-1680]
utilities/post_migration.py[1725-1759]
utilities/naming.py[50-67]
libs/providers/openshift.py[149-179]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`verify_rdm_disk_bus_types()` looks up the migrated KubeVirt `VirtualMachine` using the source VM name (`vm_config["name"]`) and a caller-provided `target_namespace`. In this repo, destination VM lookups must resolve the expected destination VM name (via `targetName` override or DNS-1123 sanitization) and must use the VM namespace stored in `plan["_vm_target_namespace"]` when present.

## Issue Context
- `check_vms()` already uses `vm_namespace = plan["_vm_target_namespace"]` and `resolve_destination_vm_name(vm)` for destination lookups.
- `OCPProvider.vm_dict()` also sanitizes destination VM names before creating a `VirtualMachine(... ensure_exists=True)`.

## Fix Focus Areas
- utilities/post_migration.py[1650-1706]
- utilities/post_migration.py[1725-1760]
- utilities/naming.py[50-67]
- libs/providers/openshift.py[149-179]

## Implementation notes
- In `verify_rdm_disk_bus_types()`, compute:
 - `dest_vm_name = resolve_destination_vm_name(vm_config)` (or at minimum `sanitize_kubernetes_name(vm_config["name"])` with `targetName` precedence).
 - `vm_namespace = plan.get("_vm_target_namespace") or target_namespace`.
- Use `dest_vm_name` and `vm_namespace` when constructing `VirtualMachine(...)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Optional flag KeyError ✗ Dismissed 🐞 Bug ☼ Reliability ⭐ New

Description

test_create_plan() passes rdm_as_lun=prepared_plan["rdm_as_lun"], which raises KeyError for
any prepared plan config that omits this optional flag, failing before plan creation. This makes
rdm_as_lun effectively required despite being documented as non-required and despite
create_plan_resource() treating it as an optional None default.

Code

tests/copyoffload/test_copyoffload_migration.py[1575]

+            rdm_as_lun=prepared_plan["rdm_as_lun"],

Relevance

●●● Strong

Team previously accepted switching optional plan flags to .get(...) to avoid KeyError when omitted.

PR-#557

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The tests now directly index prepared_plan["rdm_as_lun"], which will raise KeyError when the key
is absent. AGENTS.md explicitly marks rdm_as_lun as not required, and the prepared_plan
fixture deep-copies the plan config without adding a default for this key, so omission is possible
and would crash these tests. Additionally, create_plan_resource() defines rdm_as_lun as an
optional parameter with a None default, reinforcing that callers should not require it.

tests/copyoffload/test_copyoffload_migration.py[1569-1576]
tests/copyoffload/test_copyoffload_migration.py[1748-1754]
AGENTS.md[893-906]
conftest.py[1041-1045]
utilities/mtv_migration.py[147-151]
PR-#557

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rdm_as_lun` is documented as optional, but the copy-offload RDM tests index it directly (`prepared_plan["rdm_as_lun"]`), which can raise `KeyError` when the flag is omitted.

### Issue Context
- `AGENTS.md` documents `rdm_as_lun` as **Required: No**.
- `create_plan_resource(..., rdm_as_lun: bool | None = None)` treats the flag as optional and only sets it on the Plan CR when non-`None`.
- The `prepared_plan` fixture deep-copies the plan config and does not inject a default for `rdm_as_lun`, so omission in config propagates to runtime.

### Fix Focus Areas
- tests/copyoffload/test_copyoffload_migration.py[1575-1575]
- tests/copyoffload/test_copyoffload_migration.py[1753-1753]

### Expected change
Replace direct indexing with optional access, e.g.:
```py
rdm_as_lun=prepared_plan.get("rdm_as_lun")
```
(or `.get("rdm_as_lun", False)` if you want an explicit default), at both call sites.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. RDM bus check ignores flag ✗ Dismissed 🐞 Bug ≡ Correctness

Description

verify_rdm_disk_bus_types() always asserts at least one LUN disk exists, but its own docstring
states this should only be required when rdmAsLun/rdm_as_lun is enabled. If the helper is called for
plans where rdm_as_lun is unset/false, it will fail even when the migrated VM layout is correct.

Code

utilities/post_migration.py[R1683-1687]

+        lun_disks = [d for d in disks if d.get("lun")]
+        regular_disks = [d for d in disks if d.get("disk")]
+
+        assert lun_disks, (
+            f"No LUN-type disks found on VM '{vm_name}'. Expected at least one RDM disk mapped as LUN. Disks: {disks}"

Relevance

●●● Strong

Repo commonly gates verifications on explicit plan flags; unconditional asserts against
docstring/intent usually get fixed.

PR-#572
PR-#608

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new helper documents that the requirement applies only when rdmAsLun=true, but the
implementation never checks the plan flag and always asserts lun_disks exists.

utilities/post_migration.py[1655-1668]
utilities/post_migration.py[1683-1688]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`verify_rdm_disk_bus_types()` unconditionally asserts that `lun_disks` is non-empty. This contradicts the function contract described in the docstring (“When rdmAsLun=true is set...”), and makes the helper unsafe to call for migrations not using the feature.

## Issue Context
The helper is meant to validate a *conditional* behavior (only when the plan enables RDM-as-LUN). The implementation currently enforces the LUN expectation regardless of the plan settings.

## Fix Focus Areas
- utilities/post_migration.py[1650-1706]

## Implementation notes
- Add an early guard at the start of the function:
 - If `plan.get("rdm_as_lun") is not True`: log that the check is skipped and `return`.
- Keep the existing strict assertions when the flag is enabled.
- (Optional) If you want stricter behavior, you can also assert that the plan explicitly sets the flag when the test scenario includes RDM disks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 100 rules

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 8ba8e2b

Results up to commit 1e38146 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Wrong VM lookup name/namespace ✓ Resolved 🐞 Bug ≡ Correctness

Description

verify_rdm_disk_bus_types() queries the destination VirtualMachine using vm_config["name"] and the
passed target_namespace, but destination VMs may be renamed (targetName/DNS-1123 sanitization) and
may live in plan["_vm_target_namespace"]. This can raise NotFound/ensure_exists failures even when
migration succeeded.

Code

utilities/post_migration.py[R1675-1678]

+        migrated_vm = VirtualMachine(
+            client=destination_provider.ocp_resource.client,
+            name=vm_name,
+            namespace=target_namespace,

Relevance

●●● Strong

Team previously fixed wrong VM namespace/name lookups causing false NotFound; likely accept aligning
with plan-resolved target.

PR-#283
PR-#602

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repo already establishes that destination VM names and namespaces can differ from the source
name/plan namespace; the new helper does not follow that established resolution logic, so it can
query a non-existent resource and fail tests incorrectly.

utilities/post_migration.py[1668-1680]
utilities/post_migration.py[1725-1759]
utilities/naming.py[50-67]
libs/providers/openshift.py[149-179]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`verify_rdm_disk_bus_types()` looks up the migrated KubeVirt `VirtualMachine` using the source VM name (`vm_config["name"]`) and a caller-provided `target_namespace`. In this repo, destination VM lookups must resolve the expected destination VM name (via `targetName` override or DNS-1123 sanitization) and must use the VM namespace stored in `plan["_vm_target_namespace"]` when present.

## Issue Context
- `check_vms()` already uses `vm_namespace = plan["_vm_target_namespace"]` and `resolve_destination_vm_name(vm)` for destination lookups.
- `OCPProvider.vm_dict()` also sanitizes destination VM names before creating a `VirtualMachine(... ensure_exists=True)`.

## Fix Focus Areas
- utilities/post_migration.py[1650-1706]
- utilities/post_migration.py[1725-1760]
- utilities/naming.py[50-67]
- libs/providers/openshift.py[149-179]

## Implementation notes
- In `verify_rdm_disk_bus_types()`, compute:
 - `dest_vm_name = resolve_destination_vm_name(vm_config)` (or at minimum `sanitize_kubernetes_name(vm_config["name"])` with `targetName` precedence).
 - `vm_namespace = plan.get("_vm_target_namespace") or target_namespace`.
- Use `dest_vm_name` and `vm_namespace` when constructing `VirtualMachine(...)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. RDM bus check ignores flag ✗ Dismissed 🐞 Bug ≡ Correctness

Description

verify_rdm_disk_bus_types() always asserts at least one LUN disk exists, but its own docstring
states this should only be required when rdmAsLun/rdm_as_lun is enabled. If the helper is called for
plans where rdm_as_lun is unset/false, it will fail even when the migrated VM layout is correct.

Code

utilities/post_migration.py[R1683-1687]

+        lun_disks = [d for d in disks if d.get("lun")]
+        regular_disks = [d for d in disks if d.get("disk")]
+
+        assert lun_disks, (
+            f"No LUN-type disks found on VM '{vm_name}'. Expected at least one RDM disk mapped as LUN. Disks: {disks}"

Relevance

●●● Strong

Repo commonly gates verifications on explicit plan flags; unconditional asserts against
docstring/intent usually get fixed.

PR-#572
PR-#608

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new helper documents that the requirement applies only when rdmAsLun=true, but the
implementation never checks the plan flag and always asserts lun_disks exists.

utilities/post_migration.py[1655-1668]
utilities/post_migration.py[1683-1688]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`verify_rdm_disk_bus_types()` unconditionally asserts that `lun_disks` is non-empty. This contradicts the function contract described in the docstring (“When rdmAsLun=true is set...”), and makes the helper unsafe to call for migrations not using the feature.

## Issue Context
The helper is meant to validate a *conditional* behavior (only when the plan enables RDM-as-LUN). The implementation currently enforces the LUN expectation regardless of the plan settings.

## Fix Focus Areas
- utilities/post_migration.py[1650-1706]

## Implementation notes
- Add an early guard at the start of the function:
 - If `plan.get("rdm_as_lun") is not True`: log that the check is skipped and `return`.
- Keep the existing strict assertions when the flag is enabled.
- (Optional) If you want stricter behavior, you can also assert that the plan explicitly sets the flag when the test scenario includes RDM disks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8ba8e2b

[krcmarik]

/verified

[myakove]

/lgtm
/approve

[redhat-qe-bot2]

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