Skip to content

Backport PR #13002 to 7.x: ispec: fix cross-spec leak from fatal error integration specs#13013

Merged
yaauie merged 1 commit into7.xfrom
backport_13002_7.x
Jun 21, 2021
Merged

Backport PR #13002 to 7.x: ispec: fix cross-spec leak from fatal error integration specs#13013
yaauie merged 1 commit into7.xfrom
backport_13002_7.x

Conversation

@yaauie
Copy link
Member

@yaauie yaauie commented Jun 21, 2021

Backport PR #13002 to 7.x branch. Original message:

Release notes

[rn:skip]

What does this PR do?

FIxes cross-spec leak from "fatal error" integration specs

Because the "Fatal Error" specs specifically inject fatal errors during
execution, and do so by reacting to a "poison" event, the fatal error prevents
the poison event from being ACK'd in the underlying queue.

By specifying a one-off temporary data directory in these specs and cleaning up
after ourselves, we ensure that a PQ containing un-ACK'd events isn't leaked to
the next spec to run.

Why is it important/What is the impact to the user?

Resolves random flaky tests that were caused by being executed immediately after one of the "fatal error" tests.

In the following specific test run, the Test Logstash configuration expands environment variables in all plugin blocks spec immediately followed one of the "fatal error" specs, which had left a "poison" event containing the message "a fatal error". Since only the "fatal error" spec knew how to convert this message into an actual fatal error, the event was processed "normally". Unfortunately, the spec being executed didn't expect it to be there and its assertions failed.

14:28:29       1) Test Logstash configuration expands environment variables in all plugin blocks
14:28:29          Failure/Error: Unable to find matching line from backtrace
14:28:29          
14:28:29            expected: "74.125.176.147 - - [11/Sep/2014:21:50:37 +0000] \"GET /?flav=rss20 HTTP/1.1\" 200 29941 \"-\" \"FeedBurner/1.0 (http://www.FeedBurner.com)\" blah,environment_variables_are_evil"
14:28:29                 got: "a fatal error blah,environment_variables_are_evil74.125.176.147 - - [11/Sep/2014:21:50:37 +0000] \"G... 200 29941 \"-\" \"FeedBurner/1.0 (http://www.FeedBurner.com)\" blah,environment_variables_are_evil"
14:28:29          
14:28:29            (compared using ==)

Depending on what the next test to run validated, this "leak" may or may not end up breaking one of its unrelated assertions.

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 (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

14:28:29       1) Test Logstash configuration expands environment variables in all plugin blocks
14:28:29          Failure/Error: Unable to find matching line from backtrace
14:28:29          
14:28:29            expected: "74.125.176.147 - - [11/Sep/2014:21:50:37 +0000] \"GET /?flav=rss20 HTTP/1.1\" 200 29941 \"-\" \"FeedBurner/1.0 (http://www.FeedBurner.com)\" blah,environment_variables_are_evil"
14:28:29                 got: "a fatal error blah,environment_variables_are_evil74.125.176.147 - - [11/Sep/2014:21:50:37 +0000] \"G... 200 29941 \"-\" \"FeedBurner/1.0 (http://www.FeedBurner.com)\" blah,environment_variables_are_evil"
14:28:29          
14:28:29            (compared using ==)

Related issues

Use cases

Screenshots

Logs

Because the "Fatal Error" specs specifically inject fatal errors during
execution, and do so by reacting to a "poison" event, the fatal error prevents
the poison event from being ACK'd in the underlying queue.

By specifying a one-off temporary data directory in these specs and cleaning up
after ourselves, we ensure that a PQ containing un-ACK'd events isn't leaked to
the next spec to run.

(cherry picked from commit 6032e5f)
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

🍨

@yaauie yaauie merged commit 4dcc69d into 7.x Jun 21, 2021
@yaauie yaauie deleted the backport_13002_7.x branch June 21, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants