Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Comments

Close side panel

feat: ocp_resource plan - Add a few missing parameters#2545

Merged
myakove merged 2 commits intoRedHatQE:mainRedHatQE/openshift-python-wrapper:mainfrom
krcmarik:mtv_plankrcmarik/openshift-python-wrapper:mtv_planCopy head branch name to clipboard
Nov 12, 2025
Merged

feat: ocp_resource plan - Add a few missing parameters#2545
myakove merged 2 commits intoRedHatQE:mainRedHatQE/openshift-python-wrapper:mainfrom
krcmarik:mtv_plankrcmarik/openshift-python-wrapper:mtv_planCopy head branch name to clipboard

Conversation

@krcmarik
Copy link
Contributor

@krcmarik krcmarik commented Oct 26, 2025 •
edited by coderabbitai bot
Loading

  • volumeNameTemplate
  • networkNameTemplate
  • preserveStaticIPs
  • targetNodeSelector
  • targetLabels
  • targetAffinity

Summary by CodeRabbit

  • New Features
    • Optional naming templates for target persistent volumes and network interfaces.
    • Option to preserve static IP addresses during migration.
    • Migration targeting controls: node selection, target labels, and affinity.
    • These new settings are optional and included in exported configurations only when set.

@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025 •
edited
Loading

Walkthrough

Adds optional Plan fields for volume/network naming, static IP preservation, and target placement: extends Plan.init to accept new parameters, assigns them to instance attributes, and updates Plan.to_dict() to emit the new spec keys when provided.

Changes

Cohort / File(s) Summary
Plan resource update
ocp_resources/plan.py		 Update Plan.__init__ signature to accept volume_name_template, network_name_template, preserve_static_ips, target_node_selector, target_labels, target_affinity. Assign these attributes in __init__ and conditionally serialize them in to_dict() using camelCase keys: volumeNameTemplate, networkNameTemplate, preserveStaticIPs, targetNodeSelector, targetLabels, targetAffinity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Single file with consistent pattern: constructor params, attribute assignments, conditional serialization.
  • Pay extra attention to:
    • Correct mapping to camelCase spec keys.
    • Type handling for dict vs. None values.
    • No unintended mutation of existing spec or hook logic.

Possibly related PRs

Suggested labels

verified, can-be-merged

Suggested reviewers

  • myakove
  • rnetser
  • dbasunag

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description only lists parameter names without following the required template structure or providing context about what, why, or which issue is being fixed. Provide a comprehensive description following the template with sections for short description, details, rationale, related issues, and special notes for reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding missing parameters to the ocp_resource plan class.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a54edc and 34ae322.

📒 Files selected for processing (1)
  • ocp_resources/plan.py (5 hunks)
🔇 Additional comments (4)
ocp_resources/plan.py (4)

26-27: Documentation looks good.

The docstring additions clearly describe the purpose of each new parameter and follow the established documentation pattern in the class.

Also applies to: 35-41

98-99: LGTM!

The instance attribute assignments are straightforward and correctly follow the established pattern.

Also applies to: 104-107

174-178: Serialization logic is correct.

The implementation correctly:

  • Conditionally adds fields only when they're not None
  • Converts Python snake_case to API camelCase (including proper capitalization of "IPs")
  • Follows the established pattern for optional field serialization
  • Directly assigns values without unnecessary transformation

Also applies to: 192-202

66-67: Verify new field names against MTV/Forklift Plan CRD specification.

The parameter declarations are correctly typed and consistently implemented in the code. However, the new fields (volumeNameTemplate, networkNameTemplate, preserveStaticIPs, targetNodeSelector, targetLabels, targetAffinity) are not present in publicly available MTV/Forklift Plan CRD documentation.

Confirm these field names match the actual Plan CRD schema in the MTV/Forklift repository before merging.

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
Contributor

redhat-qe-bot commented Oct 26, 2025

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
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified

📋 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

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

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 0 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, or conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • dbasunag
  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • 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.

@krcmarik krcmarik changed the title Plan: Add a few missing parameters feat: ocp_resource plan - Add a few missing parameters Oct 26, 2025
@myakove
Copy link
Collaborator

myakove commented Oct 28, 2025

/approve
/lgtm

1 similar comment
@myakove
Copy link
Collaborator

myakove commented Oct 28, 2025

/approve
/lgtm

@krcmarik
Copy link
Contributor Author

krcmarik commented Nov 5, 2025

/verified

@krcmarik
Plan: Add a few missing parameters
34ae322 
- pvcNameTemplate
- volumeNameTemplate
- networkNameTemplate
- preserveStaticIPs
- targetNodeSelector
- targetLabels
- targetAffinity
@krcmarik
Copy link
Contributor Author

krcmarik commented Nov 5, 2025

/verified

@myakove
Copy link
Collaborator

myakove commented Nov 12, 2025

/approve
/lgtm
/verified

@myakove myakove enabled auto-merge (squash) November 12, 2025 14:21
@myakove myakove merged commit b18a08c into RedHatQE:main Nov 12, 2025
7 checks passed
@krcmarik krcmarik deleted the mtv_plan branch December 5, 2025 22:16
@servolkov
Copy link
Contributor

servolkov commented Feb 14, 2026

/cherry-pick v4.20

redhat-qe-bot pushed a commit that referenced this pull request Feb 14, 2026
@krcmarik @myakove
Plan: Add a few missing parameters (#2545)
2f0d111 
- pvcNameTemplate
- volumeNameTemplate
- networkNameTemplate
- preserveStaticIPs
- targetNodeSelector
- targetLabels
- targetAffinity

Co-authored-by: Meni Yakove <441263+myakove@users.noreply.github.com>
@redhat-qe-bot redhat-qe-bot mentioned this pull request Feb 14, 2026
Merged
@redhat-qe-bot
Copy link
Contributor

redhat-qe-bot commented Feb 14, 2026

Cherry-picked PR feat: ocp_resource plan - Add a few missing parameters into v4.20

redhat-qe-bot2 pushed a commit that referenced this pull request Feb 16, 2026
@redhat-qe-bot @krcmarik @myakove
Plan: Add a few missing parameters (#2545) (#2651)
da344ee 
- pvcNameTemplate
- volumeNameTemplate
- networkNameTemplate
- preserveStaticIPs
- targetNodeSelector
- targetLabels
- targetAffinity

Co-authored-by: Marián Krčmárik <mkrcmari@redhat.com>
Co-authored-by: Meni Yakove <441263+myakove@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rnetser rnetser Awaiting requested review from rnetser

@myakove myakove Awaiting requested review from myakove

@dbasunag dbasunag Awaiting requested review from dbasunag

Assignees

@krcmarik krcmarik

Labels

approved-myakove branch-main can-be-merged cherry-pick-v4.20 lgtm-myakove size/S verified

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@krcmarik @redhat-qe-bot @myakove @servolkov @redhat-qe-bot1 @redhat-qe-bot2 @rh-bot-1
Morty Proxy This is a proxified and sanitized view of the page, visit original site.