Skip to content

Commit 03950d1

Browse files
rubysclaude
andcommitted
Fix config reload detection when machine resumes from suspend
Previously, config reload detection compared the config file's modTime against the hook's start time. This failed to detect changes made while the machine was suspended because the file modification time predated the hook execution. Now, reload detection compares against when the config was last loaded (configLoadTime), which correctly detects any changes since the last load, including those made during machine suspension. Changes: - Add configLoadTime parameter to ShouldReloadConfig - Update ExecuteServerHooksWithReload to accept configLoadTime - Add configLoadTime field to idle.Manager and ServerLifecycle - Pass configLoadTime through CGI handler and CreateHandler - Update all tests for new signatures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b6be516 commit 03950d1

13 files changed

Lines changed: 132 additions & 106 deletions

File tree

cmd/navigator/main.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func main() {
5252
slog.Error("Failed to load configuration", "error", err)
5353
os.Exit(1)
5454
}
55+
configLoadTime := time.Now() // Track when config was loaded for reload detection
5556
slog.Info("Loaded configuration",
5657
"tenants", len(cfg.Applications.Tenants),
5758
"reverseProxies", len(cfg.Routes.ReverseProxies),
@@ -86,7 +87,7 @@ func main() {
8687

8788
// Create reload channel for resume hook config reload
8889
resumeReloadChan := make(chan string, 1)
89-
idleManager := idle.NewManager(cfg, configFile, func(path string) {
90+
idleManager := idle.NewManager(cfg, configFile, configLoadTime, func(path string) {
9091
// Non-blocking send to avoid deadlock if channel is full
9192
select {
9293
case resumeReloadChan <- path:
@@ -127,6 +128,7 @@ func main() {
127128
lifecycle := &ServerLifecycle{
128129
configFile: configFile,
129130
cfg: cfg,
131+
configLoadTime: configLoadTime,
130132
appManager: appManager,
131133
processManager: processManager,
132134
basicAuth: basicAuth,
@@ -232,6 +234,7 @@ func printHelp() {
232234
type ServerLifecycle struct {
233235
configFile string
234236
cfg *config.Config
237+
configLoadTime time.Time // When the current config was loaded (used for reload detection)
235238
appManager *process.AppManager
236239
processManager *process.Manager
237240
basicAuth *auth.BasicAuth
@@ -258,8 +261,9 @@ func (l *ServerLifecycle) Run() error {
258261
l.basicAuth,
259262
l.idleManager,
260263
l.cableHandler,
261-
func() string { return l.configFile }, // Get current config file
262-
func(path string) { l.reloadChan <- path }, // Trigger reload
264+
func() string { return l.configFile }, // Get current config file
265+
func() time.Time { return l.configLoadTime }, // Get config load time for reload detection
266+
func(path string) { l.reloadChan <- path }, // Trigger reload
263267
)
264268

265269
// Create HTTP server
@@ -290,7 +294,8 @@ func (l *ServerLifecycle) Run() error {
290294
time.Sleep(100 * time.Millisecond)
291295

292296
// Execute server ready hooks with reload check
293-
result := process.ExecuteServerHooksWithReload(l.cfg.Hooks.Ready, "ready", l.configFile)
297+
// Pass configLoadTime to detect changes since config was loaded (including during suspend)
298+
result := process.ExecuteServerHooksWithReload(l.cfg.Hooks.Ready, "ready", l.configFile, l.configLoadTime)
294299
if result.Error != nil {
295300
slog.Error("Failed to execute ready hooks", "error", result.Error)
296301
} else if result.ReloadDecision.ShouldReload {
@@ -357,10 +362,14 @@ func (l *ServerLifecycle) handleReload() {
357362
"trust_proxy", newConfig.Server.TrustProxy,
358363
"config_file", l.configFile)
359364

365+
// Replace config and update load time
366+
l.cfg = newConfig
367+
l.configLoadTime = time.Now()
368+
360369
// Update configuration in all managers
361370
l.appManager.UpdateConfig(newConfig)
362371
l.processManager.UpdateManagedProcesses(newConfig)
363-
l.idleManager.UpdateConfig(newConfig, l.configFile)
372+
l.idleManager.UpdateConfig(newConfig, l.configFile, l.configLoadTime)
364373

365374
// Update proxy settings
366375
proxy.SetTrustProxy(newConfig.Server.TrustProxy)
@@ -372,9 +381,6 @@ func (l *ServerLifecycle) handleReload() {
372381
// Update logging format if changed
373382
setupLogging(newConfig)
374383

375-
// Replace config
376-
l.cfg = newConfig
377-
378384
// Execute server start hooks BEFORE loading auth
379385
// This is important because hooks may update the htpasswd file
380386
if err := process.ExecuteServerHooks(newConfig.Hooks.Start, "start"); err != nil {
@@ -410,8 +416,9 @@ func (l *ServerLifecycle) handleReload() {
410416
l.basicAuth,
411417
l.idleManager,
412418
l.cableHandler,
413-
func() string { return l.configFile }, // Get current config file
414-
func(path string) { l.reloadChan <- path }, // Trigger reload
419+
func() string { return l.configFile }, // Get current config file
420+
func() time.Time { return l.configLoadTime }, // Get config load time for reload detection
421+
func(path string) { l.reloadChan <- path }, // Trigger reload
415422
)
416423
l.srv.Handler = newHandler
417424
}

cmd/navigator/main_ready_hook_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ logging:
6363
// Create managers
6464
appManager := process.NewAppManager(cfg)
6565
processManager := process.NewManager(cfg)
66-
idleManager := idle.NewManager(cfg, "", nil)
66+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
6767

6868
// Create lifecycle
6969
lifecycle := &ServerLifecycle{

cmd/navigator/main_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"path/filepath"
88
"strings"
99
"testing"
10+
"time"
1011

1112
"github.com/rubys/navigator/internal/auth"
1213
"github.com/rubys/navigator/internal/config"
@@ -263,7 +264,7 @@ func TestHandleConfigReload(t *testing.T) {
263264
// Create real managers to avoid nil pointer issues
264265
appManager := process.NewAppManager(cfg)
265266
processManager := process.NewManager(cfg)
266-
idleManager := idle.NewManager(cfg, "", nil)
267+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
267268

268269
// Create lifecycle with nonexistent config file
269270
lifecycle := &ServerLifecycle{
@@ -315,7 +316,7 @@ logging:
315316
// Create real managers to avoid nil pointer issues
316317
appManager := process.NewAppManager(cfg)
317318
processManager := process.NewManager(cfg)
318-
idleManager := idle.NewManager(cfg, "", nil)
319+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
319320

320321
// Create lifecycle with valid config file
321322
lifecycle := &ServerLifecycle{
@@ -393,7 +394,7 @@ logging:
393394
t.Error("Expected non-nil app manager")
394395
}
395396

396-
idleManager := idle.NewManager(cfg, "", nil)
397+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
397398
if idleManager == nil {
398399
t.Error("Expected non-nil idle manager")
399400
}
@@ -495,7 +496,7 @@ logging:
495496
// Create real managers to avoid nil pointer issues
496497
appManager := process.NewAppManager(cfg)
497498
processManager := process.NewManager(cfg)
498-
idleManager := idle.NewManager(cfg, "", nil)
499+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
499500

500501
// Create lifecycle with valid config file
501502
lifecycle := &ServerLifecycle{
@@ -598,7 +599,7 @@ logging:
598599
// Create managers
599600
appManager := process.NewAppManager(cfg)
600601
processManager := process.NewManager(cfg)
601-
idleManager := idle.NewManager(cfg, "", nil)
602+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
602603

603604
// Create lifecycle
604605
lifecycle := &ServerLifecycle{
@@ -690,7 +691,7 @@ logging:
690691
// Create real managers to avoid nil pointer issues
691692
appManager := process.NewAppManager(cfg)
692693
processManager := process.NewManager(cfg)
693-
idleManager := idle.NewManager(cfg, "", nil)
694+
idleManager := idle.NewManager(cfg, "", time.Time{}, nil)
694695

695696
// Create lifecycle with valid config file
696697
lifecycle := &ServerLifecycle{

internal/cgi/handler.go

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,20 @@ import (
2121

2222
// Handler implements CGI script execution with user switching support
2323
type Handler struct {
24-
Script string
25-
User string
26-
Group string
27-
AllowedUsers []string
28-
Env map[string]string
29-
ReloadConfig string
30-
Timeout time.Duration
31-
CurrentConfigFn func() string // Function to get current config file path
32-
TriggerReloadFn func(string) // Function to trigger config reload
24+
Script string
25+
User string
26+
Group string
27+
AllowedUsers []string
28+
Env map[string]string
29+
ReloadConfig string
30+
Timeout time.Duration
31+
CurrentConfigFn func() string // Function to get current config file path
32+
ConfigLoadTimeFn func() time.Time // Function to get when config was last loaded
33+
TriggerReloadFn func(string) // Function to trigger config reload
3334
}
3435

3536
// NewHandler creates a new CGI handler from configuration
36-
func NewHandler(cfg *config.CGIScriptConfig, currentConfigFn func() string, triggerReloadFn func(string)) (*Handler, error) {
37+
func NewHandler(cfg *config.CGIScriptConfig, currentConfigFn func() string, configLoadTimeFn func() time.Time, triggerReloadFn func(string)) (*Handler, error) {
3738
// Validate script path
3839
if cfg.Script == "" {
3940
return nil, fmt.Errorf("CGI script path is required")
@@ -58,15 +59,16 @@ func NewHandler(cfg *config.CGIScriptConfig, currentConfigFn func() string, trig
5859
})
5960

6061
return &Handler{
61-
Script: cfg.Script,
62-
User: cfg.User,
63-
Group: cfg.Group,
64-
AllowedUsers: cfg.AllowedUsers,
65-
Env: cfg.Env,
66-
ReloadConfig: cfg.ReloadConfig,
67-
Timeout: timeout,
68-
CurrentConfigFn: currentConfigFn,
69-
TriggerReloadFn: triggerReloadFn,
62+
Script: cfg.Script,
63+
User: cfg.User,
64+
Group: cfg.Group,
65+
AllowedUsers: cfg.AllowedUsers,
66+
Env: cfg.Env,
67+
ReloadConfig: cfg.ReloadConfig,
68+
Timeout: timeout,
69+
CurrentConfigFn: currentConfigFn,
70+
ConfigLoadTimeFn: configLoadTimeFn,
71+
TriggerReloadFn: triggerReloadFn,
7072
}, nil
7173
}
7274

@@ -241,9 +243,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
241243
"duration", time.Since(startTime))
242244

243245
// Check if config should be reloaded
244-
if h.ReloadConfig != "" && h.CurrentConfigFn != nil && h.TriggerReloadFn != nil {
246+
if h.ReloadConfig != "" && h.CurrentConfigFn != nil && h.ConfigLoadTimeFn != nil && h.TriggerReloadFn != nil {
245247
currentConfig := h.CurrentConfigFn()
246-
decision := utils.ShouldReloadConfig(h.ReloadConfig, currentConfig, startTime)
248+
configLoadTime := h.ConfigLoadTimeFn()
249+
decision := utils.ShouldReloadConfig(h.ReloadConfig, currentConfig, configLoadTime)
247250
if decision.ShouldReload {
248251
slog.Info("CGI script triggered config reload",
249252
"script", h.Script,

internal/cgi/handler_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77
"strings"
88
"testing"
9+
"time"
910

1011
"github.com/rubys/navigator/internal/config"
1112
)
@@ -66,7 +67,7 @@ func TestNewHandler(t *testing.T) {
6667
}
6768
}
6869

69-
handler, err := NewHandler(tt.cfg, nil, nil)
70+
handler, err := NewHandler(tt.cfg, nil, nil, nil)
7071

7172
if tt.wantError && err == nil {
7273
t.Error("Expected error but got none")
@@ -106,7 +107,7 @@ echo "Query: $QUERY_STRING"
106107
},
107108
}
108109

109-
handler, err := NewHandler(cfg, nil, nil)
110+
handler, err := NewHandler(cfg, nil, nil, nil)
110111
if err != nil {
111112
t.Fatalf("Failed to create handler: %v", err)
112113
}
@@ -195,9 +196,11 @@ echo "Config updated"
195196
ReloadConfig: configPath,
196197
}
197198

199+
configLoadTime := time.Now().Add(-1 * time.Hour) // Simulate config was loaded an hour ago
198200
handler, err := NewHandler(
199201
cfg,
200202
func() string { return configPath },
203+
func() time.Time { return configLoadTime },
201204
func(path string) {
202205
reloadTriggered = true
203206
reloadConfigPath = path
@@ -330,7 +333,7 @@ echo "Access granted"
330333
AllowedUsers: tt.allowedUsers,
331334
}
332335

333-
handler, err := NewHandler(cfg, nil, nil)
336+
handler, err := NewHandler(cfg, nil, nil, nil)
334337
if err != nil {
335338
t.Fatalf("Failed to create handler: %v", err)
336339
}

internal/idle/manager.go

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,31 @@ import (
1212

1313
// Manager tracks active requests and handles machine idle actions
1414
type Manager struct {
15-
enabled bool
16-
action string // "suspend" or "stop"
17-
idleTimeout time.Duration
18-
activeRequests int64
19-
lastActivity time.Time
20-
mutex sync.RWMutex
21-
timer *time.Timer
22-
config *config.Config
23-
configFile string // Current config file path for reload_config support
24-
reloadCallback func(configPath string) // Callback to trigger config reload
25-
idleActioned bool // Track if idle action was performed
26-
resuming bool // Track if resume hooks are currently running
27-
resumeCond *sync.Cond // Condition variable to wait for resume completion
28-
testMode bool // Prevents actual signal sending during tests
15+
enabled bool
16+
action string // "suspend" or "stop"
17+
idleTimeout time.Duration
18+
activeRequests int64
19+
lastActivity time.Time
20+
mutex sync.RWMutex
21+
timer *time.Timer
22+
config *config.Config
23+
configFile string // Current config file path for reload_config support
24+
configLoadTime time.Time // When the config was last loaded (for reload detection)
25+
reloadCallback func(configPath string) // Callback to trigger config reload
26+
idleActioned bool // Track if idle action was performed
27+
resuming bool // Track if resume hooks are currently running
28+
resumeCond *sync.Cond // Condition variable to wait for resume completion
29+
testMode bool // Prevents actual signal sending during tests
2930
}
3031

3132
// NewManager creates a new idle manager
3233
// The reloadCallback is called when a resume hook specifies reload_config and the config file was modified
33-
func NewManager(cfg *config.Config, configFile string, reloadCallback func(configPath string)) *Manager {
34+
// configLoadTime is when the config was last loaded (for detecting changes since last load)
35+
func NewManager(cfg *config.Config, configFile string, configLoadTime time.Time, reloadCallback func(configPath string)) *Manager {
3436
m := &Manager{
3537
config: cfg,
3638
configFile: configFile,
39+
configLoadTime: configLoadTime,
3740
reloadCallback: reloadCallback,
3841
lastActivity: time.Now(),
3942
}
@@ -94,10 +97,13 @@ func (m *Manager) RequestStarted() {
9497
configFile := m.configFile
9598
reloadCallback := m.reloadCallback
9699

100+
// Capture configLoadTime for the goroutine
101+
configLoadTime := m.configLoadTime
102+
97103
// Execute resume hooks asynchronously
98104
go func() {
99105
slog.Info("Executing server resume hooks")
100-
result := process.ExecuteServerHooksWithReload(m.config.Hooks.Resume, "resume", configFile)
106+
result := process.ExecuteServerHooksWithReload(m.config.Hooks.Resume, "resume", configFile, configLoadTime)
101107
if result.Error != nil {
102108
slog.Error("Failed to execute resume hooks", "error", result.Error)
103109
} else if result.ReloadDecision.ShouldReload && reloadCallback != nil {
@@ -243,12 +249,13 @@ func (m *Manager) GetStats() (activeRequests int64, lastActivity time.Time) {
243249
}
244250

245251
// UpdateConfig updates the idle manager configuration after a reload
246-
func (m *Manager) UpdateConfig(newConfig *config.Config, configFile string) {
252+
func (m *Manager) UpdateConfig(newConfig *config.Config, configFile string, configLoadTime time.Time) {
247253
m.mutex.Lock()
248254
defer m.mutex.Unlock()
249255

250256
m.config = newConfig
251257
m.configFile = configFile
258+
m.configLoadTime = configLoadTime
252259

253260
// Re-configure idle settings from new config
254261
if newConfig.Server.Idle.Action != "" && (newConfig.Server.Idle.Action == "suspend" || newConfig.Server.Idle.Action == "stop") {

0 commit comments

Comments
 (0)