Skip to content
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

Merged
merged 90 commits into from
Mar 31, 2022

Conversation

dhiaayachi
Copy link
Contributor

@dhiaayachi dhiaayachi commented Feb 14, 2022

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.

@dhiaayachi dhiaayachi marked this pull request as draft February 14, 2022 16:23
@github-actions github-actions bot added the theme/cli Flags and documentation for the CLI interface label Feb 14, 2022
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 14, 2022 18:12 Inactive
@vercel vercel bot temporarily deployed to Preview – consul February 14, 2022 18:12 Inactive
@vercel vercel bot temporarily deployed to Preview – consul February 14, 2022 21:43 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 14, 2022 21:43 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 17, 2022 21:17 Inactive
@vercel vercel bot temporarily deployed to Preview – consul February 17, 2022 21:17 Inactive
@vercel vercel bot temporarily deployed to Preview – consul February 17, 2022 21:39 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 17, 2022 21:39 Inactive
agent/agent.go Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 16:26 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 16:26 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 16:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 16:31 Inactive
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 16:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 16:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 16:46 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 16:46 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 16:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 16:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 17:01 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 17:01 Inactive
agent/agent.go Outdated
Comment on lines 716 to 717
if a.baseDeps.RuntimeConfig.AutoReloadConfig {
if len(a.baseDeps.WatchedFiles) > 0 {
Copy link
Member

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.

Comment on lines +139 to +142
w.configFilesLock.Lock()
defer w.configFilesLock.Unlock()
delete(w.configFiles, oldFile)
w.configFiles[newFile] = &watchedFile{modTime: modTime}
Copy link
Member

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)
}

Copy link
Member

@rboyer rboyer left a 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.

@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 31, 2022 18:54 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 31, 2022 18:54 Inactive
@dhiaayachi dhiaayachi merged commit 16b19dd into main Mar 31, 2022
@dhiaayachi dhiaayachi deleted the config-autoreload/autreload branch March 31, 2022 19:11
@hc-github-team-consul-core
Copy link
Contributor

🍒 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/cli Flags and documentation for the CLI interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants