Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion controller/internal/controller/lease_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"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
Expand Down Expand Up @@ -585,9 +588,37 @@ 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
}

// skipEndedPredicate returns a predicate that filters out leases carrying the
// ended label. Leases without the label are admitted so the reconciler can
// backfill it when Status.Ended is true but the label write was lost.
func skipEndedPredicate() predicate.Funcs {
Comment thread
bkhizgiy marked this conversation as resolved.
return 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)
},
}
}

// SetupWithManager sets up the controller with the Manager.
func (r *LeaseReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&jumpstarterdevv1alpha1.Lease{}).
For(&jumpstarterdevv1alpha1.Lease{}, builder.WithPredicates(skipEndedPredicate())).
Complete(r)
}
153 changes: 153 additions & 0 deletions controller/internal/controller/lease_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

Expand Down Expand Up @@ -1027,6 +1029,98 @@ var _ = Describe("orderApprovedExporters", func() {
})
})

var _ = Describe("isLeaseEnded", func() {
It("should return true when the ended label is present", func() {
lease := &jumpstarterdevv1alpha1.Lease{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
string(jumpstarterdevv1alpha1.LeaseLabelEnded): jumpstarterdevv1alpha1.LeaseLabelEndedValue,
},
},
}
Expect(isLeaseEnded(lease)).To(BeTrue())
})

It("should return false when the ended label is missing", func() {
lease := &jumpstarterdevv1alpha1.Lease{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{},
},
}
Expect(isLeaseEnded(lease)).To(BeFalse())
})

It("should return false when labels are nil", func() {
lease := &jumpstarterdevv1alpha1.Lease{}
Expect(isLeaseEnded(lease)).To(BeFalse())
})
})

var _ = Describe("skipEndedPredicate", func() {
var skipEnded predicate.Funcs

BeforeEach(func() {
skipEnded = skipEndedPredicate()
})

It("should admit creates for non-ended leases", func() {
lease := &jumpstarterdevv1alpha1.Lease{}
Expect(skipEnded.Create(event.CreateEvent{Object: lease})).To(BeTrue())
})

It("should reject creates for ended leases", func() {
lease := &jumpstarterdevv1alpha1.Lease{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
string(jumpstarterdevv1alpha1.LeaseLabelEnded): jumpstarterdevv1alpha1.LeaseLabelEndedValue,
},
},
}
Expect(skipEnded.Create(event.CreateEvent{Object: lease})).To(BeFalse())
})

It("should reject updates where new object has ended label", func() {
oldLease := &jumpstarterdevv1alpha1.Lease{}
newLease := &jumpstarterdevv1alpha1.Lease{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
string(jumpstarterdevv1alpha1.LeaseLabelEnded): jumpstarterdevv1alpha1.LeaseLabelEndedValue,
},
},
}
Expect(skipEnded.Update(event.UpdateEvent{ObjectOld: oldLease, ObjectNew: newLease})).To(BeFalse())
})

It("should admit updates for non-ended leases", func() {
oldLease := &jumpstarterdevv1alpha1.Lease{}
newLease := &jumpstarterdevv1alpha1.Lease{}
Expect(skipEnded.Update(event.UpdateEvent{ObjectOld: oldLease, ObjectNew: newLease})).To(BeTrue())
})

It("should admit updates for ended-in-status but unlabeled leases so label can be backfilled", func() {
oldLease := &jumpstarterdevv1alpha1.Lease{}
newLease := &jumpstarterdevv1alpha1.Lease{
Status: jumpstarterdevv1alpha1.LeaseStatus{
Ended: true,
},
}
// No ended label → predicate admits → reconciler runs → backfills label
Expect(skipEnded.Update(event.UpdateEvent{ObjectOld: oldLease, ObjectNew: newLease})).To(BeTrue())
})

It("should always admit deletes", func() {
lease := &jumpstarterdevv1alpha1.Lease{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
string(jumpstarterdevv1alpha1.LeaseLabelEnded): jumpstarterdevv1alpha1.LeaseLabelEndedValue,
},
},
}
Expect(skipEnded.Delete(event.DeleteEvent{Object: lease})).To(BeTrue())
})

})

var _ = Describe("Scheduled Leases", func() {
BeforeEach(func() {
createExporters(context.Background(), testExporter1DutA, testExporter2DutA, testExporter3DutB)
Expand Down Expand Up @@ -1709,6 +1803,65 @@ var _ = Describe("Scheduled Leases", func() {
})
})

When("an ended lease has the ended label", func() {
It("should be a no-op on subsequent reconciles", 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
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
Expand Down
Loading