fix: prevent lease assignment to non-ready exporters by ambient-code[bot] · Pull Request #426 · jumpstarter-dev/jumpstarter

ambient-code · 2026-04-08T09:22:41Z

Summary

Root cause fix (controller): Add filterOutNotReadyExporters to the lease controller's assignment pipeline so exporters still running hooks (AfterLeaseHook, BeforeLeaseHook, etc.) are not assigned new leases
Defense-in-depth (e2e): Wait for exporterStatus=Available in e2e test helpers before starting new tests, preventing races even if a controller bug is reintroduced

Fixes #425

Root Cause

The lease controller's reconcileStatusExporterRef only checked Online + Registered conditions before assigning a lease to an exporter. These conditions are set at initial registration and never cleared between leases. The ExporterStatusValue field (which tracks the exporter's actual operational state — Available, AfterLeaseHook, BeforeLeaseHook, LeaseReady, etc.) was completely ignored during lease assignment.

This meant an exporter still running cleanup from a previous lease (e.g., AfterLeaseHook status) could be assigned a new lease. The client would then try to Dial() the exporter, but the exporter's serve() loop was still blocked on after_lease_hook_done.wait() and couldn't process the new lease. After the 30s dial timeout, the client would fail with "Connection to exporter lost".

Controller Fix

New filterOutNotReadyExporters function in lease_controller.go filters the exporter list after the offline check and before the leased check. Only exporters with Available status (or unset status, for backwards compatibility with older exporters) are eligible for lease assignment. When all online exporters are busy, the lease gets Pending status with reason NotReady and requeues after 1 second.

E2E Fix (defense-in-depth)

The wait_for_exporter / wait_for_hooks_exporter helpers now poll .status.exporterStatus for Available in addition to checking k8s conditions, ensuring tests don't start a new lease while an exporter is still cleaning up.

Changes

controller/internal/controller/lease_controller.go: Add filterOutNotReadyExporters, integrate into assignment pipeline
controller/internal/controller/lease_controller_test.go: Add setExporterNotReady helper and two test cases (all-not-ready, partial-not-ready)
e2e/tests.bats: Refactor wait_for_exporter to poll exporterStatus=Available
e2e/tests-hooks.bats: Same exporterStatus=Available polling

Test plan

Controller unit tests pass (new tests for NotReady filtering)
E2e tests on ubuntu-24.04 — tests 47-48 should no longer flake
E2e hooks tests — no regressions

🤖 Generated with Claude Code

The wait_for_exporter helpers only checked Online+Registered k8s conditions, which are set at initial registration and never cleared between leases. This meant tests could start creating new leases while the exporter was still cleaning up from a previous one, causing intermittent "Connection to exporter lost" failures. Add polling for exporterStatus=Available after the condition checks to ensure the exporter's serve() loop has fully processed any previous lease-end before the next test runs. Fixes #425 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

netlify · 2026-04-08T09:22:46Z

✅ Deploy Preview for jumpstarter-docs ready!

Name	Link
🔨 Latest commit	`507a7ad`
🔍 Latest deploy log	https://app.netlify.com/projects/jumpstarter-docs/deploys/69d63bd60ff9280008078a0f
😎 Deploy Preview	https://deploy-preview-426--jumpstarter-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

coderabbitai · 2026-04-08T09:23:04Z

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c2ed726-4027-4028-817b-802d285a5187

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/e2e-wait-for-exporter-available

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mangelajo · 2026-04-08T09:55:26Z

@ambient-code This will probably just mask the issue. We need to understand why the exporter does not go ready, or why is it leased when still not ready.

The lease controller only checked Online+Registered conditions before assigning a lease, ignoring the ExporterStatusValue field. This meant an exporter still running afterLease/beforeLease hooks from a previous lease could be assigned a new lease, causing "Connection to exporter lost" errors when the client tried to dial. Add filterOutNotReadyExporters to the lease assignment pipeline so only exporters with Available (or unset, for backwards compat) status are eligible. This is the root cause fix for issue #425. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ambient-code · 2026-04-08T10:14:50Z

You're right — the e2e fix was masking the real issue. I've now pushed the root cause fix in the lease controller.

The problem: reconcileStatusExporterRef in lease_controller.go only checked Online + Registered conditions before assigning leases. These conditions are set once at registration and never cleared between leases. The ExporterStatusValue field (which accurately tracks Available / AfterLeaseHook / BeforeLeaseHook / etc.) was completely ignored during lease assignment.

So an exporter still running its afterLease hook cleanup could be assigned a new lease. The client would Dial() but the exporter's serve() loop was blocked on after_lease_hook_done.wait(), couldn't process the new lease, and the client timed out with "Connection to exporter lost".

The fix: Added filterOutNotReadyExporters to the lease assignment pipeline (between filterOutOfflineExporters and filterOutLeasedExporters). Only exporters with Available status (or unset, for backwards compat) are eligible. Non-ready exporters get a Pending/NotReady status with a 1s requeue.

The e2e test changes are kept as defense-in-depth.

Address review suggestion for consistency with the existing naming convention (onlineApprovedExporters). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

mangelajo · 2026-04-08T11:04:49Z

+		if len(readyApprovedExporters) == 0 {
+			lease.SetStatusPending(
+				"NotReady",
+				"There are %d online exporters, but none are ready (still cleaning up previous lease)",


The problem with this patch is that the new message "none are ready" will show even if all are leased. We should check this only once we have a list of available exporters to lease. (our current availableExporters)

Good catch — moved filterOutNotReadyExporters to run after filterOutLeasedExporters, so the "NotReady" status only fires when there are genuinely non-ready, unleased exporters. Previously it ran on all online exporters, which would incorrectly report "none are ready" when they were all simply leased.

mangelajo

see comments.

mangelajo · 2026-04-08T11:05:14Z

@ambient-code please handle comments

Move filterOutNotReadyExporters to run after filterOutLeasedExporters so the "none are ready" message only appears for genuinely non-ready, unleased exporters — not when all exporters are simply already leased. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

mangelajo · 2026-04-08T12:27:13Z

I am retriggering E2E a few times to see if stability is back.

mangelajo · 2026-04-08T12:52:51Z

2 in a row worked, triggering another...

mangelajo · 2026-04-08T13:55:18Z

ok no reds ❌ 🟢 🟢 🟢 🟢

mangelajo · 2026-04-08T13:56:59Z

@kirkbrauer based on the issue conversation I am squash-merging it, and hope for the best 🟢

… states Verify that filterOutNotReadyExporters correctly excludes exporters in HOOK_FAILED and OFFLINE states from lease assignment, serving as a regression test for the server-side safety net added in PR jumpstarter-dev#426. Ref: jumpstarter-dev#245 Generated-By: Forge/20260416_202053_681470_8c18858d_i245 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ambient-code Bot mentioned this pull request Apr 8, 2026

e2e: jmp shell fails with "Connection to exporter lost" after ~20s waiting for ready connection #425

Closed

ambient-code Bot changed the title ~~fix(e2e): wait for exporter Available status between lease operations~~ fix: prevent lease assignment to non-ready exporters Apr 8, 2026

mangelajo reviewed Apr 8, 2026

View reviewed changes

Comment thread controller/internal/controller/lease_controller.go Outdated

refactor: rename readyExporters to readyApprovedExporters

acc834d

Address review suggestion for consistency with the existing naming convention (onlineApprovedExporters). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

mangelajo enabled auto-merge (squash) April 8, 2026 10:33

mangelajo approved these changes Apr 8, 2026

View reviewed changes

mangelajo reviewed Apr 8, 2026

View reviewed changes

mangelajo requested changes Apr 8, 2026

View reviewed changes

mangelajo approved these changes Apr 8, 2026

View reviewed changes

mangelajo disabled auto-merge April 8, 2026 11:33

mangelajo requested a review from kirkbrauer April 8, 2026 11:33

mangelajo merged commit c562cf2 into main Apr 8, 2026
36 checks passed

ambient-code Bot mentioned this pull request Apr 8, 2026

hooks enabled: an exporter exiting via onFailure: exit reports itself as Available to the controller #245

Closed

raballew deleted the fix/e2e-wait-for-exporter-available branch June 5, 2026 11:37

Conversation

ambient-code Bot commented Apr 8, 2026 • edited by mangelajo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Controller Fix

E2E Fix (defense-in-depth)

Changes

Test plan

Uh oh!

netlify Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for jumpstarter-docs ready!

Uh oh!

coderabbitai Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

mangelajo commented Apr 8, 2026

Uh oh!

ambient-code Bot commented Apr 8, 2026

Uh oh!

Uh oh!

mangelajo Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ambient-code Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

mangelajo left a comment

Choose a reason for hiding this comment

Uh oh!

mangelajo commented Apr 8, 2026

Uh oh!

mangelajo commented Apr 8, 2026

Uh oh!

mangelajo commented Apr 8, 2026

Uh oh!

mangelajo commented Apr 8, 2026

Uh oh!

mangelajo commented Apr 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ambient-code Bot commented Apr 8, 2026 •

edited by mangelajo

Loading

netlify Bot commented Apr 8, 2026 •

edited

Loading

coderabbitai Bot commented Apr 8, 2026 •

edited

Loading