-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
auto-reload configuration when config files change #12329
Conversation
cc972ce
to
6014764
Compare
df3b5cc
to
26c1826
Compare
9928cdc
to
7892ff1
Compare
8553dfa
to
710aa9f
Compare
… to create events.
* tidy ups * Add tests for inode reconcile
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
agent/agent.go
Outdated
if a.baseDeps.RuntimeConfig.AutoReloadConfig { | ||
if len(a.baseDeps.WatchedFiles) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trivial:
These could be merged into a single conditional and an entire level of indentation removed.
w.configFilesLock.Lock() | ||
defer w.configFilesLock.Unlock() | ||
delete(w.configFiles, oldFile) | ||
w.configFiles[newFile] = &watchedFile{modTime: modTime} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(purely optional; codegolf)
You could move just the interaction of the lock and the map into some helpers to keep them tighter, like:
func (w *fileWatcher) replaceFile(oldFile, newFile string, modTime time.Time) {
w.configFilesLock.Lock()
defer w.configFilesLock.Unlock()
delete(w.configFiles, oldFile)
w.configFiles[newFile] = &watchedFile{modTime: modTime}
}
func (w *fileWatcher) addFile(newFile string, modTime time.Time) {
w.configFilesLock.Lock()
defer w.configFilesLock.Unlock()
w.configFiles[filename] = &watchedFile{modTime: modTime}
}
func (w *fileWatcher) removeFile(oldFile string) {
w.configFilesLock.Lock()
defer w.configFilesLock.Unlock()
delete(w.configFiles, oldFile)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Only two non-blocking comments.
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/619946. |
This add the ability to auto-reload config based on config file change. use
FileWatcher
merged in #12301.Missing tests, but ready for a first round of review.