add plugins.removeList to allow remove plugins #526
Conversation
ruanyl
requested review from
bbarani,
TheAlgo,
DandyDeveloper,
peterzhuamazon,
prudhvigodithi and
gaiksaya
as code owners
resolved opensearch-project#383 Signed-off-by: Yulong Ruan <[email protected]>
ruanyl
force-pushed
the
feature-plugin-remove-list
branch
from
38f9da3 to
704bc9d
Compare
There was a problem hiding this comment.
Hi @ruanyl ,
Please make sure you update the changelog and versions.
And address the few comments I posted. Thanks 😄
| @@ -365,6 +365,12 @@ spec: | |||
| #!/usr/bin/env bash | |||
| set -euo pipefail | |||
|
|
|||
| {{- range $plugin := .Values.plugins.removeList }} | |||
| if ./bin/opensearch-plugin list | grep -q {{ $plugin }}; then | |||
There was a problem hiding this comment.
Why is there a different behavior b/w os and osd plugin removal?
Is it necessary to do a check before OS removal but not OSD?
There was a problem hiding this comment.
Thanks for catching this, we should check if the plugin installed before remove it for both OS and OSD, now the PR is updated.
| @@ -278,6 +278,8 @@ plugins: | |||
| enabled: false | |||
| installList: [] | |||
| # - example-fake-plugin-downloadable-url | |||
| removeList: [] | |||
| # - securityDashboards | |||
There was a problem hiding this comment.
Change this to something like - examplePluginName or similar.
charts/opensearch/values.yaml
Outdated
| @@ -491,6 +491,8 @@ plugins: | |||
| enabled: false | |||
| installList: [] | |||
| # - example-fake-plugin | |||
| removeList: [] | |||
| # - opensearch-ml | |||
Signed-off-by: Yulong Ruan <[email protected]>
Signed-off-by: Yulong Ruan <[email protected]>
Thank you for the guidance, this is the first time I make change to this repo, I followed some other PRs on how to bump the version and update the change logs, let me know if I missed anything :D |
| @@ -14,6 +14,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| ### Fixed | |||
| ### Security | |||
| --- | |||
| ## [2.26.0] | |||
There was a problem hiding this comment.
Please also update the links at the end of the changelog
There was a problem hiding this comment.
I see, now updated, I thought it's something automatically generated :D
| @@ -14,6 +14,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| ### Fixed | |||
| ### Security | |||
| --- | |||
| ## [2.29.0] | |||
Signed-off-by: Yulong Ruan <[email protected]>
Description
During the development phase, it's common to deploy a cluster with plugins that are still in progress. To facilitate the installation of these plugins, the current approach involves removing existing plugins and then installing the development versions. This process often requires manual intervention.
This pull request addresses this issue by introducing the
removeListto the Helm charts. With this enhancement, we can manage the installation and removal of plugins with eliminating the need for manual intervention.Addresses the issue described in #383, where user encountered challenges related to plugin removal.
Issues Resolved
resolved #383
Check List
For any changes to files within Helm chart directories:
CHANGELOG.mdupdated to reflect changeBy 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.