chore(quality): apply kubebuilder best practices quality gate

- Fix logging conventions across all controllers (capitalize, past tense)
- Move condition type constants to api/v1alpha1/conditions.go
- Replace r.Update() with r.Patch() for finalizer add/remove
- Default --leader-elect to true in operator
- Add AGENTS.md quality gate referencing kubebuilder good-practices guide

Author: syscod3

AGENTS.md

@@ -301,6 +301,15 @@ export IMG=<registry>/<project>:<version>
 make docker-build docker-push IMG=$IMG
 ```
 
+## Quality Gate
+
+**All work on this codebase must comply with the kubebuilder good practices guide:**
+https://book.kubebuilder.io/reference/good-practices
+
+Before marking any task complete, verify compliance with every applicable practice on that page. This is a hard requirement, not a suggestion.
+
+---
+
 ## References
 
 ### Essential Reading

api/v1alpha1/conditions.go

@@ -0,0 +1,13 @@
+package v1alpha1
+
+// ImpVM condition type constants.
+const (
+	ConditionScheduled   = "Scheduled"
+	ConditionReady       = "Ready"
+	ConditionNodeHealthy = "NodeHealthy"
+)
+
+// ImpNetwork condition type constants.
+const (
+	ConditionNetworkReady = "Ready"
+)

cmd/operator/main.go

@@ -123,7 +123,7 @@ func main() {
 
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metrics endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager.")
+	flag.BoolVar(&enableLeaderElection, "leader-elect", true, "Enable leader election for controller manager.")
 	flag.BoolVar(&enableWebhooks, "enable-webhooks", true, "Enable admission webhooks.")
 
 	opts := zap.Options{Development: true}

internal/controller/events.go

@@ -20,15 +20,3 @@ const (
 	EventReasonCiliumConfigMissing  = "CiliumConfigMissing"
 	EventReasonGroupCIDRError       = "GroupCIDRError"
 )
-
-// ImpVM condition type constants.
-const (
-	ConditionScheduled   = "Scheduled"
-	ConditionReady       = "Ready"
-	ConditionNodeHealthy = "NodeHealthy"
-)
-
-// ImpNetwork condition type constants.
-const (
-	ConditionNetworkReady = "Ready"
-)

internal/controller/impnetwork_controller.go

@@ -67,8 +67,9 @@ func (r *ImpNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	if !controllerutil.ContainsFinalizer(net, finalizerImpNetwork) {
+		patch := client.MergeFrom(net.DeepCopy())
 		controllerutil.AddFinalizer(net, finalizerImpNetwork)
-		return ctrl.Result{}, r.Update(ctx, net)
+		return ctrl.Result{}, r.Patch(ctx, net, patch)
 	}
 
 	return r.sync(ctx, net)
@@ -96,8 +97,9 @@ func (r *ImpNetworkReconciler) handleDeletion(ctx context.Context, net *impdevv1
 		}
 	}
 
+	patch := client.MergeFrom(net.DeepCopy())
 	controllerutil.RemoveFinalizer(net, finalizerImpNetwork)
-	return ctrl.Result{}, r.Update(ctx, net)
+	return ctrl.Result{}, r.Patch(ctx, net, patch)
 }
 
 // reconcileCiliumPool creates or updates the CiliumPodIPPool owned by this ImpNetwork.
@@ -170,7 +172,7 @@ func (r *ImpNetworkReconciler) sync(ctx context.Context, net *impdevv1alpha1.Imp
 			r.Recorder.Eventf(net, corev1.EventTypeWarning, EventReasonCiliumConfigMissing,
 				"Cilium ipMasqAgent not configured for subnet %s — see docs/networking/cilium.md",
 				net.Spec.Subnet)
-			log.Info("CiliumConfigMissing", "subnet", net.Spec.Subnet)
+			log.Info("Cilium ipMasqAgent config missing", "subnet", net.Spec.Subnet)
 		}
 	}
 
@@ -249,7 +251,7 @@ func (r *ImpNetworkReconciler) reconcileVTEPTable(ctx context.Context, net *impd
 		return nil // no change
 	}
 
-	logf.FromContext(ctx).Info("GCing stale VTEP entries",
+	logf.FromContext(ctx).Info("Removed stale VTEP entries",
 		"network", net.Name, "before", len(net.Status.VTEPTable), "after", len(filtered))
 
 	net.Status.VTEPTable = filtered
@@ -262,7 +264,7 @@ func (r *ImpNetworkReconciler) hasCiliumMasqConfig(ctx context.Context, subnet s
 	cm := &corev1.ConfigMap{}
 	if err := r.Get(ctx, client.ObjectKey{Namespace: "kube-system", Name: "ip-masq-agent"}, cm); err != nil {
 		if !apierrors.IsNotFound(err) {
-			logf.FromContext(ctx).V(1).Info("ip-masq-agent ConfigMap lookup failed", "err", err)
+			logf.FromContext(ctx).V(1).Info("IP masq agent ConfigMap lookup failed", "err", err)
 		}
 		return false
 	}
@@ -272,7 +274,7 @@ func (r *ImpNetworkReconciler) hasCiliumMasqConfig(ctx context.Context, subnet s
 // setNetworkReady sets the Ready condition to True on an ImpNetwork.
 func setNetworkReady(net *impdevv1alpha1.ImpNetwork) {
 	apimeta.SetStatusCondition(&net.Status.Conditions, metav1.Condition{
-		Type:               ConditionNetworkReady,
+		Type:               impdevv1alpha1.ConditionNetworkReady,
 		Status:             metav1.ConditionTrue,
 		Reason:             "Reconciled",
 		Message:            "ImpNetwork reconciled successfully",
@@ -296,7 +298,7 @@ func (r *ImpNetworkReconciler) ciliumPresent() bool {
 func (r *ImpNetworkReconciler) reconcileGroupCIDRs(ctx context.Context, net *impdevv1alpha1.ImpNetwork) error {
 	desired, err := carveGroupCIDRs(net.Spec.Subnet, net.Spec.Groups)
 	if err != nil {
-		logf.FromContext(ctx).Error(err, "group CIDR carving failed", "network", net.Name)
+		logf.FromContext(ctx).Error(err, "Group CIDR carving failed", "network", net.Name)
 		r.Recorder.Eventf(net, corev1.EventTypeWarning, EventReasonGroupCIDRError,
 			"Group CIDR carving failed: %v", err)
 		return nil // don't block reconcile for this — operator continues without group CIDRs
@@ -379,7 +381,7 @@ func (r *ImpNetworkReconciler) reconcileCiliumEnrollment(ctx context.Context, ne
 		cew := &cewList.Items[i]
 		vmKey := cew.GetLabels()["imp.dev/vm-key"] // namespace/name
 		if _, ok := runningVMs[vmKey]; !ok {
-			log.Info("deleting stale CiliumExternalWorkload", "cew", cew.GetName(), "vmKey", vmKey)
+			log.Info("Deleted stale CiliumExternalWorkload", "cew", cew.GetName(), "vmKey", vmKey)
 			if err := r.Delete(ctx, cew); err != nil && !apierrors.IsNotFound(err) {
 				return err
 			}
@@ -411,11 +413,11 @@ func (r *ImpNetworkReconciler) reconcileCiliumEnrollment(ctx context.Context, ne
 		})
 		if vm.Status.IP != "" {
 			if err := unstructured.SetNestedField(cew.Object, vm.Status.IP+"/32", "spec", "ipv4AllocCIDR"); err != nil {
-				log.Error(err, "failed to set ipv4AllocCIDR", "vm", vm.Name)
+				log.Error(err, "Failed to set IPv4 alloc CIDR", "vm", vm.Name)
 			}
 		}
 
-		log.Info("creating CiliumExternalWorkload", "cew", cewName, "vm", vm.Name)
+		log.Info("Created CiliumExternalWorkload", "cew", cewName, "vm", vm.Name)
 		if err := r.Create(ctx, cew); err != nil && !apierrors.IsAlreadyExists(err) {
 			return err
 		}

internal/controller/impnetwork_controller_test.go

@@ -82,7 +82,7 @@ var _ = Describe("ImpNetwork Controller: core", func() {
 
 		updated := &impdevv1alpha1.ImpNetwork{}
 		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "core-net-2", Namespace: "default"}, updated)).To(Succeed())
-		c := apimeta.FindStatusCondition(updated.Status.Conditions, ConditionNetworkReady)
+		c := apimeta.FindStatusCondition(updated.Status.Conditions, impdevv1alpha1.ConditionNetworkReady)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionTrue))
 	})

internal/controller/impvm_conditions.go

@@ -20,32 +20,32 @@ func setCondition(vm *impdevv1alpha1.ImpVM, condType string, status metav1.Condi
 }
 
 func setScheduled(vm *impdevv1alpha1.ImpVM, nodeName string) {
-	setCondition(vm, ConditionScheduled, metav1.ConditionTrue, "NodeAssigned",
+	setCondition(vm, impdevv1alpha1.ConditionScheduled, metav1.ConditionTrue, "NodeAssigned",
 		"VM scheduled to node "+nodeName)
 }
 
 func setUnscheduled(vm *impdevv1alpha1.ImpVM) {
-	setCondition(vm, ConditionScheduled, metav1.ConditionFalse, "NoNodeAvailable",
+	setCondition(vm, impdevv1alpha1.ConditionScheduled, metav1.ConditionFalse, "NoNodeAvailable",
 		"No eligible node with available capacity")
 }
 
 func setNodeHealthy(vm *impdevv1alpha1.ImpVM) {
-	setCondition(vm, ConditionNodeHealthy, metav1.ConditionTrue, "NodeReady", "Assigned node is Ready")
+	setCondition(vm, impdevv1alpha1.ConditionNodeHealthy, metav1.ConditionTrue, "NodeReady", "Assigned node is Ready")
 }
 
 func setNodeUnhealthy(vm *impdevv1alpha1.ImpVM, reason string) {
-	setCondition(vm, ConditionNodeHealthy, metav1.ConditionFalse, "NodeNotReady", reason)
+	setCondition(vm, impdevv1alpha1.ConditionNodeHealthy, metav1.ConditionFalse, "NodeNotReady", reason)
 }
 
 func setReadyFromPhase(vm *impdevv1alpha1.ImpVM) {
 	switch vm.Status.Phase {
 	case impdevv1alpha1.VMPhaseRunning:
-		setCondition(vm, ConditionReady, metav1.ConditionTrue, "Running", "VM is running")
+		setCondition(vm, impdevv1alpha1.ConditionReady, metav1.ConditionTrue, "Running", "VM is running")
 	case impdevv1alpha1.VMPhaseSucceeded:
-		setCondition(vm, ConditionReady, metav1.ConditionFalse, "Completed", "VM completed successfully")
+		setCondition(vm, impdevv1alpha1.ConditionReady, metav1.ConditionFalse, "Completed", "VM completed successfully")
 	case impdevv1alpha1.VMPhaseFailed:
-		setCondition(vm, ConditionReady, metav1.ConditionFalse, "Failed", "VM failed")
+		setCondition(vm, impdevv1alpha1.ConditionReady, metav1.ConditionFalse, "Failed", "VM failed")
 	default:
-		setCondition(vm, ConditionReady, metav1.ConditionUnknown, "Waiting", "Waiting for VM to start")
+		setCondition(vm, impdevv1alpha1.ConditionReady, metav1.ConditionUnknown, "Waiting", "Waiting for VM to start")
 	}
 }

internal/controller/impvm_controller.go

@@ -85,8 +85,9 @@ func (r *ImpVMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resu
 
 	// 2. Ensure finalizer
 	if !controllerutil.ContainsFinalizer(vm, finalizerImp) {
+		patch := client.MergeFrom(vm.DeepCopy())
 		controllerutil.AddFinalizer(vm, finalizerImp)
-		return ctrl.Result{}, r.Update(ctx, vm)
+		return ctrl.Result{}, r.Patch(ctx, vm, patch)
 	}
 
 	// 3. Handle manual retry reset annotation
@@ -102,8 +103,8 @@ func (r *ImpVMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resu
 		if !alreadyMarked {
 			base := vm.DeepCopy()
 			vm.Status.Phase = impdevv1alpha1.VMPhaseFailed
-			setCondition(vm, ConditionScheduled, metav1.ConditionFalse, EventReasonSpecInvalid, msg)
-			setCondition(vm, ConditionReady, metav1.ConditionFalse, EventReasonSpecInvalid, msg)
+			setCondition(vm, impdevv1alpha1.ConditionScheduled, metav1.ConditionFalse, EventReasonSpecInvalid, msg)
+			setCondition(vm, impdevv1alpha1.ConditionReady, metav1.ConditionFalse, EventReasonSpecInvalid, msg)
 			if err := r.Status().Patch(ctx, vm, client.MergeFrom(base)); err != nil {
 				return ctrl.Result{}, err
 			}
@@ -119,7 +120,7 @@ func (r *ImpVMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resu
 			return ctrl.Result{}, err
 		}
 		if nodeName == "" {
-			log.Info("no node available", "vm", vm.Name)
+			log.Info("No node available", "vm", vm.Name)
 			vmCopy := vm.DeepCopy()
 			vm.Status.Phase = impdevv1alpha1.VMPhasePending
 			setUnscheduled(vm)
@@ -240,7 +241,7 @@ func (r *ImpVMReconciler) syncStatus(ctx context.Context, vm *impdevv1alpha1.Imp
 		if err == nil {
 			reason = "node is not Ready"
 		}
-		log.Info("assigned node unhealthy", "node", vm.Spec.NodeName, "reason", reason)
+		log.Info("Assigned node unhealthy", "node", vm.Spec.NodeName, "reason", reason)
 
 		if vm.Spec.Lifecycle == impdevv1alpha1.VMLifecycleEphemeral {
 			// Clear assignment — spec patch first.
@@ -345,17 +346,19 @@ func (r *ImpVMReconciler) handleDeletion(ctx context.Context, vm *impdevv1alpha1
 
 	// Agent already cleaned up (cleared spec.nodeName + set Succeeded)
 	if vm.Spec.NodeName == "" {
+		patch := client.MergeFrom(vm.DeepCopy())
 		controllerutil.RemoveFinalizer(vm, finalizerImp)
-		return ctrl.Result{}, r.Update(ctx, vm)
+		return ctrl.Result{}, r.Patch(ctx, vm, patch)
 	}
 
 	// Check for termination timeout
 	deadline := vm.DeletionTimestamp.Add(terminationTimeout)
 	if time.Now().After(deadline) {
 		r.Recorder.Event(vm, corev1.EventTypeWarning, EventReasonTerminationTimeout,
 			"Finalizer force-removed after 2min termination timeout")
+		patch := client.MergeFrom(vm.DeepCopy())
 		controllerutil.RemoveFinalizer(vm, finalizerImp)
-		return ctrl.Result{}, r.Update(ctx, vm)
+		return ctrl.Result{}, r.Patch(ctx, vm, patch)
 	}
 
 	// Signal agent by setting phase=Terminating

internal/controller/impvm_controller_test.go

@@ -265,7 +265,7 @@ var _ = Describe("ImpVM Spec Validation Fallback", func() {
 		var readyCond *metav1.Condition
 		for i := range updated.Status.Conditions {
 			c := &updated.Status.Conditions[i]
-			if c.Type == ConditionReady {
+			if c.Type == impdevv1alpha1.ConditionReady {
 				readyCond = c
 				break
 			}

internal/controller/impvm_coverage_test.go

@@ -68,7 +68,7 @@ var _ = Describe("setScheduled / setUnscheduled", func() {
 	It("sets Scheduled condition to True with correct reason", func() {
 		vm := &impdevv1alpha1.ImpVM{}
 		setScheduled(vm, "node-1")
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionScheduled)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionScheduled)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionTrue))
 		Expect(c.Reason).To(Equal("NodeAssigned"))
@@ -77,7 +77,7 @@ var _ = Describe("setScheduled / setUnscheduled", func() {
 	It("sets Scheduled condition to False via setUnscheduled", func() {
 		vm := &impdevv1alpha1.ImpVM{}
 		setUnscheduled(vm)
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionScheduled)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionScheduled)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionFalse))
 	})
@@ -87,15 +87,15 @@ var _ = Describe("setNodeHealthy / setNodeUnhealthy", func() {
 	It("sets NodeHealthy=True", func() {
 		vm := &impdevv1alpha1.ImpVM{}
 		setNodeHealthy(vm)
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionNodeHealthy)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionNodeHealthy)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionTrue))
 	})
 
 	It("sets NodeHealthy=False with reason", func() {
 		vm := &impdevv1alpha1.ImpVM{}
 		setNodeUnhealthy(vm, "node not found")
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionNodeHealthy)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionNodeHealthy)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionFalse))
 		Expect(c.Message).To(Equal("node not found"))
@@ -108,7 +108,7 @@ var _ = Describe("setReadyFromPhase", func() {
 			vm := &impdevv1alpha1.ImpVM{}
 			vm.Status.Phase = phase
 			setReadyFromPhase(vm)
-			c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionReady)
+			c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionReady)
 			Expect(c).NotTo(BeNil())
 			Expect(c.Status).To(Equal(expectedStatus))
 		},
@@ -122,7 +122,7 @@ var _ = Describe("setReadyFromPhase", func() {
 		vm := &impdevv1alpha1.ImpVM{}
 		vm.Status.Phase = impdevv1alpha1.VMPhaseSucceeded
 		setReadyFromPhase(vm)
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionReady)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionReady)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Reason).To(Equal("Completed"))
 		Expect(c.Message).To(Equal("VM completed successfully"))
@@ -291,7 +291,7 @@ var _ = Describe("applyHTTPCheck", func() {
 		changed := r.applyHTTPCheck(ctx, vm, spec)
 		Expect(changed).To(BeTrue())
 		Expect(vm.Annotations["imp/httpcheck-failures"]).To(Equal("0"))
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionReady)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionReady)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionTrue))
 	})
@@ -311,7 +311,7 @@ var _ = Describe("applyHTTPCheck", func() {
 		r.applyHTTPCheck(ctx, vm, spec)
 		Expect(vm.Annotations["imp/httpcheck-failures"]).To(Equal("1"))
 		// Not yet at threshold — Ready should not be False.
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionReady)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionReady)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).NotTo(Equal(metav1.ConditionFalse))
 	})
@@ -330,7 +330,7 @@ var _ = Describe("applyHTTPCheck", func() {
 
 		r.applyHTTPCheck(ctx, vm, spec)
 		Expect(vm.Annotations["imp/httpcheck-failures"]).To(Equal("1"))
-		c := apimeta.FindStatusCondition(vm.Status.Conditions, ConditionReady)
+		c := apimeta.FindStatusCondition(vm.Status.Conditions, impdevv1alpha1.ConditionReady)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionFalse))
 		Expect(c.Reason).To(Equal("HealthCheckFailed"))
@@ -887,7 +887,7 @@ var _ = Describe("ImpVM syncStatus: node healthy", func() {
 
 		updated := &impdevv1alpha1.ImpVM{}
 		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "sync-healthy-vm", Namespace: "default"}, updated)).To(Succeed())
-		c := apimeta.FindStatusCondition(updated.Status.Conditions, ConditionNodeHealthy)
+		c := apimeta.FindStatusCondition(updated.Status.Conditions, impdevv1alpha1.ConditionNodeHealthy)
 		Expect(c).NotTo(BeNil())
 		Expect(c.Status).To(Equal(metav1.ConditionTrue))
 	})