OCPEDGE-2766: create LVMS domain glossary and concept relationship map#2777
OCPEDGE-2766: create LVMS domain glossary and concept relationship map#2777Neilhamza wants to merge 1 commit into
Conversation
|
@Neilhamza: This pull request references OCPEDGE-2766 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 story 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. |
WalkthroughAdds two new LVMS domain docs for terminology and concepts, and updates the docs index to link them. The new concepts page covers resource flow, reconciliation, StorageClass behavior, deletion, and capacity handling. ChangesLVMS Domain Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Neilhamza The full list of commands accepted by this bot can be found 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: 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 `@docs/domain/concepts.md`:
- Around line 25-28: The Device Filter Chain description currently implies a
fixed 30s reconcile cadence, which is too broad. Update the wording in the
concepts doc to describe the actual requeue behavior instead: static-policy VGs
do not use periodic requeues, while dynamic-policy VGs requeue every 30s. Keep
the explanation aligned with the Device Filter Chain and VG Manager terminology
already used in this section.
In `@docs/domain/glossary.md`:
- Around line 23-27: The DeviceDiscoveryPolicy glossary entry currently states
that nil defaults to Dynamic, but the controller behavior distinguishes new and
existing VGs. Update the description in the DeviceDiscoveryPolicy section to
match the upstream type comment: new VGs should default to Static, while
existing VGs remain Dynamic, and keep the rest of the spec/status mapping
consistent with the glossary terms.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a679e7ce-9680-44a9-90be-ce6b897a7f99
📒 Files selected for processing (3)
AGENTS.mddocs/domain/concepts.mddocs/domain/glossary.md
| ## Device Filter Chain | ||
|
|
||
| How a raw block device becomes a VG member. VG Manager runs this on every 30s reconcile cycle. A device must pass ALL filters to be "Available": | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't imply a fixed 30s cadence for the filter chain.
This section later says static-policy VGs skip periodic requeues and dynamic-policy VGs requeue every 30s, so "every 30s reconcile cycle" is too broad. Tighten the wording to match the actual requeue rules.
🤖 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 `@docs/domain/concepts.md` around lines 25 - 28, The Device Filter Chain
description currently implies a fixed 30s reconcile cadence, which is too broad.
Update the wording in the concepts doc to describe the actual requeue behavior
instead: static-policy VGs do not use periodic requeues, while dynamic-policy
VGs requeue every 30s. Keep the explanation aligned with the Device Filter Chain
and VG Manager terminology already used in this section.
Add domain documentation capturing LVMS-specific knowledge beyond what's in Go types or CRD YAML, making it discoverable for AI agents and new contributors. New files: - docs/domain/glossary.md: 15 canonical LVMS terms with Go type references, relationships, constraints, and common gotchas - docs/domain/concepts.md: resource flow, device filter chain, reconciliation flow (both controllers), CRD relationships, StorageClass lifecycle, deletion flow, capacity/scheduling Verified against: - Current codebase: 23 claims correct, 0 contradictions - Existing repo docs: 0 contradictions, duplication reduced via cross-references to architecture.md, core-beliefs.md, design/ - Official Red Hat docs: 6 supported, 3 silent, 0 contradictions Modified: - AGENTS.md: added domain docs to documentation index Signed-off-by: Neil Hamza <nhamza@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a02ea4d to
7e117dd
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/domain/glossary.md (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReword the intro sentence.
The opening line repeats “For” three times; a small rewrite would read more cleanly.
Suggested rewrite
-Canonical LVMS terminology. Each term maps to a Go type or CRD field. For architecture, see [architecture.md](../architecture.md). For concept relationships, see [concepts.md](concepts.md). For design principles, see [core-beliefs.md](../core-beliefs.md). +Canonical LVMS terminology. Each term maps to a Go type or CRD field. See [architecture.md](../architecture.md), [concepts.md](concepts.md), and [core-beliefs.md](../core-beliefs.md).🤖 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 `@docs/domain/glossary.md` at line 3, Reword the opening sentence in the glossary introduction so it reads more cleanly and avoids repeating “For” multiple times. Update the intro line in the glossary content to keep the same meaning while improving readability, and preserve the existing links to architecture.md, concepts.md, and core-beliefs.md.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@docs/domain/glossary.md`:
- Line 3: Reword the opening sentence in the glossary introduction so it reads
more cleanly and avoids repeating “For” multiple times. Update the intro line in
the glossary content to keep the same meaning while improving readability, and
preserve the existing links to architecture.md, concepts.md, and
core-beliefs.md.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 25873303-bcdf-4f16-93c9-e50b85e5f72f
📒 Files selected for processing (3)
AGENTS.mddocs/domain/concepts.mddocs/domain/glossary.md
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
|
@Neilhamza: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
Adds domain documentation that captures LVMS-specific knowledge beyond what's in Go types or CRD YAML. Part of the Agentic SDLC MVP (OCPEDGE-2508) — completing Track 1 Contextification alongside PR #2688.
Agents can read Go struct definitions but miss the relationships, constraints, and gotchas between LVMS concepts. These docs bridge that gap.
New files
docs/domain/glossary.md— 15 canonical LVMS terms (LVMCluster, DeviceClass, DeviceSelector, VolumeGroup, ThinPoolConfig, RAIDConfig, StorageClassOptions, FilesystemType, LVMVolumeGroup, LVMVolumeGroupNodeStatus, VGStatus, LogicalVolume, ForceWipeDevicesAndDestroyAllData, @lvms tag). Each term includes the Go type reference, relationships to other terms, constraints, and common gotchas.docs/domain/concepts.md— How LVMS concepts relate:Modified
AGENTS.md— Added glossary and concepts to documentation indexReview guidance
Jira: OCPEDGE-2766
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
LVMClusterCR through operator-managed resources to node-level LVM artifacts, including reconciliation behavior.