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.

Sign up for GitHub

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

Jump to bottom

Create custom integ test file for sql plugin. #1330

Merged
merged 1 commit into from
Feb 10, 2023
Merged

Create custom integ test file for sql plugin. #1330

merged 1 commit into from
Feb 10, 2023

Conversation

vamsimanohar
Copy link
Member

@vamsimanohar vamsimanohar commented Feb 10, 2023 •
edited
Loading

Signed-off-by: Vamsi Manohar [email protected]

Description

Background:
Integ tests during opensearch version release run on a separate local cluster. For sql integ tests to work properly, we need to setup prometheus datasource configuration in keystore. Since this is a customization specific to sql plugin, infra team advised us to maintain a custom script in sql plugin itself. Build system is intelligent enough to decide on which script to run.

Our custom script is a modification of below script.
Reference integtest.sh: https://github.com/opensearch-project/opensearch-build/blob/main/scripts/default/integtest.sh

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@vamsimanohar vamsimanohar requested a review from a team as a code owner February 10, 2023 00:29
@vamsimanohar vamsimanohar self-assigned this Feb 10, 2023
joshuali925
joshuali925 reviewed Feb 10, 2023
View reviewed changes
scripts/integtest.sh Outdated
USERNAME=`echo $CREDENTIAL | awk -F ':' '{print $1}'`
PASSWORD=`echo $CREDENTIAL | awk -F ':' '{print $2}'`

OPENSEARCH_HOME=`ps -ef | grep "[o]pensearch.path.home" | uniq | tr ' ' '\n' | grep "opensearch.path.home" | grep -Eo "=.*" | sed 's/=//g'`
Copy link
Member

@joshuali925 joshuali925 Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

no issue. ps -ef | grep -o "[o]pensearch.path.home=\S\+" | cut -d= -f2- | head -n1 is shorter if grep -Po is not allowed, but it still assumes no spaces in opensearch path. is it a safe assumption?

(if grep has -P option i would probably use something like ps -ef | grep -Po "(?<=[o]pensearch.path.home=).+?(?= -D|$)" to handle spaces)

Also does this require the script to run against local running cluster only? e.g. no remote clusters, no integTest clusters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, I have asked infra team to provide the location of bin. They provided me this. I am not sure if PCRE is available. but let me confirm with them.

joshuali925
joshuali925 reviewed Feb 10, 2023
View reviewed changes
scripts/integtest.sh Outdated

OPENSEARCH_HOME=`ps -ef | grep "[o]pensearch.path.home" | uniq | tr ' ' '\n' | grep "opensearch.path.home" | grep -Eo "=.*" | sed 's/=//g'`

curl -SL https://raw.githubusercontent.com/opensearch-project/sql/2.5/integ-test/src/test/resources/datasource/datasources.json -o $OPENSEARCH_HOME/datasources.json
Copy link
Member

Choose a reason for hiding this comment

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

should this be main or 2.5 (or HEAD)?

also nit pick to not break if it has spaces

Suggested change
curl -SL https://raw.githubusercontent.com/opensearch-project/sql/2.5/integ-test/src/test/resources/datasource/datasources.json -o $OPENSEARCH_HOME/datasources.json
curl -SL https://raw.githubusercontent.com/opensearch-project/sql/2.5/integ-test/src/test/resources/datasource/datasources.json -o "$OPENSEARCH_HOME"/datasources.json

Copy link
Member Author

Choose a reason for hiding this comment

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

It can be anything...but main would be better. will make the changes.

joshuali925
joshuali925 reviewed Feb 10, 2023
View reviewed changes
scripts/integtest.sh Outdated
yes | $OPENSEARCH_HOME/bin/opensearch-keystore add-file plugins.query.federation.datasources.config $OPENSEARCH_HOME/datasources.json


curl -k --request POST --url https://localhost:9200/_nodes/reload_secure_settings --header 'content-type: application/json' --data '{"secure_settings_password":""}' --user $CREDENTIAL
Copy link
Member

Choose a reason for hiding this comment

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

use bind address and port instead of localhost:9200?

joshuali925
joshuali925 previously approved these changes Feb 10, 2023
View reviewed changes
Yury-Fridlyand
Yury-Fridlyand reviewed Feb 10, 2023
View reviewed changes
scripts/integtest.sh Outdated Show resolved Hide resolved
scripts/integtest.sh
curl -k --request POST --url https://$BIND_ADDRESS:$BIND_PORT/_nodes/reload_secure_settings --header 'content-type: application/json' --data '{"secure_settings_password":""}' --user $CREDENTIAL


./gradlew integTest -Dopensearch.version=$OPENSEARCH_VERSION -Dbuild.snapshot=$SNAPSHOT -Dtests.rest.cluster="$BIND_ADDRESS:$BIND_PORT" -Dtests.cluster="$BIND_ADDRESS:$BIND_PORT" -Dtests.clustername="opensearch-integrationtest" -Dhttps=$SECURITY_ENABLED -Duser=$USERNAME -Dpassword=$PASSWORD --console=plain
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add option to pass rest of the command line to gradlew. It allows you to specify test filter (--tests "...") or debug flag -Dtest.debug and so on.

Copy link
Member Author

@vamsimanohar vamsimanohar Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is copied from https://github.com/opensearch-project/opensearch-build/blob/main/scripts/default/integtest.sh. The intent is to customize the script to include datasource creation.

They always default on parameters. we can expand the functionality whenever we need.

@dai-chen dai-chen added the infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. label Feb 10, 2023
@vamsimanohar
Create custom integ test file for sql plugin.
97f45df 
Sql Plugin integ tests require add files to keystore to add prometheus datasource. So adding a custom integ script which can be consumed by build repo while triggering integ tests during every release.

Signed-off-by: Vamsi Manohar <[email protected]>
@vamsimanohar vamsimanohar merged commit 4df8ed7 into opensearch-project:main Feb 10, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 10, 2023
@vamsimanohar @github-actions
Create custom integ test file for sql plugin. (#1330)
a1c9a64 
Sql Plugin integ tests require add files to keystore to add prometheus datasource. So adding a custom integ script which can be consumed by build repo while triggering integ tests during every release.

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 4df8ed7)
This was referenced Feb 10, 2023
Merged
Merged
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 10, 2023
@vamsimanohar @github-actions
Create custom integ test file for sql plugin. (#1330)
8d26518 
Sql Plugin integ tests require add files to keystore to add prometheus datasource. So adding a custom integ script which can be consumed by build repo while triggering integ tests during every release.

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 4df8ed7)
vamsimanohar added a commit that referenced this pull request Feb 14, 2023
@opensearch-trigger-bot @vamsimanohar
Create custom integ test file for sql plugin. (#1330) (#1333)
017202b 
Sql Plugin integ tests require add files to keystore to add prometheus datasource. So adding a custom integ script which can be consumed by build repo while triggering integ tests during every release.

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 4df8ed7)

Co-authored-by: vamsi-amazon <[email protected]>
vamsimanohar added a commit that referenced this pull request Feb 14, 2023
@opensearch-trigger-bot @vamsimanohar
Create custom integ test file for sql plugin. (#1330) (#1332)
3350150 
Sql Plugin integ tests require add files to keystore to add prometheus datasource. So adding a custom integ script which can be consumed by build repo while triggering integ tests during every release.

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 4df8ed7)

Co-authored-by: vamsi-amazon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yury-Fridlyand Yury-Fridlyand Yury-Fridlyand approved these changes

@penghuo penghuo penghuo approved these changes

@joshuali925 joshuali925 joshuali925 left review comments

Assignees

@vamsimanohar vamsimanohar

Labels
backport 2.x backport 2.5 infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vamsimanohar @penghuo @joshuali925 @Yury-Fridlyand @dai-chen