refactor(status): one writer for the kstatus trio, one status write per reconcile

.golangci.yml

@@ -363,6 +363,14 @@ linters:
       - text: "SA1019: scheme.Builder is deprecated"
         path: 'api/v1alpha3/groupversion_info\.go'
         linters: [staticcheck]
+      # mgr.GetEventRecorderFor returns the record.EventRecorder our shared status writer emits on
+      # (a single Event per persisted Ready transition). It is the ubiquitous kubebuilder idiom; the
+      # non-deprecated GetEventRecorder returns the newer events.EventRecorder, whose Eventf demands
+      # an action verb and a note and does not fit a one-line status Event. Newer staticcheck
+      # flags the deprecation; scope the allowance to where the manager is wired.
+      - text: "SA1019: mgr.GetEventRecorderFor is deprecated"
+        path: 'cmd/main\.go'
+        linters: [staticcheck]
       # Allow fmt.Print* in test/e2e (debugging outputs)
       - path: 'test/e2e/.*\.go'
         linters: [forbidigo]

api/v1alpha3/clusterwatchrule_types.go

@@ -200,7 +200,7 @@ type ClusterWatchRule struct {
 
 	// metadata is a standard object metadata.
 	// +optional
-	metav1.ObjectMeta `json:"metadata,omitempty"`
+	metav1.ObjectMeta `json:"metadata,omitempty,omitzero"`
 
 	// spec defines the desired state of ClusterWatchRule.
 	// +required

api/v1alpha3/gitprovider_types.go

@@ -143,7 +143,9 @@ type GitProviderStatus struct {
 	// +optional
 	// +listType=map
 	// +listMapKey=type
-	Conditions []metav1.Condition `json:"conditions,omitempty"`
+	// +patchMergeKey=type
+	// +patchStrategy=merge
+	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// SigningPublicKey is the operator's SSH signing public key in authorized_keys format.
 	// Register this as a signing key on your git platform.

api/v1alpha3/gittarget_types.go

@@ -195,10 +195,6 @@ type GitTargetStatus struct {
 	// +patchStrategy=merge
 	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
-	// LastReconcileTime is the timestamp of the most recent reconcile attempt.
-	// +optional
-	LastReconcileTime metav1.Time `json:"lastReconcileTime,omitempty"`
-
 	// LastPushTime is the timestamp of the last successful push.
 	// +optional
 	LastPushTime *metav1.Time `json:"lastPushTime,omitempty"`
@@ -228,7 +224,11 @@ type GitTargetStatus struct {
 // GitTargetStreamsStatus is a bounded roll-up of the stream readiness state for the
 // types this GitTarget tracks.
 type GitTargetStreamsStatus struct {
-	// Summary is the display-only ready/total ratio.
+	// Summary is the display-only ready/total ratio, e.g. "3/4".
+	//
+	// It restates Ready and Total, which the API conventions would normally rule out. It exists
+	// solely to feed the Streams printer column: a column can read one JSONPath, not format two.
+	// Do not compute anything from it — read ready and total.
 	// +optional
 	Summary string `json:"summary,omitempty"`
 
@@ -243,10 +243,6 @@ type GitTargetStreamsStatus struct {
 
 	// Blocked is how many tracked types cannot currently be watched.
 	Blocked int32 `json:"blocked"`
-
-	// ObservedTime is when this roll-up was last computed.
-	// +optional
-	ObservedTime *metav1.Time `json:"observedTime,omitempty"`
 }
 
 // Design rationale, kept out of the generated CRD description by the blank line below.
@@ -282,9 +278,8 @@ type GitTargetRetentionStatus struct {
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
-// +kubebuilder:printcolumn:name="Provider",type=string,JSONPath=`.spec.providerRef.name`
-// +kubebuilder:printcolumn:name="Branch",type=string,JSONPath=`.spec.branch`
-// +kubebuilder:printcolumn:name="Path",type=string,JSONPath=`.spec.path`
+// Seven default-priority columns wrapped `kubectl get gittargets` on any normal terminal.
+// Flux ships three or four; the identity fields stay one `-o wide` away.
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
 // +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason`
 // +kubebuilder:printcolumn:name="Streams",type=string,JSONPath=`.status.streams.summary`
@@ -296,6 +291,9 @@ type GitTargetRetentionStatus struct {
 // +kubebuilder:printcolumn:name="ClusterProviderReady",type=string,JSONPath=`.status.conditions[?(@.type=="ClusterProviderReady")].status`,priority=1
 // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].message`,priority=1
 // +kubebuilder:printcolumn:name="Encryption",type=string,JSONPath=`.spec.encryption.provider`,priority=1
+// +kubebuilder:printcolumn:name="Provider",type=string,JSONPath=`.spec.providerRef.name`,priority=1
+// +kubebuilder:printcolumn:name="Branch",type=string,JSONPath=`.spec.branch`,priority=1
+// +kubebuilder:printcolumn:name="Path",type=string,JSONPath=`.spec.path`,priority=1
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
 // GitTarget is the Schema for the gittargets API.

api/v1alpha3/watchrule_types.go

@@ -235,10 +235,6 @@ type WatchRuleStreamsStatus struct {
 	// +optional
 	// +kubebuilder:validation:MaxItems=5
 	PendingSample []string `json:"pendingSample,omitempty"`
-
-	// ObservedTime is when this roll-up was last computed.
-	// +optional
-	ObservedTime *metav1.Time `json:"observedTime,omitempty"`
 }
 
 // Design rationale, kept out of the generated CRD description by the blank line below.
@@ -274,7 +270,7 @@ type WatchRule struct {
 
 	// metadata is a standard object metadata
 	// +optional
-	metav1.ObjectMeta `json:"metadata,omitempty"`
+	metav1.ObjectMeta `json:"metadata,omitempty,omitzero"`
 
 	// spec defines the desired state of WatchRule
 	// +required

cmd/main.go

@@ -187,6 +187,7 @@ func main() {
 		Scheme:       mgr.GetScheme(),
 		RuleStore:    ruleStore,
 		WatchManager: watchMgr,
+		Recorder:     mgr.GetEventRecorderFor("watchrule"),
 	}).SetupWithManager(mgr), "unable to create controller", "controller", "WatchRule")
 
 	// ClusterWatchRule controller (with WatchManager reference for dynamic reconciliation)
@@ -195,6 +196,7 @@ func main() {
 		Scheme:       mgr.GetScheme(),
 		RuleStore:    ruleStore,
 		WatchManager: watchMgr,
+		Recorder:     mgr.GetEventRecorderFor("clusterwatchrule"),
 	}).SetupWithManager(mgr), "unable to create controller", "controller", "ClusterWatchRule")
 
 	// Valkey/Redis is optional. When configured it holds each GitTarget's watch resume cursor so work
@@ -282,6 +284,7 @@ func main() {
 		Client:      mgr.GetClient(),
 		Scheme:      mgr.GetScheme(),
 		SSHHostKeys: cfg.sshHostKeys,
+		Recorder:    mgr.GetEventRecorderFor("gitprovider"),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "GitProvider")
 		os.Exit(1)
@@ -291,6 +294,7 @@ func main() {
 		Scheme:            mgr.GetScheme(),
 		OperatorNamespace: os.Getenv("POD_NAMESPACE"),
 		KubeConfigSafety:  cfg.kubeConfigSafety,
+		Recorder:          mgr.GetEventRecorderFor("clusterprovider"),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterProvider")
 		os.Exit(1)
@@ -300,6 +304,7 @@ func main() {
 		Scheme:        mgr.GetScheme(),
 		WorkerManager: workerManager,
 		EventRouter:   eventRouter,
+		Recorder:      mgr.GetEventRecorderFor("gittarget"),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "GitTarget")
 		os.Exit(1)

config/crd/bases/configbutler.ai_clusterwatchrules.yaml

@@ -275,10 +275,6 @@ spec:
                       be watched.
                     format: int32
                     type: integer
-                  observedTime:
-                    description: ObservedTime is when this roll-up was last computed.
-                    format: date-time
-                    type: string
                   pendingSample:
                     description: PendingSample is a bounded sample of types not yet
                       ready.

config/crd/bases/configbutler.ai_gittargets.yaml

@@ -15,15 +15,6 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - jsonPath: .spec.providerRef.name
-      name: Provider
-      type: string
-    - jsonPath: .spec.branch
-      name: Branch
-      type: string
-    - jsonPath: .spec.path
-      name: Path
-      type: string
     - jsonPath: .status.conditions[?(@.type=="Ready")].status
       name: Ready
       type: string
@@ -65,6 +56,18 @@ spec:
       name: Encryption
       priority: 1
       type: string
+    - jsonPath: .spec.providerRef.name
+      name: Provider
+      priority: 1
+      type: string
+    - jsonPath: .spec.branch
+      name: Branch
+      priority: 1
+      type: string
+    - jsonPath: .spec.path
+      name: Path
+      priority: 1
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date
@@ -444,11 +447,6 @@ spec:
                   push.
                 format: date-time
                 type: string
-              lastReconcileTime:
-                description: LastReconcileTime is the timestamp of the most recent
-                  reconcile attempt.
-                format: date-time
-                type: string
               observedGeneration:
                 description: ObservedGeneration is the latest generation observed
                   by the controller.
@@ -494,10 +492,6 @@ spec:
                       be watched.
                     format: int32
                     type: integer
-                  observedTime:
-                    description: ObservedTime is when this roll-up was last computed.
-                    format: date-time
-                    type: string
                   ready:
                     description: Ready is how many tracked types are Streaming.
                     format: int32
@@ -508,7 +502,12 @@ spec:
                     format: int32
                     type: integer
                   summary:
-                    description: Summary is the display-only ready/total ratio.
+                    description: |-
+                      Summary is the display-only ready/total ratio, e.g. "3/4".
+
+                      It restates Ready and Total, which the API conventions would normally rule out. It exists
+                      solely to feed the Streams printer column: a column can read one JSONPath, not format two.
+                      Do not compute anything from it — read ready and total.
                     type: string
                   total:
                     description: Total is how many types this target tracks.

config/crd/bases/configbutler.ai_watchrules.yaml

@@ -287,10 +287,6 @@ spec:
                       be watched.
                     format: int32
                     type: integer
-                  observedTime:
-                    description: ObservedTime is when this roll-up was last computed.
-                    format: date-time
-                    type: string
                   pendingSample:
                     description: PendingSample is a bounded sample of types not yet
                       ready.

config/rbac/role.yaml

@@ -4,6 +4,13 @@ kind: ClusterRole
 metadata:
   name: gitops-reverser
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - ""
   resources:

docs/UPGRADING.md

@@ -7,6 +7,51 @@ guidance that the changelog's breaking-change entries link to.
 We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with
 `bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it.
 
+## Unreleased: status vocabulary and two removed status fields (next minor; status-only change)
+
+Nothing in `spec` changed and no manifest you wrote needs editing. Three things a **reader of
+status** may notice:
+
+**1. The generic condition reason is now `Succeeded`, not `OK` or `Ready`.** A reason that restates
+the condition type (`Ready=True, reason=Ready`) answers nothing. The generic reasons are now aliases
+of [`github.com/fluxcd/pkg/apis/meta`](https://pkg.go.dev/github.com/fluxcd/pkg/apis/meta)
+(`Succeeded`, `Failed`, `Progressing`, `DependencyNotReady`), so one alerting rule works across every
+kind here and across every Flux kind in the same cluster. Domain reasons (`UnsupportedContent`,
+`WriteBoundaryRefused`, `NoAdmittedSourceNamespaces`, …) are unchanged.
+
+If you match on a reason string, update:
+
+| Was | Now |
+|---|---|
+| `Ready=True`, reason `OK` (GitTarget, `Validated` too) | reason `Succeeded` |
+| `Ready=True`, reason `Ready` (GitProvider, ClusterProvider, WatchRule, ClusterWatchRule) | reason `Succeeded` |
+
+Matching on `status` rather than `reason` (the usual case, and what
+`kubectl wait --for=condition=Ready` does) needs no change.
+
+**2. `GitTarget.status.lastReconcileTime` and `status.streams.observedTime` were removed.** Both were
+stamped with the current time on every pass, which made every reconcile a status write, and every
+status write a fresh watch event that re-queued the object. A condition's `lastTransitionTime` plus
+the `controller_runtime_reconcile_total` metric answer the same question without making every object
+mutable on read. `status.lastPushTime` (a real event) and `status.retention.observedTime` (the
+data plane's own observation time, which you need to tell a stale zero from a live one) both stay.
+
+**3. A GitTarget with no WatchRules now reports `Ready=True`.** It used to sit at
+`Ready=False`/`Reconciling=True`/`NoResolvedTypes` forever, a state the documented setup flow passes
+through, since you create the GitTarget before its rules. Nothing was pending and nothing ever would
+be, so `kubectl wait --for=condition=Ready` never returned and the object re-reconciled every 10
+seconds for its whole life. "Nothing to mirror" is a converged state. `status.streams.summary` still
+reads `0/0` and `StreamsRunning` still carries reason `NoResolvedTypes`, so the zero stays visible.
+
+If you have automation that treats "GitTarget Ready" as "GitTarget is mirroring something", assert
+`status.streams.total > 0` as well.
+
+**Also in this change, with no action needed:** GitTarget's `kubectl get` output drops to four
+columns (`Ready`, `Reason`, `Streams`, `Age`); `Provider`, `Branch` and `Path` moved to
+`-o wide`. And every controller now emits a Kubernetes Event on each persisted `Ready` transition,
+visible in `kubectl describe` and routable by anything that consumes Events. The operator's
+ServiceAccount gained `create`/`patch` on `events` for it, which `helm upgrade` applies.
+
 ## Unreleased — a resync no longer deletes Git documents by default (next minor; behavior change)
 
 **`GitTarget` gained `spec.prune.mode`, and its effective default changes what a resync does.**

docs/design/reconcile-triggering.md

@@ -44,11 +44,19 @@ Facts first, then the fix, then the gaps, then the toolbox.
 | Controller | Watches (besides `For`) | Predicate | Happy-path requeue | Data-plane edge |
 |---|---|---|---|---|
 | **GitProvider** | *(none)* | `GenerationChanged` on self | 5 min | ❌ |
-| **GitTarget** | GitProvider, WatchRule, ClusterWatchRule | `GenerationChanged` on deps | 5 min | ✅ GitPath channel (new) |
-| **WatchRule** | GitTarget, GitProvider | `GenerationChanged` | 5 min | ❌ |
-| **ClusterWatchRule** | GitTarget, GitProvider | `GenerationChanged` | 5 min | ❌ |
+| **GitTarget** | GitProvider, ClusterProvider, Namespace, WatchRule, ClusterWatchRule | `GenerationChanged` on **self** and on deps | 5 min converged / 10 s settling | ✅ GitPath channel |
+| **WatchRule** | GitTarget, GitProvider, ClusterProvider | `GenerationChanged` on **self** and on deps | 5 min converged / 10 s settling | ✅ source-namespace channel |
+| **ClusterWatchRule** | GitTarget, GitProvider | `GenerationChanged` on **self** and on deps | 5 min converged / 10 s settling | ❌ |
 | **CommitRequest** | *(none)* | — | polls every 2 s | polls worker, no push |
 
+The `For()` predicate on GitTarget, WatchRule and ClusterWatchRule is not an
+optimisation, it closes a self-triggering edge: a status write bumps `resourceVersion`
+and fires an Update watch event that `EnqueueRequestForObject` turns straight back into
+a queued request, un-rate-limited, so every reconcile cost roughly two. The shared status
+writer already suppresses no-op writes (see
+[`../spec/status-conditions-guide.md`](../spec/status-conditions-guide.md)); the predicate
+makes it structural, and matches what GitProvider and ClusterProvider already did.
+
 The GitTarget no longer watches Secrets: the encryption-Secret watch was removed so the
 process stops retaining every Secret value in the cluster; generated-age-Secret recovery
 and out-of-band age-key updates ride the 5-minute reconcile instead.
@@ -221,13 +229,22 @@ status heartbeat. The fix for B1 is the same shape: `predicate.Or(
 GenerationChangedPredicate{}, gitTargetReadinessChangedPredicate{})` rather than
 removing the predicate (which reintroduces the storm).
 
-### F4. Conditions **and** Events, every loop (fixes Gap A surfacing)
+### F4. Conditions **and** Events, every loop (fixes Gap A surfacing) — SHIPPED
 
 Flux always patches kstatus conditions (`fluxcd/pkg/runtime/conditions` + `patch`)
 **and** emits a Kubernetes Warning Event (`fluxcd/pkg/runtime/events`, which also
 forwards to notification-controller), then requeues at a shorter failure interval.
 A failure is immediately visible and alertable, not buried in a metric.
 
+**Built.** Every reconciler takes an `EventRecorder` and the shared status writer
+(`internal/controller/status.go`) emits one Event per **persisted** `Ready`
+transition: `Normal` when `Ready=True`, `Warning` otherwise, carrying the condition's
+reason and message. Emitting after the patch rather than beside each condition write
+is deliberate — a reconcile may write `Ready` several times and only the last is
+stored, so intermediate values never reach `kubectl describe`. That also means the
+suppressed no-op status write emits nothing, which is the intended behavior: nothing
+changed, so nothing happened.
+
 ### F5. Generalize the data-plane nudge (the §2 recipe, reused)
 
 Extend the GitPath channel into a small shared "status nudge": the data plane