Updated integ-tests implementation to run all the OSD plugins. #3316

Divyaasm · 2023-03-16T00:08:59Z

Fix to issue #2144

Description

-Made changes to run OSD plugins altogether or individually using --component arg.
-OSD integtests when triggered from buildrepo are now executed via opensearch-dashboards-functional-test repo.
-Updated test-cases accordingly.

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.

codecov-commenter · 2023-03-16T00:14:27Z

Codecov Report

Merging #3316 (305a8ab) into main (9720afd) will decrease coverage by 0.32%.
The diff coverage is 55.55%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3316      +/-   ##
==========================================
- Coverage   91.74%   91.42%   -0.32%     
==========================================
  Files         172      172              
  Lines        4991     5017      +26     
==========================================
+ Hits         4579     4587       +8     
- Misses        412      430      +18

Impacted Files	Coverage Δ
...workflow/integ_test/integ_test_suite_opensearch.py	`65.85% <27.27%> (-26.15%)`	⬇️
src/test_workflow/integ_test/integ_test_suite.py	`94.00% <50.00%> (+8.28%)`	⬆️
...teg_test/integ_test_suite_opensearch_dashboards.py	`95.31% <89.28%> (-4.69%)`	⬇️

gaiksaya

I believe the methods need to be separated into child classes due to -t option. It only applies to dashboards and not opensearch.

gaiksaya · 2023-03-16T00:22:22Z

src/test_workflow/integ_test/integ_test_suite.py

@@ -114,7 +116,8 @@ def custom_node_endpoint_encoder(node_endpoint: NodeEndpoint) -> dict:

    def execute_integtest_sh(self, endpoint: str, port: int, security: bool, test_config: str) -> int:
        cluster_endpoint_port = [ClusterEndpoint("cluster1", [NodeEndpoint(endpoint, port, 9300)], [])]
-        return self.multi_execute_integtest_sh(cluster_endpoint_port, security, test_config)
+        script = ScriptFinder.find_integ_test_script("functionalTestDashboards", "https://github.com/opensearch-project/opensearch-dashboards-functional-test.git")


This will break for opensearch plugins. functional test only applies to dashboards. Also the I don't see ref anywhere, this will by default clone main. You need to get the repo link and ref from input manifest, https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.6.0/opensearch-dashboards-2.6.0.yml#L13-L15

I see the function execute_integtest_sh in used only for OSD which calls multi_execute_integtest_sh, whereas OS directly uses multi_execute_integtest_sh.

Yeah we need to change that behavior. Can you fix that as well? Adding @peterzhuamazon @prudhvigodithi for more input

Use the git lib in build repo to checkout ftrepo, do not need to use a find script call like this, because if cwd already have ftrepo content, you can just call the find script for integtest.sh and it will find it in root.

Sure Peter, started working on this change. Trying to fetch the integtest.sh from ftrepo based on ref branch from input manifest.

gaiksaya · 2023-03-16T00:22:45Z

src/test_workflow/integ_test/integ_test_suite.py

        if os.path.exists(script):
            if len(cluster_endpoints) == 1:
                single_data_node = cluster_endpoints[0].data_nodes[0]
-                cmd = f"bash {script} -b {single_data_node.endpoint} -p {single_data_node.port} -s {str(security).lower()} -v {self.bundle_manifest.build.version}"
+                cmd = f"bash {script} -b {single_data_node.endpoint} -p {single_data_node.port} -s {str(security).lower()} -t {self.component.name} -v {self.bundle_manifest.build.version}"


This is again not true for opensearch components,

I can make multi_execute_integtest_sh abstract and implement it separately for OS and OSD.

Just use a filter to remove -t if not needed.

Another problem is what if user does not enter any component, then it should default run everything in the yml. Therefore, you method might not work as it will include ftrepo as a component to run.

Hey @peterzhuamazon , the plan is to implement the multi_execute_integtest_sh in the child classes for OS and OSD. If that's the case I think the implementaion works well for OS too, let me know if I just need to add a filter to remove -t instead of having different implementations OS and OSD.

Currently I can see the tests are running sequentially for all OSD and OS plugins in yml if we do not provide --component names.

peterzhuamazon

Hi @Divyaasm,
Some comments here.
Thanks.

peterzhuamazon · 2023-03-17T16:55:22Z

src/test_workflow/integ_test/integ_test_suite.py

        if os.path.exists(script):
            if len(cluster_endpoints) == 1:
                single_data_node = cluster_endpoints[0].data_nodes[0]
-                cmd = f"bash {script} -b {single_data_node.endpoint} -p {single_data_node.port} -s {str(security).lower()} -v {self.bundle_manifest.build.version}"
+                cmd = f"bash {script} -b {single_data_node.endpoint} -p {single_data_node.port} -s {str(security).lower()} -t {self.component.name} -v {self.bundle_manifest.build.version}"


Just use a filter to remove -t if not needed.

peterzhuamazon · 2023-03-17T16:57:42Z

src/test_workflow/integ_test/integ_test_suite.py

@@ -114,7 +116,8 @@ def custom_node_endpoint_encoder(node_endpoint: NodeEndpoint) -> dict:

    def execute_integtest_sh(self, endpoint: str, port: int, security: bool, test_config: str) -> int:
        cluster_endpoint_port = [ClusterEndpoint("cluster1", [NodeEndpoint(endpoint, port, 9300)], [])]
-        return self.multi_execute_integtest_sh(cluster_endpoint_port, security, test_config)
+        script = ScriptFinder.find_integ_test_script("functionalTestDashboards", "https://github.com/opensearch-project/opensearch-dashboards-functional-test.git")


Use the git lib in build repo to checkout ftrepo, do not need to use a find script call like this, because if cwd already have ftrepo content, you can just call the find script for integtest.sh and it will find it in root.

peterzhuamazon · 2023-03-17T16:58:36Z

src/test_workflow/integ_test/integ_test_suite_opensearch.py

@@ -87,7 +88,8 @@ def __setup_cluster_and_execute_test_config(self, config: str) -> int:
        ) as (endpoints):
            os.chdir(self.work_dir)
            self.pretty_print_message("Running integration tests for " + self.component.name + " " + config)
-            return self.multi_execute_integtest_sh(endpoints, security, config)
+            script = ScriptFinder.find_integ_test_script(self.component.name, self.repo.working_directory)


Why do we need to call the scriptfinder here?

To make sure I separate the ftrepo clone for dashboards, Will change it back in the child class implementations.

peterzhuamazon · 2023-03-17T16:59:41Z

src/test_workflow/integ_test/integ_test_suite.py

        if os.path.exists(script):
            if len(cluster_endpoints) == 1:
                single_data_node = cluster_endpoints[0].data_nodes[0]
-                cmd = f"bash {script} -b {single_data_node.endpoint} -p {single_data_node.port} -s {str(security).lower()} -v {self.bundle_manifest.build.version}"
+                cmd = f"bash {script} -b {single_data_node.endpoint} -p {single_data_node.port} -s {str(security).lower()} -t {self.component.name} -v {self.bundle_manifest.build.version}"


Another problem is what if user does not enter any component, then it should default run everything in the yml. Therefore, you method might not work as it will include ftrepo as a component to run.

peterzhuamazon · 2023-03-17T18:58:58Z

Have offline discussion with @Divyaasm on this and there are several issues:

The ftrepo is not checked out at all as the initial implementation assuming only components within the manifest get checked out, so removing it from the manifest would cause it not to be checked out and actually trying to use the default gradle integtest to run cypress OSD test.
@Divyaasm will do a filter none check before sending the component to -t. So that we can make sure the component has value before using -t, else remove.
https://github.com/dblock/opensearch-build/blob/8babdde32e2262b9e2de3e3672daa198bef74c4f/src/build_workflow/builder_from_source.py#L48
Refine how the script finder is called once ftrepo is checked out.

Service is available
2023-03-17 18:43:05 INFO     ===============================================
2023-03-17 18:43:05 INFO     Running integration tests for customImportMapDashboards
2023-03-17 18:43:05 INFO     ===============================================
2023-03-17 18:43:05 INFO     Executing "bash /local/home/divyaasm/divyaasm/opensearch-build/scripts/default/integtest.sh -b localhost -p 5601 -s true -t customImportMapDashboards -v 2.6.0" in /tmp/tmpv4kasw4_/customImportMapDashboards
2023-03-17 18:43:05 INFO     Recording component test results for customImportMapDashboards at /local/home/divyaasm/divyaasm/opensearch-build/test-results/2e5262448e9e454cb9790bc9892f6415/integ-test/customImportMapDashboards/with-security/test-results
2023-03-17 18:43:05 INFO     Sending SIGKILL to PID 27123
2023-03-17 18:43:05 INFO     Process killed with exit code None
2023-03-17 18:43:05 INFO     Cleanup /tmp/tmpv4kasw4_/local-test-cluster/* content after the test
2023-03-17 18:43:06 INFO     Sending SIGKILL to PID 27241
2023-03-17 18:43:06 INFO     Process killed with exit code None
2023-03-17 18:43:06 INFO     Cleanup /tmp/tmpv4kasw4_/local-test-cluster/* content after the test
2023-03-17 18:43:06 INFO     Recording local cluster logs for customImportMapDashboards with test configuration as with-security at /local/home/divyaasm/divyaasm/opensearch-build/test-results/2e5262448e9e454cb9790bc9892f6415/integ-test/customImportMapDashboards/with-security/local-cluster-logs
2023-03-17 18:43:06 INFO     Recording local cluster logs for customImportMapDashboards with test configuration as with-security at /local/home/divyaasm/divyaasm/opensearch-build/test-results/2e5262448e9e454cb9790bc9892f6415/integ-test/customImportMapDashboards/with-security/local-cluster-logs
2023-03-17 18:43:06 INFO     opensearch distribution: tar
2023-03-17 18:43:06 INFO     opensearch-dashboards distribution: tar
2023-03-17 18:43:06 INFO     Downloading bundle artifact
2023-03-17 18:43:06 INFO     Downloading https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.6.0/5520/linux/x64/tar/dist/opensearch-dashboards/opensearch-dashboards-2.6.0-linux-x64.tar.gz into /tmp/tmpv4kasw4_/local-test-cluster/opensearch-dashboards-2.6.0-linux-x64.tar.gz ...
2023-03-17 18:43:07 INFO     Downloaded bundle to /tmp/tmpv4kasw4_/local-test-cluster/opensearch-dashboards-2.6.0-linux-x64.tar.gz
2023-03-17 18:43:07 INFO     Installing /tmp/tmpv4kasw4_/local-test-cluster/opensearch-dashboards-2.6.0-linux-x64.tar.gz in /tmp/tmpv4kasw4_/local-test-cluster/opensearch-dashboards-2.6.0
Removing securityDashboards...
Plugin removal complete
2023-03-17 18:43:30 INFO

Thanks.

gaiksaya

Looks good, Some minor improvements.

src/test_workflow/integ_test/integ_test_suite.py

gaiksaya · 2023-03-30T18:29:14Z

src/test_workflow/integ_test/integ_test_suite_opensearch_dashboards.py

@@ -81,6 +94,38 @@ def __setup_cluster_and_execute_test_config(self, config: str) -> int:
            os.chdir(self.work_dir)
            return self.execute_integtest_sh(endpoint, port, security, config)

+    def multi_execute_integtest_sh(self, cluster_endpoints: list, security: bool, test_config: str) -> int:
+        script = ScriptFinder.find_integ_test_script(self.component.name, self.repo.working_directory)


Add tests for this asserting that self.repo is always functionalTestDashboards for opensearch dashboards

gaiksaya · 2023-03-30T18:30:04Z

src/test_workflow/integ_test/integ_test_suite_opensearch_dashboards.py

+            build_manifest_opensearch_dashboards.components["functionalTestDashboards"].repository,
+            build_manifest_opensearch_dashboards.components["functionalTestDashboards"].commit_id,


nit: Use single quotes

Sure Sayali

peterzhuamazon

Hi @Divyaasm ,

Some good improvements, just some nits.

Thanks.

src/test_workflow/integ_test/integ_test_suite_opensearch_dashboards.py

peterzhuamazon · 2023-04-12T20:07:19Z

Thanks @Divyaasm this is a great addition to OSD test so that it can run parallel like OS test.

Divyaasm · 2023-04-20T23:16:50Z

src/test_workflow/integ_test/integ_test_suite_opensearch_dashboards.py

+            return {"endpoint": node_endpoint.endpoint, "port": node_endpoint.port, "transport": node_endpoint.transport}
+        if os.path.exists(script):
+            single_node = cluster_endpoints[0].data_nodes[0]
+            cmd = f"bash {script} -b {single_node.endpoint} -p {single_node.port} -s {str(security).lower()} -t {self.component.name} -v {self.bundle_manifest.build.version}"


Hi @tianleh , seems there is an issue in running integtests when -t is passed from python code base @line 107.
This command which is executed inside the cloned ftrepo is running integtests for all the components instead of the specified component.

Checked with @peterzhuamazon . Please raise the PR to FTRep once ready.

rishabh6788 · 2023-04-26T04:51:28Z

How about we introduce a new parameter, say sub-component that is only acknowledged when the calling class is IntegTestSuiteOpenSearchDashboards.
This way we don't have to have multiple implementations of custom_node_endpoint_encoder method here.

All we need to do would be add a condition to test which sub-class is calling this method and if it comes from IntegTestSuiteOpenSearchDashboards and if a subcomponent has been passed in the argument then add that to the command. We wouldn't have to change anything in the manifests as well.
Since I was debugging the issue with OSD integ test running for all components even after passing the specific component, to test I tried this slightly different approach to achieve this. Here is a draft PR just for the demo purpose.
I am okay with going ahead with the current implementation as well.
Thoughts? @peterzhuamazon @Divyaasm @gaiksaya @dblock

gaiksaya · 2023-04-26T18:40:01Z

Thoughts? @peterzhuamazon @Divyaasm @gaiksaya @dblock

The idea is good but the sub-component is actually the component of dashboards product. That's why in order to avoid confusion we went with refactoring the current set up. It is also true that they are sub-components of functional-test repo which is quite confusing but that is just from test perspective. For build workflow, we have similar thing where we build specific components. Wanted to keep the same for test as well.

rishabh6788 · 2023-04-26T20:45:09Z

The logic is such that if sub-component is passed when running OS integ-test it will be ignored, it will only be acknowledged when the test suite is executed from OSD class.

gaiksaya · 2023-04-26T21:20:54Z

The logic is such that if sub-component is passed when running OS integ-test it will be ignored, it will only be acknowledged when the test suite is executed from OSD class.

Yeah got that but again will be confusing for component as well as other teams between components and sub-components for OS and OSD. I am happy to revisit this logic in future. The reason we turnedmulti_execute_integtest_sh into abstract method was because eventually the way integration tests are run for OS and OSD may vary in future as these two are different products. This is the start of diverging path.

rishabh6788

LGTM!

gaiksaya · 2023-04-26T21:23:40Z

src/test_workflow/integ_test/integ_test_suite_opensearch_dashboards.py

+            return {"endpoint": node_endpoint.endpoint, "port": node_endpoint.port, "transport": node_endpoint.transport}
+        if os.path.exists(script):
+            single_node = cluster_endpoints[0].data_nodes[0]
+            cmd = f"bash {script} -b {single_node.endpoint} -p {single_node.port} -s {str(security).lower()} -t {self.component.name} -v {self.bundle_manifest.build.version} -o default"


if self.component.name is empty will the tests still run for all components by default?

The list of components is fetched here based on input argument --component if that's not passed it runs for all the components in the test manifest. self.component.name has the plugin name and has it's value from the manifest and is not empty in any case.

We alter the plugin names passed to self.component.name based on --component argument to the workflow but run them individually each time with -t component.name as per test manifest.

Thanks! This makes it clear.

gaiksaya

I believe opensearch-project/opensearch-dashboards-functional-test#644 needs to go in first before this PR can be merged?

Divyaasm · 2023-04-26T22:19:17Z

I believe opensearch-project/opensearch-dashboards-functional-test#644 needs to go in first before this PR can be merged?

Yes, and Peter seem to have PR related to integ-test OSD in line

Signed-off-by: Divya Madala <[email protected]> Changes updated Signed-off-by: Divya Madala <[email protected]> add -o parameter Signed-off-by: Divya Madala <[email protected]> Updated test cases Signed-off-by: Divya Madala <[email protected]>

Divyaasm requested review from dblock, peterzhuamazon, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi and tianleh as code owners March 16, 2023 00:08

github-actions bot added the seasoned-contributor label Mar 16, 2023

Divyaasm changed the title ~~Updated integ-tests implementation to all the run OSD plugins.~~ Updated integ-tests implementation to run all the OSD plugins. Mar 16, 2023

gaiksaya requested changes Mar 16, 2023

View reviewed changes

peterzhuamazon reviewed Mar 17, 2023

View reviewed changes

Divyaasm requested review from peterzhuamazon and gaiksaya March 30, 2023 17:49

gaiksaya requested changes Mar 30, 2023

View reviewed changes

peterzhuamazon requested changes Mar 30, 2023

View reviewed changes

Divyaasm force-pushed the integ-osd branch from 0e098be to f1a0790 Compare April 3, 2023 20:00

gaiksaya approved these changes Apr 10, 2023

View reviewed changes

peterzhuamazon approved these changes Apr 12, 2023

View reviewed changes

Divyaasm commented Apr 20, 2023

View reviewed changes

peterzhuamazon mentioned this pull request Apr 26, 2023

[META] RPM, Deb and Windows integration testing lacks jenkins automation #2994

Closed

18 tasks

Divyaasm mentioned this pull request Apr 26, 2023

Add an optional parameter -o to integtest.sh opensearch-project/opensearch-dashboards-functional-test#644

Merged

1 task

rishabh6788 approved these changes Apr 26, 2023

View reviewed changes

gaiksaya reviewed Apr 26, 2023

View reviewed changes

gaiksaya approved these changes Apr 26, 2023

View reviewed changes

Divyaasm mentioned this pull request Apr 27, 2023

Integtests are executed for all components even when -t component is passed when TEST_TYPE is checked to manifest. opensearch-project/opensearch-dashboards-functional-test#648

Closed

Updated integ-tests implementation for OSD

5dfa985

Signed-off-by: Divya Madala <[email protected]> Changes updated Signed-off-by: Divya Madala <[email protected]> add -o parameter Signed-off-by: Divya Madala <[email protected]> Updated test cases Signed-off-by: Divya Madala <[email protected]>

Divyaasm force-pushed the integ-osd branch from 305a8ab to 5dfa985 Compare April 27, 2023 20:43

Divyaasm mentioned this pull request May 3, 2023

Jenkins Job to run OSDInteg in Parallel #3465

Merged

1 task

peterzhuamazon approved these changes May 4, 2023

View reviewed changes

peterzhuamazon merged commit 904abbc into opensearch-project:main May 4, 2023

Divyaasm mentioned this pull request May 5, 2023

Add 3.0.0 osd test manifest #3477

Merged

Divyaasm deleted the integ-osd branch May 5, 2023 21:58

This was referenced May 8, 2023

Add jenkins file running all OSD integration tests in parallel #3412

Closed

[Enhancement] Run Dashboards integTest in parallel on Jenkins #2144

Closed

		build_manifest_opensearch_dashboards.components["functionalTestDashboards"].repository,
		build_manifest_opensearch_dashboards.components["functionalTestDashboards"].commit_id,

Updated integ-tests implementation to run all the OSD plugins. #3316

Updated integ-tests implementation to run all the OSD plugins. #3316

Conversation

Divyaasm commented Mar 16, 2023

Description

codecov-commenter commented Mar 16, 2023 • edited Loading

Codecov Report

gaiksaya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterzhuamazon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterzhuamazon commented Mar 17, 2023 • edited Loading

gaiksaya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterzhuamazon left a comment

Choose a reason for hiding this comment

peterzhuamazon commented Apr 12, 2023

Divyaasm Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rishabh6788 commented Apr 26, 2023

gaiksaya commented Apr 26, 2023

rishabh6788 commented Apr 26, 2023

gaiksaya commented Apr 26, 2023

rishabh6788 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaiksaya left a comment

Choose a reason for hiding this comment

Divyaasm commented Apr 26, 2023

codecov-commenter commented Mar 16, 2023 •

edited

Loading

peterzhuamazon left a comment •

edited

Loading

peterzhuamazon commented Mar 17, 2023 •

edited

Loading

Divyaasm Apr 20, 2023 •

edited

Loading