Skip to content

[codex] Fix Kubernetes config watch reloads #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
winhowes merged 2 commits into from
Apr 24, 2026
Merged

[codex] Fix Kubernetes config watch reloads #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c362c4
Fix Kubernetes config watch reloads
winhowes Apr 23, 2026
9a605b5
Improve watcher test coverage
winhowes Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 81 additions & 28 deletions app/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"net/url"
"os"
"os/signal"
"path/filepath"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -1159,6 +1160,43 @@ var newWatcher = func() (fileWatcher, error) {
return fswatcher{w}, nil
}

var watchAbsPath = filepath.Abs

const watchDebounceDelay = 100 * time.Millisecond

type watchDebouncer struct {
delay time.Duration
timer *time.Timer
timerC <-chan time.Time
}

func newWatchDebouncer(delay time.Duration) *watchDebouncer {
return &watchDebouncer{delay: delay}
}

func (d *watchDebouncer) trigger() <-chan time.Time {
if d.timer == nil {
d.timer = time.NewTimer(d.delay)
d.timerC = d.timer.C
return d.timerC
}
d.timer.Stop()
d.timer.Reset(d.delay)
d.timerC = d.timer.C
return d.timerC
}

func (d *watchDebouncer) fired() {
d.timerC = nil
}

func (d *watchDebouncer) stop() {
if d.timer == nil {
return
}
d.timer.Stop()
}

func watchFiles(ctx context.Context, files []string, out chan<- struct{}) {
w, err := newWatcher()
if err != nil {
Expand All @@ -1167,46 +1205,46 @@ func watchFiles(ctx context.Context, files []string, out chan<- struct{}) {
}
defer w.Close()

watchedDirs := make(map[string]struct{})
for _, f := range files {
if err := w.Add(f); err != nil {
logger.Error("watch add failed", "file", f, "error", err)
abs, err := watchAbsPath(f)
if err != nil {
logger.Error("watch path failed", "file", f, "error", err)
continue
}
watchedDirs[filepath.Dir(filepath.Clean(abs))] = struct{}{}
}

for dir := range watchedDirs {
if err := w.Add(dir); err != nil {
logger.Error("watch add failed", "dir", dir, "error", err)
Comment on lines +1218 to +1220
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep watching symlinked config files themselves

Switching from w.Add(file) to w.Add(dir) drops reload notifications when a configured file path is a symlink whose target lives outside the symlink’s parent directory. In that setup, writes update the target inode but do not emit directory-entry events in the symlink directory, so -watch no longer reloads unless the symlink entry itself changes.

Useful? React with 👍 / 👎.

}
}

debouncer := newWatchDebouncer(watchDebounceDelay)
defer debouncer.stop()
var timerC <-chan time.Time
notify := func() {
select {
case out <- struct{}{}:
default:
}
}

for {
select {
case <-ctx.Done():
return
case <-timerC:
timerC = nil
debouncer.fired()
notify()
case ev, ok := <-w.Events():
if !ok {
return
}
if ev.Op&(fsnotify.Rename|fsnotify.Remove) != 0 {
go func(name string) {
for i := 0; i < 50; i++ {
if err := w.Add(name); err == nil {
return
} else if !os.IsNotExist(err) {
logger.Error("watch re-add failed", "file", name, "error", err)
return
}
select {
case <-ctx.Done():
return
case <-time.After(10 * time.Millisecond):
}
}
}(ev.Name)
} else if ev.Op&fsnotify.Create != 0 {
if err := w.Add(ev.Name); err != nil && !os.IsNotExist(err) {
logger.Error("watch re-add failed", "file", ev.Name, "error", err)
}
}
if ev.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename) != 0 {
select {
case out <- struct{}{}:
default:
}
if watchEventInDirs(ev, watchedDirs) && ev.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename|fsnotify.Remove|fsnotify.Chmod) != 0 {
timerC = debouncer.trigger()
Comment on lines +1246 to +1247
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict reload triggers to configured file paths

The new filter treats any event in a watched directory as relevant, so changing unrelated sibling files now triggers config reloads even when config.yaml/allowlist.yaml/denylist.yaml are untouched. This is a behavioral regression from file-level watching and can cause noisy reload loops in directories like /etc or /tmp; with frequent unrelated writes, the debounce timer can keep resetting and delay real config reloads.

Useful? React with 👍 / 👎.

}
case err, ok := <-w.Errors():
if !ok {
Expand All @@ -1217,6 +1255,21 @@ func watchFiles(ctx context.Context, files []string, out chan<- struct{}) {
}
}

func watchEventInDirs(ev fsnotify.Event, watchedDirs map[string]struct{}) bool {
if ev.Name == "" {
return false
}
name := filepath.Clean(ev.Name)
if !filepath.IsAbs(name) {
abs, err := filepath.Abs(name)
if err == nil {
name = abs
}
}
_, ok := watchedDirs[filepath.Dir(name)]
return ok
}

// healthzHandler reports server readiness.
func healthzHandler(w http.ResponseWriter, r *http.Request) {
w.Header().Set("X-Last-Reload", metrics.LastReloadTime.Value())
Expand Down
Loading
Loading