From 752e6cbac1ee85ec3d5bf6e474bddc447ecf283f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ste=CC=81phan=20Taljaard?= Date: Wed, 9 Sep 2026 20:10:22 +0200 Subject: [PATCH 1/2] fix(idlewatcher): don't adopt a dependency's config on reload NewWatcher reuses an existing watcher by key and copies the incoming IdlewatcherConfigBase over the stored one when the incoming idle timeout is positive. neverTick, which marks a dependency that must never auto-sleep, is math.MaxInt64 and so passes that check. A route that is both its own idle route and another route's dependency therefore loses its own idle_timeout, wake_timeout, stop_method and stop_signal to the dependency's inherited base, depending on the order NewWatcher is called in. Co-Authored-By: Claude Opus 5 --- internal/idlewatcher/watcher.go | 5 ++- .../idlewatcher/watcher_dependencies_test.go | 35 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/internal/idlewatcher/watcher.go b/internal/idlewatcher/watcher.go index 6d6cce81..3f8a36ef 100644 --- a/internal/idlewatcher/watcher.go +++ b/internal/idlewatcher/watcher.go @@ -134,7 +134,10 @@ func NewWatcher(parent task.Parent, r routing.Route, cfg *Config) (*Watcher, err watcherMapMu.RUnlock() if exists { - if cfg.IdleTimeout > 0 { + // neverTick is positive, so a dependency-synthesized config would pass a + // bare `> 0` check and overwrite the route's own base config, depending + // on the order NewWatcher happens to be called in. + if cfg.IdleTimeout > 0 && cfg.IdleTimeout != neverTick { w.cfg.IdlewatcherConfigBase = cfg.IdlewatcherConfigBase } cfg = w.cfg diff --git a/internal/idlewatcher/watcher_dependencies_test.go b/internal/idlewatcher/watcher_dependencies_test.go index d168a372..e31d348e 100644 --- a/internal/idlewatcher/watcher_dependencies_test.go +++ b/internal/idlewatcher/watcher_dependencies_test.go @@ -298,3 +298,38 @@ func (r *idlewatcherTestRoute) ReverseProxy() *reverseproxy.ReverseProxy { } func (r *idlewatcherTestRoute) ServeHTTP(http.ResponseWriter, *http.Request) {} func (r *idlewatcherTestRoute) MarshalZerologObject(*zerolog.Event) {} + +// A watcher that is both its own idle route and another route's dependency gets +// NewWatcher called a second time with a dependency-synthesized config. That +// config carries neverTick so the dependency never auto-sleeps, and adopting it +// would silently disable the route's own idle timeout. +func TestNewWatcherReloadIgnoresDependencyConfig(t *testing.T) { + w, parent, mainRoute, _ := newDependencyReloadTest(t, "dep-clobber", nil) + require.Equal(t, time.Hour, w.cfg.IdleTimeout) + + depCfg := idlewatcherTestConfig("dep-clobber", nil) + depCfg.IdleTimeout = neverTick + depCfg.WakeTimeout = 42 * time.Second + + reloaded, err := NewWatcher(parent, mainRoute, depCfg) + require.NoError(t, err) + require.Same(t, w, reloaded) + require.Equal(t, time.Hour, reloaded.cfg.IdleTimeout, "route keeps its own idle timeout") + require.Equal(t, time.Second, reloaded.cfg.WakeTimeout, "and the rest of its own base config") +} + +// A genuine reload must still be adopted. +func TestNewWatcherReloadAdoptsUpdatedConfig(t *testing.T) { + w, parent, mainRoute, _ := newDependencyReloadTest(t, "adopt-cfg", nil) + require.Equal(t, time.Hour, w.cfg.IdleTimeout) + + newCfg := idlewatcherTestConfig("adopt-cfg", nil) + newCfg.IdleTimeout = 2 * time.Hour + newCfg.WakeTimeout = 5 * time.Second + + reloaded, err := NewWatcher(parent, mainRoute, newCfg) + require.NoError(t, err) + require.Same(t, w, reloaded) + require.Equal(t, 2*time.Hour, reloaded.cfg.IdleTimeout) + require.Equal(t, 5*time.Second, reloaded.cfg.WakeTimeout) +} From 7807dfa19c329814bf0eefc41d7225b83b29ceb4 Mon Sep 17 00:00:00 2001 From: yusing Date: Thu, 10 Sep 2026 02:58:39 +0000 Subject: [PATCH 2/2] fix(idlewatcher): retain dependency-only config reloads --- internal/idlewatcher/watcher.go | 7 +- .../idlewatcher/watcher_dependencies_test.go | 90 ++++++++++++++++++- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/internal/idlewatcher/watcher.go b/internal/idlewatcher/watcher.go index 3f8a36ef..cc474eb7 100644 --- a/internal/idlewatcher/watcher.go +++ b/internal/idlewatcher/watcher.go @@ -134,10 +134,9 @@ func NewWatcher(parent task.Parent, r routing.Route, cfg *Config) (*Watcher, err watcherMapMu.RUnlock() if exists { - // neverTick is positive, so a dependency-synthesized config would pass a - // bare `> 0` check and overwrite the route's own base config, depending - // on the order NewWatcher happens to be called in. - if cfg.IdleTimeout > 0 && cfg.IdleTimeout != neverTick { + // Dependency configs must not overwrite a route's own base config, but + // dependency-only watchers still need updated wake and stop settings. + if cfg.IdleTimeout > 0 && (cfg.IdleTimeout != neverTick || w.cfg.IdleTimeout == neverTick) { w.cfg.IdlewatcherConfigBase = cfg.IdlewatcherConfigBase } cfg = w.cfg diff --git a/internal/idlewatcher/watcher_dependencies_test.go b/internal/idlewatcher/watcher_dependencies_test.go index e31d348e..9fb33542 100644 --- a/internal/idlewatcher/watcher_dependencies_test.go +++ b/internal/idlewatcher/watcher_dependencies_test.go @@ -310,12 +310,16 @@ func TestNewWatcherReloadIgnoresDependencyConfig(t *testing.T) { depCfg := idlewatcherTestConfig("dep-clobber", nil) depCfg.IdleTimeout = neverTick depCfg.WakeTimeout = 42 * time.Second + depCfg.StopTimeout = 10 * time.Second + depCfg.StopMethod = idlewatchertypes.ContainerStopMethodKill + depCfg.StopSignal = "SIGKILL" + want := w.cfg.IdlewatcherConfigBase reloaded, err := NewWatcher(parent, mainRoute, depCfg) require.NoError(t, err) require.Same(t, w, reloaded) require.Equal(t, time.Hour, reloaded.cfg.IdleTimeout, "route keeps its own idle timeout") - require.Equal(t, time.Second, reloaded.cfg.WakeTimeout, "and the rest of its own base config") + require.Equal(t, want, reloaded.cfg.IdlewatcherConfigBase, "route keeps all of its own base config") } // A genuine reload must still be adopted. @@ -326,10 +330,92 @@ func TestNewWatcherReloadAdoptsUpdatedConfig(t *testing.T) { newCfg := idlewatcherTestConfig("adopt-cfg", nil) newCfg.IdleTimeout = 2 * time.Hour newCfg.WakeTimeout = 5 * time.Second + newCfg.StopTimeout = 10 * time.Second + newCfg.StopMethod = idlewatchertypes.ContainerStopMethodKill + newCfg.StopSignal = "SIGKILL" reloaded, err := NewWatcher(parent, mainRoute, newCfg) require.NoError(t, err) require.Same(t, w, reloaded) require.Equal(t, 2*time.Hour, reloaded.cfg.IdleTimeout) - require.Equal(t, 5*time.Second, reloaded.cfg.WakeTimeout) + require.Equal(t, newCfg.IdlewatcherConfigBase, reloaded.cfg.IdlewatcherConfigBase) +} + +func TestNewWatcherReloadDependencyOnlyConfig(t *testing.T) { + for _, tc := range []struct { + name string + idleTimeout time.Duration + adopt bool + }{ + {"refresh", neverTick, true}, + {"promote_to_route", 2 * time.Hour, true}, + {"zero_unchanged", 0, false}, + {"negative_unchanged", -1, false}, + } { + t.Run(tc.name, func(t *testing.T) { + w, parent, mainRoute, _ := newDependencyReloadTest(t, "dep-only", []string{"old"}) + dep := w.dependsOn[0].Watcher + require.Equal(t, neverTick, dep.cfg.IdleTimeout) + want := dep.cfg.IdlewatcherConfigBase + cfg := idlewatcherTestConfig("old-id", nil) + cfg.IdleTimeout = tc.idleTimeout + cfg.WakeTimeout = 42 * time.Second + cfg.StopTimeout = 10 * time.Second + cfg.StopMethod = idlewatchertypes.ContainerStopMethodKill + cfg.StopSignal = "SIGKILL" + if tc.adopt { + want = cfg.IdlewatcherConfigBase + } + depRoute, ok := mainRoute.provider.GetRoute("old") + require.True(t, ok) + reloaded, err := NewWatcher(parent, depRoute, cfg) + require.NoError(t, err) + require.Same(t, dep, reloaded) + require.Equal(t, want, reloaded.cfg.IdlewatcherConfigBase) + }) + } +} + +func TestNewWatcherReloadRefreshesDependencySettings(t *testing.T) { + for _, tc := range []struct { + name string + explicit bool + idleTimeout time.Duration + }{ + {name: "inherited"}, + {name: "explicit_zero", explicit: true}, + {name: "explicit_negative", explicit: true, idleTimeout: -1}, + } { + t.Run(tc.name, func(t *testing.T) { + w, parent, mainRoute, _ := newDependencyReloadTest(t, "parent-reload", []string{"old"}) + dep := w.dependsOn[0].Watcher + require.Equal(t, neverTick, dep.cfg.IdleTimeout) + cfg := idlewatcherTestConfig("parent-reload", []string{"old"}) + updated := idlewatchertypes.IdlewatcherConfigBase{ + IdleTimeout: neverTick, + WakeTimeout: 42 * time.Second, + StopTimeout: 10 * time.Second, + StopMethod: idlewatchertypes.ContainerStopMethodKill, + StopSignal: "SIGKILL", + } + if tc.explicit { + depRoute, ok := mainRoute.provider.GetRoute("old") + require.True(t, ok) + depCfg := idlewatcherTestConfig("old-id", nil) + depCfg.IdlewatcherConfigBase = updated + depCfg.IdleTimeout = tc.idleTimeout + depRoute.(*idlewatcherTestRoute).cfg = depCfg + } else { + cfg.IdlewatcherConfigBase = updated + cfg.IdleTimeout = time.Hour + } + + reloaded, err := NewWatcher(parent, mainRoute, cfg) + require.NoError(t, err) + require.Same(t, w, reloaded) + require.Len(t, reloaded.dependsOn, 1) + require.Same(t, dep, reloaded.dependsOn[0].Watcher) + require.Equal(t, updated, dep.cfg.IdlewatcherConfigBase) + }) + } }