Skip to content

Commit

Permalink
Refactor the rest of lease reconciler
Browse files Browse the repository at this point in the history
  • Loading branch information
@NickCao
NickCao committed Oct 17, 2024
1 parent 550ed6a commit 3ac241b
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 120 deletions.
208 changes: 93 additions & 115 deletions internal/controller/lease_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,117 +53,136 @@ type LeaseReconciler struct {
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
func (r *LeaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := log.FromContext(ctx)
logger := log.FromContext(ctx)

var lease jumpstarterdevv1alpha1.Lease
if err := r.Get(ctx, req.NamespacedName, &lease); err != nil {
log.Error(err, "unable to fetch Lease")
logger.Error(err, "Reconcile: unable to get lease", "lease", req.NamespacedName)
return ctrl.Result{}, client.IgnoreNotFound(err)
}

// lease already ended, ignoring
if lease.Status.Ended {
return ctrl.Result{}, nil
var result ctrl.Result
if err := r.reconcileStatusExporterRef(ctx, &result, &lease); err != nil {
return result, err
}

// force release lease
if lease.Spec.Release {
log.Info("lease released early", "lease", lease.Name)
now := time.Now()
if err := r.reconcileStatusBeginTime(ctx, &lease); err != nil {
return result, err
}

if lease.Labels == nil {
lease.Labels = make(map[string]string)
}
lease.Labels[string(jumpstarterdevv1alpha1.LeaseLabelEnded)] = jumpstarterdevv1alpha1.LeaseLabelEndedValue
if err := r.reconcileStatusEnded(ctx, &result, &lease); err != nil {
return result, err
}

if err := r.Update(ctx, &lease); err != nil {
log.Error(err, "unable to update Lease labels")
return ctrl.Result{}, err
}
if err := r.Status().Update(ctx, &lease); err != nil {
return result, err
}

lease.Status.Ended = true
lease.Status.EndTime = &metav1.Time{
Time: now,
}
meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{
Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady),
Status: metav1.ConditionFalse,
ObservedGeneration: lease.Generation,
LastTransitionTime: metav1.Time{
Time: now,
},
Reason: "Released",
})
if lease.Labels == nil {
lease.Labels = make(map[string]string)
}
if lease.Status.Ended {
lease.Labels[string(jumpstarterdevv1alpha1.LeaseLabelEnded)] = jumpstarterdevv1alpha1.LeaseLabelEndedValue
}

if err := r.Status().Update(ctx, &lease); err != nil {
log.Error(err, "unable to update Lease status")
return ctrl.Result{}, err
if lease.Status.ExporterRef != nil {
var exporter jumpstarterdevv1alpha1.Exporter
if err := r.Get(ctx, types.NamespacedName{
Namespace: lease.Namespace,
Name: lease.Status.ExporterRef.Name,
}, &exporter); err != nil {
return result, err
}
if err := controllerutil.SetControllerReference(&exporter, &lease, r.Scheme); err != nil {
logger.Error(err, "Reconcile: failed to update lease controller reference", "lease", lease)
return result, err
}
}

return ctrl.Result{}, nil
if err := r.Update(ctx, &lease); err != nil {
logger.Error(err, "Reconcile: failed to update lease metadata", "lease", lease)
return result, err
}

// 1. newly created lease
if lease.Status.BeginTime == nil || lease.Status.EndTime == nil || lease.Status.ExporterRef == nil {
return r.ReconcileNewLease(ctx, lease)
} else {
// 2. expired lease
if time.Now().After(lease.Status.EndTime.Time) {
log.Info("lease expired", "lease", lease.Name)
if lease.Labels == nil {
lease.Labels = make(map[string]string)
}
lease.Labels[string(jumpstarterdevv1alpha1.LeaseLabelEnded)] = jumpstarterdevv1alpha1.LeaseLabelEndedValue
return result, nil
}

if err := r.Update(ctx, &lease); err != nil {
log.Error(err, "unable to update Lease labels")
return ctrl.Result{}, err
}
// also manages EndTime and LeaseConditionTypeReady
// nolint:unparam
func (r *LeaseReconciler) reconcileStatusEnded(
ctx context.Context,
result *ctrl.Result,
lease *jumpstarterdevv1alpha1.Lease,
) error {
logger := log.FromContext(ctx)

lease.Status.Ended = true
now := time.Now()
if !lease.Status.Ended {
if lease.Spec.Release {
logger.Info("reconcileStatusEndTime: force releasing lease", "lease", lease)
meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{
Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady),
Status: metav1.ConditionFalse,
ObservedGeneration: lease.Generation,
LastTransitionTime: metav1.Time{
Time: time.Now(),
Time: now,
},
Reason: "Expired",
Reason: "Released",
})

if err := r.Status().Update(ctx, &lease); err != nil {
log.Error(err, "unable to update Lease status")
return ctrl.Result{}, err
lease.Status.Ended = true
lease.Status.EndTime = &metav1.Time{
Time: now,
}
return nil
} else if lease.Status.BeginTime != nil {
expiration := lease.Status.BeginTime.Add(lease.Spec.Duration.Duration)
if expiration.Before(now) {
logger.Info("reconcileStatusEndTime: lease expired", "lease", lease)
meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{
Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady),
Status: metav1.ConditionFalse,
ObservedGeneration: lease.Generation,
LastTransitionTime: metav1.Time{
Time: time.Now(),
},
Reason: "Expired",
})
lease.Status.Ended = true
lease.Status.EndTime = &metav1.Time{
Time: now,
}
return nil
} else {
result.RequeueAfter = expiration.Sub(now)
return nil
}

return ctrl.Result{}, nil
} else {
// 3. acquired lease
// Requeue acquire lease on EndTime
return ctrl.Result{
RequeueAfter: time.Until(lease.Status.EndTime.Time),
}, nil
}
}

return nil
}

func (r *LeaseReconciler) reconcileStatusBeginEndTime(
// nolint:unparam
func (r *LeaseReconciler) reconcileStatusBeginTime(
ctx context.Context,
lease *jumpstarterdevv1alpha1.Lease,
) error {
logger := log.FromContext(ctx)

now := time.Now()
if lease.Status.BeginTime == nil && lease.Status.ExporterRef != nil {
logger.Info("reconcileStatusBeginEndTime: updating begin time", "lease", lease)
logger.Info("reconcileStatusBeginTime: updating begin time", "lease", lease)
meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{
Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady),
Status: metav1.ConditionTrue,
ObservedGeneration: lease.Generation,
LastTransitionTime: metav1.Time{
Time: now,
},
Reason: "Ready",
})
lease.Status.BeginTime = &metav1.Time{
Time: time.Now(),
}
}

if lease.Status.EndTime == nil && lease.Status.BeginTime != nil {
logger.Info("reconcileStatusBeginEndTime: updating end time", "lease", lease)
lease.Status.EndTime = &metav1.Time{
Time: lease.Status.BeginTime.Add(lease.Spec.Duration.Duration),
Time: now,
}
}

Expand Down Expand Up @@ -273,47 +292,6 @@ func (r *LeaseReconciler) reconcileStatusExporterRef(
return nil
}

func (r *LeaseReconciler) ReconcileNewLease(
ctx context.Context,
lease jumpstarterdevv1alpha1.Lease,
) (ctrl.Result, error) {
logger := log.FromContext(ctx)

var result ctrl.Result
if err := r.reconcileStatusExporterRef(ctx, &result, &lease); err != nil {
return result, err
}

if err := r.reconcileStatusBeginEndTime(ctx, &lease); err != nil {
return result, err
}

if err := r.Status().Update(ctx, &lease); err != nil {
logger.Error(err, "unable to update Lease status")
return result, err
}

if lease.Status.ExporterRef != nil {
var exporter jumpstarterdevv1alpha1.Exporter
if err := r.Get(ctx, types.NamespacedName{
Namespace: lease.Namespace,
Name: lease.Status.ExporterRef.Name,
}, &exporter); err != nil {
return result, err
}
if err := controllerutil.SetControllerReference(&exporter, &lease, r.Scheme); err != nil {
logger.Error(err, "unable to update Lease owner reference")
return result, err
}
if err := r.Update(ctx, &lease); err != nil {
logger.Error(err, "unable to update Lease status")
return result, err
}
}

return result, nil
}

// SetupWithManager sets up the controller with the Manager.
func (r *LeaseReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down
5 changes: 0 additions & 5 deletions internal/controller/lease_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ var _ = Describe("Lease Controller", func() {
Expect(updatedLease.Status.ExporterRef).NotTo(BeNil())
Expect(updatedLease.Status.ExporterRef.Name).To(BeElementOf([]string{testExporter1DutA.Name, testExporter2DutA.Name}))
Expect(updatedLease.Status.BeginTime).NotTo(BeNil())
Expect(updatedLease.Status.EndTime).NotTo(BeNil())

updatedExporter := getExporter(ctx, updatedLease.Status.ExporterRef.Name)
Expect(updatedExporter.Status.LeaseRef).NotTo(BeNil())
Expand Down Expand Up @@ -167,7 +166,6 @@ var _ = Describe("Lease Controller", func() {
Expect(updatedLease.Status.ExporterRef).NotTo(BeNil())
Expect(updatedLease.Status.ExporterRef.Name).To(BeElementOf([]string{testExporter2DutA.Name}))
Expect(updatedLease.Status.BeginTime).NotTo(BeNil())
Expect(updatedLease.Status.EndTime).NotTo(BeNil())

updatedExporter := getExporter(ctx, updatedLease.Status.ExporterRef.Name)
Expect(updatedExporter.Status.LeaseRef).NotTo(BeNil())
Expand Down Expand Up @@ -265,16 +263,13 @@ var _ = Describe("Lease Controller", func() {
// we should consider adding a flag on the spec to do this, or look at the duration too
updatedLease.Spec.Release = true

endTime := updatedLease.Status.EndTime

Expect(k8sClient.Update(ctx, updatedLease)).To(Succeed())

_ = reconcileLease(ctx, updatedLease)

updatedLease = getLease(ctx, lease.Name)
Expect(updatedLease.Status.ExporterRef).NotTo(BeNil())
Expect(updatedLease.Status.Ended).To(BeTrue())
Expect(updatedLease.Status.EndTime).ToNot(Equal(endTime))

updatedExporter := getExporter(ctx, exporterName)
Expect(updatedExporter.Status.LeaseRef).To(BeNil())
Expand Down

0 comments on commit 3ac241b

Please sign in to comment.