Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add performance test scripts #1671
Add performance test scripts #1671
Changes from 17 commits
7affa4d
e93c8c7
7247194
440051d
1347f5c
5513bbf
9fef51e
7648533
4f06122
57c8977
e65e19e
eee81d0
dea8876
f9f5674
c01ea40
75e8d53
74bf99a
a19db11
fad13db
f79d5bd
a2a40d5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we get security from bundle manifest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would prevent us from parallelizing the tests in the future. We want to kick off two tests for the security based bundles, which can be done from the Jenkins pipeline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like security enabled/disabled should be decided based on
security
param inmanifest.components
. To parallelize the test we can pass 2 different bundle manifest, one with security enabled and other disabled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will have to publish these manifests to our buckets and distributions, which we do not currently.
Moving it out additionally gives more flexibility for extensibility moving forward for other use cases rather than updating/adding manifests for every build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do publish manifests to our buckets.
without-security
tests is something that will run irrespective of what's in the manifest but forwith-security
can we add a check in the codebase and then start the test only if the security component is present?If the performance tests are to be run nightly it is highly possible that security component is not present initially in the manifest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refer to the comment below - #1671 (comment)
What I was trying to say is - we have a single manifest which might or might not have security in the components. By enabling the logic at the Jenkins level, we can execute the two runs in parallel, giving us more control over the test runs and statuses.
Also, it is based directly on the manifest. Look here for more - https://github.com/opensearch-project/opensearch-build/pull/1671/files#diff-4debf5e3ece07145d8395f15df88f49b8b784a274cead94e2a94c8b7152c11efR75
All I am doing is pulling out that logic for better control at the top level of job execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we expose retry options to the command line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems like a good option to have. I do have a few things to follow up on post - based on the test run outcomes once everything is up and running.
Added an issue here for tracking - opensearch-project/OpenSearch#2718