Repository navigation
fix(provider): recover and clean up closed leases - #438
Conversation
AfterFunc timers have no channel to drain. Waiting on that channel after a timer fires blocks monitor removal and provider shutdown indefinitely. Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
Recover missed closure events by querying the monitored lease before checking funds or withdrawing. Confirmed terminal leases request local teardown and leave monitoring; active and reclaiming leases remain. Retry uncertain queries without withdrawing or deleting deployments. Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
A persisted manifest can outlive its lease. Retire its funds monitor and run normal teardown when recovery confirms a terminal lease state. Register monitoring before recovery starts so fast cleanup cannot be followed by a stale registration. Keep missing, unknown, and mismatched lease responses distinct from a confirmed closure. Active and reclaiming leases remain deployable. Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
Propagate manifest deletion failures so persisted deployments cannot be silently recovered after cleanup reports success. Treat missing objects as already removed and retain errors from both deletion operations. Keep the deployment manager and resource reservation while cleanup fails, and retry instead of retiring the manager after exhausted attempts. Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
An organization admin can review the limit in your organization's usage and billing settings.
Once the limit resets or is raised, reopen this pull request to trigger a review.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. WalkthroughThe balance checker validates lease states before querying deployment escrow, includes active and reclaiming lease costs, and publishes closure events. The deployment manager handles inactive leases through teardown and retries failures. Kubernetes lease teardown preserves errors from both deletion operations. ChangesLease lifecycle reconciliation
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant BalanceChecker
participant LeaseQuery
participant EventBus
BalanceChecker->>LeaseQuery: query lease state
LeaseQuery-->>BalanceChecker: return lease state
BalanceChecker->>EventBus: publish closure event for closed or insufficient-funds lease
Suggested reviewers: Merge Risk: 🔵 Low · up to The change recovers and cleans up closed leases, and the supplied context shows no concrete defect. It has not been tested together with the companion SDK change, so verify the combined build before or soon after merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change improves cleanup and explicitly preserves active and reclaiming leases. Recovery after interrupted cleanup, endpoint trust, and combined-release validation remain incompletely established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
I’m a rabbit, counting leases in the moonlit glow Comment |
Run the escrow check for both active and reclaiming leases, counting both states in the shared account burn rate. Low funds still trigger a withdrawal so the chain decides whether to close the lease. Cover funded, exhausted, shared-escrow, scheduled-withdrawal, and query-error cases, including disabled periodic withdrawals. Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
2 participants
Closed leases can leave workloads running when closure events are missed or occur while the provider is offline. Check chain state during balance checks and startup, fix funds-monitor shutdown deadlocks, and retry failed Kubernetes cleanup. Only confirmed terminal leases trigger local teardown; uncertain queries do not trigger deletion.
Active and reclaiming leases continue through funds checks and withdrawals. Count both states when estimating the shared escrow burn rate so exhausted reclaiming leases still trigger settlement even when periodic withdrawals are disabled.
Complements akash-network/chain-sdk#361 by recovering existing zombies and handling cleanup failures.
Validation: local unit/race tests for the provider, cluster, and Kubernetes packages passed; scoped lint passed. Regression coverage includes funded and exhausted reclaiming leases, shared escrow, scheduled withdrawals, and query failures. The exhausted-lease test failed before the fix and passes afterward. Earlier testnet validation covered normal closure, missed-event recovery, restart recovery, and active-lease controls; the reclamation withdrawal fix is validated locally. The combined SDK build has not been tested on a live cluster.