Script to run all plugins tests and try an install, rooting in local Logstash #15018

andsel · 2023-04-21T14:47:04Z

Release notes

[rn:skip]

What does this PR do?

Implements a script to test all supported plugins against Logstash/JRuby, in particular it uses the JRuby(and JDK) bundled with Logstash to execute the unit tests, create the gem and install the gem on local Logstash.

This doesn't start a real pipeline run into the local Logstash, but should be enough to reveal macroscopic Ruby syntax compatibility errors.

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

The introduced script permit to test supported plugins in the context of Logstash.
It provides a couple of command line options to select the plugins to test.
Using -p followed by a plugin name verify only that plugin. To test logstash-integration-jdbc it can be specified as:

ci/test_supported_plugins.sh -p logstash-integration-jdbc

Using -t (tier) and -k (kind) a set of plugins can be selected, for example: all input and output plugins of tier1 and tier2 could expressed as

ci/test_supported_plugins.sh -k input,output -t tier1,tier2

-h (halt) option is a boolean switch to stop on the first error instead of continue with all the plugins.

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~~

Author's Checklist

[ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

yaauie

I think this will be super useful, and spent a bit of time bike-shedding where the list of supported plugins should be (I don't believe the current nested structured embedding is the "right" place).

I ran into a bit of a problem though because this script pollutes the Gemfile and Gemfile.lock files that are used by Logstash's plugin manager, and the ci/test_supported_plugins.sh entry-point invokes ./gradlew installDefaultGems which ensures that most (all?) of these gems are already previously installed from their relevant rubygems artifact.

Questions to that end:

do we expect this to run on top of the default gem installation, and if so, what are we doing to validate that the local-git-checkout of the plugin is what is effectively installed?
are we trying to test installing each in isolation (that is, snapshot the Gemfile and Gemfile.lock before each plugin we are validating and restore the snapshot before proceeding to the next), or are we trying to test installing them together (if so, we should likely randomize the order in which we do so)

ci/test_supported_plugins.rb

andsel · 2023-05-22T07:54:28Z

Thanks @yaauie for reviewing it!

I ran into a bit of a problem though because this script pollutes the Gemfile and Gemfile.lock files that are used by Logstash's plugin manager, and the ci/test_supported_plugins.sh entry-point invokes ./gradlew installDefaultGems which ensures that most (all?) of these gems are already previously installed from their relevant rubygems artifact.

Ahh right, but the bundle install executed during the testing phase shouldn't overwrite with the latest dependencies required by the plugin itself? https://github.com/elastic/logstash/pull/15018/files#diff-a23b8397e5c96e0774b4b531cb29933248fb8278670d575b0acd4a5575985770R59 in that case it should test the plugin with the right dependencies, I think.

do we expect this to run on top of the default gem installation, and if so, what are we doing to validate that the local-git-checkout of the plugin is what is effectively installed?

You are right, maybe before installing the locally created gem the script should remove the plugin from Logstash with a bin/logstash-plugin uninstall <plugin pre-installed>, WDYT, coule be enough?

are we trying to test installing each in isolation..., or are we trying to test installing them together
Actually the script install one after the other without isolation of installs a "copy & restore" of Gemfile and Gemfile.lock would help a lot into this.

…ns or provide directly the name of the plugin

…h it

…ion)

Co-authored-by: Ry Biesemeyer <[email protected]>

yaauie

Excellent.

It would be good to have a singular canonical reference of plugins by support tier that can be referenced by this script instead of hard-coding it, but that can be deferred to a separate effort.

ci/test_supported_plugins.rb

yaauie · 2023-05-31T17:15:16Z

ci/test_supported_plugins.rb

+  def git_clone
+    if File.directory?(plugin_name)
+      puts "#{plugin_name} already cloned locally"
+      return


should we git pull in this case? do we need to worry about "dirty" repositories with untracked or modified files?

Yes, great idea thanks. Implemented with commit e23677b

yaauie · 2023-05-31T17:21:02Z

ci/test_supported_plugins.rb

+
+# it has to be out of logstash local close else plugins' Gradle script
+# would interfere with Logstash's one
+base_folder = "/tmp"


I would rather avoid dumping into /tmp, so maybe an environment variable would be a good hook point?

We should use the OS-independent Dir::tmpdir instead of hard-coding /tmp:

Suggested change

base_folder = "/tmp"

base_folder = ENV['LOGSTASH_PLUGINS_TMP'] || (require 'tmpdir'; Dir.tmpdir)

Good point, fixed with commit d3df637

yaauie · 2023-05-31T17:27:14Z

ci/test_supported_plugins.rb

+# Test the 
+# - build (rspec), 
+# - packaging (gem build)
+# - and deploy (bin/logstash-plugin install)
+# of a plugins inside the current Logstash, using its JRuby
+# Usage example:
+# bin/ruby ci/test_supported_plugins.rb -p logstash-integration-jdbc
+# bin/ruby ci/test_supported_plugins.rb -t tier1 -k input,codec,integration


🤔 the usage is missing a note about needing a clean bootstrap + installDevelopmentGems.

we need the Logstash JRuby to be present, so at least here we should execute ./gradlew assemble, to setup the environment to be able to run the Ruby script.

We can split this in 2:

in the shell script invoke the minimal Gradle task that setup the environment to be able to run the Ruby script

in the Ruby script executes all the steps that are fundamental to have Logstash in right posture to execute the tests; which means executing the Gradle's installDevelopmentGems tasks

handled with b30ad38

yaauie · 2023-05-31T17:28:06Z

ci/test_supported_plugins.sh

+export JRUBY_OPTS="-J-Xmx1g"
+export GRADLE_OPTS="-Xmx4g -Dorg.gradle.jvmargs=-Xmx4g -Dorg.gradle.daemon=false -Dorg.gradle.logging.level=info -Dfile.encoding=UTF-8"
+
+./gradlew installDevelopmentGems


would it be simpler to move this pre-requisite over to the ruby script so that everything is all in one place?

Co-authored-by: Ry Biesemeyer <[email protected]>

…plugin repository was already cloned

…er for all plugin cloning activities

…table from the phase to setup a local Logstash to be used in the test

andsel · 2023-06-01T10:47:59Z

Hi @yaauie

It would be good to have a singular canonical reference of plugins by support tier that can be referenced by this script instead of hard-coding it, but that can be deferred to a separate effort.

I agree with you. My idea would be to move those tiers and plugins inventory into a separate YAML file, maybe in the root of the Logstash project.

A sample could be:

tier1:
  inputs:
    - logstash-input-azure_event_hubs
    - logstash-input-beats
  codecs:
   - logstash-codec-generator
  filters:
    - ...
  outputs:
   - ...
  integrations:
   - logstash_integration_kafka

tier2: ...

I think that would be better addressed in a follow up PR. WDYT?

andsel · 2023-06-01T14:06:32Z

Eventually we could use this: https://github.com/elastic/logstash/blob/main/rakelib/plugins-metadata.json as plugin repository. Every plugins that has default-plugins to true is bundled in Logstash default distribution, so it's supported plugin.

andsel · 2023-06-14T14:32:16Z

Hi @yaauie , sorry to nudge you, let me know what you think about latest changes to this PR

yaauie

Implementation LGTM.

Let's merge as-is file an issue about the shared source-of-truth for plugins and their tiers so we don't lose track of it.

andsel · 2023-07-18T08:47:17Z

Created the follow up issue #15190 to track the request to use a common plugins inventory.

andsel added the status:work-in-progress label Apr 21, 2023

andsel self-assigned this Apr 21, 2023

andsel mentioned this pull request May 16, 2023

[Testing] Create script to test build all supported plugins #15017

Closed

andsel changed the title ~~First rough idea~~ Script to run all plugins tests and try an install, rooting in local Logstash May 17, 2023

andsel marked this pull request as ready for review May 18, 2023 10:22

andsel added status:needs-review and removed status:work-in-progress labels May 18, 2023

yaauie reviewed May 18, 2023

View reviewed changes

ci/test_supported_plugins.rb Outdated Show resolved Hide resolved

ci/test_supported_plugins.rb Outdated Show resolved Hide resolved

andsel and others added 10 commits May 31, 2023 17:34

First rough idea

fb35ddd

Some minor changes to generalize

c261d6a

Introduced command line arguments to specify kinds and tiers of plugi…

619b348

…ns or provide directly the name of the plugin

Moved the ruby script into ci folder and added a bash script to launc…

baf6330

…h it

Fixed join of not existing selection for tier-kind (es tier2-integrat…

da06602

…ion)

Fixed tier1 input names

4a4af04

Added immediate halt option to stop on first verification error

6f053c6

Remove unless...else to more direct if..else with ternary ? operator

fcd3644

Co-authored-by: Ry Biesemeyer <[email protected]>

Output also sterr when the instal gem command fails

be85408

Isolated plugin executions leveraging git to snapshot Logstash build

0d263fb

andsel force-pushed the feature/create_utility_to_bulk_test_all_plugins branch from 2d5568d to 0d263fb Compare May 31, 2023 15:35

yaauie reviewed May 31, 2023

View reviewed changes

andsel and others added 4 commits June 1, 2023 10:48

Make the script working directory independent

1324e65

Co-authored-by: Ry Biesemeyer <[email protected]>

Updates a local existing clone, after an eventual clean, in case the …

e23677b

…plugin repository was already cloned

Uses OS's temporary folder or one provided by user to use a base fold…

d3df637

…er for all plugin cloning activities

Splitted the phase to setup the environment to have local JRuby execu…

b30ad38

…table from the phase to setup a local Logstash to be used in the test

andsel requested a review from yaauie June 1, 2023 10:48

yaauie approved these changes Jul 17, 2023

View reviewed changes

roaksoax added status:approved and removed status:needs-review labels Jul 17, 2023

andsel merged commit 5d6aa10 into elastic:main Jul 18, 2023

andsel added the v8.10.0 label Jul 18, 2023

andsel mentioned this pull request Jul 18, 2023

Avoid hardcoded plugin inventory in bulk plugins test #15190

Open

roaksoax removed the status:approved label Jul 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Script to run all plugins tests and try an install, rooting in local Logstash #15018

Script to run all plugins tests and try an install, rooting in local Logstash #15018

andsel commented Apr 21, 2023 •

edited

Loading

yaauie left a comment

andsel commented May 22, 2023

yaauie left a comment

yaauie May 31, 2023

andsel Jun 1, 2023

yaauie May 31, 2023

andsel Jun 1, 2023

yaauie May 31, 2023

andsel Jun 1, 2023

andsel Jun 1, 2023

yaauie May 31, 2023

andsel commented Jun 1, 2023

andsel commented Jun 1, 2023

andsel commented Jun 14, 2023

yaauie left a comment

andsel commented Jul 18, 2023

	base_folder = "/tmp"
	base_folder = ENV['LOGSTASH_PLUGINS_TMP'] \|\| (require 'tmpdir'; Dir.tmpdir)

Script to run all plugins tests and try an install, rooting in local Logstash #15018

Script to run all plugins tests and try an install, rooting in local Logstash #15018

Conversation

andsel commented Apr 21, 2023 • edited Loading

Release notes

What does this PR do?

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

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

yaauie left a comment

Choose a reason for hiding this comment

andsel commented May 22, 2023

yaauie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andsel commented Jun 1, 2023

andsel commented Jun 1, 2023

andsel commented Jun 14, 2023

yaauie left a comment

Choose a reason for hiding this comment

andsel commented Jul 18, 2023

andsel commented Apr 21, 2023 •

edited

Loading