feat: add rdmAsLun parameter to Plan resource by krcmarik · Pull Request #2754 · RedHatQE/openshift-python-wrapper

krcmarik · 2026-07-02T09:59:58Z

Add rdm_as_lun parameter to the Plan class to support mapping RDM (Raw Device Mapping) disks as LUN devices with SCSI bus on the target VM during MTV migrations. Only applies to vSphere source providers.

Summary by CodeRabbit

New Features
- Added support for an optional rdm_as_lun setting when creating a plan.
- When enabled, this value is included in the generated plan configuration.
- Documented that this option applies to vSphere-based source providers only.

coderabbitai · 2026-07-02T10:00:19Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b40c5b7e-2047-498f-9276-d2ef62ffaa88

📥 Commits

Reviewing files that changed from the base of the PR and between 343f639 and de5a2d2.

📒 Files selected for processing (1)

ocp_resources/plan.py

💤 Files with no reviewable changes (1)

ocp_resources/plan.py

Walkthrough

Adds an optional rdm_as_lun boolean parameter to Plan in ocp_resources/plan.py, documents its vSphere-only behavior, stores it on the instance, and includes spec["rdmAsLun"] in the generated spec when set.

Changes

Plan rdm_as_lun support

Layer / File(s)	Summary
rdm_as_lun parameter and spec generation `ocp_resources/plan.py`	Documents `rdm_as_lun` in the class docstring, adds it to `Plan.__init__`, stores it on the instance, and conditionally emits `spec["rdmAsLun"]` in `to_dict()` when set.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

RedHatQE/openshift-python-wrapper#2741: Also extends ocp_resources/plan.py to add an optional field to Plan.__init__ and emit it from to_dict().

Suggested reviewers: myakove

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is too brief and misses the required template sections like Short description, More details, and issue references.	Expand the PR description to fill the template sections, including purpose, details, issue links, reviewer notes, and bug context.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: adding rdmAsLun support to the Plan resource.
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

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

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

qodo-code-review · 2026-07-02T10:00:45Z

PR Summary by Qodo

Add rdmAsLun option to Plan resource for vSphere RDM migrations

✨ Enhancement 🕐 10-20 Minutes

AI Description

• Add rdm_as_lun parameter to the Plan resource API.
• Serialize the option into the Plan spec as rdmAsLun when provided.
• Document behavior: map vSphere RDM disks as SCSI LUNs on the target VM.

Diagram

graph TD
  A["Caller code"] --> B["Plan class"] --> C["Plan.to_dict()"] --> D["Plan CR spec"] --> E["MTV/Forklift controller"]

High-Level Assessment

The PR’s approach (add an explicit optional parameter and map it to the CRD field name during serialization) is the most direct and consistent with how other Plan fields are modeled, while keeping API usage discoverable via type hints and docstrings.

Files changed (1) +9 / -0

Enhancement (1) +9 / -0

plan.pyAdd rdm_as_lun Plan option and serialize as rdmAsLun +9/-0

Add rdm_as_lun Plan option and serialize as rdmAsLun

• Introduces a new optional 'rdm_as_lun' constructor parameter on 'Plan' and stores it on the instance. Updates 'to_dict()' to include 'spec.rdmAsLun' when set, and extends the class docstring to document vSphere-only RDM-as-LUN behavior.

ocp_resources/plan.py

redhat-qe-bot1 · 2026-07-02T10:02:28Z

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
/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 python-module-install - Test Python package installation
/retest conventional-title - Validate commit message format
/retest all - Run all available tests

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:

Approval: /approve from at least one approver
Status Checks: All required status checks must pass
No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

myakove
rnetser

Reviewers:

myakove
rnetser

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)

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
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 · 2026-07-02T10:03:18Z

Code Review by Qodo

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

1. vSphere-only flag not enforced 🐞 Bug ⚙ Maintainability

Description

Plan documents rdm_as_lun as “Only applies to vSphere source providers”, but to_dict() will
serialize spec["rdmAsLun"] whenever the parameter is set, regardless of source provider type. This
lets callers generate Plan specs that contradict the class’ documented constraint, making the API
contract ambiguous.

Code

ocp_resources/plan.py[R223-224]

+            if self.rdm_as_lun is not None:
+                spec["rdmAsLun"] = self.rdm_as_lun

Relevance

⭐ Low
Team often rejects adding spec validation/constraints in to_dict (e.g., required/non-empty checks
rejected in PRs #2529, #2520).
PR-#2529
PR-#2520
PR-#2545

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docstring explicitly states the option is vSphere-only, but the serializer adds rdmAsLun with
no conditional logic tied to source provider type.

ocp_resources/plan.py[45-48]
ocp_resources/plan.py[223-224]

Agent prompt

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

### Issue description
`Plan` claims `rdm_as_lun` only applies to vSphere sources, but the implementation always emits `spec.rdmAsLun` when set. This creates an ambiguous contract: the class documentation implies a constraint that the code does not enforce.

### Issue Context
The class currently does not track the source provider type (only the provider name/namespace), so it cannot enforce the constraint unless additional context is supplied.

### Fix Focus Areas
- ocp_resources/plan.py[45-48]
- ocp_resources/plan.py[223-224]

### Suggested fix approaches (pick one)
1) **Enforce the constraint**: add an optional `source_provider_type` (or similar) parameter to `Plan.__init__`, and if `rdm_as_lun is not None` and `source_provider_type != ProviderType.VSPHERE`, raise `ValueError`.
2) **Clarify documentation**: if enforcement is intentionally out-of-scope for this class, adjust the docstring wording to indicate that the field is *only meaningful/used by MTV* for vSphere sources, but will still be serialized if provided.

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

myakove · 2026-07-08T10:35:55Z

Code Review by Qodo

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

vSphere-only flag not enforced 🐞 Bug ⚙ Maintainability

Description

Plan documents rdm_as_lun as “Only applies to vSphere source providers”, but to_dict() will
serialize spec["rdmAsLun"] whenever the parameter is set, regardless of source provider type. This
lets callers generate Plan specs that contradict the class’ documented constraint, making the API
contract ambiguous.
Code

[ocp_resources/plan.py[R223-224]](https://github.com/RedHatQE/openshift-python-wrapper/pull/2754/files#diff-9cea905c5aa47482df948a3f10f6c25725e5cce578cca3ade7c0caa6373ac23eR223-R224)
+            if self.rdm_as_lun is not None:
+                spec["rdmAsLun"] = self.rdm_as_lun
Relevance

⭐ Low
Team often rejects adding spec validation/constraints in to_dict (e.g., required/non-empty checks
rejected in PRs #2529, #2520).

PR-#2529
PR-#2520
PR-#2545
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docstring explicitly states the option is vSphere-only, but the serializer adds rdmAsLun with
no conditional logic tied to source provider type.

[ocp_resources/plan.py[45-48]](https://github.com/RedHatQE/openshift-python-wrapper/blob/343f639cb8addfa0f164bd1e79b48bd83931709e/ocp_resources/plan.py/#L45-L48)
[ocp_resources/plan.py[223-224]](https://github.com/RedHatQE/openshift-python-wrapper/blob/343f639cb8addfa0f164bd1e79b48bd83931709e/ocp_resources/plan.py/#L223-L224)
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Plan` claims `rdm_as_lun` only applies to vSphere sources, but the implementation always emits `spec.rdmAsLun` when set. This creates an ambiguous contract: the class documentation implies a constraint that the code does not enforce.

### Issue Context
The class currently does not track the source provider type (only the provider name/namespace), so it cannot enforce the constraint unless additional context is supplied.

### Fix Focus Areas
- ocp_resources/plan.py[45-48]
- ocp_resources/plan.py[223-224]

### Suggested fix approaches (pick one)
1) **Enforce the constraint**: add an optional `source_provider_type` (or similar) parameter to `Plan.__init__`, and if `rdm_as_lun is not None` and `source_provider_type != ProviderType.VSPHERE`, raise `ValueError`.
2) **Clarify documentation**: if enforcement is intentionally out-of-scope for this class, adjust the docstring wording to indicate that the field is *only meaningful/used by MTV* for vSphere sources, but will still be serialized if provided.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@qodo-code-review[bot] we do not force sent args based on what it taied to, if openshift api accept rdmAsLun without vsphare resriction we allow it

qodo-code-review · 2026-07-08T10:36:08Z

Code Review by Qodo

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

vSphere-only flag not enforced 🐞 Bug ⚙ Maintainability

Description

Plan documents rdm_as_lun as “Only applies to vSphere source providers”, but to_dict() will
serialize spec["rdmAsLun"] whenever the parameter is set, regardless of source provider type. This
lets callers generate Plan specs that contradict the class’ documented constraint, making the API
contract ambiguous.
Code

[ocp_resources/plan.py[R223-224]](https://github.com/RedHatQE/openshift-python-wrapper/pull/2754/files#diff-9cea905c5aa47482df948a3f10f6c25725e5cce578cca3ade7c0caa6373ac23eR223-R224)
+            if self.rdm_as_lun is not None:
+                spec["rdmAsLun"] = self.rdm_as_lun
Relevance

⭐ Low
Team often rejects adding spec validation/constraints in to_dict (e.g., required/non-empty checks
rejected in PRs #2529, #2520).

PR-#2529
PR-#2520
PR-#2545
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docstring explicitly states the option is vSphere-only, but the serializer adds rdmAsLun with
no conditional logic tied to source provider type.

[ocp_resources/plan.py[45-48]](https://github.com/RedHatQE/openshift-python-wrapper/blob/343f639cb8addfa0f164bd1e79b48bd83931709e/ocp_resources/plan.py/#L45-L48)
[ocp_resources/plan.py[223-224]](https://github.com/RedHatQE/openshift-python-wrapper/blob/343f639cb8addfa0f164bd1e79b48bd83931709e/ocp_resources/plan.py/#L223-L224)
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Plan` claims `rdm_as_lun` only applies to vSphere sources, but the implementation always emits `spec.rdmAsLun` when set. This creates an ambiguous contract: the class documentation implies a constraint that the code does not enforce.

### Issue Context
The class currently does not track the source provider type (only the provider name/namespace), so it cannot enforce the constraint unless additional context is supplied.

### Fix Focus Areas
- ocp_resources/plan.py[45-48]
- ocp_resources/plan.py[223-224]

### Suggested fix approaches (pick one)
1) **Enforce the constraint**: add an optional `source_provider_type` (or similar) parameter to `Plan.__init__`, and if `rdm_as_lun is not None` and `source_provider_type != ProviderType.VSPHERE`, raise `ValueError`.
2) **Clarify documentation**: if enforcement is intentionally out-of-scope for this class, adjust the docstring wording to indicate that the field is *only meaningful/used by MTV* for vSphere sources, but will still be serialized if provided.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
@qodo-code-review[bot] we do not force sent args based on what it taied to, if openshift api accept rdmAsLun without vsphare resriction we allow it

Understood — if the OpenShift API accepts rdmAsLun without a vSphere-side restriction, then serializing it unconditionally is fine.

In that case, finding 1 is really a docs/contract clarity issue, not a bug in the serializer. The best fix would be to soften the docstring so it says the field is vSphere-specific in meaning, but not client-enforced.

If you want, I can suggest precise wording for that doc update.

Add rdm_as_lun parameter to the Plan class to support mapping RDM (Raw Device Mapping) disks as LUN devices with SCSI bus on the target VM during MTV migrations. Only applies to vSphere source providers.

myakove · 2026-07-08T13:20:45Z

/lgtm
/approve

krcmarik · 2026-07-08T15:05:52Z

/verified

redhat-qe-bot1 requested a review from myakove July 2, 2026 10:02

redhat-qe-bot1 assigned krcmarik Jul 2, 2026

redhat-qe-bot1 requested a review from rnetser July 2, 2026 10:02

redhat-qe-bot1 added branch-main size/XS labels Jul 2, 2026

redhat-qe-bot added the needs-rebase label Jul 6, 2026

redhat-qe-bot1 added the has-conflicts label Jul 8, 2026

feat: add rdmAsLun parameter to Plan resource

de5a2d2

Add rdm_as_lun parameter to the Plan class to support mapping RDM (Raw Device Mapping) disks as LUN devices with SCSI bus on the target VM during MTV migrations. Only applies to vSphere source providers.

krcmarik force-pushed the rdmAsLun branch from 343f639 to de5a2d2 Compare July 8, 2026 13:07

redhat-qe-bot removed has-conflicts needs-rebase labels Jul 8, 2026

myakove enabled auto-merge (squash) July 8, 2026 13:20

redhat-qe-bot1 added approved-myakove lgtm-myakove labels Jul 8, 2026

redhat-qe-bot added the verified label Jul 8, 2026

rh-bot-1 added the can-be-merged label Jul 8, 2026

myakove merged commit e147ff8 into RedHatQE:main Jul 8, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add rdmAsLun parameter to Plan resource#2754

feat: add rdmAsLun parameter to Plan resource#2754
myakove merged 1 commit into
RedHatQE:mainfrom
krcmarik:rdmAsLun

krcmarik commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Review ran into problems

Uh oh!

qodo-code-review Bot commented Jul 2, 2026

Uh oh!

redhat-qe-bot1 commented Jul 2, 2026

Uh oh!

qodo-code-review Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

myakove commented Jul 8, 2026

Code Review by Qodo

Uh oh!

qodo-code-review Bot commented Jul 8, 2026

Code Review by Qodo

Uh oh!

myakove commented Jul 8, 2026

Uh oh!

krcmarik commented Jul 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

krcmarik commented Jul 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Review ran into problems

Uh oh!

qodo-code-review Bot commented Jul 2, 2026

PR Summary by Qodo

Uh oh!

redhat-qe-bot1 commented Jul 2, 2026

Welcome! 🎉

🔄 Automatic Actions

📋 Available Commands

PR Status Management

Review & Approval

Testing & Validation

Cherry-pick Operations

Branch Management

Label Management

✅ Merge Requirements

📊 Review Process

💡 Tips

Uh oh!

qodo-code-review Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

myakove commented Jul 8, 2026

Code Review by Qodo

Uh oh!

qodo-code-review Bot commented Jul 8, 2026

Code Review by Qodo

Uh oh!

myakove commented Jul 8, 2026

Uh oh!

krcmarik commented Jul 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

krcmarik commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 2, 2026 •

edited

Loading