Skip to content

Add comprehensive controller development rules, reconciliation flow library, and refactor API/controllers #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dmgtn merged 46 commits into from
Jan 16, 2026
Merged

Add comprehensive controller development rules, reconciliation flow library, and refactor API/controllers #495

dmgtn merged 46 commits into from
Jan 16, 2026

Conversation

@dmgtn
Copy link
Member

@dmgtn dmgtn commented Dec 31, 2025 •
edited
Loading

Add comprehensive controller development rules, reconciliation flow library, and refactor API/controllers

Summary

This PR introduces comprehensive Cursor AI rules for controller development, a new flow package for standardized reconciliation patterns, a reusable objutilv1 utilities package, and refactors the controller architecture with a unified rv_controller.

Key Changes

Cursor Rules (.cursor/rules/) — +5,741 lines

Comprehensive documentation for AI-assisted controller development following RFC 2119/8174 conventions:

API Rules:

  • api-types.mdc — Type definitions, optional scalars, kubebuilder markers
  • api-conditions.mdc — Condition type/reason/message conventions
  • api-labels-and-finalizers.mdc — Label keys, finalizer naming
  • api-file-structure.mdc — File organization (<kind>_types.go, <kind>_conditions.go)
  • api-codegen.mdc — Code generation workflow

Controller Rules:

  • controller-terminology.mdc — Shared terminology (intended/actual/target/report, patch domains)
  • controller-file-structure.mdc — Package structure (controller.go, predicates.go, reconciler.go)
  • controller-controller.mdc — Builder chain, watches, predicates wiring
  • controller-predicate.mdc — Predicate implementation patterns
  • controller-reconciliation.mdc — Reconciliation pipeline, I/O boundaries
  • controller-reconciliation-flow.mdc — Flow package usage patterns

ReconcileHelper Rules (9 files):

  • controller-reconcile-helper.mdc — Overview and naming conventions
  • controller-reconcile-helper-compute.mdc — compute* helpers (non-I/O)
  • controller-reconcile-helper-construction.mdc — new*/build*/make* helpers
  • controller-reconcile-helper-is-in-sync.mdc — is*InSync comparison helpers
  • controller-reconcile-helper-apply.mdc — apply* mutation helpers
  • controller-reconcile-helper-ensure.mdc — ensure* helpers with change tracking
  • controller-reconcile-helper-get.mdc — get* single-call read helpers
  • controller-reconcile-helper-create.mdc / delete.mdc / patch.mdc — Write helpers

General Rules:

  • rfc-like-mdc.mdc — RFC-style writing conventions for .mdc files
  • go.mdc, go-tests.mdc — Go coding conventions
  • repo-wide.mdc, tooling.mdc — Repository-wide patterns

Reconciliation Flow Library (internal/reconciliation/flow/) — +1,749 lines

New flow package providing typed scopes for reconciliation phases with standardized logging, panic handling, and error context:

  • ReconcileFlow — For reconcile methods, returns ReconcileOutcome (requeue control + error)
  • EnsureFlow — For ensure helpers, returns EnsureOutcome (change tracking + optimistic lock intent)
  • StepFlow — For plain steps returning error

Features:

  • Phase-scoped logging (phase start / phase end + duration)
  • Panic recovery with re-panic after logging
  • Structured outcome types with ShouldReturn() and ToCtrl() converters

Object Utilities (api/objutilv1/) — +1,065 lines

New reusable package for Kubernetes object manipulation:

  • conditions.go — Condition comparison, semantic equality, batch operations
  • finalizers.go — Add/remove/check finalizers
  • labels.go — Label manipulation utilities
  • ownerrefs.go — Owner reference helpers
  • interfaces.go — StatusConditionObject interface for polymorphic condition access

All utilities include comprehensive tests.

Controller Refactoring (images/controller/)

New rv_controller:

  • Unified controller handling ReplicatedVolume metadata and device minor allocation
  • Replaces deleted rv_metadata and rv_status_config_device_minor controllers

Generic ID Pool (idpool/):

  • Type-safe IDPool[T] for allocating unique numeric IDs
  • Conflict detection with typed errors (DuplicateIDError, PoolExhaustedError, NameConflictError)
  • Used for DRBD device minor allocation

API Reorganization (api/v1alpha1/)

  • Renamed type files: replicated_volume.go → rv_types.go, etc.
  • Separated condition definitions to *_conditions.go files
  • Removed deprecated conditions.go, errors.go, *_consts.go
  • Added GetStatusConditions()/SetStatusConditions() methods for objutilv1 compatibility

Other Changes

  • Updated CRDs with new condition types
  • Optimized agent drbd_config predicates
  • Adapted megatest for ReplicatedVolumeAttachment
  • Various test updates

David Magton added 5 commits December 31, 2025 13:03
Implement indexing for ReplicatedVolumeReplica by replicatedVolumeName
89021c1 
Signed-off-by: David Magton <david.magton@flant.com>
[controller] Introduce rv-controller: RV metadata + deviceMinor alloc…
2f9f672 
…ation

Signed-off-by: David Magton <david.magton@flant.com>
refactor(api): rename status.conditions constants and add Cursor nami…
e6d77ce 
…ng rules

- Renamed condition Type/Reason constants for all API types that have status.conditions (RV/RVR/RVA/RSC/RSP) to:
  - <ObjPrefix>Cond<...>Type
  - <ObjPrefix>Cond<...>Reason<...>
  - string values unchanged.
- Updated all usages across controllers/agent/CSI/megatest and tests; split shared reasons per condition type while keeping the same string values.
- Added Cursor rules in .cursor/api_conditions_rules.mdc.

Signed-off-by: David Magton <david.magton@flant.com>
[dev] Add Cursor Go coding/testing rules
1e8dfa1 
- Add .cursor/go_rules.mdc, .cursor/go_test_rules.mdc, .cursor/api_codegen_rules.mdc
- Update .cursor/rules.mdc with commit message requirements

Signed-off-by: David Magton <david.magton@flant.com>
[api] Reorganize v1alpha1 types and conditions
0e31413 
- Split API into *_types.go and *_conditions.go; add shared helpers
- Rename condition constants to object-scoped Replicated*Cond* identifiers
- Update controllers/tests to use the new condition/type names
- Regenerate CRDs and move Cursor rules under .cursor/rules/*

Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from 127f13a to 0e31413 Compare January 2, 2026 14:27
David Magton added 9 commits January 2, 2026 18:28
[tooling] Add direnv caches and migrate Cursor rules to .mdc
67e1072 
- Rename Cursor rules from RULE.md to *.mdc
- Add tooling.mdc with canonical commands and "must ask" boundaries
- Add .envrc to keep Go/golangci-lint caches under .cache/
- Ignore .cache/ and .direnv/ in .gitignore

Signed-off-by: David Magton <david.magton@flant.com>
[controller] Type-safe device minor allocation
87fff30 
- API: represent ReplicatedVolume status.deviceMinor as v1alpha1.DeviceMinor and add range validation.
- API: add DeviceMinorOutOfRangeError and exclude it from controller-gen object/deepcopy generation.
- Controller: genericize rv_controller/idpool and use IDPool[v1alpha1.DeviceMinor] for allocation.
- Codegen: regenerate deepcopy outputs.\n- Tooling: document non-API type generation hygiene in Cursor api-types rules.

Signed-off-by: David Magton <david.magton@flant.com>
[api] Add objutilv1 helpers and migrate usages from v1alpha1
9069534 
Introduce api/objutilv1 (conditions, labels, finalizers, ownerrefs, interfaces) with tests,
and update API types to implement StatusConditionObject adapters (GetStatusConditions/SetStatusConditions).

Refactor controller/agent code to use objutilv1 (imported as obju):
- condition comparisons use semantic equality with ObservedGeneration (SpecAware)
- label updates use objutilv1.SetLabel
- finalizer checks use objutilv1.HasFinalizer/HasFinalizersOtherThan

Update API rules docs to require StatusConditionObject adapters on root objects.

Signed-off-by: David Magton <david.magton@flant.com>
[controller] Introduce reconciliation flow and refactor RV controller
a985a6e 
- Add internal/reconciliation/flow with Outcome helpers, phase logging, and tests
- Refactor rv_controller reconciler to use flow.Begin/BeginPhase, split main vs status, and improve error aggregation
- Switch ReplicatedVolume label sync helpers to objutilv1 label operations
- Add go.work wiring for nested modules and make hack/run-tests.sh workspace-aware

Signed-off-by: David Magton <david.magton@flant.com>
[internal] Encapsulate flow.Outcome and extend objutilv1 helpers
f74112d 
- flow: make Outcome fields private (result/err) and update docs/tests accordingly
- objutilv1: add condition comparison helpers (by semantic meaning / by Type+Status) with tests
- objutilv1: add missing godoc for labels/finalizers/ownerrefs helpers
- cursor: add controller wiring/file-structure rules for IDE guidance

Signed-off-by: David Magton <david.magton@flant.com>
[dev] Clarify controller.go wiring rules and commit sign-off policy
1335e25 
- Add TL;DR + explicit ALLOW/DENY boundaries for controller.go (wiring vs business logic)
- Clarify predicate best practices: generation vs metadata-only changes; prefer client.Object getters; use obju for conditions
- Update repo-wide commit-message rules to require sign-off and remind about it when generating commit messages

Signed-off-by: David Magton <david.magton@flant.com>
[controller] Refine ReplicatedVolume update predicate
afb3842 
Signed-off-by: David Magton <david.magton@flant.com>
[reconciliation/flow] Track changes and optimistic-lock requirement i…
73373c3 
…n Outcome

- Add change tracking API on flow.Outcome (ReportChanged*, DidChange, RequireOptimisticLock, OptimisticLockRequired, Error)
- Aggregate change tracking state in flow.Merge
- Refactor and extend tests (external flow_test + internal guard test for unsupported Requeue=true)

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Require commit messages to be output in a code block
0c382f1 
Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from a2d6b45 to 78141bb Compare January 4, 2026 23:58
[dev] Document controller reconcile helper contracts; add flow.Outcom…
2137cbd 
…e.Errorf

- Add Cursor rules for ReconcileHelper categories (compute/apply/ensure/is-up-to-date/create/delete/patch)
- Extend controller file-structure rules to formalize Reconcile vs ReconcileHelper vs other code in reconciler.go
- Move controller reconciliation rules document to repo root (disable alwaysApply in frontmatter)
- Add flow.Outcome.Errorf() helper + unit tests

Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from 78141bb to 2137cbd Compare January 5, 2026 00:21
David Magton added 7 commits January 5, 2026 04:30
[docs] Expand controller ReconcileHelper guidelines
598ec32 
Update and extend ReconcileHelper category docs (compute/apply/ensure/create/delete/patch/isUpToDate) and the shared helper conventions.

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Refine controller reconciliation guide
223b150 
- Make controller reconciliation rules always-applied and scope them via globs
- Restructure and tighten the document (patterns, DeepCopy/base rules, phases, checklist)

Signed-off-by: David Magton <david.magton@flant.com>
[dev] Refine controller reconciliation rules for error context
fe0a70c 
- Clarify error context layering: Reconcile methods add action/phase context, helpers must not include primary reconcile identity
- Tighten error-handling guidance across apply/compute/ensure/create/delete/patch/isUpToDate helper categories
- Add guidance on when to extract helpers and document deterministic reconciler-owned state exceptions (e.g., idpool/cache)

Signed-off-by: David Magton <david.magton@flant.com>
[flow] Add phase context helpers and Outcome.OnErrorf
712aa45 
- Record phase metadata in ctx; log phase start/end at V(1)
- Add Outcome.OnErrorf for local wrapping + logging + phase-aware wrapping
- Rename Outcome.Errorf -> Wrapf; add Outcome.Merge
- Update flow tests

Signed-off-by: David Magton <david.magton@flant.com>
[docs] Clarify flow.Outcome usage in reconcile helpers
e61f9fa 
- Document compute helper restrictions for change/optimistic-lock reporting
- Add recommended ensure helper reporting patterns
- Minor wording fix in patch helper doc

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Add controller terminology and flow usage rules
ae6be19 
- Add `controller-terminology.mdc` and `controller-reconciliation-flow.mdc`
- Refine reconciliation, compute and ensure helper guidelines

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Refine controller rule docs structure and terminology
c4f2967 
- Make TL;DR sections non-normative (“Summary only”) and move requirements into explicit sections
- Align helper category docs (apply/compute/create/delete/ensure/is-up-to-date/patch) with consistent terminology, naming and keywords
- Expand `controller-terminology.mdc` and apply consistent bolding for defined terms

Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from ce4191e to c4f2967 Compare January 6, 2026 16:09
David Magton added 4 commits January 6, 2026 20:07
[flow] Validate phase kv pairs and stabilize error context
de8dfb0 
- Change BeginPhase kv to string key/value pairs and panic on odd-length input
- Render phase kv in errors as "phase <name> [k=v ...]" consistently
- Add tests for kv validation and error formatting

Signed-off-by: David Magton <david.magton@flant.com>
[flow] Log phase errors once; remove ContinueErr
68f535e 
- Move error logging to EndPhase and track it via Outcome.ErrorLogged to avoid duplicates
- Rename Outcome.Wrapf -> Enrichf and adjust docs/examples accordingly
- Make Merge treat any error as Fail and drop ContinueErr/ContinueErrf
- Update rv_controller reconciler and controller .mdc rules to match the new flow contract

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Normalize controller docs terminology and formatting
da43af7 
- Bold standardized controller terminology across rule documents
- Clarify wording around wiring-only vs reconciliation logic and predicate scope
- Keep content consistent across helper category docs (compute/apply/ensure/create/delete/patch)

Signed-off-by: David Magton <david.magton@flant.com>
[controller] Refactor RV reconciler patching and error propagation
54a6d16 
- Run main/status reconciliation sequentially with explicit flow phases and documented patterns
- Extract RV patch helpers (main vs status) and deviceMinor desired-state helpers
- Tighten tests to assert returned

Signed-off-by: David Magton <david.magton@flant.com>
David Magton added 5 commits January 7, 2026 03:11
[controller] Inline storageClass label sync for RV and drop API helpers
5fc978d 
- Move replicated-storage-class label sync logic into rv_controller reconcileMain
- Remove ReplicatedVolume label helper methods from api/v1alpha1

Signed-off-by: David Magton <david.magton@flant.com>
[controller] Refactor RV device-minor allocation and idpool API
e7fa7f8 
- rv_controller: switch root Reconcile to orchestration style; rework status deviceMinor reconciliation via allocateDM phase
- idpool: replace GetOrCreate/GetOrCreateWithID/BulkAdd with EnsureAllocated/Fill; return OutOfRangeError instead of panicking; add Is/AsOutOfRange helpers and update tests
- api: drop DeviceMinor validation/error and ReplicatedVolumeStatus deviceMinor helpers from v1alpha1; adjust objutilv1 condition semantic equality to default ObservedGeneration from object
- docs: update controller rule docs and add ConstructionReconcileHelper guidelines

Signed-off-by: David Magton <david.magton@flant.com>
[controller] Rename IsUpToDate helpers to IsInSync
4ba7daa 
- Update controller helper naming/contracts docs: IsUpToDate -> IsInSync
- Adjust rv_controller status drift check helper to isDMInSync

Signed-off-by: David Magton <david.magton@flant.com>
[controller] RV reconciler: treat NotFound as deleted and normalize n…
e60a40b 
…aming

- On ReplicatedVolume NotFound, set rv=nil to allow cleanup paths (e.g. device-minor release)
- Align reconciliation pattern comments (Pure orchestration / Target-state driven)
- Normalize device-minor phase name and helper/variable naming (targetDM, newDeviceMinorAssignedCondition)

Signed-off-by: David Magton <david.magton@flant.com>
[cursor] Extend controller rules globs to rv_attach_controller
05946b9 
Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from 889ac1c to 05946b9 Compare January 12, 2026 16:20
David Magton added 9 commits January 12, 2026 19:42
[cursor] Add RFC-style writing rules for .mdc documents
f40c804 
Signed-off-by: David Magton <david.magton@flant.com>
[rules] Split predicate rules into predicate.go and add GetReconcileH…
7fede95 
…elper docs

- Make controller wiring docs require predicates to live in predicate.go (wired via builder.WithPredicates).
- Add GetReconcileHelper contract and align helper-category listings/terminology.
- Tighten flow and controller terminology docs (phase naming, root/non-root phase rules, controller name kebab-case).

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Normalize .mdc frontmatter and centralize RFC-style conventions
7cebad9 
- Convert rule scoping to single-string `globs` and stop `alwaysApply` for scoped docs
- Link controller rule docs to `rfc-like-mdc.mdc` and remove duplicated keyword sections
- Align predicate file naming in rules (`predicate.go` -> `predicates.go`)

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Align controller rule docs with RFC keyword formatting
92ca3d2 
- Remove emphasis from BCP 14 keywords (MUST/SHOULD/MAY, etc.) across controller rules
- Adjust section headings and bullets to match `rfc-like-mdc.mdc` conventions

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Define API short kind names for controller helper/predicate n…
5660f63 
…aming

- Document canonical short kind names (RV/RVR/RVA/RSC/RSP) in `controller-terminology.mdc`
- Require using short kind names for repo API kinds across controller predicates and reconcile helper naming rules

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Document reconciler.go layout and helper ordering conventions
7e9ca2c 
- Define call-graph based ordering for Reconcile methods and non-I/O helper blocks
- Specify canonical grouping/sorting for I/O helpers (get/create/patch/delete) at file end

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Add controller reconciliation rules for optional scalars and …
66ef1e3 
…state variable naming

- Require `*T` for `omitempty` non-nil-able fields in controller POV artifacts to preserve “unset”
- Define canonical naming for intended/actual/target/report variables and forbid new “desired” usage

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Document optional scalar fields as *T across API and reconc…
5882b5c 
…iliation rules

- Specify that `omitempty` scalar fields MUST be represented as pointers to preserve unset vs zero
- Add `ptr.Equal`/nil handling guidance and update the TimeoutSeconds example

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Add split-client determinism guidance to reconciliation docs
3c7cb0c 
- Document controller-runtime split client semantics and stale-read implications
- Require idempotent, deterministic reconciliation under retry and cache lag
- Outline protections for create/update operations to avoid non-deterministic duplicates

Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn changed the title [controller] rv_controller Add comprehensive controller development rules, reconciliation flow library, and refactor API/controllers Jan 16, 2026
David Magton added 5 commits January 16, 2026 14:16
[flow] Introduce typed flow scopes for reconcile/ensure/steps
8a77d79 
- Refactor internal reconciliation flow into explicit scopes (`ReconcileFlow`, `EnsureFlow`, `StepFlow`) with dedicated outcome types and consistent phase logging/validation.
- Update `rv_controller` reconciler to use the new API (`BeginRootReconcile` / `BeginReconcile`) and adjust tests accordingly.
- Mark controller-related Cursor rules as always-applied and refine rule descriptions.

Signed-off-by: David Magton <david.magton@flant.com>
Merge branch 'astef-prototype' into astef-prototype-rv-controller
7c9f8db 
Signed-off-by: David Magton <david.magton@flant.com>
[controller] Preserve deviceMinor on pool errors
2f2262e 
- Keep existing status.deviceMinor when the device-minor pool source is unavailable and report the failure via DeviceMinorAssigned condition
- Add coverage for the pool-not-ready error path
- Normalize Cursor .mdc rule frontmatter descriptions and relax alwaysApply where appropriate

Signed-off-by: David Magton <david.magton@flant.com>
[rules] Refactor flow API: split Outcome into ReconcileOutcome and En…
d999da2 
…sureOutcome

Redesign the reconciliation flow API in controller .mdc rules:

- Split single `flow.Outcome` type into specialized types:
  - `flow.ReconcileOutcome` for Reconcile methods
  - `flow.EnsureOutcome` for ensure helpers (carries change/lock flags)
  - Plain `error` for create/delete/patch/step helpers

- Replace phase API:
  - `BeginPhase/EndPhase` → scoped constructors with `OnEnd` methods:
    - `BeginRootReconcile` (root Reconcile, no OnEnd)
    - `BeginReconcile` + `rf.OnEnd(&outcome)` (non-root Reconcile)
    - `BeginEnsure` + `ef.OnEnd(&outcome)` (ensure helpers)
    - `BeginStep` + `sf.OnEnd(&err)` (step helpers returning error)

- Update terminology throughout:
  - "phase" → "phase scope"
  - "Outcome" → "ReconcileOutcome" / "EnsureOutcome"

- Simplify helper return types:
  - Create/Delete/Patch helpers: MUST return `error`, NOT outcome
  - Compute helpers: MUST NOT return EnsureOutcome
  - Ensure helpers: MUST create ensure phase scope, accept ctx

- Restructure controller-reconciliation-flow.mdc for clarity

Signed-off-by: David Magton <david.magton@flant.com>
[cursor-rules] Improve controller reconcile helper documentation
77d3b5e 
- Unify placeholder type names: rename SKN/SomeKindName/EON to
  EK/ExampleKind for consistency across all helper rules
- Add explicit inline comments to bad examples clarifying why ctx and
  client.Client are forbidden in apply/construction/is-in-sync helpers
- Rewrite ensure helper examples to use proper flow.BeginEnsure() +
  defer ef.OnEnd(&outcome) pattern
- Fix flow.Outcome → flow.EnsureOutcome naming in ensure rules
- Align terminology: "desired" → "intended/target" in compute rules
- Add implementation example section to patch helper rules showing
  optimistic lock handling
- Fix confusing step function example in reconciliation-flow rules
- Remove duplicate line in controller-reconcile-helper.mdc

Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from eb5ffb1 to 77d3b5e Compare January 16, 2026 11:18
[ci] Add all image modules to go.work and fix internal dependency
e76bc15 
Image modules were not listed in go.work, causing CI test failures with
"directory prefix . does not contain modules listed in go.work" error.

Additionally, images/controller was missing the internal module
dependency required for internal/reconciliation/flow import.

Changes:
- Add all images/*/go.mod modules to go.work use() directive
- Update hack/run-tests.sh case pattern to stay in sync with go.work
- Add replace directive and require for internal module in
  images/controller/go.mod
- Add internal to includePaths in images/controller/werf.inc.yaml so
  the directory is available during container build

Signed-off-by: David Magton <david.magton@flant.com>
@dmgtn dmgtn force-pushed the astef-prototype-rv-controller branch from ae0044a to e76bc15 Compare January 16, 2026 11:53
@dmgtn dmgtn marked this pull request as ready for review January 16, 2026 12:17
@dmgtn dmgtn merged commit a774ae1 into astef-prototype Jan 16, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dmgtn