fix: avoid reconciling ended leases

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
Assited-by: claude-opus-4.6

Author: bennyz

controller/internal/controller/lease_controller.go

@@ -34,7 +34,9 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
 // LeaseReconciler reconciles a Lease object
@@ -77,6 +79,10 @@ func (r *LeaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		)
 	}
 
+	if lease.Status.Ended && isLeaseEnded(&lease) {
+		return ctrl.Result{}, nil
+	}
+
 	var result ctrl.Result
 	if err := r.reconcileStatusExporterRef(ctx, &result, &lease); err != nil {
 		return result, err
@@ -585,9 +591,36 @@ func filterOutOfflineExporters(approvedExporters []ApprovedExporter) []ApprovedE
 	return onlineExporters
 }
 
+// isLeaseEnded checks whether a lease object carries the ended label.
+func isLeaseEnded(obj client.Object) bool {
+	if v, ok := obj.GetLabels()[string(jumpstarterdevv1alpha1.LeaseLabelEnded)]; ok {
+		return v == jumpstarterdevv1alpha1.LeaseLabelEndedValue
+	}
+	return false
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *LeaseReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	// Skip ended leases: they are immutable and never need further reconciliation.
+	// The Update funnel still admits the transition into ended so the label
+	// and status update written by the reconciler itself is not swallowed.
+	skipEnded := predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			return !isLeaseEnded(e.Object)
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return !isLeaseEnded(e.ObjectNew)
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			return true
+		},
+		GenericFunc: func(e event.GenericEvent) bool {
+			return !isLeaseEnded(e.Object)
+		},
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&jumpstarterdevv1alpha1.Lease{}).
+		WithEventFilter(skipEnded).
 		Complete(r)
 }

controller/internal/controller/lease_controller_test.go

@@ -1709,6 +1709,65 @@ var _ = Describe("Scheduled Leases", func() {
 		})
 	})
 
+	When("an ended lease has the ended label", func() {
+		It("should short-circuit without running reconciliation logic", func() {
+			lease := leaseDutA2Sec.DeepCopy()
+			lease.Spec.Duration = &metav1.Duration{Duration: 100 * time.Millisecond}
+
+			ctx := context.Background()
+			Expect(k8sClient.Create(ctx, lease)).To(Succeed())
+			_ = reconcileLease(ctx, lease)
+
+			Eventually(func() bool {
+				_ = reconcileLease(ctx, lease)
+				return getLease(ctx, lease.Name).Status.Ended
+			}).WithTimeout(500 * time.Millisecond).WithPolling(50 * time.Millisecond).Should(BeTrue())
+
+			updatedLease := getLease(ctx, lease.Name)
+			Expect(updatedLease.Labels).To(HaveKeyWithValue(
+				string(jumpstarterdevv1alpha1.LeaseLabelEnded),
+				jumpstarterdevv1alpha1.LeaseLabelEndedValue,
+			))
+
+			// Reconciling again should be a no-op (short-circuit)
+			result := reconcileLease(ctx, lease)
+			Expect(result.RequeueAfter).To(BeZero())
+		})
+	})
+
+	When("an ended lease is missing the ended label", func() {
+		It("should backfill the label on the next reconcile", func() {
+			lease := leaseDutA2Sec.DeepCopy()
+			lease.Spec.Duration = &metav1.Duration{Duration: 100 * time.Millisecond}
+
+			ctx := context.Background()
+			Expect(k8sClient.Create(ctx, lease)).To(Succeed())
+			_ = reconcileLease(ctx, lease)
+
+			Eventually(func() bool {
+				_ = reconcileLease(ctx, lease)
+				return getLease(ctx, lease.Name).Status.Ended
+			}).WithTimeout(500 * time.Millisecond).WithPolling(50 * time.Millisecond).Should(BeTrue())
+
+			// Simulate split-brain: remove the ended label as if metadata update failed
+			updatedLease := getLease(ctx, lease.Name)
+			delete(updatedLease.Labels, string(jumpstarterdevv1alpha1.LeaseLabelEnded))
+			Expect(k8sClient.Update(ctx, updatedLease)).To(Succeed())
+
+			// Verify label is gone
+			updatedLease = getLease(ctx, lease.Name)
+			Expect(updatedLease.Labels).NotTo(HaveKey(string(jumpstarterdevv1alpha1.LeaseLabelEnded)))
+
+			// Reconcile should backfill the label
+			_ = reconcileLease(ctx, lease)
+			updatedLease = getLease(ctx, lease.Name)
+			Expect(updatedLease.Labels).To(HaveKeyWithValue(
+				string(jumpstarterdevv1alpha1.LeaseLabelEnded),
+				jumpstarterdevv1alpha1.LeaseLabelEndedValue,
+			))
+		})
+	})
+
 	// UpdateLease mutation tests
 	// Note: These tests simulate what UpdateLease does via gRPC by directly
 	// modifying the lease spec and calling ReconcileLeaseTimeFields