Add native plugin validation framework #5529

Divyaasm · 2025-05-20T17:49:55Z

Description

Add native plugin validation framework for all distributions

Issues Resolved

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.

prudhvigodithi · 2025-05-20T18:11:08Z

src/validation_workflow/validation.py

+            plugin_list = [i["name"] for i in response if i["name"] not in installed_plugins_list]
+            plugin_list.remove("examples")
+            plugin_list.remove("build.gradle")
+            plugin_list.remove("identity-shiro")  # Assuming security plugin enabled in the artifacts


@Divyaasm can you add some more details like since identity-shiro is also an identity plugin and 2 similar identity plugins cannot be installed.

Signed-off-by: Divya Madala <[email protected]>

codecov · 2025-05-20T20:11:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (6eba98d) to head (3d432f1).
Report is 22 commits behind head on main.

❌ Your project status has failed because the head coverage (0.00%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@     Coverage Diff      @@
##   main   #5529   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gaiksaya

I would still recommend to add gradle task in core repo to list native plugins as projects and grab those instead of parsing the GitHub. Wondering if core maintainers (@andrross @reta ) have any better mechanism or preference for this.

Thanks!

gaiksaya · 2025-05-22T19:50:32Z

src/validation_workflow/docker/validation_docker.py

+                            native_plugins_list = self.get_native_plugin_list(self.tmp_dir.name)
+                            for container in ["opensearch-node1", "opensearch-node2"]:
+                                for native_plugin in native_plugins_list:
+                                    command = f'docker exec -it {container} sh .' + os.sep + 'bin' + os.sep + f'opensearch-plugin install --batch {native_plugin}'


Why not add RUN/CMD opensearch-plugin install --batch {native_plugin} in the docker compose file.
This is not how user might be doint it right?

This can be done in both ways! I have opted this as it seemed straight-forward, we have container details to run the command and we restart docker compose file after installing plugins. So we should be good AFAIK.

For the approach you have suggested, Existing compose file needs to be modified, we will need to update the docker-compose file dynamically and bring docker up!

That's how the users will do it. Add the required plugins in the docker-compose files for their set up. I believe its the right way to go than executing a command against container.

I think we should not change dockerfile or dockercomposefiles especially the latter one is now the official docker compose files on our website since the WP migration.

I am ok with having cmd in py to do this test as it is just a test, not related to functionalities of dockerfile and dockercomposefiles.

Thanks.

gaiksaya · 2025-05-22T19:52:50Z

src/validation_workflow/tar/validation_tar.py

+                try:
+                    self.filename = os.path.basename(self.args.file_path.get(project))
+                    execute('mkdir ' + os.path.join(self.tmp_dir.path, project) + ' | tar -xzf ' + os.path.join(str(self.tmp_dir.path), self.filename) + ' -C ' + os.path.join(self.tmp_dir.path, project) + ' --strip-components=1', ".", True, False)  # noqa: E501
+                    if self.args.validate_native_plugin:


We want to test everytime no matter the arg right? Wondering why we have if condition here.

Agree. I think we should always test native plugin installation as part of validation workflow.

Or maybe we could have an argument to pass in specific native plugin we want to test on instead of the whole lists. By default, it would test the entire list. I think it could be helpful. WDYT? @gaiksaya @prudhvigodithi @rishabh6788

I assumed that, this will be an argument passed from jenkins. We can set it to true/false by default based on our requirement.

I believe its better to pass the whole list. I know discovery-ec2 and repository-s3 are some of the commonly used plugins but better to test everything while we are in there. It's just installation so should be fast.

src/validation_workflow/validation.py

gaiksaya · 2025-05-22T19:54:57Z

src/validation_workflow/validation.py

+            plugin_list = [i["name"] for i in response if i["name"] not in installed_plugins_list]
+            plugin_list.remove("examples")
+            plugin_list.remove("build.gradle")
+            plugin_list.remove("identity-shiro")  # Since the security plugin is enabled in the artifacts and identity-shiro is also an identity plugin, we cannot have both the plugins installed together. # noqa: E501


Suggested change

plugin_list.remove("identity-shiro") # Since the security plugin is enabled in the artifacts and identity-shiro is also an identity plugin, we cannot have both the plugins installed together. # noqa: E501

# Since the security plugin is enabled in the artifacts and identity-shiro is also an identity plugin, we cannot have both the plugins installed together.

plugin_list.remove("identity-shiro") # noqa: E501

zelinh

Can you also update our README file since we will be introducing new functionality?
https://github.com/opensearch-project/opensearch-build/blob/main/src/validation_workflow/README.md

zelinh · 2025-05-22T20:29:49Z

src/validation_workflow/tar/validation_tar.py

+                try:
+                    self.filename = os.path.basename(self.args.file_path.get(project))
+                    execute('mkdir ' + os.path.join(self.tmp_dir.path, project) + ' | tar -xzf ' + os.path.join(str(self.tmp_dir.path), self.filename) + ' -C ' + os.path.join(self.tmp_dir.path, project) + ' --strip-components=1', ".", True, False)  # noqa: E501
+                    if self.args.validate_native_plugin:


Agree. I think we should always test native plugin installation as part of validation workflow.

Or maybe we could have an argument to pass in specific native plugin we want to test on instead of the whole lists. By default, it would test the entire list. I think it could be helpful. WDYT? @gaiksaya @prudhvigodithi @rishabh6788

zelinh · 2025-05-22T20:32:03Z

src/validation_workflow/validation.py

        path = os.path.exists(os.path.join(work_dir, "plugins", "opensearch-security"))
        return path

+    def install_native_plugin(self, path: str) -> None:


Wonder how do we validate the cluster can be exit gracefully after native plugin installation. I think we saw an issue when exiting previously.

At the end of validation, cleanup() method stops the cluster before finishing the job, if there is any issue an exception is raised.

reta · 2025-05-24T20:56:12Z

I would still recommend to add gradle task in core repo to list native plugins as projects and grab those instead of parsing the GitHub. Wondering if core maintainers (@andrross @reta ) have any better mechanism or preference for this.

Thanks!

Thanks @gaiksaya , I think just listing the repo plugins folder (https://github.com/opensearch-project/OpenSearch/tree/main/plugins) would the simplest way to get all list "builtin" plugins.

peterzhuamazon · 2025-05-29T18:28:01Z

src/validation_workflow/docker/validation_docker.py

+                            native_plugins_list = self.get_native_plugin_list(self.tmp_dir.name)
+                            for container in ["opensearch-node1", "opensearch-node2"]:
+                                for native_plugin in native_plugins_list:
+                                    command = f'docker exec -it {container} sh .' + os.sep + 'bin' + os.sep + f'opensearch-plugin install --batch {native_plugin}'


I think we should not change dockerfile or dockercomposefiles especially the latter one is now the official docker compose files on our website since the WP migration.

I am ok with having cmd in py to do this test as it is just a test, not related to functionalities of dockerfile and dockercomposefiles.

Thanks.

peterzhuamazon · 2025-05-29T18:30:30Z

src/validation_workflow/validation.py

+    def get_native_plugin_list(self, workdir: str) -> list:
+        bundle_manifest = BundleManifest.from_path(os.path.join(workdir, "manifest.yml"))
+        commit_id = bundle_manifest.components["OpenSearch"].commit_id
+        plugin_url = f"https://api.github.com/repos/opensearch-project/OpenSearch/contents/plugins?ref={commit_id}"


Is the native plugin list not in manifest that you have to retrieve from github api?

The github api returns the plugin(native plugins list) folder based on the commit id from the bundle manifest.

Yeah but build manifest at least include all the plugin names without the need to go online again.
I am not sure about the bundle manifest tho.

Yes, but we need native-pugins list https://github.com/opensearch-project/OpenSearch/tree/main/plugins, not the os plugins which are in the manifest.

No, if you check bundle manifest under opensearch core section, there are core-plugins key that contains all the core plugins.

peterzhuamazon · 2025-05-29T18:31:13Z

src/validation_workflow/docker/validation_docker.py

+                            subprocess.run(f'docker cp opensearch-node1:/usr/share/opensearch/plugins {self.tmp_dir.name}/plugins',
+                                           shell=True, stdout=PIPE, stderr=PIPE, universal_newlines=True)


Can you explain why do you need to copy the plugins folder, unlike other dists?

Yeah so, the query-insights in 2.x releases is os plugin as well as in native plugin folder. So inorder to restrict installing the plugin twice. I'm removing plugins list in the artifact from the native plugins list retrieved from github api. Which will need plugins folder available locally for the operations. So I've copied it to tmp folder from the container for getting the final list of native plugins which will be later iterated and installed.

Cant you just get the list and save to a var?
I still couldnt wrap my head around why we need to docker cp the plugin folders again.

Thanks.

Signed-off-by: Divya Madala <[email protected]>

peterzhuamazon

Thanks @Divyaasm !

Please wrap possible improvements in an issue and attached to this PR.

Before merging, Thanks!

Divyaasm requested review from gaiksaya, peterzhuamazon, prudhvigodithi, rishabh6788, tianleh and zelinh as code owners May 20, 2025 17:49

github-project-automation bot added this to Engineering Effectiveness Board May 20, 2025

github-actions bot added the distinguished-contributor label May 20, 2025

Divyaasm force-pushed the nativeplugin branch from 4dce708 to 5d2f954 Compare May 20, 2025 17:58

prudhvigodithi reviewed May 20, 2025

View reviewed changes

Add native plugin validation framework

190e34d

Signed-off-by: Divya Madala <[email protected]>

Divyaasm force-pushed the nativeplugin branch from 5d2f954 to 190e34d Compare May 20, 2025 18:28

fix tests

70ca68d

Signed-off-by: Divya Madala <[email protected]>

Divyaasm force-pushed the nativeplugin branch from 69cfa86 to 70ca68d Compare May 20, 2025 18:44

gaiksaya reviewed May 22, 2025

View reviewed changes

zelinh reviewed May 22, 2025

View reviewed changes

peterzhuamazon reviewed May 29, 2025

View reviewed changes

Add requested changes

3d432f1

Signed-off-by: Divya Madala <[email protected]>

Divyaasm mentioned this pull request Jun 5, 2025

[Bug]: Native plugin installation can't be done on multiple nodes during docker validation. #5561

Open

peterzhuamazon approved these changes Jun 6, 2025

View reviewed changes

github-project-automation bot moved this to 👀 In Review in Engineering Effectiveness Board Jun 6, 2025

Divyaasm merged commit 1bfa697 into opensearch-project:main Jun 6, 2025
13 checks passed

github-project-automation bot moved this from 👀 In Review to ✅ Done in Engineering Effectiveness Board Jun 6, 2025

Divyaasm mentioned this pull request Jul 7, 2025

Validate native plugin installation #2859

Closed

2 tasks

	plugin_list.remove("identity-shiro") # Since the security plugin is enabled in the artifacts and identity-shiro is also an identity plugin, we cannot have both the plugins installed together. # noqa: E501
	# Since the security plugin is enabled in the artifacts and identity-shiro is also an identity plugin, we cannot have both the plugins installed together.
	plugin_list.remove("identity-shiro") # noqa: E501

		subprocess.run(f'docker cp opensearch-node1:/usr/share/opensearch/plugins {self.tmp_dir.name}/plugins',
		shell=True, stdout=PIPE, stderr=PIPE, universal_newlines=True)

Add native plugin validation framework #5529

Add native plugin validation framework #5529

Uh oh!

Conversation

Divyaasm commented May 20, 2025

Description

Issues Resolved

Uh oh!

prudhvigodithi May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gaiksaya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zelinh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reta commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterzhuamazon May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Divyaasm May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Divyaasm May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterzhuamazon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

prudhvigodithi May 20, 2025 •

edited

Loading

codecov bot commented May 20, 2025 •

edited

Loading

reta commented May 24, 2025 •

edited

Loading

peterzhuamazon May 29, 2025 •

edited

Loading

Divyaasm May 29, 2025 •

edited

Loading

Divyaasm May 29, 2025 •

edited

Loading