Incremental state step 2: central recompute and diff

.agents/skills/implement-conformance-test/journal/central-recompute-and-diff.md

@@ -0,0 +1,69 @@
+# Conformance & Architecture Journal: Incremental State Step 2 - Central Recompute and Diff
+
+## 1. Overview
+- **Objective**: Implement Step 2 of `docs/incremental-state.md` ("Central recompute and diff").
+- **Key Goals**:
+  1. Record all inputs (including `GatewayClass` and `GatewayAddresses`) into `State`.
+  2. Feed `State` straight from informer event handlers, marking state synced only after all informer registrations report `HasSynced() == true`.
+  3. Pure computation of all outputs via `ComputeOutputs(inputs ModelInputs) *Outputs` without reading previously written statuses.
+  4. Recompute centrally with debouncing/coalescing in `State`, diff desired status against previous outputs, and push events directly into controller workqueues via custom `EventSource` (never dropping events under load).
+  5. Reconcilers only write status from computed outputs, skipping Gateways whose GatewayClass is not managed by us.
+  6. Execute callbacks (`UpdateConfig`, `UpdateCertificates`, `onGatewaysUpdate`) outside `s.mu` to prevent deadlocks.
+  7. Deterministic `status.parents` ordering (#627) preserved with no unnecessary API writes for order-only differences.
+  8. Delete unneeded non-status reconcilers (Secret, Namespace, ConfigMap, Service, ReferenceGrant) and all 19 cross-object mapping functions.
+  9. Add unit tests and recomputation benchmark.
+
+## 2. Issue / Architectural Analysis
+- **Previous Architecture**:
+  - Each controller manually fetched dependency objects or re-ran partial compilation in its own reconciler.
+  - 19 hand-written `EnqueueRequestsFromMapFunc` watches existed across Gateway, HTTPRoute, ListenerSet, BackendTLSPolicy, and ConfigMap reconcilers.
+  - Reconcilers invoked `updateProxy()` independently, causing duplicate compilations and proxy updates.
+  - `CompileModel` checked `len(gw.Status.Addresses) > 0`, making computation depend on status writes rather than pure recorded inputs.
+- **Root Cause & Design Solutions**:
+  - Centralized input tracking with a monotonic revision counter stamped on genuine input modifications (ignoring status-only echoes).
+  - Provided clean separation: `ComputeOutputs` purely calculates desired statuses and proxy configs; `State.Recompute()` diffs and notifies; reconcilers merge status (preserving `LastTransitionTime` and foreign controller entries) and write to API server.
+  - Feed `State` directly from informer event handlers (`AddEventHandler`), eliminating redundant reconcilers.
+  - Custom `EventSource` sends reconcile requests directly to controller workqueues, avoiding channel drops.
+
+## 3. Implementation Details
+- **Informer Event Handlers & Sync (`pkg/controller/utils.go`, `pkg/state/state.go`)**:
+  - Added informer event handlers for all 10 input types (`GatewayClass`, `Gateway`, `ListenerSet`, `HTTPRoute`, `BackendTLSPolicy`, `Service`, `Secret`, `ConfigMap`, `Namespace`, `ReferenceGrant`).
+  - `State.Start` waits for all registrations to report `HasSynced() == true` before enabling output computation (`SetSynced(true)`), preventing partial-world flapping and startup 404s.
+- **Lock-Free Callbacks (`pkg/state/state.go`)**:
+  - Captured proxy configuration and gateway slice under `s.mu`, released `s.mu`, and executed `proxy.UpdateConfig`, `proxy.UpdateCertificates`, and `onGatewaysUpdate` outside the lock.
+- **EventSource Workqueue Delivery (`pkg/state/state.go`)**:
+  - Implemented `EventSource` implementing `source.TypedSource[reconcile.Request]` that enqueues directly to controller rate-limiting workqueues.
+- **GatewayClass Filtering & Gateway Reconciler (`pkg/controller/gateway_controller.go`)**:
+  - `GatewayReconciler` verifies `gc.Spec.ControllerName == r.ControllerName`. If not ours, deletes from `State` and exits without calling `AddressProvider` or writing status.
+  - `CompileModel` initializes `managedGatewayClasses` to an empty map so Gateways are not compiled until their GatewayClass is known.
+- **Deterministic Route Parents Ordering (`pkg/state/compiled.go`, `pkg/state/conditions.go`)**:
+  - Kept #627's deterministic parentRef sorting and other controller preservation in `UpdateRouteParentStatuses` / `MergeHTTPRouteStatus`.
+- **Testing & Benchmarks**:
+  - Added unit tests in `pkg/state/state_test.go` and `pkg/controller/controller_test.go`.
+  - Added `BenchmarkState_Recompute` in `pkg/state/state_benchmark_test.go` testing 100 Gateways and 1000 HTTPRoutes (~2.7ms per full recompute).
+
+## 4. Key Files Modified / Added / Deleted
+- `pkg/state/state.go`
+- `pkg/state/compiled.go`
+- `pkg/state/conditions.go`
+- `pkg/state/gateway.go`
+- `pkg/state/referencegrant.go`
+- `pkg/state/state_test.go`
+- `pkg/state/state_benchmark_test.go`
+- `pkg/controller/gateway_controller.go`
+- `pkg/controller/httproute_controller.go`
+- `pkg/controller/listenerset_controller.go`
+- `pkg/controller/backendtlspolicy_controller.go`
+- `pkg/controller/utils.go`
+- `pkg/controller/controller_test.go`
+- (Deleted: `configmap_controller.go`, `namespace_controller.go`, `referencegrant_controller.go`, `secret_controller.go`, `service_controller.go`)
+- `.agents/skills/implement-conformance-test/journal/central-recompute-and-diff.md`
+
+## 5. Validation & Results
+- **Unit Tests & Benchmark**:
+  - All unit tests across all packages pass (`pkg/controller`, `pkg/gari`, `pkg/provisioning/singlepod`, `pkg/proxy`, `pkg/state`, `snigateway`).
+  - `BenchmarkState_Recompute` executed at ~2.7ms for 100 Gateways and 1000 HTTPRoutes.
+- **Linting & Code Generation**:
+  - `ap generate` and `ap lint` passed cleanly with 0 errors/warnings.
+- **Conformance & E2E**:
+  - All Gateway API conformance tests and E2E suites passed under `ap e2e`.

pkg/controller/backendtlspolicy_controller.go

@@ -16,305 +16,59 @@ package controller
 
 import (
 	"context"
-	"encoding/pem"
 	"fmt"
-	"reflect"
 
-	"github.com/gke-labs/gateway-api-reference-implementation/pkg/proxy"
 	"github.com/gke-labs/gateway-api-reference-implementation/pkg/state"
-	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 )
 
 type BackendTLSPolicyReconciler struct {
 	client.Client
-	Scheme           *runtime.Scheme
-	State            *state.State
-	Proxy            *proxy.Proxy
-	ControllerName   string
-	OnGatewaysUpdate func([]*gatewayv1.Gateway)
+	Scheme         *runtime.Scheme
+	State          *state.State
+	ControllerName string
 }
 
 func (r *BackendTLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	l := log.FromContext(ctx)
 
-	controllerName := r.ControllerName
-
 	policy := &gatewayv1.BackendTLSPolicy{}
 	if err := r.Get(ctx, req.NamespacedName, policy); err != nil {
-		if apierrors.IsNotFound(err) {
-			r.State.DeleteBackendTLSPolicy(req.NamespacedName)
-			r.updateProxy()
-		}
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Find all Gateways that use this policy (via Services referenced in HTTPRoutes)
-	gateways := r.State.GetGateways()
-	routes := r.State.GetHTTPRoutes()
-	allPolicies := r.State.GetBackendTLSPolicies()
-
-	// Conflict resolution
-	isConflicted := false
-	var conflictingPolicy string
-	for _, targetRef := range policy.Spec.TargetRefs {
-		if string(targetRef.Group) != "" && string(targetRef.Group) != "gateway.networking.k8s.io" {
-			continue
-		}
-		if string(targetRef.Kind) != "Service" {
-			continue
-		}
-
-		targetSvcNamespace := policy.Namespace
-		targetSvcName := string(targetRef.Name)
-
-		for _, p := range allPolicies {
-			if p.Namespace == policy.Namespace && p.Name == policy.Name {
-				continue
-			}
-
-			for _, t := range p.Spec.TargetRefs {
-				if (string(t.Group) == "" || string(t.Group) == "gateway.networking.k8s.io") && string(t.Kind) == "Service" {
-					if p.Namespace == targetSvcNamespace && string(t.Name) == targetSvcName {
-						// Found another policy targeting the same service
-						// Check if it's older
-						if p.CreationTimestamp.Time.Before(policy.CreationTimestamp.Time) {
-							isConflicted = true
-							conflictingPolicy = fmt.Sprintf("%s/%s", p.Namespace, p.Name)
-							break
-						}
-						if p.CreationTimestamp.Time.Equal(policy.CreationTimestamp.Time) {
-							if p.Namespace < policy.Namespace || (p.Namespace == policy.Namespace && p.Name < policy.Name) {
-								isConflicted = true
-								conflictingPolicy = fmt.Sprintf("%s/%s", p.Namespace, p.Name)
-								break
-							}
-						}
-					}
-				}
-			}
-			if isConflicted {
-				break
-			}
-		}
-		if isConflicted {
-			break
-		}
-	}
-
-	// Validate CA certificates
-	var caCerts [][]byte
-	var unresolvedRefs []string
-	for _, caRef := range policy.Spec.Validation.CACertificateRefs {
-		if string(caRef.Group) == "" && string(caRef.Kind) == "ConfigMap" {
-			cm := &corev1.ConfigMap{}
-			if err := r.Get(ctx, types.NamespacedName{Namespace: policy.Namespace, Name: string(caRef.Name)}, cm); err != nil {
-				unresolvedRefs = append(unresolvedRefs, string(caRef.Name))
-			} else {
-				var data []byte
-				if d, ok := cm.Data["ca.crt"]; ok {
-					data = []byte(d)
-				} else if d, ok := cm.BinaryData["ca.crt"]; ok {
-					data = d
-				}
-
-				if len(data) == 0 {
-					unresolvedRefs = append(unresolvedRefs, string(caRef.Name))
-				} else {
-					// Verify it's a valid PEM block
-					block, _ := pem.Decode(data)
-					if block == nil || block.Type != "CERTIFICATE" {
-						unresolvedRefs = append(unresolvedRefs, string(caRef.Name))
-					} else {
-						caCerts = append(caCerts, data)
-					}
-				}
-			}
-		} else {
-			unresolvedRefs = append(unresolvedRefs, string(caRef.Name))
-		}
-	}
-
-	acceptedStatus := metav1.ConditionTrue
-	acceptedReason := gatewayv1.PolicyReasonAccepted
-	acceptedMessage := "Policy accepted"
-
-	resolvedRefsStatus := metav1.ConditionTrue
-	resolvedRefsReason := gatewayv1.BackendTLSPolicyReasonResolvedRefs
-	resolvedRefsMessage := "All references resolved"
-
-	if len(unresolvedRefs) > 0 {
-		acceptedStatus = metav1.ConditionFalse
-		acceptedReason = "NoValidCACertificate"
-		acceptedMessage = fmt.Sprintf("Unresolved or invalid CA certificate references: %v", unresolvedRefs)
-
-		resolvedRefsStatus = metav1.ConditionFalse
-		resolvedRefsReason = "InvalidCACertificateRef"
-		resolvedRefsMessage = fmt.Sprintf("Unresolved or invalid CA certificate references: %v", unresolvedRefs)
-	}
-
-	if isConflicted {
-		acceptedStatus = metav1.ConditionFalse
-		acceptedReason = gatewayv1.PolicyReasonConflicted
-		acceptedMessage = fmt.Sprintf("Conflicted with older policy: %s", conflictingPolicy)
-	}
-
-	var ancestors []gatewayv1.PolicyAncestorStatus
-
-	for _, gw := range gateways {
-		usesPolicy := false
-		for _, route := range routes {
-			if route.MatchesGateway(gw.Gateway, controllerName) {
-				for _, rule := range route.Spec.Rules {
-					for _, backendRef := range rule.BackendRefs {
-						if string(state.ValueOf(backendRef.Kind)) == "Service" || state.ValueOf(backendRef.Kind) == "" {
-							backendSvcKey := ResolveNamespacedName(backendRef.Namespace, backendRef.Name, route)
-
-							for _, targetRef := range policy.Spec.TargetRefs {
-								if backendSvcKey.Namespace == policy.Namespace && backendSvcKey.Name == string(targetRef.Name) {
-									usesPolicy = true
-									break
-								}
-							}
-						}
-						if usesPolicy {
-							break
-						}
-					}
-					if usesPolicy {
-						break
-					}
-				}
-			}
-			if usesPolicy {
-				break
-			}
-		}
-
-		if usesPolicy {
-			ancestors = append(ancestors, gatewayv1.PolicyAncestorStatus{
-				AncestorRef: gatewayv1.ParentReference{
-					Group:     state.Ptr(gatewayv1.Group("gateway.networking.k8s.io")),
-					Kind:      state.Ptr(gatewayv1.Kind("Gateway")),
-					Namespace: state.Ptr(gatewayv1.Namespace(gw.Namespace)),
-					Name:      gatewayv1.ObjectName(gw.Name),
-				},
-				ControllerName: gatewayv1.GatewayController(controllerName),
-				Conditions: []metav1.Condition{
-					{
-						Type:               string(gatewayv1.PolicyConditionAccepted),
-						Status:             acceptedStatus,
-						ObservedGeneration: policy.Generation,
-						LastTransitionTime: metav1.Now(),
-						Reason:             string(acceptedReason),
-						Message:            acceptedMessage,
-					},
-					{
-						Type:               string(gatewayv1.BackendTLSPolicyConditionResolvedRefs),
-						Status:             resolvedRefsStatus,
-						ObservedGeneration: policy.Generation,
-						LastTransitionTime: metav1.Now(),
-						Reason:             string(resolvedRefsReason),
-						Message:            resolvedRefsMessage,
-					},
-				},
-			})
+	if r.State != nil {
+		desired, ok := r.State.GetDesiredBackendTLSPolicyStatus(req.NamespacedName)
+		if !ok {
+			return ctrl.Result{}, nil
 		}
-	}
 
-	updated := false
-	if len(policy.Status.Ancestors) != len(ancestors) {
-		updated = true
-	} else {
-		for i := range ancestors {
-			if !reflect.DeepEqual(policy.Status.Ancestors[i].AncestorRef, ancestors[i].AncestorRef) ||
-				string(policy.Status.Ancestors[i].ControllerName) != string(ancestors[i].ControllerName) ||
-				len(policy.Status.Ancestors[i].Conditions) != len(ancestors[i].Conditions) {
-				updated = true
-				break
-			}
-			for j := range ancestors[i].Conditions {
-				matched := false
-				for k := range policy.Status.Ancestors[i].Conditions {
-					if policy.Status.Ancestors[i].Conditions[k].Type == ancestors[i].Conditions[j].Type {
-						if policy.Status.Ancestors[i].Conditions[k].Status == ancestors[i].Conditions[j].Status &&
-							policy.Status.Ancestors[i].Conditions[k].ObservedGeneration == ancestors[i].Conditions[j].ObservedGeneration &&
-							policy.Status.Ancestors[i].Conditions[k].Reason == ancestors[i].Conditions[j].Reason &&
-							policy.Status.Ancestors[i].Conditions[k].Message == ancestors[i].Conditions[j].Message {
-							matched = true
-						}
-						break
-					}
-				}
-				if !matched {
-					updated = true
-					break
-				}
-			}
-			if updated {
-				break
+		if state.MergeBackendTLSPolicyStatus(&policy.Status, desired, gatewayv1.GatewayController(r.ControllerName)) {
+			if err := r.Status().Update(ctx, policy); err != nil {
+				l.Error(err, "unable to update BackendTLSPolicy status")
+				return ctrl.Result{}, err
 			}
 		}
 	}
 
-	if updated {
-		policy.Status.Ancestors = ancestors
-		if err := r.Status().Update(ctx, policy); err != nil {
-			l.Error(err, "unable to update BackendTLSPolicy status")
-			return ctrl.Result{}, err
-		}
-	}
-
-	r.State.UpsertBackendTLSPolicy(policy)
-	_ = caCerts // keep for now
-	r.updateProxy()
-
-	l.Info("Updated BackendTLSPolicy status and proxy")
-
 	return ctrl.Result{}, nil
 }
 
-func (r *BackendTLSPolicyReconciler) updateProxy() {
-	updateProxy(r.State, r.Proxy, r.ControllerName, r.OnGatewaysUpdate)
-}
-
 func (r *BackendTLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	if r.ControllerName == "" {
 		return fmt.Errorf("ControllerName is required")
 	}
-	return ctrl.NewControllerManagedBy(mgr).
-		For(&gatewayv1.BackendTLSPolicy{}).
-		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []ctrl.Request {
-			cm := obj.(*corev1.ConfigMap)
-			// Find all BackendTLSPolicies that reference this ConfigMap
-			var list gatewayv1.BackendTLSPolicyList
-			if err := mgr.GetClient().List(ctx, &list, client.InNamespace(cm.Namespace)); err != nil {
-				return nil
-			}
-			var requests []ctrl.Request
-			for _, policy := range list.Items {
-				for _, caRef := range policy.Spec.Validation.CACertificateRefs {
-					if string(caRef.Group) == "" && string(caRef.Kind) == "ConfigMap" && string(caRef.Name) == cm.Name {
-						requests = append(requests, ctrl.Request{
-							NamespacedName: types.NamespacedName{
-								Namespace: policy.Namespace,
-								Name:      policy.Name,
-							},
-						})
-						break
-					}
-				}
-			}
-			return requests
-		})).
-		Complete(r)
+	bldr := ctrl.NewControllerManagedBy(mgr).
+		For(&gatewayv1.BackendTLSPolicy{})
+
+	if r.State != nil {
+		bldr.WatchesRawSource(r.State.BackendTLSPolicySource())
+	}
+
+	return bldr.Complete(r)
 }