Skip to content

Commit 1495697

Browse files
committed
[rules] Refactor flow API: split Outcome into ReconcileOutcome and EnsureOutcome
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 <[email protected]>
1 parent efca350 commit 1495697

13 files changed

+434
-678
lines changed

.cursor/rules/controller-reconcile-helper-apply.mdc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,12 @@ func applyFoo(
167167

168168
---
169169

170-
## Flow phases and **Outcome**
170+
## Flow phase scopes and outcomes
171171

172-
- **ApplyReconcileHelpers** MUST NOT create a `reconcile/flow` **phase** (they do not accept `ctx context.Context`; see `controller-reconcile-helper.mdc`).
173-
- **ApplyReconcileHelpers** MUST NOT return **Outcome** (in code: `flow.Outcome`) (they are “in-memory write” steps).
174-
- If a failure is possible, return `error` and let the calling function convert it into `flow.Fail(err)` (or equivalent **flow** handling).
172+
- **ApplyReconcileHelpers** MUST NOT create a `reconcile/flow` **phase scope** (they do not accept `ctx context.Context`; see `controller-reconcile-helper.mdc`).
173+
- **ApplyReconcileHelpers** MUST NOT return **ReconcileOutcome** (`flow.ReconcileOutcome`) or **EnsureOutcome** (`flow.EnsureOutcome`) (they are “in-memory write” steps).
174+
- If a failure is possible, return `error` and let the caller convert it into a flow result in its own scope
175+
(for example, `rf.Fail(err)` in a reconcile scope or `ef.Err(err)` in an ensure scope).
175176

176177
---
177178

@@ -212,11 +213,12 @@ func applyFoo(obj *v1alpha1.Foo, target TargetFoo) {
212213
}
213214
```
214215

215-
❌ Returning `flow.Outcome` / doing flow control inside apply:
216+
❌ Returning a reconcile/ensure outcome / doing flow control inside apply:
216217
```go
217-
func applyFoo(obj *v1alpha1.Foo, target TargetFoo) flow.Outcome {
218+
func applyFoo(obj *v1alpha1.Foo, target TargetFoo) flow.ReconcileOutcome {
219+
var rf flow.ReconcileFlow
218220
obj.Spec = target.Spec
219-
return flow.Continue() // forbidden: apply helpers do not return flow control
221+
return rf.Continue() // forbidden: apply helpers do not return flow control
220222
}
221223
```
222224

.cursor/rules/controller-reconcile-helper-compute.mdc

Lines changed: 48 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ Summary only; if anything differs, follow normative sections below.
2323
- They MAY use **ConstructionReconcileHelpers** (`new*`, `build*`, `make*`, `compose*`) for internal in-memory construction, as long as the compute helper’s purity/determinism/non-I/O contract remains satisfied.
2424
- They treat `obj` and all caller-provided inputs as **read-only inputs** and MUST NOT mutate them (including via **Aliasing** of maps/slices; **Clone** before modifying derived maps/slices).
2525
- They MUST NOT perform **Kubernetes API I/O**, call **DeepCopy**, execute patches, or make any **patch ordering** / **patch type decision** decisions.
26-
- If a **ComputeReconcileHelper** returns **Outcome** (in code: `flow.Outcome`), it MUST use it only for **flow control** (continue/done/requeue) and/or **errors**.
27-
- A **ComputeReconcileHelper** MUST NOT use **Outcome** change tracking (`ReportChanged`, `ReportChangedIf`) or **Optimistic-lock signaling** (`RequireOptimisticLock`).
26+
- A **ComputeReconcileHelper** MUST return computed values (and optionally `error`) and MUST NOT report object mutations or optimistic-lock intent.
27+
In particular, a **ComputeReconcileHelper** MUST NOT return `flow.EnsureOutcome` and MUST NOT call `ReportChanged*` / `RequireOptimisticLock`.
2828
- If `computeTarget*` derives **target** values for **both** **patch domains** (**main patch domain** + **status patch domain**) that will later be used by **IsInSyncReconcileHelper** and/or **ApplyReconcileHelper**, it MUST return **two separate** values (**target main** + **target status**), not a mixed struct.
2929
- New code MUST NOT introduce `computeDesired*` helpers. Replace legacy “desired” helpers with **intended**/**target**/**report** helpers.
3030
- If a **ComputeReconcileHelper** depends on previous compute output, the dependency MUST be explicit in the signature as args **after `obj`**.
@@ -159,44 +159,36 @@ Or, for **report** computations when the helper needs data from `Reconciler`:
159159
func (r *Reconciler) computeFooReport(obj *v1alpha1.Foo, intendedFoo IntendedFoo, actualFoo ActualFoo, targetFoo TargetFoo) (FooReport, error)
160160
```
161161

162-
### Complex compute with flow control
163-
Prefer returning **Outcome** (in code, the type is `flow.Outcome`) and writing to `out`:
164-
```go
165-
func computeIntendedFoo(ctx context.Context, obj *v1alpha1.Foo, out *IntendedFoo) flow.Outcome
166-
```
167-
168-
Or, if a compute helper needs data from `Reconciler`:
169-
```go
170-
func (r *Reconciler) computeIntendedFoo(ctx context.Context, obj *v1alpha1.Foo, out *IntendedFoo) flow.Outcome
171-
```
162+
### Complex compute with structured logging
172163

173-
Or, for **actual** computations:
174-
```go
175-
func computeActualFoo(ctx context.Context, obj *v1alpha1.Foo, out *ActualFoo) flow.Outcome
176-
```
164+
When a compute helper is large and benefits from phase-scoped logging/panic logging, it SHOULD:
165+
- accept `ctx context.Context`,
166+
- compute into explicit `out` args,
167+
- return `error`,
168+
- and use a step scope (`flow.BeginStep`) for standardized `phase start/end` logs.
177169

178-
Or, for **actual** computations when the helper needs data from `Reconciler`:
170+
Preferred signature:
179171
```go
180-
func (r *Reconciler) computeActualFoo(ctx context.Context, obj *v1alpha1.Foo, out *ActualFoo) flow.Outcome
172+
func computeIntendedFoo(ctx context.Context, obj *v1alpha1.Foo, out *IntendedFoo) error
181173
```
182174

183-
Or, for **target** computations:
175+
Or, if a compute helper needs data from `Reconciler`:
184176
```go
185-
func computeTargetFoo(ctx context.Context, obj *v1alpha1.Foo, intendedFoo IntendedFoo, actualFoo ActualFoo, out *TargetFoo) flow.Outcome
177+
func (r *Reconciler) computeIntendedFoo(ctx context.Context, obj *v1alpha1.Foo, out *IntendedFoo) error
186178
```
187179

188180
Or, for **target** computations that also emit a **report** in one pass:
189181
```go
190-
func computeTargetFoo(ctx context.Context, obj *v1alpha1.Foo, intendedFoo IntendedFoo, actualFoo ActualFoo, outTarget *TargetFoo, outReport *FooReport) flow.Outcome
182+
func computeTargetFoo(
183+
ctx context.Context,
184+
obj *v1alpha1.Foo,
185+
intendedFoo IntendedFoo,
186+
actualFoo ActualFoo,
187+
outTarget *TargetFoo,
188+
outReport *FooReport,
189+
) error
191190
```
192191

193-
Or, for **report** computations:
194-
```go
195-
func computeFooReport(ctx context.Context, obj *v1alpha1.Foo, intendedFoo IntendedFoo, actualFoo ActualFoo, targetFoo TargetFoo, out *FooReport) flow.Outcome
196-
```
197-
198-
> This keeps the call site clean and avoids `(flow.Outcome, DesiredFoo, error)` tuples.
199-
200192
### Dependent compute
201193
If a compute helper depends on previous compute output, the dependency MUST be explicit and come **after `obj`**:
202194
```go
@@ -257,7 +249,7 @@ See the common determinism contract in `controller-reconcile-helper.mdc`.
257249
In particular, avoid producing “equivalent but different” outputs across runs (e.g., unstable ordering).
258250
- **ComputeReconcileHelpers** MAY use extracted computation/caching components owned by the reconciler (e.g. “world view” / “planner” / “topology scorer”, caches), as described in `controller-file-structure.mdc` (“Additional components”), as long as they do not violate the I/O boundaries above.
259251
- Note: cache population is a side effect and an additional source of state; therefore, the helper is deterministic only relative to that state. For the same explicit inputs and the same state of these components, the result MUST be the same.
260-
- If a **ComputeReconcileHelper** returns **Outcome** (in code: `flow.Outcome`), its **flow decision** and **error** MUST be stable for the same inputs and object state.
252+
- Errors (when returned) MUST be stable for the same inputs and object state (no nondeterministic branching / hidden I/O).
261253

262254
> Practical reason: nondeterminism creates patch churn and flaky tests.
263255

@@ -277,44 +269,42 @@ See the common read-only contract in `controller-reconcile-helper.mdc` (especial
277269

278270
---
279271

280-
## Flow phases and **Outcome**
272+
## Phase scopes (optional)
281273

282-
- A **ComputeReconcileHelper** MUST NOT create a `reconcile/flow` **phase** by default.
283-
- A **large** **ComputeReconcileHelper** MAY create a `reconcile/flow` **phase** (`flow.BeginPhase` / `flow.EndPhase`) **only when it improves structure or diagnostics**.
284-
- Otherwise (small/straightforward compute), it MUST NOT create a **phase**.
285-
- If it creates a **phase** (or writes logs), it MUST accept `ctx context.Context` (see `controller-reconcile-helper.mdc`).
286-
- If a **ComputeReconcileHelper** returns **Outcome** (in code: `flow.Outcome`), it MUST use helpers from `internal/reconciliation/flow`:
287-
- `flow.Continue()`, `flow.Done()`, `flow.Fail(err)`, `flow.RequeueAfter(dur)`.
274+
- A **ComputeReconcileHelper** MUST NOT create a phase scope by default.
275+
- A **large** **ComputeReconcileHelper** MAY create a step phase scope (`flow.BeginStep` + deferred `sf.OnEnd(&err)`)
276+
**only when it improves structure or diagnostics**.
277+
- Otherwise (small/straightforward compute), it MUST NOT create a phase scope.
278+
- If it creates a step phase scope (or writes logs), it MUST accept `ctx context.Context` (see `controller-reconcile-helper.mdc`).
279+
- Step scope placement rules are defined in `controller-reconciliation-flow.mdc`.
288280

289-
### **Outcome** change / optimistic-lock reporting
281+
### Change reporting and optimistic-lock signaling
290282

291-
**ComputeReconcileHelpers** MUST NOT report object changes or optimistic-lock requirements via **Outcome** (in code: `flow.Outcome`):
283+
**ComputeReconcileHelpers** MUST NOT report object changes or optimistic-lock requirements:
284+
- MUST NOT return `flow.EnsureOutcome`
292285
- MUST NOT call `ReportChanged` / `ReportChangedIf`
293286
- MUST NOT call `RequireOptimisticLock`
294287

295-
Rationale: `Outcome.DidChange()` / `Outcome.OptimisticLockRequired()` semantically mean
288+
Rationale: change reporting / optimistic-lock intent semantically mean
296289
“this helper already mutated the target object and the subsequent save of that mutation must use **Optimistic locking** semantics”.
297290
**ComputeReconcileHelpers** do not mutate `obj` by contract.
298291

299-
---
300-
301-
### Returning results when using **Outcome**
302-
303-
If a **ComputeReconcileHelper** returns **Outcome** (in code: `flow.Outcome`), it MAY write its computed result into an explicit output argument passed by pointer (e.g. `*DesiredState` / `*ActualState`) instead of returning that result as an additional return value.
304-
305-
- It MUST NOT write the result into `obj`.
292+
### Step scope pattern (illustrative)
306293

307-
Example pattern (illustrative):
308294
```go
309-
func (r *Reconciler) computeIntendedX(ctx context.Context, obj *v1alpha1.X, out *IntendedX) flow.Outcome {
295+
func computeIntendedFoo(ctx context.Context, obj *v1alpha1.Foo, out *IntendedFoo) (err error) {
296+
sf := flow.BeginStep(ctx, "compute-intended-foo")
297+
defer sf.OnEnd(&err)
298+
ctx = sf.Ctx()
299+
310300
if out == nil {
311-
return flow.Fail(fmt.Errorf("out is nil"))
301+
return sf.Errf("out is nil")
312302
}
313303

314304
// compute into *out (pure)
315-
*out = IntendedX{ /* ... */ }
305+
*out = IntendedFoo{ /* ... */ }
316306

317-
return flow.Continue()
307+
return nil
318308
}
319309
```
320310

@@ -368,7 +358,6 @@ Notes (SHOULD):
368358

369359
- See the common error handling rules in `controller-reconcile-helper.mdc`.
370360
- **ComputeReconcileHelpers** SHOULD generally return errors as-is.
371-
- If a **ComputeReconcileHelper** returns **Outcome** (in code: `flow.Outcome`), use `flow.Fail(err)` for errors.
372361

373362
**Allowed (rare)**: when propagating a **non-local** error (e.g., from parsing/validation libs or injected pure components) and additional context is necessary to **disambiguate multiple different error sources** within the same calling **Reconcile method**, a **ComputeReconcileHelper** MAY wrap with small, local context:
374363
- prefer `fmt.Errorf("<local-action>: %w", err)`
@@ -487,10 +476,14 @@ func computeActualFoo(obj *v1alpha1.Foo) ActualFoo {
487476
}
488477
```
489478

490-
❌ Using `flow.Outcome` change / optimistic-lock reporting in compute:
479+
❌ Using change reporting / optimistic-lock signaling in compute:
491480
```go
492-
func computeTargetFoo(ctx context.Context, obj *v1alpha1.Foo, intendedFoo IntendedFoo, actualFoo ActualFoo, out *TargetFoo) flow.Outcome {
481+
func computeTargetFoo(ctx context.Context, obj *v1alpha1.Foo, intendedFoo IntendedFoo, actualFoo ActualFoo, out *TargetFoo) error {
493482
*out = TargetFoo{ /* ... */ }
494-
return flow.Continue().ReportChanged().RequireOptimisticLock() // forbidden in compute
483+
484+
// forbidden: compute helpers do not mutate obj and must not signal persistence semantics
485+
_ = flow.EnsureOutcome{}.ReportChanged() // (illustrative) forbidden category mixing
486+
487+
return nil
495488
}
496489
```

.cursor/rules/controller-reconcile-helper-construction.mdc

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ Summary only; if anything differs, follow normative sections below.
2626
- Clone maps/slices before editing; avoid returning references that alias caller-owned storage unless explicitly documented and safe.
2727
- MUST NOT:
2828
- do Kubernetes API I/O, filesystem/network/env reads, or use time/random sources,
29-
- log/print, accept `context.Context`, start `reconcile/flow` phases, or call `DeepCopy`,
30-
- return `flow.Outcome` or make flow/patch orchestration decisions (patch ordering/strategy/execution).
29+
- log/print, accept `context.Context`, start `reconcile/flow` phase scopes (`flow.BeginReconcile` / `flow.BeginEnsure` / `flow.BeginStep`), or call `DeepCopy`,
30+
- return `flow.ReconcileOutcome` / `flow.EnsureOutcome` or make flow/patch orchestration decisions (patch ordering/strategy/execution).
3131
- MUST be plain functions (no `Reconciler` receiver) and may only call other **construction** helpers.
3232
- If the primary goal is a reconciliation pipeline artifact (**intended/actual/target/report**) or domain decision-making, prefer **ComputeReconcileHelper** (`compute*`) and use construction helpers only as sub-steps.
3333

@@ -244,10 +244,10 @@ Important distinctions:
244244

245245
---
246246

247-
## Flow phases and **Outcome**
247+
## Flow phase scopes and outcomes
248248

249-
- **ConstructionReconcileHelpers** MUST NOT create a `reconcile/flow` **phase**.
250-
- **ConstructionReconcileHelpers** MUST NOT return **Outcome** (in code: `flow.Outcome`).
249+
- **ConstructionReconcileHelpers** MUST NOT create a `reconcile/flow` **phase scope** (`flow.BeginReconcile` / `flow.BeginEnsure` / `flow.BeginStep`).
250+
- **ConstructionReconcileHelpers** MUST NOT return **ReconcileOutcome** (`flow.ReconcileOutcome`) or **EnsureOutcome** (`flow.EnsureOutcome`).
251251
- **ConstructionReconcileHelpers** MUST NOT log (they do not accept `ctx context.Context`).
252252

253253
---
@@ -284,16 +284,18 @@ func newFoo(ctx context.Context, c client.Client, obj *v1alpha1.Foo) (FooOut, er
284284
func buildFoo(ctx context.Context, obj *v1alpha1.Foo) FooOut {
285285
l := log.FromContext(ctx)
286286
l.Info("building foo") // forbidden: no logging/phases in construction helpers
287-
flow.BeginPhase(ctx, "buildFoo") // forbidden
287+
rf := flow.BeginReconcile(ctx, "build-foo") // forbidden: no flow phase scopes in construction helpers
288+
_ = rf
288289
return FooOut{}
289290
}
290291
```
291292

292293
❌ Returning `flow.Outcome` / doing flow control:
293294

294295
```go
295-
func makeFoo(obj *v1alpha1.Foo) flow.Outcome {
296-
return flow.Continue() // forbidden: construction helpers do not return Outcome
296+
func makeFoo(obj *v1alpha1.Foo) flow.ReconcileOutcome {
297+
var rf flow.ReconcileFlow
298+
return rf.Continue() // forbidden: construction helpers do not return reconcile outcomes
297299
}
298300
```
299301

.cursor/rules/controller-reconcile-helper-create.mdc

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,6 @@ Typical create helpers are used for child resources to encapsulate the mechanica
5858

5959
### Simple create
6060
```go
61-
func (r *Reconciler) createSKN(
62-
ctx context.Context,
63-
obj *v1alpha1.SomeKindName,
64-
) flow.Outcome
65-
```
66-
67-
Or, if **Outcome** (in code, the type is `flow.Outcome`) is intentionally not used:
68-
```go
6961
func (r *Reconciler) createSKN(
7062
ctx context.Context,
7163
obj *v1alpha1.SomeKindName,
@@ -148,20 +140,18 @@ See the common read-only contract in `controller-reconcile-helper.mdc` (especial
148140

149141
---
150142

151-
## Flow phases and **Outcome**
143+
## Flow phase scopes and outcomes
152144

153-
- **CreateReconcileHelpers** MUST NOT create a `reconcile/flow` **phase** — they should stay mechanical and short.
154-
- If a **CreateReconcileHelper** returns **Outcome** (in code: `flow.Outcome`), it SHOULD use helpers from `internal/reconciliation/flow`:
155-
- `flow.Continue()`, `flow.Done()`, `flow.Fail(err)`, `flow.RequeueAfter(dur)`.
156-
- Prefer encoding retry/requeue policy explicitly in the returned **Outcome**.
145+
- **CreateReconcileHelpers** MUST NOT create a `reconcile/flow` **phase scope** — they should stay mechanical and short.
146+
- **CreateReconcileHelpers** MUST return `error` and MUST NOT return **ReconcileOutcome** (`flow.ReconcileOutcome`) or **EnsureOutcome** (`flow.EnsureOutcome`).
147+
- Any retry/requeue policy belongs to the calling **Reconcile method** (use `ReconcileFlow` there).
157148

158149
---
159150

160151
## Error handling
161152

162153
- See the common error handling rules in `controller-reconcile-helper.mdc`.
163154
- A **CreateReconcileHelper** SHOULD be mechanically thin: if the single `Create(...)` call fails, return the error **without wrapping**.
164-
- If returning **Outcome** (in code: `flow.Outcome`), use `flow.Fail(err)` (or equivalent) with the original `err`.
165155
- A **CreateReconcileHelper** MUST NOT enrich errors with additional context (including **object identity** such as `namespace/name`, UID, object key).
166156
- Error enrichment (action + **object identity** + **phase**) is the calling **Reconcile method**’s responsibility.
167157

@@ -171,18 +161,18 @@ See the common read-only contract in `controller-reconcile-helper.mdc` (especial
171161

172162
❌ Doing existence checks (`Get/List`) or any extra Kubernetes API calls:
173163
```go
174-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) flow.Outcome {
164+
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
175165
// forbidden: extra API call
176166
var existing v1alpha1.EON
177167
if err := r.client.Get(ctx, client.ObjectKeyFromObject(obj), &existing); err == nil {
178-
return flow.Continue() // "already exists" decision belongs to Reconcile methods
168+
return nil // "already exists" decision belongs to Reconcile methods
179169
}
180170

181171
// forbidden: second API call in the same helper if create proceeds
182172
if err := r.client.Create(ctx, obj); err != nil {
183-
return flow.Fail(err)
173+
return err
184174
}
185-
return flow.Continue()
175+
return nil
186176
}
187177
```
188178

0 commit comments

Comments
 (0)