Repository navigation
OSAC-1270: PRD: Base OS Management for Bare Metal Instances - #135
Conversation
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com>
WalkthroughAdds a PRD defining how BMaaS selects, references, provisions, and protects DiskImage resources, including scope boundaries, user stories, dependencies, UI/API expectations, E2E coverage, and provenance metadata. ChangesBMaaS Base OS Management
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…nceCatalogItems Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com>
AI EP Review: EP-135Score: 10/10 | Verdict: PASS
Verdict: A well-focused, clearly written PRD that describes concrete user-facing capabilities for DiskImage integration into BMaaS with strong business justification, though user stories underrepresent the stated scope and cross-cutting dimensions are not addressed. Feedback: User stories only cover catalog item creation and deletion blocking, but In Scope describes much broader capabilities (browsing, lifecycle management, deprecation UX) — add stories for these so each In Scope capability maps to a verifiable user story. CPA and TA stories are near-identical; differentiate them or justify the duplication. Consider addressing relevant cross-cutting dimensions (documentation needs, E2E testing scope, installation impact) even briefly, as the OSAC dimensions checklist expects these for feature PRDs. Critical (0)None. Important (3)
Suggestions (3)
Review costModel: claude-opus-4-6 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md`:
- Around line 20-21: Update the DiskImage deletion requirements for both Cloud
Provider Admins and Tenant Admins to apply the same deletion guard to global and
tenant-scoped images. Explicitly define which BaremetalInstance and
BaremetalInstanceCatalogItem lifecycle states are considered active and
therefore block deletion.
- Line 1: Update the document title and all compound modifier usages in the PRD
to consistently use “bare-metal” with a hyphen, including user-facing
requirements; preserve standalone noun usages where “bare metal” is
grammatically correct.
- Line 18: Clarify the DiskImage requirement in the BaremetalInstance creation
behavior: resolve a user-provided DiskImage first, then fall back to the
catalog-item default, and reject creation only when neither provides a
reference. Update the mandatory wording so it describes this effective-reference
requirement rather than requiring direct user input.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: da192b16-f4ae-4844-bc62-486bf8fbd1d5
📒 Files selected for processing (1)
enhancements/OSAC-1270-base-os-management-bmaas/prd.md
adriengentil
changed the title
|
@adriengentil: This pull request references OSAC-1270 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
- Clarify effective DiskImage resolution (user input or catalog default) - Explicitly define deletion-blocking scope (any non-deleted BMI or catalog item) - Differentiate CPA vs Tenant Admin catalog item stories (global vs tenant-scoped) - Fix bare-metal hyphenation as compound modifier Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com>
|
Addressing the AI EP review feedback: User stories underrepresent In Scope capabilities — intentional. This PRD covers only the BMaaS-specific integration of DiskImage: the mandatory effective-reference at BaremetalInstance creation, catalog item default, and deletion protection extension. Browsing, lifecycle management (deprecate/obsolete/reactivate), deprecation warnings, and obsolete filtering are unchanged behaviors defined in OSAC-2540 and not duplicated here. The In Scope section references OSAC-2540 explicitly for this reason. Cross-cutting dimensions (docs, E2E, installation) — these are addressed at the Design EP stage per OSAC convention, not the PRD. The PRD template used here ( Jira target version warning — will set fix version to 5.0.0 on OSAC-1270. |
AI EP Review: EP-135Score: 10/10 | Verdict: PASS
Verdict: A well-written, focused PRD that clearly describes user-observable capabilities for DiskImage integration into BMaaS, with concrete justification, clean persona coverage, no design leakage, and tightly coupled scope. Feedback: Minor improvements: the user stories are narrower than the In Scope — CPA and TA stories only cover catalog item creation and deletion blocking, but not the core registration/update/deprecation lifecycle or image browsing. Adding 1-2 stories per persona for those capabilities would strengthen completeness. Consider briefly addressing cross-cutting dimensions (installation prerequisites, E2E testing scope, documentation needs) even if just to declare them out of scope or deferred. Critical (0)None. Important (0)None. Suggestions (3)
Review costModel: claude-opus-4-6 |
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com>
AI EP Review: EP-135Score: 10/10 | Verdict: PASS
Verdict: A strong PRD with clear user-observable outcomes, concrete business justification, specific and testable requirements, and well-scoped interdependent capabilities across three personas. Feedback: The user stories are somewhat narrow compared to the In Scope items — key user journeys like browsing/discovering images and seeing deprecation warnings lack corresponding 'As a...' stories, which makes it harder for reviewers to verify persona coverage is complete. Consider adding a Tenant User story for browsing available images and a Cloud Provider Admin story for the full register/deprecate/obsolete lifecycle to ensure all In Scope capabilities trace back to explicit user needs. Critical (0)None. Important (1)
Suggestions (2)
Review costModel: claude-opus-4-6 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md`:
- Around line 21-22: Revise the global and tenant-scoped DiskImage admin stories
to clarify that “update” is limited to mutable metadata and lifecycle fields.
Explicitly exclude changes to immutable source_type and source_ref, while
preserving the existing registration, deprecation, obsolescence, reactivation,
and deletion permissions and constraints.
- Around line 18-25: Trim the OSAC-1270 scope bullets to BMaaS-specific
behavior: BaremetalInstance DiskImage picker/default resolution, provisioning,
and reference-aware deletion effects. Remove requirements for DiskImage
browsing, lifecycle operations, warnings/filtering, full lifecycle UI, and
standalone E2E coverage; reference OSAC-2540 for shared DiskImage behavior, and
move deferred E2E tracking to the Design EP if applicable.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: be462247-94cb-4545-9139-4df59846a3b7
📒 Files selected for processing (1)
enhancements/OSAC-1270-base-os-management-bmaas/prd.md
| - Cloud Provider Admins can register, update, deprecate, obsolete, reactivate, and delete global DiskImages available to all tenants for bare-metal provisioning. Deletion of a global DiskImage is blocked when any BaremetalInstance (in any non-deleted state) or any BaremetalInstanceCatalogItem references it. | ||
| - Tenant Admins can register, update, deprecate, obsolete, reactivate, and delete tenant-scoped DiskImages visible only within their organization. Deletion of a tenant DiskImage is blocked when any BaremetalInstance (in any non-deleted state) or any BaremetalInstanceCatalogItem references it. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Constrain “update” to mutable DiskImage fields.
OSAC-2540 makes source_type and source_ref immutable. Clarify that these admin stories permit metadata/lifecycle updates only; otherwise “update” can be interpreted as replacing the backing image reference and conflicts with the upstream contract.
🤖 Prompt for 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.
In `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md` around lines 21 - 22,
Revise the global and tenant-scoped DiskImage admin stories to clarify that
“update” is limited to mutable metadata and lifecycle fields. Explicitly exclude
changes to immutable source_type and source_ref, while preserving the existing
registration, deprecation, obsolescence, reactivation, and deletion permissions
and constraints.
| - Tenant Admins can register, update, deprecate, obsolete, reactivate, and delete tenant-scoped DiskImages visible only within their organization. Deletion of a tenant DiskImage is blocked when any BaremetalInstance (in any non-deleted state) or any BaremetalInstanceCatalogItem references it. | ||
| - UI support for the full DiskImage lifecycle (image list, image picker in BaremetalInstance creation, image detail, and lifecycle management controls) for all affected personas. | ||
| - E2E test coverage for DiskImage selection at bare-metal instance provision time, added to the existing bare-metal test suite. | ||
| - DiskImages for bare metal use the same resource, metadata schema, image source format, and two-tier visibility model (global + tenant-scoped) as defined in OSAC-2540. |
There was a problem hiding this comment.
A lot of these items seem like they're not specific to BMaaS, and are general to DiskImages. Is there some additional BMaaS/DiskImage interaction - being able to specify a DiskImage as appropriate for bare metal use, or something like that?
There was a problem hiding this comment.
not really all that is the integration of DiskImage with BMaaS. I'll try to trim a little more, thanks
Remove OSAC-2540-owned bullets (browsing, lifecycle warnings, lifecycle UI, management CRUD). Keep only BaremetalInstance effective-reference requirement, deletion blocking extension, BMaaS creation UI/API, and E2E scope. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com>
AI EP Review: EP-135Score: 10/10 | Verdict: PASS
Verdict: A strong, focused PRD that clearly describes DiskImage integration into BMaaS with concrete user-observable outcomes, per-persona user stories, and testable requirements — no design leakage or scope bloat. Feedback: The PRD is ready for design work. Two minor improvements: add a Tenant User story for the defaulting path (e.g., 'As a Tenant User, I want my bare-metal instance to receive the catalog item's default OS image when I don't explicitly select one, so that provisioning is simple for standard workloads'). Also consider explicitly noting which cross-cutting dimensions (documentation, installation, networking, storage) are not applicable to this feature to preempt reviewer questions. Critical (0)None. Important (1)
Suggestions (2)
Review costModel: claude-opus-4-6 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adriengentil, tzumainn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
enhancements/OSAC-1270-base-os-management-bmaas/prd.md (1)
29-30: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign the default-image contract.
BareMetalInstanceTemplateSpecDefaults.imageis the only default OS base image field in the API docs, but this PRD routes the effectiveDiskImagedefault throughBaremetalInstanceCatalogItemwhile also sayingBaremetalInstanceTemplatehas noDiskImagefield. Define where that default is stored, or move the default to the template/spec-defaults path.🤖 Prompt for 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. In `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md` around lines 29 - 30, Align the PRD’s default OS image contract by choosing one authoritative location for the effective DiskImage default: either define how BaremetalInstanceCatalogItem stores and supplies it, or route it through BareMetalInstanceTemplateSpecDefaults.image. Update the statements about BaremetalInstanceTemplate and catalog-item parameters so they consistently describe that chosen source.
🤖 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 `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md`:
- Line 18: Clarify the DiskImage requirement in the BaremetalInstance creation
contract by explicitly defining how existing raw-URL values are handled. State
whether raw URLs are rejected, converted to DiskImage references, or remain
supported, and ensure the catalog and instance behavior is consistent with that
decision.
---
Outside diff comments:
In `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md`:
- Around line 29-30: Align the PRD’s default OS image contract by choosing one
authoritative location for the effective DiskImage default: either define how
BaremetalInstanceCatalogItem stores and supplies it, or route it through
BareMetalInstanceTemplateSpecDefaults.image. Update the statements about
BaremetalInstanceTemplate and catalog-item parameters so they consistently
describe that chosen source.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b6cecf29-2eb8-4575-9021-b4ce729d5f59
📒 Files selected for processing (1)
enhancements/OSAC-1270-base-os-management-bmaas/prd.md
|
|
||
| ## In Scope | ||
|
|
||
| - A BaremetalInstance must have an effective DiskImage reference at creation time — either explicitly selected by the user or defaulted from the BaremetalInstanceCatalogItem. Creation is rejected when neither provides a reference. The instance is provisioned with the OS from that image. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define the raw-URL compatibility path.
Line 14 documents the existing raw URL behavior, while Line 18 makes a DiskImage reference mandatory. Specify whether raw URLs are rejected, migrated to DiskImages, or remain supported; otherwise the API and existing catalog/instance data contract is ambiguous.
🤖 Prompt for 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.
In `@enhancements/OSAC-1270-base-os-management-bmaas/prd.md` at line 18, Clarify
the DiskImage requirement in the BaremetalInstance creation contract by
explicitly defining how existing raw-URL values are handled. State whether raw
URLs are rejected, converted to DiskImage references, or remain supported, and
ensure the catalog and instance behavior is consistent with that decision.
3 participants
PRD: Base OS Management for Bare Metal Instances
Jira: https://redhat.atlassian.net/browse/OSAC-1270
Summary
This PRD covers the integration of the DiskImage resource (OSAC-2540) into BMaaS. It does not change DiskImage behavior — that is fully specified in OSAC-2540. The feature adds a mandatory DiskImage reference to BaremetalInstance creation, extends DiskImage deletion protection to cover active BaremetalInstances, and allows Cloud Provider Admins and Tenant Admins to reference a default DiskImage in BaremetalInstanceCatalogItems.
Requesting Review On
Summary by CodeRabbit