Skip to content
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

allow adding plugins and change defaultmode for opensearch dashboards #342

Merged
merged 11 commits into from
Dec 8, 2022

Conversation

BionIT
Copy link
Contributor

@BionIT BionIT commented Oct 27, 2022

Signed-off-by: Lu Yu nluyu@amazon.com

Description

Similar to #71 , the change would allow installing external plugins when container starts for opensearch dashboards.
In addition, this change allows to specify defaultMode for the opensearch_dashboards.yml file which is mounted as configmap
Changelog and Readme are updated.
Changes were tested using k8s cluster using the chart

Issues Resolved

#341

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • Helm chart version bumped
  • Helm chart CHANGELOG.md updated to reflect change

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.

… yaml file

Signed-off-by: Lu Yu <nluyu@amazon.com>
Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BionIT Can you refactor the PR according to contribution guidelines mentioned in CONTRIBUTING.md

@BionIT
Copy link
Contributor Author

BionIT commented Oct 28, 2022

@BionIT Can you refactor the PR according to contribution guidelines mentioned in CONTRIBUTING.md

@TheAlgo Sure! Do we need to bump appVersion from 2.3.0 to 2.3.1 as well or just version? The linked PR did both, but OSD latest release is at 2.3.0

@peterzhuamazon
Copy link
Member

peterzhuamazon commented Oct 28, 2022

@BionIT Can you refactor the PR according to contribution guidelines mentioned in CONTRIBUTING.md

@TheAlgo Sure! Do we need to bump appVersion from 2.3.0 to 2.3.1 as well or just version? The linked PR did both, but OSD latest release is at 2.3.0

appversion keep 2.3.0, just bump version.

Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
@BionIT
Copy link
Contributor Author

BionIT commented Oct 28, 2022

@BionIT Can you refactor the PR according to contribution guidelines mentioned in CONTRIBUTING.md

@TheAlgo Sure! Do we need to bump appVersion from 2.3.0 to 2.3.1 as well or just version? The linked PR did both, but OSD latest release is at 2.3.0

appversion keep 2.3.0, just bump version.

Thanks @peterzhuamazon and @TheAlgo ! I have bumped version for osd chart, linter was complaining about version bump for os, but I only updated documentation for os chart, and per CONTRIBUTING.md: Chart version should be bumped for every code change except docoumentation change. Please let me know if any advice~

@TheAlgo
Copy link
Member

TheAlgo commented Oct 29, 2022

@BionIT Can you refactor the PR according to contribution guidelines mentioned in CONTRIBUTING.md

@TheAlgo Sure! Do we need to bump appVersion from 2.3.0 to 2.3.1 as well or just version? The linked PR did both, but OSD latest release is at 2.3.0

appversion keep 2.3.0, just bump version.

Thanks @peterzhuamazon and @TheAlgo ! I have bumped version for osd chart, linter was complaining about version bump for os, but I only updated documentation for os chart, and per CONTRIBUTING.md: Chart version should be bumped for every code change except docoumentation change. Please let me know if any advice~

Yes this is not ideal but this is a backlog for this repo. Our linter is configured this way. @prudhvigodithi / @peterzhuamazon We need to fix this ASAP. In the meanwhile @BionIT can you bump minor version for OS chart as well so that linter is happy

Signed-off-by: Lu Yu <nluyu@amazon.com>
@BionIT
Copy link
Contributor Author

BionIT commented Oct 29, 2022

@BionIT Can you refactor the PR according to contribution guidelines mentioned in CONTRIBUTING.md

@TheAlgo Sure! Do we need to bump appVersion from 2.3.0 to 2.3.1 as well or just version? The linked PR did both, but OSD latest release is at 2.3.0

appversion keep 2.3.0, just bump version.

Thanks @peterzhuamazon and @TheAlgo ! I have bumped version for osd chart, linter was complaining about version bump for os, but I only updated documentation for os chart, and per CONTRIBUTING.md: Chart version should be bumped for every code change except docoumentation change. Please let me know if any advice~

Yes this is not ideal but this is a backlog for this repo. Our linter is configured this way. @prudhvigodithi / @peterzhuamazon We need to fix this ASAP. In the meanwhile @BionIT can you bump minor version for OS chart as well so that linter is happy

Sure, @TheAlgo! Thanks for clarifying! I have bumped version for os chart as well and the linter is happy now :).

@@ -47,6 +47,9 @@ spec:
- name: config
configMap:
name: {{ template "opensearch-dashboards.fullname" . }}-config
{{- if .Values.opensearchDashboardsYml.defaultMode }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use some other intuitive name here. I don't mind directly adding defaultMode as well. @prudhvigodithi @DandyDeveloper @peterzhuamazon Opinions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple configmaps, and I think being explicit would help if we introduce more custom configurations about defaultmode for those configmaps. Open to a better naming if we have any

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterzhuamazon
Copy link
Member

Adding @prudhvigodithi to take a look at this.

@TheAlgo
Copy link
Member

TheAlgo commented Nov 19, 2022

@prudhvigodithi @DandyDeveloper Please take a look when free. Thanks

@seanneumann
Copy link

Heya, what's the latest on getting this reviewed? I'd like to get this merged.

set -e
{{- range $plugin := .Values.plugins.installList }}
./bin/opensearch-dashboards-plugin install {{ $plugin }}
{{- end }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @BionIT quick question, for OpenSearch we have a --batch option to force install without a prompt, is there a same flag or scenario for opensearch-dashboards-plugin, I dont see it in the document though https://opensearch.org/docs/latest/dashboards/install/plugins/, please test this and share your update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @prudhvigodithi, --batch is not supported by opensearch-dashboards-plugin(error: unknown option '--batch' ), I have been using this script to install plugins and never had prompts.

@prudhvigodithi
Copy link
Collaborator

prudhvigodithi commented Nov 23, 2022

Hey @BionIT thanks for the PR and contribution, please check the following points.

  • Adding default mode is not a bad idea but can you share what issues faced for this to get added, if not lets keep what kubernetes sets to the defaultMode to the volume.
    - Since the change is particular to the OpenSearch Dashboard, we dont have to increment the versions in chart.yaml for OpenSearch, only for the dashboard chart is required.
  • Can you also please resolve the conflicts?

@TheAlgo @DandyDeveloper @peterzhuamazon @bbarani @seraphjiang

Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
@BionIT
Copy link
Contributor Author

BionIT commented Nov 25, 2022

Hey @BionIT thanks for the PR and contribution, please check the following points.

  • Adding default mode is not a bad idea but can you share what issues faced for this to get added, if not lets keep what kubernetes sets to the defaultMode to the volume.
    - Since the change is particular to the OpenSearch Dashboard, we dont have to increment the versions in chart.yaml for OpenSearch, only for the dashboard chart is required.
  • Can you also please resolve the conflicts?

@TheAlgo @DandyDeveloper @peterzhuamazon @bbarani @seraphjiang

Per request, resolved conflicts.
Per allowing users to set defaultMode, k8s allow users to specify defaultmode for volumes, why our helm chart want to restrict it to readonly? We have use case to dynamically change the values in the yaml and the current set up prevents that.

Signed-off-by: Lu Yu <nluyu@amazon.com>
@peterzhuamazon
Copy link
Member

Pinging @DandyDeveloper @TheAlgo @prudhvigodithi again for the checks on @BionIT latest changes.
I am ok with this approach, and if any of you give an approval we can follow up with a merge.

Thanks.

Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, thanks for being patient on this.
@rishabh6788 please approve.

Thanks.

@peterzhuamazon
Copy link
Member

Trying to fix the issues with linting.

@peterzhuamazon
Copy link
Member

peterzhuamazon commented Dec 7, 2022

Related to kind action need to update to 1.3.0.

@peterzhuamazon peterzhuamazon merged commit 867d5fd into opensearch-project:main Dec 8, 2022
@peterzhuamazon
Copy link
Member

Hi @BionIT please have a follow-up backport PR to 1.x.
Thanks.

@peterzhuamazon
Copy link
Member

Backport 1.x: #359

prathaptce pushed a commit to prathaptce/helm-charts that referenced this pull request Mar 24, 2023
…opensearch-project#342)

* allow adding plugins and change defaultmode for opensearch dashboards yaml file

Signed-off-by: Lu Yu <nluyu@amazon.com>

* bump version and update changelog

Signed-off-by: Lu Yu <nluyu@amazon.com>

* add new line

Signed-off-by: Lu Yu <nluyu@amazon.com>

* bump version for os

Signed-off-by: Lu Yu <nluyu@amazon.com>

* resolve conflict in changelog

Signed-off-by: Lu Yu <nluyu@amazon.com>

* trigger build

Signed-off-by: Lu Yu <nluyu@amazon.com>

Signed-off-by: Lu Yu <nluyu@amazon.com>
Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
bbarani added a commit that referenced this pull request Apr 24, 2023
* Update appVersion for 2.4.0 release (#350)

Signed-off-by: Zelin Hao <zelinhao@amazon.com>

Signed-off-by: Zelin Hao <zelinhao@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Add .whitesource configuration file (#353)

Co-authored-by: mend-for-github-com[bot] <50673670+mend-for-github-com[bot]@users.noreply.github.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Resolve Kind Cluster not able to be built in PR checks (#356)

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Fix the kindest/node docker images versions (#357)

* Resolve Kind Cluster not able to be built in PR checks

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

* Fix the kindest/node versions on docker images

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Resolve Kind Cluster not able to be built in PR checks (#358)

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* allow adding plugins and change defaultmode for opensearch dashboards (#342)

* allow adding plugins and change defaultmode for opensearch dashboards yaml file

Signed-off-by: Lu Yu <nluyu@amazon.com>

* bump version and update changelog

Signed-off-by: Lu Yu <nluyu@amazon.com>

* add new line

Signed-off-by: Lu Yu <nluyu@amazon.com>

* bump version for os

Signed-off-by: Lu Yu <nluyu@amazon.com>

* resolve conflict in changelog

Signed-off-by: Lu Yu <nluyu@amazon.com>

* trigger build

Signed-off-by: Lu Yu <nluyu@amazon.com>

Signed-off-by: Lu Yu <nluyu@amazon.com>
Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Fix path in securityConfig section on OpenSearch (values.yaml) (#344)

* fix securityConfig.path

Signed-off-by: Ruslan Gainanov <gromrx1@gmail.com>

* add link to issue

Signed-off-by: Ruslan Gainanov <gromrx1@gmail.com>

Signed-off-by: Ruslan Gainanov <gromrx1@gmail.com>
Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Update appVersion to 2.4.1 (#363)

* Update appVersion to 2.4.1

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

* Update appVersion to 2.4.1

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

* Fix changelog

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

* Fix changelog

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

* Fix version

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Fix version for OpenSearch

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Fix version for OpenSearch-Dasboards

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Add hostPort support for http- and transport-ports (#336)

Signed-off-by: Christian Kuhn <phello@gmx.de>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Updated MAINTAINERS.md to match recommended opensearch-project format. (#367)

Signed-off-by: dblock <dblock@amazon.com>

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Bump OS and OSD version to 2.5.0 (#373)

Signed-off-by: Rishabh Singh <sngri@amazon.com>

Signed-off-by: Rishabh Singh <sngri@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Created untriaged issue workflow. (#382)

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Bump OpenSearch and Dashboards to 2.6.0 (#393)

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Updating the CODEOWNERS file (#399)

Signed-off-by: bbarani <bbarani@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Add lifecycle support in opensearch container (#376)

* Add lifecycle support in opensearch container

Signed-off-by: josephteddick <josephteddick@gmail.com>

* lifecycle support PR cleanup

Signed-off-by: josephteddick <josephteddick@gmail.com>

---------

Signed-off-by: josephteddick <josephteddick@gmail.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* feat: Add user-defined labels option to ingress (#390)

Signed-off-by: Jason Witkowski <jwitkowski@zscaler.com>
Co-authored-by: Jason Witkowski <jwitkowski@zscaler.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Update deployment.yaml

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Update values.yaml

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Update values.yaml

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Update statefulset.yaml

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Updating chart version and changelog.md

Signed-off-by: Prathap Mahalingam (Nokia) <prathap.mahalingam@nokia.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Add lifecycle support in opensearch container (#376)

* Add lifecycle support in opensearch container

Signed-off-by: josephteddick <josephteddick@gmail.com>

* lifecycle support PR cleanup

Signed-off-by: josephteddick <josephteddick@gmail.com>

---------

Signed-off-by: josephteddick <josephteddick@gmail.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Add github-merit-badger.yml (#408)

Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Service port for performance analyzer (#346)

* Performance analyzer port mapping

Signed-off-by: Philipp Hölscher <phoelsch@outlook.de>

* Performance analyzer port on ci-values

Signed-off-by: Philipp Hölscher <phoelsch@outlook.de>

* Update changelog

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>

---------

Signed-off-by: Philipp Hölscher <phoelsch@outlook.de>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Incorporated the review comments

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Update values.yaml

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Incorporated the review comments

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Correcting the merge conflicts

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Updated the README.md for OpenSearch & Dashboard

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Updated the changelog message

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

* Updating the version number

Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>

---------

Signed-off-by: Zelin Hao <zelinhao@amazon.com>
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Ruslan Gainanov <gromrx1@gmail.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Christian Kuhn <phello@gmx.de>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: Rishabh Singh <sngri@amazon.com>
Signed-off-by: bbarani <bbarani@amazon.com>
Signed-off-by: josephteddick <josephteddick@gmail.com>
Signed-off-by: Jason Witkowski <jwitkowski@zscaler.com>
Signed-off-by: Prathap Mahalingam (Nokia) <prathap.mahalingam@nokia.com>
Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
Signed-off-by: Philipp Hölscher <phoelsch@outlook.de>
Co-authored-by: Zelin Hao <87548827+zelinh@users.noreply.github.com>
Co-authored-by: mend-for-github-com[bot] <50673670+mend-for-github-com[bot]@users.noreply.github.com>
Co-authored-by: Peter Zhu <zhujiaxi@amazon.com>
Co-authored-by: Lu Yu <yulu.nju@gmail.com>
Co-authored-by: Ruslan Gainanov <gromrx1@gmail.com>
Co-authored-by: Sayali Gaikawad <61760125+gaiksaya@users.noreply.github.com>
Co-authored-by: Sayali Gaikawad <gaiksaya@amazon.com>
Co-authored-by: Christian Kuhn <86721442+ph311o@users.noreply.github.com>
Co-authored-by: Daniel (dB.) Doubrovkine <dblock@dblock.org>
Co-authored-by: Rishabh Singh <rishabhksingh@gmail.com>
Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Co-authored-by: Barani <70038446+bbarani@users.noreply.github.com>
Co-authored-by: Joseph Teddick <43552317+josephteddick@users.noreply.github.com>
Co-authored-by: Jason Witkowski <jason@witkow.ski>
Co-authored-by: Jason Witkowski <jwitkowski@zscaler.com>
Co-authored-by: Prudhvi Godithi <pgodithi@amazon.com>
Co-authored-by: Philipp Hölscher <46397932+Phoelsch@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants