qa: use clean expansion of LS tarball per fixture instance by yaauie · Pull Request #17082 · elastic/logstash

yaauie · 2025-02-13T15:44:37Z

Release notes

[rn: skip]

What does this PR do?

Ensures that Logstash integration tests are run against a clean expansion of the built tarball.

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

Because QA tests can modify the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball.

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

Run integration tests that invoke the Logstash service (or any of its sub-command features like the plugin manager):

ci/integration_tests.sh specs/cli/remove_spec.rb

Logs

see expunging, indicating that the old was removed
see expanding, indicating a fresh expansion
see Using...as LS_HOME, indicating that the fresh expansion is being used

    CLI > logstash-plugin remove
    expunging(/Users/rye/src/elastic/logstash@main/build/qa-fixture/logstash-9.1.0-SNAPSHOT)
    expanding(/Users/rye/src/elastic/logstash@main/build/logstash-9.1.0-SNAPSHOT.tar.gz)
    Using /Users/rye/src/elastic/logstash@main/build/qa-fixture/logstash-9.1.0-SNAPSHOT as LS_HOME
    Setting up services
    Setting up logstash service
    Setup script not found for logstash
    logstash service setup complete
      when other plugins depends on this plugin
        refuses to remove the plugin and display the plugin that depends on it.
      when no other plugins depends on this plugin
        successfully remove the plugin

    Finished in 2 minutes 0.2 seconds (files took 0.98941 seconds to load)
    2 examples, 0 failures

Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball.

yaauie · 2025-02-13T17:41:41Z

2025-02-13 16:52:01 UTC	    An error occurred in an `after(:context)` hook.
2025-02-13 16:52:01 UTC	    Failure/Error: FileUtils.mv("#{@fixture.get_service('logstash').application_settings_file}.original", @fixture.get_service('logstash').application_settings_
file)
2025-02-13 16:52:01 UTC	
2025-02-13 16:52:01 UTC	    Errno::ENOENT:
2025-02-13 16:52:01 UTC	      No such file or directory - /buildkite/builds/bk-agent-prod-k8s-1739464416407722128/elastic/logstash-pull-request-pipeline/build/qa-fixture/logstash-9.1.0
-SNAPSHOT/config/logstash.yml.original or /buildkite/builds/bk-agent-prod-k8s-1739464416407722128/elastic/logstash-pull-request-pipeline/build/qa-fixture/logsta
sh-9.1.0-SNAPSHOT/config/logstash.yml
2025-02-13 16:52:01 UTC	    # ./specs/logstash_to_logstash_spec.rb:35:in `block in <main>'
2025-02-13 16:52:01 UTC	    # ./rspec.rb:36:in `<main>'

-- https://buildkite.com/elastic/logstash-pull-request-pipeline/builds/2300#01950028-7702-4dcf-b3ee-8b5ebbda643f/2070-3724

The specs/logstash_to_logstash_spec.rb does LogstashService::new directly, twice, and the mid-test expunge in LogstashService#initialize deletes files that it expects to still exist.

I'll need to re-work that test, or accommodate it somehow.

yaauie · 2025-02-13T18:23:25Z

qa/integration/specs/logstash_to_logstash_spec.rb

    @fixture.teardown
  }

-  def change_logstash_setting(logstash_service, name, value)


This stomps on the existing config in the logstash installation under test, which is shared across multiple tests and may contain useful bits in the event of feature flags.

Since it is only ever used to set the api.http.port in advance of spawning the logstash process, we can simply use the equivalent --api.http.port command-line flag and avoid modifying the logstash config.

elastic-sonarqube · 2025-02-13T18:28:54Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

yaauie · 2025-02-13T18:29:01Z

qa/integration/specs/logstash_to_logstash_spec.rb

  end

  def run_logstash_instance(config_name, options = {})
-    api_port = 9600 + rand(1000)


random allocation means occasional collisions.

I've elected to keep track of an offset in a range of 1000, looping back to 0 if we ever have enough tests to do so.

elasticmachine · 2025-02-13T18:38:19Z

💚 Build Succeeded

Buildkite Build
Commit: 5609260

History

💔 Build #2300 failed 1cedfbd

robbavey

LGTM!

yaauie · 2025-02-14T15:54:07Z

@logstashmachine backport 8.x

yaauie · 2025-02-14T15:54:27Z

@logstashmachine backport 8.18

* qa: use clean expansion of LS tarball per fixture instance Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball. * qa: enable safe reuse of ls_home in ls_to_ls tests (cherry picked from commit d20eb4d)

…17094) * qa: use clean expansion of LS tarball per fixture instance Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball. * qa: enable safe reuse of ls_home in ls_to_ls tests (cherry picked from commit d20eb4d) Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>

…17095) * qa: use clean expansion of LS tarball per fixture instance Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball. * qa: enable safe reuse of ls_home in ls_to_ls tests (cherry picked from commit d20eb4d) Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>

yaauie · 2025-02-18T20:11:13Z

@logstashmachine backport 9.0

* qa: use clean expansion of LS tarball per fixture instance Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball. * qa: enable safe reuse of ls_home in ls_to_ls tests (cherry picked from commit d20eb4d)

…17113) * qa: use clean expansion of LS tarball per fixture instance Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball. * qa: enable safe reuse of ls_home in ls_to_ls tests (cherry picked from commit d20eb4d) Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>

…7082) * qa: use clean expansion of LS tarball per fixture instance Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball. * qa: enable safe reuse of ls_home in ls_to_ls tests

…gify * upstream/main: (27 commits) Add Windows 2025 to CI (elastic#17133) Update container acceptance tests with stdout/stderr changes (elastic#17138) entrypoint: avoid polluting stdout (elastic#17125) Fix acceptance test assertions for updated plugin remove (elastic#17126) Fix acceptance test assertions for updated plugin `remove` (elastic#17122) plugins: improve `remove` command to support multiple plugins (elastic#17030) spec: improve ls2ls spec (elastic#17114) allow concurrent Batch deserialization (elastic#17050) CPM handle 404 response gracefully with user-friendly log (elastic#17052) qa: use clean expansion of LS tarball per fixture instance (elastic#17082) Allow capturing heap dumps in DRA BK jobs (elastic#17081) Use centralized source of truth for active branches (elastic#17063) Update logstash_releases.json (elastic#17055) fix logstash-keystore to accept spaces in values when added via stdin (elastic#17039) Don't honor VERSION_QUALIFIER if set but empty (elastic#17032) Release note placeholder might be empty, making parsing lines nil tolerant. (elastic#17026) Fix BufferedTokenizer to properly resume after a buffer full condition respecting the encoding of the input string (elastic#16968) Add short living 9.0 next and update main in CI release version definition. (elastic#17008) Core version bump to 9.1.0 (elastic#16991) Add 9.0 branch to the CI branches definition (elastic#16997) ...

qa: use clean expansion of LS tarball per fixture instance

1cedfbd

Because QA tests can _modify_ the Logstash installation (e.g. those that invoke the plugin manager), it is important that the service wrapper begins with a clean expansion of the logstash tarball.

yaauie added the tests label Feb 13, 2025

qa: enable safe reuse of ls_home in ls_to_ls tests

5609260

yaauie commented Feb 13, 2025

View reviewed changes

robbavey approved these changes Feb 13, 2025

View reviewed changes

yaauie merged commit d20eb4d into elastic:main Feb 14, 2025

github-actions bot mentioned this pull request Feb 14, 2025

Backport PR #17082 to 8.x: qa: use clean expansion of LS tarball per fixture instance #17094

Merged

2 tasks

github-actions bot added the v8.19.0 label Feb 14, 2025

github-actions bot added the v8.18.0 label Feb 14, 2025

github-actions bot mentioned this pull request Feb 14, 2025

Backport PR #17082 to 8.18: qa: use clean expansion of LS tarball per fixture instance #17095

Merged

2 tasks

github-actions bot added the v9.0.0 label Feb 18, 2025

github-actions bot mentioned this pull request Feb 18, 2025

Backport PR #17082 to 9.0: qa: use clean expansion of LS tarball per fixture instance #17113

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qa: use clean expansion of LS tarball per fixture instance#17082

qa: use clean expansion of LS tarball per fixture instance#17082
yaauie merged 2 commits intoelastic:mainfrom
yaauie:qa-clean-logstash-service-per-fixture-instance

yaauie commented Feb 13, 2025

Uh oh!

yaauie commented Feb 13, 2025

Uh oh!

yaauie Feb 13, 2025

Uh oh!

elastic-sonarqube bot commented Feb 13, 2025

Uh oh!

yaauie Feb 13, 2025 •

edited

Loading

Uh oh!

elasticmachine commented Feb 13, 2025

Uh oh!

robbavey left a comment

Uh oh!

yaauie commented Feb 14, 2025

Uh oh!

yaauie commented Feb 14, 2025

Uh oh!

yaauie commented Feb 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yaauie commented Feb 13, 2025

Release notes

What does this PR do?

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

Checklist

How to test this PR locally

Logs

Uh oh!

yaauie commented Feb 13, 2025

Uh oh!

yaauie Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

elastic-sonarqube bot commented Feb 13, 2025

Quality Gate passed

Uh oh!

yaauie Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 13, 2025

💚 Build Succeeded

History

Uh oh!

robbavey left a comment

Choose a reason for hiding this comment

Uh oh!

yaauie commented Feb 14, 2025

Uh oh!

yaauie commented Feb 14, 2025

Uh oh!

yaauie commented Feb 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yaauie Feb 13, 2025 •

edited

Loading