From 778930911ca1765e86824baf4c92754e4a2c73bb Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:52:36 +0100 Subject: [PATCH] fix(proxies): keep trying to start a copy the cap released until it starts A copy whose start failed when the cap lifted kept its mark and waited for the next reset, up to a month. The once-a-minute pass now retries every marked copy of an uncapped proxy, recording the failure only once. The mid-stop path marks the copy when its start back fails, so it is retried too instead of staying stopped with no mark. --- docs/proxies.md | 5 +- internal/services/proxies.go | 23 +++++---- internal/services/proxies_test.go | 78 +++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/docs/proxies.md b/docs/proxies.md index 625f4ca..a027ffc 100644 --- a/docs/proxies.md +++ b/docs/proxies.md @@ -164,8 +164,9 @@ proxy is stopped and you get a notice. Only the copies that were running are marked as stopped by the cap, so a copy you had already stopped stays stopped after the reset. While the proxy is capped, Start is disabled on its copies: reset the usage or raise the cap first. A reset starts every marked copy; one -whose start fails keeps its mark, the failure shows in its events, and the next -reset or a Start by hand tries again. +whose start fails keeps its mark, the failure shows once in its events, and the +app tries again every minute until it starts, or until you start or stop it by +hand. ## Tailscale exit nodes diff --git a/internal/services/proxies.go b/internal/services/proxies.go index e3d358b..4d0d456 100644 --- a/internal/services/proxies.go +++ b/internal/services/proxies.go @@ -220,7 +220,7 @@ func (m *Manager) SaveProxy(ctx context.Context, id int64, values map[string]str return proxies.View{}, err } if release { - if _, err := m.releaseCapped(ctx, id, d); err != nil { + if _, err := m.releaseCapped(ctx, id, d, true); err != nil { return proxies.View{}, err } } @@ -562,13 +562,15 @@ func (m *Manager) ResetProxy(ctx context.Context, id int64) ([]string, error) { if err != nil { return nil, err } - return m.releaseCapped(ctx, id, d) + return m.releaseCapped(ctx, id, d, true) } // releaseCapped starts the copies the cap stopped. A copy the owner stopped stays // stopped. An error means the copies could not even be listed or reached; a copy -// that failed to start is recorded as an event and keeps its flag. -func (m *Manager) releaseCapped(ctx context.Context, id int64, d proxies.Definition) ([]string, error) { +// that failed to start keeps its flag, so the next ProxyTick tries it again. Only +// a first attempt (record) writes the failure as an event: the retries run every +// minute, and events feed the health score. +func (m *Manager) releaseCapped(ctx context.Context, id int64, d proxies.Definition, record bool) ([]string, error) { m.proxy.lastMu.Lock() delete(m.proxy.unstopped, id) m.proxy.lastMu.Unlock() @@ -594,7 +596,9 @@ func (m *Manager) releaseCapped(ctx context.Context, id int64, d proxies.Definit continue } if _, err := p.StartProxied(ctx, c.Slug, id, m.stopTimeout(c.Slug), specOf(d), ownIP); err != nil { - m.store.RecordEvent(c.Slug, "proxied_start_error", maskSecrets(err.Error(), d)) + if record { + m.store.RecordEvent(c.Slug, "proxied_start_error", maskSecrets(err.Error(), d)) + } continue } _ = m.store.SetProxiedCopyCapped(id, c.Slug, false) @@ -706,10 +710,11 @@ func (m *Manager) ProxyTick(ctx context.Context) ([]CapNotice, error) { if saveErr != nil { continue } - if reset && next.CappedAt == "" { - _, _ = m.releaseCapped(ctx, id, d) // the next reset or a start by hand retries - } if next.CappedAt == "" { + // A reset starts what the cap stopped. On every other pass this retries a + // copy whose start failed when the cap lifted (Docker down, the proxy not + // answering yet): it still carries the cap's mark. + _, _ = m.releaseCapped(ctx, id, d, reset) continue } targets := map[string]bool{} @@ -776,6 +781,8 @@ func (m *Manager) stopCapped(ctx context.Context, p runtime.ProxyProvider, id in } else if loadErr == nil { if _, err := p.StartProxied(ctx, slug, id, m.stopTimeout(slug), specOf(fresh), detectOwnIP(ctx)); err != nil { m.store.RecordEvent(slug, "proxied_start_error", maskSecrets(err.Error(), fresh)) + // Marked, so the next pass starts it; unmarked it would stay stopped. + _ = m.store.SetProxiedCopyCapped(id, slug, true) } } } diff --git a/internal/services/proxies_test.go b/internal/services/proxies_test.go index 1c9a3bb..795d148 100644 --- a/internal/services/proxies_test.go +++ b/internal/services/proxies_test.go @@ -28,6 +28,8 @@ type fakeProxyProvider struct { specs []runtime.ProxySpec removed []string allGone []string + startErr error // every start fails with it while set + stopFn func() // runs inside a stop, before it returns } func newFakeProxyProvider() *fakeProxyProvider { @@ -48,11 +50,17 @@ func (f *fakeProxyProvider) StopProxied(_ context.Context, slug string, id int64 f.stops = append(f.stops, key(slug, id)) was := f.running[key(slug, id)] f.running[key(slug, id)] = false + if f.stopFn != nil { + f.stopFn() + } return was, nil } func (f *fakeProxyProvider) StartProxied(_ context.Context, slug string, id int64, _ int, spec runtime.ProxySpec, _ string) (runtime.Exit, error) { f.starts = append(f.starts, key(slug, id)) f.specs = append(f.specs, spec) + if f.startErr != nil { + return runtime.Exit{}, f.startErr + } f.running[key(slug, id)] = true return runtime.Exit{IP: "203.0.113.7"}, nil } @@ -444,3 +452,73 @@ func TestProxyRefusalNamesTheNativeRuntime(t *testing.T) { t.Fatalf("a failed lookup must be the refusal, got %q", got) } } + +// A copy whose start failed when the cap lifted is started by a later pass, not left +// stopped until the next reset a month away. Only the first failure is an event. +func TestACappedCopyThatDidNotStartIsRetriedEveryPass(t *testing.T) { + m, p, st := newProxyManager(t) + ctx := context.Background() + deployExample(t, st) + view, _ := m.SaveProxy(ctx, 0, map[string]string{"name": "Gate", "kind": "socks5", "host": "203.0.113.50", "port": "7000", "cap_gb": "1"}) + if _, err := m.DeployProxied(ctx, "example", view.ID); err != nil { + t.Fatal(err) + } + k := key("example", view.ID) + p.rx[k] = 950_000_000 + if _, err := m.ProxyTick(ctx); err != nil || p.running[k] { + t.Fatalf("capped: %v", err) + } + p.startErr = errors.New("Cannot connect to the Docker daemon") + if started, err := m.ResetProxy(ctx, view.ID); err != nil || len(started) != 0 { + t.Fatalf("a failed start is not reported as started: %v %v", started, err) + } + if _, err := m.ProxyTick(ctx); err != nil || p.running[k] { + t.Fatalf("still failing: %v", err) + } + // Each *_error event costs the health score 20 points. + if got := st.HealthScores(1)["example"].Crashes; got != 1 { + t.Fatalf("%d start-error events; a retry every minute must not add one each time", got) + } + p.startErr = nil + p.starts = nil + if _, err := m.ProxyTick(ctx); err != nil { + t.Fatal(err) + } + if !p.running[k] || len(p.starts) != 1 { + t.Fatalf("the next pass starts the copy the reset could not: running=%v starts=%v", p.running[k], p.starts) + } + if _, err := m.ProxyTick(ctx); err != nil || len(p.starts) != 1 { + t.Fatalf("a started copy loses the mark and is not started again: %v %v", p.starts, err) + } +} + +// The cap lifts while a stop is on its way and the start back fails: the copy gets +// the mark, so the next pass starts it. +func TestACopyWhoseStartBackFailedMidStopIsRetried(t *testing.T) { + m, p, st := newProxyManager(t) + ctx := context.Background() + deployExample(t, st) + view, _ := m.SaveProxy(ctx, 0, map[string]string{"name": "Gate", "kind": "socks5", "host": "203.0.113.50", "port": "7000", "cap_gb": "1"}) + if _, err := m.DeployProxied(ctx, "example", view.ID); err != nil { + t.Fatal(err) + } + k := key("example", view.ID) + p.rx[k] = 950_000_000 + p.startErr = errors.New("the proxy did not answer") + p.stopFn = func() { + p.stopFn = nil + if _, err := m.ResetProxy(ctx, view.ID); err != nil { + t.Error(err) + } + } + if _, err := m.ProxyTick(ctx); err != nil || p.running[k] { + t.Fatalf("the start back failed: %v", err) + } + p.startErr = nil + if _, err := m.ProxyTick(ctx); err != nil { + t.Fatal(err) + } + if !p.running[k] { + t.Fatal("the next pass starts the copy the failed start back left stopped") + } +}