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

Pass plugin-params also for modules #1640

Merged
merged 1 commit into from
Dec 21, 2022

Conversation

danielmitterdorfer
Copy link
Member

In #1624 we've ensured that plugins that have turned into modules are now skipped from the mandatory plugin check. However, we've also not passed any plugin parameters to the provisioner and this made post installation hooks fail, if they expected parameters to be passed on the command line via --plugin-params (e.g. repository-gcs). With this commit we make sure that --plugin-params is still honored even in these cases.

Relates #1624

In elastic#1624 we've ensured that plugins that have turned into modules are
now skipped from the mandatory plugin check. However, we've also not
passed any plugin parameters to the provisioner and this made post
installation hooks fail, if they expected parameters to be passed on the
command line via `--plugin-params` (e.g. `repository-gcs`). With this
commit we make sure that `--plugin-params` is still honored even in
these cases.

Relates elastic#1624
@danielmitterdorfer danielmitterdorfer added bug Something's wrong :Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch labels Dec 16, 2022
@danielmitterdorfer danielmitterdorfer added this to the 2.7.1 milestone Dec 16, 2022
Copy link
Member

@b-deam b-deam left a comment

Choose a reason for hiding this comment

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

Thank you for catching this.

Funny that I missed this, given I extensively tested the PR in conjunction with esbench use-cases. It turns out we leverage CSP specific magic to handle snapshot repo auth on both AWS and GCP, so much magic that we actually don't even pass in any --plugin-params at all!

Thank you for including a test case too.

NB: Tested with..

Master:

$ esrally install --revision "@2022-02-04" --elasticsearch-plugins repository-s3,repository-gcs,repository-azure --plugin-params="s3_client_name:mys3client,s3_access_key:XXXXX,s3_secret_key:YYYYY,s3_session_token:ZZZZZ"

$ esrally start --installation-id f6bd6223-029c-45e6-812f-8d6e1f680589 --race-id testing

$ ./elasticsearch-keystore list
warning: ignoring JAVA_HOME=/Library/Java/JavaVirtualMachines/temurin-18.jdk/Contents/Home; using bundled JDK
keystore.seed

PR/1640:

$ esrally install --revision "@2022-02-04" --elasticsearch-plugins repository-s3,repository-gcs,repository-azure --plugin-params="s3_client_name:mys3client,s3_access_key:XXXXX,s3_secret_key:YYYYY,s3_session_token:ZZZZZ"

$ esrally start --installation-id 54117e74-c42a-43c3-bd83-e30b0cac9c54 --race-id testing

$ ./elasticsearch-keystore list
warning: ignoring JAVA_HOME=/Library/Java/JavaVirtualMachines/temurin-18.jdk/Contents/Home; using bundled JDK
keystore.seed
s3.client.mys3client.access_key
s3.client.mys3client.secret_key
s3.client.mys3client.session_token

@danielmitterdorfer danielmitterdorfer merged commit 4c7141a into elastic:master Dec 21, 2022
@danielmitterdorfer danielmitterdorfer deleted the pass-vars branch December 21, 2022 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch bug Something's wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants