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.

Sign up for GitHub

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

Jump to bottom

Add better handling for Kubernetes Update and Delete watcher events #18882

Merged
merged 13 commits into from
Jun 17, 2020
Merged

Add better handling for Kubernetes Update and Delete watcher events #18882

merged 13 commits into from
Jun 17, 2020

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Jun 1, 2020 •
edited
Loading

Improvements for #11834 described on #11834 (comment)

Changes:

  1. Retry on configCheck failures in order to cover stop/start events.
  2. Ignore Update events when status of Pod is Pending.
  3. Emit delete anywayeven if the obj cannot be found in thestore` at

    beats/libbeat/common/kubernetes/watcher.go

    Line 202 in 95626b8

    o, exists, err := w.store.GetByKey(key)
    , so as to ensure that states are cleaned anyway.

@ChrsMark
Fix race conditions and config errors
43bc4f8 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 1, 2020
@ChrsMark ChrsMark mentioned this pull request Jun 1, 2020
Closed
@ChrsMark ChrsMark self-assigned this Jun 1, 2020
@ChrsMark ChrsMark added the Team:Platforms Label for the Integrations - Platforms team label Jun 1, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 1, 2020
@ChrsMark
log level
188e970 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 1, 2020 •
edited
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #18882 updated]

  • Start Time: 2020-06-16T11:23:29.636+0000

  • Duration: 86 min 41 sec

Test stats 🧪

Test Results
Failed 0
Passed 9441
Skipped 1574
Total 11015

@ChrsMark
Add delete-anyway logic as well as retry on stop/start
638f96d 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Add specific check for InputNotFinished errors
078c279 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Add License
bf14d18 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Fix state
ad14e50 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Merge remote-tracking branch 'upstream/master' into k8s_autodiscover_…
5aaba62 
…race
@ChrsMark
Remove input related stuff
d8c70f9 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
changelog
b6681ee 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

Reverted checks for input realted errors handling (see content 078c279).

@exekias @jsoriano what do you think about adding these changes for now since we have found them, regardless of what we do for input related issues(maybe we need to wait for inputs' refactoring in order to completely resolve 11834).

@ChrsMark ChrsMark marked this pull request as ready for review June 15, 2020 07:46
@ChrsMark ChrsMark changed the title Fix race conditions and config errors Add better handling for Kubernetes Update and Delete watcher events Jun 15, 2020
jsoriano
jsoriano reviewed Jun 15, 2020
View reviewed changes
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree on adding these changes now that we can see that they can improve the situation. Added a couple of small suggestions.

libbeat/common/kubernetes/watcher.go Outdated Show resolved Hide resolved
libbeat/autodiscover/providers/kubernetes/pod.go Outdated Show resolved Hide resolved
@ChrsMark
Review changes
2c56f4b 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Bring back removed line
943825f 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

Thanks for the suggestions @jsoriano!, PR updated.

@ChrsMark
replace if/else with switch
9c46945 
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark merged commit d573e8c into elastic:master Jun 17, 2020
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Jun 17, 2020
@ChrsMark
Add better handling for Kubernetes Update and Delete watcher events (e…
7beb7e6 
…lastic#18882)

(cherry picked from commit d573e8c)
@ChrsMark ChrsMark mentioned this pull request Jun 17, 2020
Merged
ChrsMark added a commit that referenced this pull request Jun 17, 2020
@ChrsMark
Add better handling for Kubernetes Update and Delete watcher events (#…
999fb5c 
…18882) (#19232)

(cherry picked from commit d573e8c)
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Jul 30, 2020
@ChrsMark
Add better handling for Kubernetes Update and Delete watcher events (e…
1c77f73 
…lastic#18882)

(cherry picked from commit d573e8c)
@ChrsMark ChrsMark mentioned this pull request Jul 30, 2020
Closed
@ChrsMark ChrsMark mentioned this pull request Aug 12, 2020
Closed
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@ChrsMark @melchiormoulin
Add better handling for Kubernetes Update and Delete watcher events (e…
ae17a28 
…lastic#18882)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsoriano jsoriano jsoriano approved these changes

Assignees

@ChrsMark ChrsMark

Labels
autodiscovery Team:Platforms Label for the Integrations - Platforms team v7.8.2 v7.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ChrsMark @elasticmachine @jsoriano @andresrc