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

winlogbeat: allow termination while attempting to open channels #35474

Merged
merged 1 commit into from
May 16, 2023

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented May 15, 2023

What does this PR do?

Currently, if winlogbeat is attempting to open a channel that is not available because it is not found or there is a recoverable error, it will loop and retry. Since signals have been sent to notification handlers by this point, cancellation of the loop is not possible without agreement. So add a cancellation select at the head of the retry loop to allow termination.

Why is it important?

The current behaviour requires heroic user intervention when attempting to bring down a running winlogbeat that has not been able to get its input channels.

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
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@efd6 efd6 self-assigned this May 15, 2023
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 15, 2023
Currently, if winlogbeat is attempting to open a channel that is not
available because it is not found or there is a recoverable error, it
will loop and retry. Since signals have been sent to notification
handlers by this point, cancellation of the loop is not possible without
agreement. So add a cancellation select at the head of the retry loop to
allow termination.
@efd6 efd6 force-pushed the s3385-winlogbeat branch from b9a9bf6 to 3041a51 Compare May 15, 2023 21:29
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-05-15T21:29:40.851+0000

  • Duration: 37 min 19 sec

Test stats 🧪

Test Results
Failed 0
Passed 930
Skipped 9
Total 939

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@efd6 efd6 marked this pull request as ready for review May 15, 2023 22:08
@efd6 efd6 requested a review from a team as a code owner May 15, 2023 22:08
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@taylor-swanson taylor-swanson added the backport-v8.7.0 Automated backport with mergify label May 16, 2023
@efd6 efd6 merged commit a927de0 into elastic:main May 16, 2023
mergify bot pushed a commit that referenced this pull request May 16, 2023
Currently, if winlogbeat is attempting to open a channel that is not
available because it is not found or there is a recoverable error, it
will loop and retry. Since signals have been sent to notification
handlers by this point, cancellation of the loop is not possible without
agreement. So add a cancellation select at the head of the retry loop to
allow termination.

(cherry picked from commit a927de0)
mergify bot pushed a commit that referenced this pull request May 16, 2023
Currently, if winlogbeat is attempting to open a channel that is not
available because it is not found or there is a recoverable error, it
will loop and retry. Since signals have been sent to notification
handlers by this point, cancellation of the loop is not possible without
agreement. So add a cancellation select at the head of the retry loop to
allow termination.

(cherry picked from commit a927de0)
efd6 added a commit that referenced this pull request May 16, 2023
… to open channels (#35491)

* winlogbeat: allow termination while attempting to open channels (#35474)

Currently, if winlogbeat is attempting to open a channel that is not
available because it is not found or there is a recoverable error, it
will loop and retry. Since signals have been sent to notification
handlers by this point, cancellation of the loop is not possible without
agreement. So add a cancellation select at the head of the retry loop to
allow termination.

(cherry picked from commit a927de0)

* Update CHANGELOG.next.asciidoc

---------

Co-authored-by: Dan Kortschak <[email protected]>
efd6 added a commit that referenced this pull request May 16, 2023
… to open channels (#35492)

* winlogbeat: allow termination while attempting to open channels (#35474)

Currently, if winlogbeat is attempting to open a channel that is not
available because it is not found or there is a recoverable error, it
will loop and retry. Since signals have been sent to notification
handlers by this point, cancellation of the loop is not possible without
agreement. So add a cancellation select at the head of the retry loop to
allow termination.

(cherry picked from commit a927de0)

* Update CHANGELOG.next.asciidoc

---------

Co-authored-by: Dan Kortschak <[email protected]>
@nicpenning
Copy link
Contributor

Any idea if this same issue exists with filebeat and needs to be corrected?

@taylor-swanson
Copy link
Contributor

@nicpenning, I took a look at the run loop for filebeat (winlog), and that one properly handles context cancellation. Looks like this issue only affected winlogbeat.

@nicpenning
Copy link
Contributor

Thank you for being thorough!

chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
Currently, if winlogbeat is attempting to open a channel that is not
available because it is not found or there is a recoverable error, it
will loop and retry. Since signals have been sent to notification
handlers by this point, cancellation of the loop is not possible without
agreement. So add a cancellation select at the head of the retry loop to
allow termination.
@reakaleek reakaleek mentioned this pull request Jul 19, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.9-candidate backport-v8.7.0 Automated backport with mergify backport-v8.8.0 Automated backport with mergify bug Winlogbeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants