-
Notifications
You must be signed in to change notification settings - Fork 234
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
Customize opensearch deployment annotation through values.yaml #407
Customize opensearch deployment annotation through values.yaml #407
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, OpenSearch deployment is not customizable using values.yaml . I'm trying to add deployment annotation in the values.yaml , which will be used by the deployment.yaml and configures the metadata annotation of the deployment.
Use case: In the DR site deployments, we want to control the deployment of OpenSearch based on a few Deployment annotations. which is not possible now as deployment annotations cannot be passed through values.yaml
@bbarani , Hi, I'm new to this review process. I have submitted the changes needed for customizing the OpenSearch deployment. Can someone review these changes? thanks |
Hey @prathaptce thanks for the contribution, please check the following points.
Apart from this I will review the technical aspects of the chart shortly, thank you. |
05da68f
to
e67418e
Compare
Now, DCO check is passing, helm chart version is bumped & 'CHANGELOG.md' is updated with the changes done for this PR |
charts/opensearch/values.yaml
Outdated
@@ -131,6 +131,9 @@ image: | |||
|
|||
podAnnotations: {} | |||
# iam.amazonaws.com/role: es-cluster | |||
|
|||
# Deployment annotations | |||
annotations: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Can we please use as openSearchAnnotations
?, also please change the comment to # OpenSearch Statefulset annotations
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to openSearchAnnotations & comments to stateful set annotations
@@ -67,6 +67,9 @@ secretMounts: [] | |||
|
|||
podAnnotations: {} | |||
|
|||
# Deployment annotations | |||
annotations: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Can we please use as dashboardAnnotations
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed this comment as well
Hey @prathaptce apart from my previous suggestions, can you take care of the lint errors also please update the README files in the chart with the new annotation changes. |
@@ -3,6 +3,10 @@ kind: Deployment | |||
metadata: | |||
name: {{ template "opensearch-dashboards.fullname" . }} | |||
labels: {{- include "opensearch-dashboards.labels" . | nindent 4 }} | |||
{{- with .Values.annotations }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be as dashboardAnnotations
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made the necessary changes here as well
@@ -7,6 +7,9 @@ metadata: | |||
{{- include "opensearch.labels" . | nindent 4 }} | |||
annotations: | |||
majorVersion: "{{ include "opensearch.majorVersion" . }}" | |||
{{- with .Values.annotations }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be as openSearchAnnotations
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
incorporated the changes
I'm not able to execute ct lint locally, is there a way I can trigger the ct lint job in GitHub & check the error? I'm not sure which line is creating the lint error |
a151749
to
a24b1b8
Compare
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>
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>
…oject#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>
* 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>
…oject#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>
…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>
…earch-project#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 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>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com> Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com> Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
…t#336) Signed-off-by: Christian Kuhn <phello@gmx.de> Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
opensearch-project#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>
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>
@prudhvigodithi, I have addressed all the comments. Let me know if the lint error still exists. thanks |
@@ -13,6 +13,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
### Fixed | |||
### Security | |||
--- | |||
## [2.9.2] | |||
### Added | |||
- Added custom opensearch deployment annotation through values.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NIT] Thanks @prathaptce noticed one more, please check
- Added custom opensearch and dashboard annotations through values.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks. updated the same
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
Thanks @prathaptce , @TheAlgo and @peterzhuamazon can you please review ? |
@TheAlgo @peterzhuamazon, can you please review? thank you |
Hey @prathaptce I did notice one thing the |
Signed-off-by: prathaptce <86703966+prathaptce@users.noreply.github.com>
Just now pumped up the version numbers in Chart.yaml. Please check now |
Pinging @TheAlgo and @peterzhuamazon :) |
hi @TheAlgo @peterzhuamazon, Can you please review the changes & let me know your comments? |
Can this PR merged? if no other comments. thanks |
Hey @TheAlgo @peterzhuamazon please add your approval. Thanks for your patience @prathaptce |
Will check this week. Apologies for delays. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@TheAlgo .Thank you. Can you please merge this PR? |
Merging the PR as its approved. |
Hey @prathaptce can you please backport to 1.x? as I see this feature is useful for 1.x version as well. |
Sure @prudhvigodithi, I will raise the PR for 1.x as well |
Description
[Currently, OpenSearch deployment is not customizable using values.yaml . I'm trying to add deployment annotation in the values.yaml , which will be used by the deployment.yaml and configures the metadata annotation of the deployment.]
Issues Resolved
[https://github.com/opensearch-project/opensearch-devops/issues/115]
Check List
For any changes to files within Helm chart directories:
CHANGELOG.md
updated 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.