diff --git a/internal/controller/lease_controller.go b/internal/controller/lease_controller.go index 4a8b738..aba7b85 100644 --- a/internal/controller/lease_controller.go +++ b/internal/controller/lease_controller.go @@ -23,9 +23,11 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -52,276 +54,245 @@ type LeaseReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.18.4/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") + if !apierrors.IsNotFound(err) { + 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 } - // 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 + if err := r.Update(ctx, &lease); err != nil { + logger.Error(err, "Reconcile: failed to update lease metadata", "lease", lease) + return result, err + } - if err := r.Update(ctx, &lease); err != nil { - log.Error(err, "unable to update Lease labels") - return ctrl.Result{}, err - } + return result, nil +} - lease.Status.Ended = true +// 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) + + 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) ReconcileNewLease( +// nolint:unparam +func (r *LeaseReconciler) reconcileStatusBeginTime( ctx context.Context, - lease jumpstarterdevv1alpha1.Lease, -) (ctrl.Result, error) { - log := log.FromContext(ctx) - - selector, err := metav1.LabelSelectorAsSelector(&lease.Spec.Selector) - if err != nil { - log.Error(err, "Error creating selector for label selector") - return ctrl.Result{}, err - } - - // List all Exporter matching selector - var matchedExporters jumpstarterdevv1alpha1.ExporterList - if err := r.List( - ctx, - &matchedExporters, - client.InNamespace(lease.Namespace), - client.MatchingLabelsSelector{Selector: selector}, - ); err != nil { - log.Error(err, "Error listing exporters") - return ctrl.Result{}, err - } - - onlineExporters := slices.DeleteFunc( - matchedExporters.Items, - func(exporter jumpstarterdevv1alpha1.Exporter) bool { - return !(true && - meta.IsStatusConditionTrue( - exporter.Status.Conditions, - string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered), - ) && - meta.IsStatusConditionTrue( - exporter.Status.Conditions, - string(jumpstarterdevv1alpha1.ExporterConditionTypeOnline), - )) - }, - ) - - // No Exporter available, lease unsatisfiable - if len(onlineExporters) == 0 { - lease.Status = jumpstarterdevv1alpha1.LeaseStatus{ - BeginTime: nil, - EndTime: nil, - ExporterRef: nil, - Ended: true, - } + lease *jumpstarterdevv1alpha1.Lease, +) error { + logger := log.FromContext(ctx) + now := time.Now() + if lease.Status.BeginTime == nil && lease.Status.ExporterRef != nil { + logger.Info("reconcileStatusBeginTime: updating begin time", "lease", lease) meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{ - Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable), + Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), Status: metav1.ConditionTrue, ObservedGeneration: lease.Generation, LastTransitionTime: metav1.Time{ - Time: time.Now(), + Time: now, }, - Reason: "NoExporter", + Reason: "Ready", }) - - if err := r.Status().Update(ctx, &lease); err != nil { - log.Error(err, "unable to update Lease status") - return ctrl.Result{}, client.IgnoreNotFound(err) + lease.Status.BeginTime = &metav1.Time{ + Time: now, } - - return ctrl.Result{}, nil } - var leases jumpstarterdevv1alpha1.LeaseList - err = r.List( - ctx, - &leases, - client.InNamespace(lease.Namespace), - MatchingActiveLeases(), - ) - if err != nil { - log.Error(err, "Error listing leases") - return ctrl.Result{}, err - } + return nil +} - // Find available exporter - for _, exporter := range onlineExporters { - taken := false - for _, existingLease := range leases.Items { - // if lease is active and is referencing an exporter - if !existingLease.Status.Ended && existingLease.Status.ExporterRef != nil { - // if lease is referencing this exporter - if existingLease.Status.ExporterRef.Name == exporter.Name { - taken = true - } - } - } - // Exporter taken by lease - if taken { - continue +// Also manages LeaseConditionTypeUnsatisfiable and LeaseConditionTypePending +func (r *LeaseReconciler) reconcileStatusExporterRef( + ctx context.Context, + result *ctrl.Result, + lease *jumpstarterdevv1alpha1.Lease, +) error { + logger := log.FromContext(ctx) + + if lease.Status.ExporterRef == nil { + logger.Info("reconcileStatusExporterRef: looking for matching exporter", "lease", lease) + + selector, err := metav1.LabelSelectorAsSelector(&lease.Spec.Selector) + if err != nil { + logger.Error(err, "reconcileStatusExporterRef: failed to create selector from label selector", "lease", lease) + return err } - beginTime := time.Now() - - lease.Status = jumpstarterdevv1alpha1.LeaseStatus{ - BeginTime: &metav1.Time{ - Time: beginTime, - }, - EndTime: &metav1.Time{ - Time: beginTime.Add(lease.Spec.Duration.Duration), - }, - ExporterRef: &corev1.LocalObjectReference{ - Name: exporter.Name, - }, - Ended: false, + // List all Exporter matching selector + var matchingExporters jumpstarterdevv1alpha1.ExporterList + if err := r.List( + ctx, + &matchingExporters, + client.InNamespace(lease.Namespace), + client.MatchingLabelsSelector{Selector: selector}, + ); err != nil { + logger.Error(err, "reconcileStatusExporterRef: failed to list exporters matching selector", "lease", lease) + return err } - meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{ - Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), - Status: metav1.ConditionTrue, - ObservedGeneration: lease.Generation, - LastTransitionTime: metav1.Time{ - Time: beginTime, + // Filter out offline exporters + onlineExporters := slices.DeleteFunc( + matchingExporters.Items, + func(exporter jumpstarterdevv1alpha1.Exporter) bool { + return !(true && + meta.IsStatusConditionTrue( + exporter.Status.Conditions, + string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered), + ) && + meta.IsStatusConditionTrue( + exporter.Status.Conditions, + string(jumpstarterdevv1alpha1.ExporterConditionTypeOnline), + )) }, - Reason: "Acquired", - }) + ) - if err := r.Status().Update(ctx, &lease); err != nil { - log.Error(err, "unable to update Lease status") - return ctrl.Result{}, client.IgnoreNotFound(err) - } - - if err := controllerutil.SetControllerReference(&exporter, &lease, r.Scheme); err != nil { - log.Error(err, "unable to update Lease owner reference") - return ctrl.Result{}, err + // No matching exporter online, lease unsatisfiable + if len(onlineExporters) == 0 { + meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{ + Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable), + Status: metav1.ConditionTrue, + ObservedGeneration: lease.Generation, + LastTransitionTime: metav1.Time{ + Time: time.Now(), + }, + Reason: "NoExporter", + }) + return nil } - if err := r.Update(ctx, &lease); err != nil { - log.Error(err, "unable to update Lease") - return ctrl.Result{}, err + var leases jumpstarterdevv1alpha1.LeaseList + if err := r.List( + ctx, + &leases, + client.InNamespace(lease.Namespace), + MatchingActiveLeases(), + ); err != nil { + logger.Error(err, "reconcileStatusExporterRef: failed to list active leases", "lease", lease) + return err } - // Requeue at EndTime - return ctrl.Result{ - RequeueAfter: time.Until(lease.Status.EndTime.Time), - }, nil - } - - lease.Status = jumpstarterdevv1alpha1.LeaseStatus{ - BeginTime: nil, - EndTime: nil, - ExporterRef: nil, - Ended: false, - } - meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{ - Type: string(jumpstarterdevv1alpha1.LeaseConditionTypePending), - Status: metav1.ConditionTrue, - ObservedGeneration: lease.Generation, - LastTransitionTime: metav1.Time{ - Time: time.Now(), - }, - Reason: "NotAvailable", - }) - meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{ - Type: string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), - Status: metav1.ConditionFalse, - ObservedGeneration: lease.Generation, - LastTransitionTime: metav1.Time{ - Time: time.Now(), - }, - Reason: "Pending", - }) + availableExporters := slices.DeleteFunc(onlineExporters, func(exporter jumpstarterdevv1alpha1.Exporter) bool { + for _, existingLease := range leases.Items { + // if the lease is referencing the current exporter + if existingLease.Status.ExporterRef != nil && existingLease.Status.ExporterRef.Name == exporter.Name { + return true + } + } + return false + }) - if err := r.Status().Update(ctx, &lease); err != nil { - log.Error(err, "unable to update Lease status") - return ctrl.Result{}, client.IgnoreNotFound(err) + if len(availableExporters) == 0 { + meta.SetStatusCondition(&lease.Status.Conditions, metav1.Condition{ + Type: string(jumpstarterdevv1alpha1.LeaseConditionTypePending), + Status: metav1.ConditionTrue, + ObservedGeneration: lease.Generation, + LastTransitionTime: metav1.Time{ + Time: time.Now(), + }, + Reason: "NotAvailable", + }) + result.RequeueAfter = time.Second + return nil + } else { + lease.Status.ExporterRef = &corev1.LocalObjectReference{ + Name: availableExporters[0].Name, + } + return nil + } } - // No exporter available - // Try again later - return ctrl.Result{ - RequeueAfter: time.Second, - }, nil + return nil } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/controller/lease_controller_test.go b/internal/controller/lease_controller_test.go index 18c9b88..73d9741 100644 --- a/internal/controller/lease_controller_test.go +++ b/internal/controller/lease_controller_test.go @@ -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()) @@ -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()) @@ -265,8 +263,6 @@ 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) @@ -274,7 +270,6 @@ var _ = Describe("Lease Controller", func() { 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())