Repository navigation
Conversation
…tart deploy() removed the old earner and tunnel before the new tunnel started, so a tunnel that failed to start (an image missing locally, a proxy that does not answer) left the proxy with no copy at all. The new tunnel now starts under a staged name next to the running copy. Only once it has proved an exit does the old pair go and the tunnel take the final name, under a lock the heartbeat's status() shares. The heartbeat never reports a staged tunnel, and a worker start sweeps one a dead deploy left behind.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #632 +/- ##
==========================================
+ Coverage 93.96% 93.98% +0.01%
==========================================
Files 74 74
Lines 15687 15731 +44
==========================================
+ Hits 14741 14784 +43
- Misses 946 947 +1
🚀 New features to boost your workflow:
|
This was referenced Nov 7, 2026
GeiserX
added a commit
to GeiserX/CashPilot-Desktop
that referenced
this pull request
Nov 7, 2026
…tart (#267) * fix(proxies): keep the running copy when a redeploy's tunnel cannot start deployProxied removed the old pair before the new tunnel started, so a tunnel that failed to start left no copy. The new tunnel now starts as <name>-next beside the running copy; once it has an exit, the old pair goes and the tunnel is renamed, under a lock the status read shares. The status read skips staged tunnels and sweeps stale ones. Mirrors GeiserX/CashPilot#632. * test(proxies): cover a failed promotion of the staged tunnel * fix(proxies): run one deploy per proxied copy at a time * fix(proxies): never remove a staged tunnel another deploy still owns A deploy removed any staged tunnel of ours as a leftover, and the status read swept any older than five minutes. A second app on the same Docker host, or a deploy still waiting for the swap, could lose its tunnel after the old pair was gone. A deploy now refuses a staged tunnel younger than the sweep age, and the sweep skips one whose copy is being deployed in this app.
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Redeploying a proxied copy could leave the proxy with no copy at all.
deploy()removed the old earner and tunnel first, then started the new tunnel. If the new tunnel failed to start, for example because its image was not available locally or the proxy did not answer within the ready timeout, the old copy was already gone. Only changing the order of the calls does not work: the tunnel name is deterministic, so Docker refuses a second container with that name while the old one exists.The fix:
cashpilot-tunnel-<slug>-p<id>-next, with the final hostname, while the old pair keeps running. If it cannot prove an exit, it is removed and the running copy is untouched.status()share a lock, so a heartbeat cannot start an earner that is being removed or read a half-swapped pair.status()skips staged tunnels, so their probe traffic never counts toward a cap.Tests in
tests/test_proxy_tunnel.py(all five fail on main):test_sidecar_is_ready_before_the_earner_is_created: the order is now staged ready, old removed, renamed, earner.test_a_sidecar_that_cannot_start_keeps_the_running_copytest_a_failed_promotion_removes_the_staged_sidecartest_a_staged_sidecar_left_behind_is_replaced_but_never_a_strangertest_a_staged_sidecar_of_a_deploy_that_died_is_swepttest_the_heartbeat_never_reports_a_staged_sidecaralso covers thestatus()filter.Follows up the review thread on #576 at app/proxy_tunnel.py:344. Desktop's proxied deploy had the same order and gets the same fix in GeiserX/CashPilot-Desktop#267.
Review follow-ups: the worker-start sweep removes only staged sidecars older than 300 s (
STAGED_STALE_SECONDS), well past the ready timeout, so a deploy still in flight keeps its sidecar. Deploys of one copy run one at a time under a per-copy lock. Tests:test_a_young_staged_sidecar_is_left_to_its_deployandtest_two_deploys_of_one_copy_take_turns.