diff --git a/controller/internal/controller/lease_controller_test.go b/controller/internal/controller/lease_controller_test.go index 42b6d035e..99d6936d5 100755 --- a/controller/internal/controller/lease_controller_test.go +++ b/controller/internal/controller/lease_controller_test.go @@ -1947,3 +1947,86 @@ var _ = Describe("Scheduled Leases", func() { }) }) }) + +var _ = Describe("filterOutNotReadyExporters", func() { + makeExporter := func(name string, status string) ApprovedExporter { + return ApprovedExporter{ + Policy: jumpstarterdevv1alpha1.Policy{Priority: 0}, + Exporter: jumpstarterdevv1alpha1.Exporter{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Status: jumpstarterdevv1alpha1.ExporterStatus{ + ExporterStatusValue: status, + }, + }, + } + } + + When("all exporters are in hook-failed states", func() { + It("should exclude them all", func() { + exporters := []ApprovedExporter{ + makeExporter("exp-before-failed", jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHookFailed), + makeExporter("exp-after-failed", jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHookFailed), + } + + result := filterOutNotReadyExporters(exporters) + Expect(result).To(BeEmpty()) + }) + }) + + When("exporters are in Offline status", func() { + It("should exclude them", func() { + exporters := []ApprovedExporter{ + makeExporter("exp-offline", jumpstarterdevv1alpha1.ExporterStatusOffline), + } + + result := filterOutNotReadyExporters(exporters) + Expect(result).To(BeEmpty()) + }) + }) + + When("exporters are in mixed states", func() { + It("should only keep Available and unset exporters", func() { + exporters := []ApprovedExporter{ + makeExporter("exp-available", jumpstarterdevv1alpha1.ExporterStatusAvailable), + makeExporter("exp-unset", ""), + makeExporter("exp-unspecified", jumpstarterdevv1alpha1.ExporterStatusUnspecified), + makeExporter("exp-offline", jumpstarterdevv1alpha1.ExporterStatusOffline), + makeExporter("exp-before-hook", jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHook), + makeExporter("exp-after-hook", jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHook), + makeExporter("exp-before-failed", jumpstarterdevv1alpha1.ExporterStatusBeforeLeaseHookFailed), + makeExporter("exp-after-failed", jumpstarterdevv1alpha1.ExporterStatusAfterLeaseHookFailed), + } + + result := filterOutNotReadyExporters(exporters) + Expect(result).To(HaveLen(3)) + + names := make([]string, len(result)) + for i, ae := range result { + names[i] = ae.Exporter.Name + } + Expect(names).To(ConsistOf("exp-available", "exp-unset", "exp-unspecified")) + }) + }) + + When("all exporters are Available", func() { + It("should keep them all", func() { + exporters := []ApprovedExporter{ + makeExporter("exp1", jumpstarterdevv1alpha1.ExporterStatusAvailable), + makeExporter("exp2", jumpstarterdevv1alpha1.ExporterStatusAvailable), + } + + result := filterOutNotReadyExporters(exporters) + Expect(result).To(HaveLen(2)) + }) + }) + + When("input is empty", func() { + It("should return empty", func() { + result := filterOutNotReadyExporters([]ApprovedExporter{}) + Expect(result).To(BeEmpty()) + }) + }) +}) diff --git a/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py b/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py index a25c496e2..31041c7d6 100644 --- a/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py +++ b/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py @@ -391,6 +391,59 @@ async def test_finally_sets_before_lease_hook_on_early_cancel(self): ) +class TestStopRequestedGuard: + async def test_cleanup_does_not_report_available_when_stop_requested_with_skip(self): + """When _stop_requested is True and skip_after_lease_hook is True, + _cleanup_after_lease must NOT report AVAILABLE. This prevents the + controller from assigning new leases to a dying exporter (issue #245).""" + lease_ctx = make_lease_context() + lease_ctx.before_lease_hook.set() + lease_ctx.skip_after_lease_hook = True + + statuses = [] + + async def track_status(status, message=""): + statuses.append(status) + + exporter = make_exporter(lease_ctx) + exporter._stop_requested = True + exporter._report_status = AsyncMock(side_effect=track_status) + + await exporter._cleanup_after_lease(lease_ctx) + + available_statuses = [s for s in statuses if s == ExporterStatus.AVAILABLE] + assert len(available_statuses) == 0, ( + f"AVAILABLE must NOT be reported when _stop_requested is True, " + f"got statuses: {statuses}" + ) + assert lease_ctx.after_lease_hook_done.is_set() + + async def test_cleanup_does_not_report_available_when_stop_requested_no_hooks(self): + """When _stop_requested is True and no hook executor is configured, + _cleanup_after_lease must NOT report AVAILABLE. Even without hooks, + the _stop_requested guard prevents AVAILABLE during shutdown.""" + lease_ctx = make_lease_context() + lease_ctx.before_lease_hook.set() + + statuses = [] + + async def track_status(status, message=""): + statuses.append(status) + + exporter = make_exporter(lease_ctx, hook_executor=None) + exporter._stop_requested = True + exporter._report_status = AsyncMock(side_effect=track_status) + + await exporter._cleanup_after_lease(lease_ctx) + + available_statuses = [s for s in statuses if s == ExporterStatus.AVAILABLE] + assert len(available_statuses) == 0, ( + f"AVAILABLE must NOT be reported when _stop_requested is True " + f"(no hooks), got statuses: {statuses}" + ) + assert lease_ctx.after_lease_hook_done.is_set() + + class TestIdempotentLeaseEnd: async def test_duplicate_cleanup_is_noop(self): """Calling _cleanup_after_lease twice for the same LeaseContext