Select polling based watcher for docker log file watcher on Windows#37412
Select polling based watcher for docker log file watcher on Windows#37412thaJeztah merged 1 commit intomoby:masterfrom
Conversation
daemon/logger/loggerutils/logfile.go
Outdated
There was a problem hiding this comment.
This ends up duplicating a lot.
Can we set fileWatcher to a polling watcher for Windows and keep using the rest of this function?
There was a problem hiding this comment.
Thank you @cpuguy83 I updated the method as per your comments.
Codecov Report
@@ Coverage Diff @@
## master #37412 +/- ##
=========================================
Coverage ? 34.93%
=========================================
Files ? 610
Lines ? 44875
Branches ? 0
=========================================
Hits ? 15678
Misses ? 27078
Partials ? 2119 |
1f479a4 to
0de6721
Compare
daemon/logger/loggerutils/logfile.go
Outdated
There was a problem hiding this comment.
Can probably keep this check outside the if-block. Not particularly harmful to error out twice in the case of Windows.
There was a problem hiding this comment.
Not sure I followed this comment. For non Windows we retry with polling right?
There was a problem hiding this comment.
Right. So some pseudo-code if windows { fileWatcher = getPoller } else { fileWatcher = getGeneric}; fileWatcher.Add(...) || fileWatcher = getPoller; fileWatcer.Add(...) || exit
There was a problem hiding this comment.
Wouldn't the fallback logic be duplicate for Windows?
There was a problem hiding this comment.
Yes, but doesn't really hurt anything.
There was a problem hiding this comment.
Ok. I updated the code as per your comment.
Signed-off-by: Tejaswini Duggaraju <[email protected]>
0de6721 to
df84cdd
Compare
|
The test case that failed is My change is not directly related to it. Is it expected or do I need to investigate? |
|
Unrelated testing bug. |
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
ping @johnstep @jhowardmsft
|
LGTM |
Signed-off-by: Tejaswini Duggaraju [email protected]
Fixes Windows docker -f doesn't follow instantly #30046
What I did
I updated logger to use polling based watcher for Windows . This is because the file watcher based on syscall notifications doesn't work because of file caching.
Based on runtime OS, I selected poller based watcher for Windows.
docker run -it --name <containername> <container image>docker logs -f <containername>This should stream the logs of the container continuously.
Check for the runtime OS and use poller based watcher to work around the file caching issue in Windows