Skip to content

OCPEDGE-2766: create LVMS domain glossary and concept relationship map#2777

Open
Neilhamza wants to merge 1 commit into
openshift:mainfrom
Neilhamza:OCPEDGE-2766/domain-glossary-and-concepts
Open

OCPEDGE-2766: create LVMS domain glossary and concept relationship map#2777
Neilhamza wants to merge 1 commit into
openshift:mainfrom
Neilhamza:OCPEDGE-2766/domain-glossary-and-concepts

Conversation

@Neilhamza

@Neilhamza Neilhamza commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

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:

    • Resource flow: LVMCluster → DeviceClass → VG → ThinPool → LV → PV → PVC
    • Device filter chain: 7 filters with pass/reject logic
    • Reconciliation flow: LVMCluster controller (1min) and VG Manager (30s) loops
    • CRD relationships: ownership hierarchy and finalizer structure
    • StorageClass lifecycle: SSA creation with field ownership
    • Deletion flow: gates, finalizer cleanup, ManualCleanupRequired
    • Capacity and scheduling: TopoLVM topology-aware scheduling

Modified

  • AGENTS.md — Added glossary and concepts to documentation index

Review guidance

  • Are the 15 term definitions accurate? Do they match your understanding?
  • Are there missing terms that agents would need?
  • Do the concept relationship diagrams match the current reconciliation flow?

Jira: OCPEDGE-2766

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

Summary by CodeRabbit

  • Documentation
    • Added a new glossary covering core LVMS terminology, including how concepts map to user configuration and reported status.
    • Added a concepts guide detailing resource flow from the LVMCluster CR through operator-managed resources to node-level LVM artifacts, including reconciliation behavior.
    • Documented device filtering logic used during reconciliation, plus StorageClass generation and deletion/capacity/scheduling workflows.
    • Updated the documentation index to include the new domain pages.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 29, 2026
@openshift-ci-robot

Copy link
Copy Markdown

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

Details

In response to this:

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:

  • Resource flow: LVMCluster → DeviceClass → VG → ThinPool → LV → PV → PVC

  • Device filter chain: 7 filters with pass/reject logic

  • Reconciliation flow: LVMCluster controller (1min) and VG Manager (30s) loops

  • CRD relationships: ownership hierarchy and finalizer structure

  • StorageClass lifecycle: SSA creation with field ownership

  • Deletion flow: gates, finalizer cleanup, ManualCleanupRequired

  • Capacity and scheduling: TopoLVM topology-aware scheduling

Modified

  • AGENTS.md — Added glossary and concepts to documentation index

Review guidance

  • Are the 15 term definitions accurate? Do they match your understanding?
  • Are there missing terms that agents would need?
  • Do the concept relationship diagrams match the current reconciliation flow?

Jira: OCPEDGE-2766

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

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.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Walkthrough

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

Changes

LVMS Domain Documentation

Layer / File(s) Summary
Glossary intro and index
AGENTS.md, docs/domain/glossary.md
Adds the glossary page header and registers the glossary and concepts pages in the documentation index.
Core cluster and selection terms
docs/domain/glossary.md
Defines the main cluster and device-selection terms, including documented gotchas for validation, renaming, and policy/status behavior.
Provisioning and storage terms
docs/domain/glossary.md
Defines volume-group and provisioning terms, including thin pool, RAID, and StorageClass option semantics.
Status, filesystem, and tag terms
docs/domain/glossary.md
Defines filesystem behavior, internal status resources, logical volumes, wipe semantics, and @lvms tag behavior.
Resource flow and device filtering
docs/domain/concepts.md
Describes the top-level resource flow from LVMCluster to node-side artifacts and the VG Manager device filter chain.
Reconciliation and ownership
docs/domain/concepts.md
Documents the controller and VG Manager reconciliation loops, CRD ownership and finalizers, node status shape, and StorageClass creation rules.
Deletion and capacity behavior
docs/domain/concepts.md
Describes LVMCluster deletion handling, retain-policy cleanup paths, stale node finalizers, and capacity and scheduling behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested labels

ready-for-human-review

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding an LVMS glossary and concept relationship map.
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.
Stable And Deterministic Test Names ✅ Passed PR is docs-only; no new or changed Ginkgo titles with dynamic values were introduced.
Test Structure And Quality ✅ Passed PR is docs-only: AGENTS.md adds links to docs/domain/glossary.md and concepts.md; no Ginkgo/test files were modified.
Microshift Test Compatibility ✅ Passed Only docs/AGENTS changed; no new Ginkgo e2e tests or MicroShift-unsafe API usage were added.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Diff only adds docs/AGENTS.md; no new Go e2e tests or SNO-sensitive Ginkgo specs were added.
Topology-Aware Scheduling Compatibility ✅ Passed Only AGENTS.md and domain docs changed; no manifests, operators, or controllers were modified, so no new scheduling constraints were introduced.
Ote Binary Stdout Contract ✅ Passed Docs-only change: AGENTS.md plus new markdown docs; no main/init/TestMain/Ginkgo setup or stdout logging code was added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR only adds/updates docs; no new Ginkgo tests or network code were added in the diff.
No-Weak-Crypto ✅ Passed PR only adds/updates docs; exact-word scan of changed files found no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or secret-comparison code.
Container-Privileges ✅ Passed PASS: This PR only adds/updates markdown docs (AGENTS.md, docs/domain/*); no container/K8s manifests or privilege settings are introduced.
No-Sensitive-Data-In-Logs ✅ Passed Docs-only PR; reviewed AGENTS.md, concepts.md, and glossary.md and found no log statements or sensitive-data examples.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 29, 2026
@openshift-ci openshift-ci Bot requested review from jaypoulz and jerpeter1 June 29, 2026 11:29
@coderabbitai coderabbitai Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Jun 29, 2026
@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Neilhamza
Once this PR has been reviewed and has the lgtm label, please assign jaypoulz for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 07cc0d3 and a02ea4d.

📒 Files selected for processing (3)
  • AGENTS.md
  • docs/domain/concepts.md
  • docs/domain/glossary.md

Comment thread docs/domain/concepts.md
Comment on lines +25 to +28
## 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":

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread docs/domain/glossary.md Outdated
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>
@Neilhamza Neilhamza force-pushed the OCPEDGE-2766/domain-glossary-and-concepts branch from a02ea4d to 7e117dd Compare June 29, 2026 11:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
docs/domain/glossary.md (1)

3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reword 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

📥 Commits

Reviewing files that changed from the base of the PR and between a02ea4d and 7e117dd.

📒 Files selected for processing (3)
  • AGENTS.md
  • docs/domain/concepts.md
  • docs/domain/glossary.md
✅ Files skipped from review due to trivial changes (1)
  • AGENTS.md

@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@Neilhamza: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants