fix: pre-NodeUpdate cleanup — metrics, deletion comment, drift-gated plans (#93)

Three cleanup items identified during the comprehensive architecture review:

1. Fix controllerName() in planner metrics — GVK is stripped by
   controller-runtime on fetched objects, so all metric labels were
   "unknown". Now uses a type switch on the known resource types.

2. Document the deletion path's separate Status().Patch() call — it's an
   intentional exception to the single-patch model, not an oversight.

3. Running-phase plans are now drift-gated — buildRunningPlan returns nil
   when spec.image == status.currentImage (no drift). This eliminates
   unnecessary plan creation/flush/execution every 30s for idle Running
   nodes, and establishes the extension point for NodeUpdate plans.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: bdchatham

internal/controller/node/controller.go

@@ -216,6 +216,8 @@ func (r *SeiNodeReconciler) handleNodeDeletion(ctx context.Context, node *seiv1a
 		return ctrl.Result{}, nil
 	}
 
+	// Deletion path: separate patch is intentional — this runs before the
+	// main reconcile flow and must set Terminating before cleaning up resources.
 	patch := client.MergeFromWithOptions(node.DeepCopy(), client.MergeFromWithOptimisticLock{})
 	node.Status.Phase = seiv1alpha1.PhaseTerminating
 	if err := r.Status().Patch(ctx, node, patch); err != nil {

internal/planner/metrics.go

@@ -1,11 +1,11 @@
 package planner
 
 import (
-	"strings"
-
 	"github.com/prometheus/client_golang/prometheus"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
+
+	seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1"
 )
 
 var (
@@ -42,13 +42,17 @@ func init() {
 	)
 }
 
-// controllerName derives a lowercase controller label from the object's GVK Kind.
+// controllerName returns the controller label for metrics. Uses a type switch
+// because controller-runtime strips GVK from objects fetched via client.Get.
 func controllerName(obj client.Object) string {
-	kind := obj.GetObjectKind().GroupVersionKind().Kind
-	if kind == "" {
+	switch obj.(type) {
+	case *seiv1alpha1.SeiNode:
+		return "seinode"
+	case *seiv1alpha1.SeiNodeDeployment:
+		return "seinodedeployment"
+	default:
 		return "unknown"
 	}
-	return strings.ToLower(kind)
 }
 
 // CleanupPlanMetrics removes the plan_active gauge for a deleted resource.

internal/planner/planner.go

@@ -443,14 +443,20 @@ func commonOverrides(node *seiv1alpha1.SeiNode) map[string]string {
 	}
 }
 
-// buildRunningPlan builds a convergence plan for a Running node.
-// It ensures the StatefulSet and Service match the current spec.
+// buildRunningPlan returns a plan for a Running node only when the controller
+// recognizes a scenario that requires action. Returns nil when the node is
+// in steady state (no drift detected).
 //
-// FailedPhase is deliberately empty: a convergence failure should not
-// transition the node out of Running. The executor still sets a PlanFailed
-// condition for observability, and the next reconcile will build a fresh
-// convergence plan to retry.
+// Currently detects image drift (spec.image != status.currentImage). This is
+// the extension point for future drift types (config changes, peer changes).
 func buildRunningPlan(node *seiv1alpha1.SeiNode) (*seiv1alpha1.TaskPlan, error) {
+	if node.Spec.Image == node.Status.CurrentImage {
+		return nil, nil
+	}
+
+	// Image drift detected — build a convergence plan to update the StatefulSet.
+	// FailedPhase is deliberately empty: a failure retries on the next reconcile
+	// rather than transitioning the node out of Running.
 	prog := []string{task.TaskTypeApplyStatefulSet, task.TaskTypeApplyService}
 
 	planID := uuid.New().String()