-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Validate config before init NewInput to avoid uneccessory cleanup and kubernetes watcher leak #18630
Validate config before init NewInput to avoid uneccessory cleanup and kubernetes watcher leak #18630
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
1 similar comment
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💔 Tests FailedExpand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
@exekias Hi, PTAL |
Thank you so much for contributing @DanielQujun! This is looking good to me, pinging @urso just in case you see something I miss |
Pinging @elastic/integrations (Team:Integrations) |
@DanielQujun could you please add a new line to the |
e8aee9d
to
3bd083c
Compare
thanks for your review, updated done. |
@urso Hi, would you please take a look at your convenience. |
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. Always +1 to validate input/configuration before creating objects/resources. Thanks for contributing.
jenkins run tests |
💔 Build FailedExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
…t's misconfigured(elastic#18629) fix log format missing in libbeat/common/kubernetes/util.go Signed-off-by: 屈骏 <[email protected]>
3bd083c
to
a93628b
Compare
@exekias Hi, I have rebased with master branch, Do I get a flaky test? |
It seems the test was unrelated Thank you for contributing!! 🎉 |
…t's misconfigured(elastic#18629) (elastic#18630) fix log format missing in libbeat/common/kubernetes/util.go Signed-off-by: 屈骏 <[email protected]> (cherry picked from commit 74d81c2)
…t's misconfigured(#18629) (#18630) (#18810) fix log format missing in libbeat/common/kubernetes/util.go Signed-off-by: 屈骏 <[email protected]> (cherry picked from commit 74d81c2) Co-authored-by: Daniel <[email protected]>
Great! Thanks~ |
An alternative Fix #18629
When Filebeat using
add_kubernetes_metadata
processor and enabledinput.reload
, It will cause Kubernetes watcher goroutine leaks when the input config is wrong.I looked through the code in
filebeat/input/log/input.go
, notice that there is a defer function to cleanup when executingNewInput
failed.beats/filebeat/input/log/input.go
Line 76 in d11d609
But it seems not easy to add Kubernetes watcher stopping function here, So I move the validate section to the top, then
NewInput
will return directly if the config not qualified.