Remove unnecessary restarts of metricsets while using Node autodiscover#19974
Remove unnecessary restarts of metricsets while using Node autodiscover#19974ChrsMark merged 3 commits intoelastic:masterfrom
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? |
32d6c6c to
f205ee0
Compare
|
Pinging @elastic/integrations-platforms (Team:Platforms) |
|
jenkins run the tests please |
|
Thank you for opening, I agree @ChrsMark we could do something similar with Pods (an the others). A note on using reflect: I've checked this discussion kubernetes-sigs/kubebuilder#592 and it seems that there are better alternatives: Option 1 (I think this one is preferred, as we probably have this package already): Option 2: Using https://github.com/banzaicloud/k8s-objectmatcher, which doesn't look bad either WDYT? |
f205ee0 to
57af22b
Compare
|
@exekias done. |
💔 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
|
|
Test failure looks related: https://travis-ci.org/github/elastic/beats/jobs/709358439#L703 |
57af22b to
bfc2c5c
Compare
|
@ChrsMark , fixed. |
There was a problem hiding this comment.
apologies. removing
libbeat/common/kubernetes/watcher.go
Outdated
There was a problem hiding this comment.
isUpdated will be nil for all other resources (Pod, Service...), so this will never be true for them.
Perhaps we should have a default isUpdated function, when the option is not present that does just the old != new check?
bfc2c5c to
ba464e1
Compare
|
jenkins run the tests |
|
https://travis-ci.org/github/elastic/beats/builds/712100705 seems like a flake. |
|
jenkins run the tests please |
|
Failing tests seem unrelated, merging. Thanks for contributing this @vjsamuel ! |
…er (elastic#19974) (cherry picked from commit 06f05ca)
…ne-2.0 * upstream/master: (29 commits) Add an explicit system test for processes on unix systems (elastic#20320) [Autodiscovery] Ignore ErrInputNotFinished errors in autodiscover config checks (elastic#20305) [CI] Update README.md with CI references (elastic#20316) Add ECK doc links to Heartbeat docs (elastic#20284) [Filebeat] Add export tests to x-pack/filebeat (elastic#20156) feat(ci): support building docker images for PRs (elastic#20323) Update system tests dependency (elastic#20287) [Libbeat] Log debug message if the Kibana dashboard can not be imported from the archive (elastic#12211) (elastic#20150) [Filebeat][Gsuite] Transform all dates to timestamp with processor (elastic#20308) Infer types in Prometheus remote_write (elastic#19944) Remove unnecessary restarts of metricsets while using Node autodiscover (elastic#19974) docs: update changelog on master branch (elastic#20259) feat(ci): support storing artifacts for PRs in separate dirs (elastic#20282) [CI] Change upstream reference (elastic#20296) [Filebeat] Updates to Suricata module (elastic#20220) [docs] Fix Windows download link for agent (elastic#20258) [docs] Rename release highlights to what's new (elastic#20255) fix: update the display name of the multibranch job (elastic#20265) [Elastic Agent] Add basic protocol to control Elastic Agent. (elastic#20146) Cisco ASA: Fix message 106100 (elastic#20245) ...
Bug
What does this PR do?
Everytime the kubelet does a check for node health it posts an update to the node status. Each change even if overall status doesnt change then it causes the metricset to restart. This PR adds a watcher option to register a func that checks in detail if the update is valid or not.
Why is it important?
This makes sure that the kubelet status updates dont prompt a restart unless the overall status changes.
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Author's Checklist
How to test this PR locally
Related issues