file watcher to be used for configuration auto-reload feature #12301
Conversation
github-actions
bot
added
pr/dependencies
dhiaayachi
added
the
pr/no-changelog
| func randomStr(length int) string { | ||
| const charset = "abcdefghijklmnopqrstuvwxyz" + | ||
| "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" | ||
| var seededRand *rand.Rand = rand.New( | ||
| rand.NewSource(time.Now().UnixNano())) | ||
| b := make([]byte, length) | ||
| for i := range b { | ||
| b[i] = charset[seededRand.Intn(len(charset))] | ||
| } | ||
| return string(b) | ||
| } |
There was a problem hiding this comment.
Added this helper to generate random strings, but if we already have something similar I could reuse.
agent/config/file_watcher.go
Outdated
| } | ||
|
|
||
| // Stop the file watcher | ||
| // calling Stop multiple times is a noop |
There was a problem hiding this comment.
Also might be useful to say "Stop must only be called after Start" since the sequence of Stop, Start, Stop would leave the goroutine running.
| require.NoError(t, assertEvent(file.Name(), w.EventsCh, defaultTimeout)) | ||
| } | ||
|
|
||
| func TestEventWatcherRead(t *testing.T) { |
There was a problem hiding this comment.
Is this testing that reading from a file (which may touch atime) doesn't notify you?
There was a problem hiding this comment.
yes, this should not trigger any event but just in case another filesystem do something special.
agent/config/file_watcher_test.go
Outdated
| _ = w.Stop() | ||
| }() | ||
|
|
||
| file.Chmod(0777) |
| recreated, err := os.Create(filepath) | ||
| require.NoError(t, err) | ||
| _, err = recreated.WriteString("config 2") |
There was a problem hiding this comment.
It's interesting you include this test setup since the order of operations here is actually the "unsafe" one you discuss elsewhere. In this case the filesystem sees:
- file name=X is erased
- file name=X is created and is empty
- file name=X is updated to not be empty
as 3 separate events, i guess here you're only going to see one of these. You could i guess end the test with an assert that times out to prove that you don't wake up more times than you expect.
dhiaayachi
force-pushed
the
config-autoreload/file-watcher
branch
from
1a3b21d
to
479b602
Compare
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
|
🍒 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/590786. |
This PR add a file watcher that combine the use of https://github.com/fsnotify/fsnotify and a reconcile loop that id files by inode ID in linux and Creation Time in Windows to detect if file has been recreated and call a handler with the event accordingly.
For the time being this is not yet used in Consul but the the intent is to use it to automatically reload some of the configuration options when the file get recreated.
The 2 events that could trigger the handlers are:
Symlinks are not supported as fsnotify/fsnotify and the go filesystem packages are not consistent on how to treat symlinks and could get different behaviours (follow or not follow) depending on OS. If a symlink is added an error will be returned.