diff --git a/controller/api/v1alpha1/lease_helpers.go b/controller/api/v1alpha1/lease_helpers.go index 63f81ca6d..2bb956b97 100644 --- a/controller/api/v1alpha1/lease_helpers.go +++ b/controller/api/v1alpha1/lease_helpers.go @@ -307,6 +307,18 @@ func (l *Lease) SetStatusInvalid(reason, messageFormat string, a ...any) { l.SetStatusCondition(LeaseConditionTypeInvalid, true, reason, messageFormat, a...) } +func (l *Lease) SetStatusBeforeLeaseHook(status bool, reason, messageFormat string, a ...any) { + l.SetStatusCondition(LeaseConditionTypeBeforeLeaseHook, status, reason, messageFormat, a...) +} + +func (l *Lease) SetStatusAfterLeaseHook(status bool, reason, messageFormat string, a ...any) { + l.SetStatusCondition(LeaseConditionTypeAfterLeaseHook, status, reason, messageFormat, a...) +} + +func (l *Lease) SetStatusHookFailed(reason, messageFormat string, a ...any) { + l.SetStatusCondition(LeaseConditionTypeHookFailed, true, reason, messageFormat, a...) +} + func (l *Lease) SetStatusCondition( condition LeaseConditionType, status bool, diff --git a/controller/api/v1alpha1/lease_types.go b/controller/api/v1alpha1/lease_types.go index dfd8e9a9d..0db0a08ed 100644 --- a/controller/api/v1alpha1/lease_types.go +++ b/controller/api/v1alpha1/lease_types.go @@ -61,10 +61,13 @@ type LeaseStatus struct { type LeaseConditionType string const ( - LeaseConditionTypePending LeaseConditionType = "Pending" - LeaseConditionTypeReady LeaseConditionType = "Ready" - LeaseConditionTypeUnsatisfiable LeaseConditionType = "Unsatisfiable" - LeaseConditionTypeInvalid LeaseConditionType = "Invalid" + LeaseConditionTypePending LeaseConditionType = "Pending" + LeaseConditionTypeReady LeaseConditionType = "Ready" + LeaseConditionTypeUnsatisfiable LeaseConditionType = "Unsatisfiable" + LeaseConditionTypeInvalid LeaseConditionType = "Invalid" + LeaseConditionTypeBeforeLeaseHook LeaseConditionType = "BeforeLeaseHook" + LeaseConditionTypeAfterLeaseHook LeaseConditionType = "AfterLeaseHook" + LeaseConditionTypeHookFailed LeaseConditionType = "HookFailed" ) type LeaseLabel string diff --git a/controller/internal/controller/lease_controller.go b/controller/internal/controller/lease_controller.go index a4dc2d527..c32cbae9d 100755 --- a/controller/internal/controller/lease_controller.go +++ b/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/handler" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) // LeaseReconciler reconciles a Lease object @@ -86,6 +88,10 @@ func (r *LeaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return result, err } + if err := r.reconcileStatusHookPhase(ctx, &lease); err != nil { + return result, err + } + if err := r.reconcileStatusEnded(ctx, &result, &lease); err != nil { return result, err } @@ -175,6 +181,49 @@ func (r *LeaseReconciler) reconcileStatusBeginEndTimes( logger.Info("Updating begin time for lease", "lease", lease.Name, "exporter", lease.GetExporterName(), "client", lease.GetClientName()) now := time.Now() lease.Status.BeginTime = &metav1.Time{Time: now} + } + + return nil +} + +// nolint:unparam +func (r *LeaseReconciler) reconcileStatusHookPhase( + ctx context.Context, + lease *jumpstarterdevv1alpha1.Lease, +) error { + if lease.Status.ExporterRef == nil || lease.Status.Ended { + return nil + } + + var exporter jumpstarterdevv1alpha1.Exporter + if err := r.Get(ctx, types.NamespacedName{ + Namespace: lease.Namespace, + Name: lease.Status.ExporterRef.Name, + }, &exporter); err != nil { + return fmt.Errorf("reconcileStatusHookPhase: failed to get exporter: %w", err) + } + + switch exporter.Status.ExporterStatusValue { + case jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHook: + lease.SetStatusBeforeLeaseHook(true, "BeforeLeaseHook", "The beforeLease hook is executing on the exporter") + lease.SetStatusReady(false, "BeforeLeaseHook", "Waiting for beforeLease hook to complete") + lease.SetStatusAfterLeaseHook(false, "Inactive", "The afterLease hook is not running") + case jumpstarterdevv1alpha1.ExporterStatusLeaseReady: + lease.SetStatusReady(true, "Ready", "An exporter has been acquired for the client") + lease.SetStatusBeforeLeaseHook(false, "Completed", "The beforeLease hook has completed") + lease.SetStatusAfterLeaseHook(false, "Inactive", "The afterLease hook is not running") + case jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHook: + lease.SetStatusAfterLeaseHook(true, "AfterLeaseHook", "The afterLease hook is executing on the exporter") + lease.SetStatusReady(false, "AfterLeaseHook", "The afterLease hook is executing") + lease.SetStatusBeforeLeaseHook(false, "Completed", "The beforeLease hook has completed") + case jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHookFailed: + lease.SetStatusHookFailed("BeforeLeaseHookFailed", "The beforeLease hook failed: %s", exporter.Status.StatusMessage) + lease.SetStatusReady(false, "HookFailed", "The beforeLease hook failed") + lease.SetStatusBeforeLeaseHook(false, "Failed", "The beforeLease hook failed") + case jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHookFailed: + lease.SetStatusHookFailed("AfterLeaseHookFailed", "The afterLease hook failed: %s", exporter.Status.StatusMessage) + lease.SetStatusAfterLeaseHook(false, "Failed", "The afterLease hook failed") + default: lease.SetStatusReady(true, "Ready", "An exporter has been acquired for the client") } @@ -566,9 +615,39 @@ func filterOutOfflineExporters(approvedExporters []ApprovedExporter) []ApprovedE return onlineExporters } +func (r *LeaseReconciler) findLeasesForExporter(ctx context.Context, obj client.Object) []reconcile.Request { + exporter, ok := obj.(*jumpstarterdevv1alpha1.Exporter) + if !ok { + return nil + } + + var leases jumpstarterdevv1alpha1.LeaseList + if err := r.List(ctx, &leases, + client.InNamespace(exporter.Namespace), + MatchingActiveLeases(), + ); err != nil { + return nil + } + + var requests []reconcile.Request + for _, lease := range leases.Items { + if lease.Status.ExporterRef != nil && lease.Status.ExporterRef.Name == exporter.Name { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: lease.Name, + Namespace: lease.Namespace, + }, + }) + } + } + return requests +} + // SetupWithManager sets up the controller with the Manager. func (r *LeaseReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&jumpstarterdevv1alpha1.Lease{}). + Watches(&jumpstarterdevv1alpha1.Exporter{}, + handler.EnqueueRequestsFromMapFunc(r.findLeasesForExporter)). Complete(r) } diff --git a/controller/internal/controller/lease_controller_test.go b/controller/internal/controller/lease_controller_test.go index 42b6d035e..a697097b4 100755 --- a/controller/internal/controller/lease_controller_test.go +++ b/controller/internal/controller/lease_controller_test.go @@ -694,6 +694,189 @@ func deleteLeases(ctx context.Context, leases ...string) { } } +var _ = Describe("Lease Hook Phase Propagation", func() { + BeforeEach(func() { + createExporters(context.Background(), testExporter1DutA, testExporter2DutA, testExporter3DutB) + setExporterOnlineConditions(context.Background(), testExporter1DutA.Name, metav1.ConditionTrue) + setExporterOnlineConditions(context.Background(), testExporter2DutA.Name, metav1.ConditionTrue) + setExporterOnlineConditions(context.Background(), testExporter3DutB.Name, metav1.ConditionTrue) + }) + AfterEach(func() { + ctx := context.Background() + deleteExporters(ctx, testExporter1DutA, testExporter2DutA, testExporter3DutB) + deleteLeases(ctx, lease1Name, lease2Name, lease3Name) + }) + + When("the exporter transitions to BeforeLeaseHook after lease assignment", func() { + It("should set BeforeLeaseHook condition on the lease and Ready to False", func() { + lease := leaseDutA2Sec.DeepCopy() + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + exporterName := updatedLease.Status.ExporterRef.Name + + setExporterStatus(ctx, exporterName, jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHook, "Running beforeLease hook") + _ = reconcileLease(ctx, lease) + + updatedLease = getLease(ctx, lease.Name) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeBeforeLeaseHook), + )).To(BeTrue()) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), + )).To(BeFalse()) + }) + }) + + When("the exporter transitions from BeforeLeaseHook to LeaseReady", func() { + It("should set Ready to True and BeforeLeaseHook to False", func() { + lease := leaseDutA2Sec.DeepCopy() + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + exporterName := updatedLease.Status.ExporterRef.Name + + setExporterStatus(ctx, exporterName, jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHook, "Running beforeLease hook") + _ = reconcileLease(ctx, lease) + + setExporterStatus(ctx, exporterName, jumpstarterdevv1alpha1.ExporterStatusLeaseReady, "Lease ready") + _ = reconcileLease(ctx, lease) + + updatedLease = getLease(ctx, lease.Name) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), + )).To(BeTrue()) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeBeforeLeaseHook), + )).To(BeFalse()) + }) + }) + + When("the exporter transitions to AfterLeaseHook", func() { + It("should set AfterLeaseHook condition on the lease and Ready to False", func() { + lease := leaseDutA2Sec.DeepCopy() + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + exporterName := updatedLease.Status.ExporterRef.Name + + setExporterStatus(ctx, exporterName, jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHook, "Running afterLease hook") + _ = reconcileLease(ctx, lease) + + updatedLease = getLease(ctx, lease.Name) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeAfterLeaseHook), + )).To(BeTrue()) + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), + )).To(BeFalse()) + }) + }) + + When("the exporter transitions to BeforeLeaseHookFailed", func() { + It("should set HookFailed condition with beforeLease message", func() { + lease := leaseDutA2Sec.DeepCopy() + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + exporterName := updatedLease.Status.ExporterRef.Name + + setExporterStatus(ctx, exporterName, jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHookFailed, "Hook script exited with code 1") + _ = reconcileLease(ctx, lease) + + updatedLease = getLease(ctx, lease.Name) + condition := meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeHookFailed), + ) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + Expect(condition.Reason).To(Equal("BeforeLeaseHookFailed")) + Expect(condition.Message).To(ContainSubstring("beforeLease")) + }) + }) + + When("the exporter transitions to AfterLeaseHookFailed", func() { + It("should set HookFailed condition with afterLease message", func() { + lease := leaseDutA2Sec.DeepCopy() + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + exporterName := updatedLease.Status.ExporterRef.Name + + setExporterStatus(ctx, exporterName, jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHookFailed, "Cleanup script failed") + _ = reconcileLease(ctx, lease) + + updatedLease = getLease(ctx, lease.Name) + condition := meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeHookFailed), + ) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + Expect(condition.Reason).To(Equal("AfterLeaseHookFailed")) + Expect(condition.Message).To(ContainSubstring("afterLease")) + }) + }) + + When("the exporter has Available status (backward compatibility)", func() { + It("should set Ready to True with no hook conditions", func() { + lease := leaseDutA2Sec.DeepCopy() + ctx := context.Background() + Expect(k8sClient.Create(ctx, lease)).To(Succeed()) + _ = reconcileLease(ctx, lease) + + updatedLease := getLease(ctx, lease.Name) + Expect(updatedLease.Status.ExporterRef).NotTo(BeNil()) + + Expect(meta.IsStatusConditionTrue( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeReady), + )).To(BeTrue()) + Expect(meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeBeforeLeaseHook), + )).To(BeNil()) + Expect(meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeAfterLeaseHook), + )).To(BeNil()) + Expect(meta.FindStatusCondition( + updatedLease.Status.Conditions, + string(jumpstarterdevv1alpha1.LeaseConditionTypeHookFailed), + )).To(BeNil()) + }) + }) +}) + +func setExporterStatus(ctx context.Context, name string, status string, message string) { + exporter := getExporter(ctx, name) + exporter.Status.ExporterStatusValue = status + exporter.Status.StatusMessage = message + Expect(k8sClient.Status().Update(ctx, exporter)).To(Succeed()) +} + var _ = Describe("orderApprovedExporters", func() { When("approved exporters are under a lease", func() { It("should put them last", func() {