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.

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

[BUG] Use Pod.Status.Phase for pod updates in kubernetes autodiscovery #17223

Merged
merged 2 commits into from
Mar 25, 2020

Conversation

brunotm
Copy link
Contributor

@brunotm brunotm commented Mar 24, 2020

What does this PR do?

This change fixes several issues with beats loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout and leading to
early termination of running beats.

Why is it important?

Avoids missing log, audit and metrics data when using kubernetes autodiscovery.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
    - [] I have made corresponding changes to the documentation
    - [] I have made corresponding change to the default configuration files
    - [] I have added tests that prove my fix is effective or that my feature works

Fixes #17246

@elasticmachine
Copy link
Collaborator

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
@elasticmachine
Copy link
Collaborator

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?

@cla-checker-service
Copy link

cla-checker-service bot commented Mar 24, 2020

💚 CLA has been signed

This change fixes several issues with filebeat loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout leading to
early termination of running beats.
@exekias exekias added containers Related to containers use case review Team:Platforms Label for the Integrations - Platforms team labels Mar 25, 2020
@elasticmachine
Copy link
Collaborator

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

@exekias
Copy link
Contributor

exekias commented Mar 25, 2020

This looks good to me, any other opinion @vjsamuel @ChrsMark @blakerouse?

@exekias
Copy link
Contributor

exekias commented Mar 25, 2020

jenkins test this please

@exekias
Copy link
Contributor

exekias commented Mar 25, 2020

I don't think they are related to this change, so let's ignore them for now

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

I agree, this looks good!

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Going to set to request changes, only so an issue can be created and add an entry in the CHANGELOG.next.asciidoc referencing this pull request.

Please file an issue for what this fixes so it can be tracked what this is solving.

@exekias exekias added the needs_backport PR is waiting to be backported to other branches. label Mar 25, 2020
@brunotm brunotm requested a review from blakerouse March 25, 2020 16:11
Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the issue and the changelog entry.

@blakerouse blakerouse merged commit 70237a7 into elastic:master Mar 25, 2020
@blakerouse blakerouse added v7.8.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 25, 2020
blakerouse pushed a commit to blakerouse/beats that referenced this pull request Mar 25, 2020
elastic#17223)

* Use Pod.Status.Phase for pod updates in kubernetes autodiscovery

This change fixes several issues with filebeat loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout leading to
early termination of running beats.

* add issue PR reference to changelog

(cherry picked from commit 70237a7)
blakerouse pushed a commit to blakerouse/beats that referenced this pull request Mar 25, 2020
elastic#17223)

* Use Pod.Status.Phase for pod updates in kubernetes autodiscovery

This change fixes several issues with filebeat loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout leading to
early termination of running beats.

* add issue PR reference to changelog

(cherry picked from commit 70237a7)
@brunotm
Copy link
Contributor Author

brunotm commented Mar 25, 2020

Thanks for the quick feedback on this one.

@exekias
Copy link
Contributor

exekias commented Mar 25, 2020

Thank you for contributing!!

blakerouse added a commit that referenced this pull request Mar 26, 2020
#17223) (#17247)

* Use Pod.Status.Phase for pod updates in kubernetes autodiscovery

This change fixes several issues with filebeat loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout leading to
early termination of running beats.

* add issue PR reference to changelog

(cherry picked from commit 70237a7)

Co-authored-by: Bruno Moura <brunotm@gmail.com>
blakerouse added a commit that referenced this pull request Mar 26, 2020
#17223) (#17248)

* Use Pod.Status.Phase for pod updates in kubernetes autodiscovery

This change fixes several issues with filebeat loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout leading to
early termination of running beats.

* add issue PR reference to changelog

(cherry picked from commit 70237a7)

Co-authored-by: Bruno Moura <brunotm@gmail.com>
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
elastic#17223) (elastic#17248)

* Use Pod.Status.Phase for pod updates in kubernetes autodiscovery

This change fixes several issues with filebeat loosing events when
using kubernetes autodiscovery by incorrectly handling of pod states.

Switch the pod status verification in OnUpdate() from
ObjectMeta.DeletionTimestamp (which is present only for deleted pods)
to Pod.Status.Phase in order to correctly handle pod states.

ObjectMeta.DeletionTimestamp is only present for deleted pods and when a
pod runs to completion (eg. pods generated by conjobs), OnUpdate()
will emit a pod stop event disrespecting the CleanupTimeout leading to
early termination of running beats.

* add issue PR reference to changelog

(cherry picked from commit b919f23)

Co-authored-by: Bruno Moura <brunotm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
containers Related to containers use case review Team:Platforms Label for the Integrations - Platforms team v7.7.0 v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Beats can loose events when using kubernetes autodiscovery
4 participants