Add new plugins onboarding to smoke tests framework #5255
Conversation
Signed-off-by: Zelin Hao <zelinhao@amazon.com>
zelinh
requested review from
dblock,
peterzhuamazon,
gaiksaya,
rishabh6788,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
There was a problem hiding this comment.
Thanks for adding more components to smoke tests. Couple of high level questions:
- If no files are found under 2.x folder, what is the expected behavior of the smoke tests framework? Will it look into default? If yes, instead of duplicating would recommend to keep as default for now.
- Wondering if we should get this reviewed from related component teams. Since they own the components, it would be a ideal to get feedback from them as to what qualifies as smoke testing for those components.
cc: @reta
src/test_workflow/smoke_test/smoke_tests_spec/2.x/index-management.yml
Outdated
Show resolved
Hide resolved
Signed-off-by: Zelin Hao <zelinhao@amazon.com>
Thanks @gaiksaya , I think it would make perfect sense: we probably need 1 (max 2) key flows that would give us high confidence that plugin works (or doesn't). |
|
@cwperks @shiv0408 @vikasvb90 @VijayanB @navneet1v @LantaoJin could you please review the suggested smoke tests for your components folks? Thank you! |
@reta would like to know bit more on the purpose of these tests. I might not be following all the conversations here. |
Oh sorry @navneet1v , the basic idea is to have a (small) set of smoke tests over the OpenSearch distribution with all core plugins installed (not individuals plugin as it is done now). We've been caught in the past that some plugins work perfectly in isolation but fail when installed at the same time. In scope of this change, there are smoke tests for k-nn plugin (among few others), if you could help to validate this part, does it make sense or not, would be much appreciated. Thank you. |
| @@ -0,0 +1,90 @@ | |||
| --- | |||
There was a problem hiding this comment.
Where can I find the cluster configuration for the cluster that all of these tests are run against?
I think these APIs are a good start and cover the core APIs of the security plugin. If we want to get more exhaustive with test coverage we can either try to derive tests from the open-api-specification repo or cover the APIs outlined on https://opensearch.org/docs/latest/security/access-control/api/ (note: some of these APIs require additional opensearch.yml settings in order to call and are disabled by default like the config update API)
There was a problem hiding this comment.
The smoke tests are intended to be light weighted so it runs on default configuration without any customized cluster configuration at this time.
There was a problem hiding this comment.
Hey @cwperks do you have any other suggestions on these API test cases for security plugins? Let me know if there is anything else you want me to onboard. Thanks!
There was a problem hiding this comment.
This covers some of the core APIs so I think this is a good base to start with.
Thanks that make sense. Thanks you for providing the details. Let me look at the smoke tests |
| @@ -0,0 +1,91 @@ | |||
| --- | |||
| info: | |||
There was a problem hiding this comment.
can I know some details from where we identified these tests? These tests will need modification as they are testing the feature. @reta what would be a process to fix these tests?
There was a problem hiding this comment.
We would like your inputs for this test as well. We want to test the essential APIs that won't cause any conflicts on the cluster.
There was a problem hiding this comment.
@reta what would be a process to fix these tests?
@navneet1v I am sorry but I don't know where these tests came from in the first place.
There was a problem hiding this comment.
I believe the tests came from here? https://github.com/opensearch-project/opensearch-api-specification/blob/main/spec/namespaces/knn.yaml
@zelinh Please correct if I am wrong.
There was a problem hiding this comment.
Yeah I added the tests from both that file and the documentation from https://opensearch.org/docs/latest/search-plugins/knn/api/ here as well.
I think for k-NN plugin, I didn't test too much as using k-NN plugin to train the models will somehow stop the cluster that blocks following tests.
I'm willing to get some inputs/helps from k-NN maintainers regarding of how we could contribute here.
There was a problem hiding this comment.
Hey @navneet1v Do you have any other suggestions on these smoke test cases for k-NN plugin? We tried to make it light weighted so I was hesitate about adding a model training. So far I run the API for get stats. Please let me know if you think there is something else I can add to this onboarding step. Thanks!
| @@ -0,0 +1,73 @@ | |||
| --- | |||
There was a problem hiding this comment.
These are one category of tests on policy APIs but we should also add one rest test on an ISM action which mutates cluster state such as rollover. Can you please try adding one such test as I am short on bandwidth?
Tagging some maintainers if they can help @shiv0408 , @bowenlan-amzn @soosinha
There was a problem hiding this comment.
@vikasvb90 make sense. @zelinh Can you try to add a test for rollover as well? Let me know if you need any help with that.
There was a problem hiding this comment.
@shiv0408 Could you help provide what APIs test are good to have here? i think you may talk about _plugins/_ism/policies/rollover_policy?
Please help list out some of them so I can add into this test framework. Thanks!
There was a problem hiding this comment.
Added additional rollover test cases for ISM. Please help take a look. Thanks! @shiv0408 @vikasvb90
Signed-off-by: Zelin Hao <zelinhao@amazon.com>
| 'Adams' | ||
| } | ||
| ] | ||
| /_plugins/_sql: |
There was a problem hiding this comment.
FYI, here is the documentation for ppl endpoints which are also essential for sql plugin. https://opensearch.org/docs/latest/search-plugins/sql/sql-ppl-api/#sample-explain-request-for-a-ppl-query
There was a problem hiding this comment.
Hey @RyanL1997 thanks for suggestions. I have added corresponding ppl tests. Please help take another look. Thanks!
Signed-off-by: Zelin Hao <zelinhao@amazon.com>
There was a problem hiding this comment.
Thanks @zelinh
This is a good start. Let's create issues for remaining components along with his PR as a sample, onboarding guide and make it part of on-boarding steps for new plugin as well when it gets added to the distribution.
Signed-off-by: Zelin Hao <zelinhao@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5255 +/- ##
==========================================
- Coverage 92.06% 92.02% -0.04%
==========================================
Files 202 202
Lines 7017 7020 +3
==========================================
Hits 6460 6460
- Misses 557 560 +3 ☔ View full report in Codecov by Sentry. |
Sure. I have created a PR for onboarding guide here. Will create issues for rest of the components shortly. Merging this PR for now. Thanks. |
Description
Add new plugins to our current smoke tests framework.
New onboarding plugins include
index-management,sql,securityandk-NNplugins. Each of them is onboarded with few essential API that could be run with smoke tests.Issues Resolved
#5247
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.