-
Notifications
You must be signed in to change notification settings - Fork 217
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
Refactor opensearch-cluster helm chart #754
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
FYI as this PR was prepared a few months ago it didn't include some templates for the CRDs. If you will give it a green light in terms of concept and implementation I will add all missing templates. |
Hey @evheniyt thanks for your contribution, I like the idea to control the yaml files using the helm that can be used to create the cluster via the operator, can you include all the CRD's and fix the conflicts, we can go ahead and merge it. |
Sure, I will add all the missing CRDs soon. Also, will need to add upgrade instructions. |
Thanks @evheniyt with existing setup and release process, the helm charts are updated to the appVersion (operator version) https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/.github/workflows/release.yaml#L26-L31. We should seperate out this sooner to ensure the helm charts development and release is fast paced. WDYT @salyh @jochenkressin @pchmielnik @swoehrl-mw ? |
There is something to be said for keeping the versioning of cluster chart and operator the same to make it clear which versions match and are compatible. But on the other hand I can understand the cluster chart might need a faster/different release cycle than the operator. |
I agree |
# Conflicts: # charts/opensearch-cluster/Chart.yaml # charts/opensearch-cluster/templates/opensearch-cluster-cr.yaml
Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
Added OpenSearchISMPolicy template |
Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>
I have updated Readme. Also, I have tested helm chart upgrade process from v2 to v3 with default config from Now it's ready for review @prudhvigodithi @swoehrl-mw @salyh |
When is this going to be merged / released? |
Description
We found that the current
opensearch-cluster
helm chart supports onlyOpenSearchCluster
configuration, also configuration itself is very limited because not all possible options were defined in the template.For our needs, we have decided to improve this helm chart and think that our changes could be useful for the community.
The main differences in the new helm chart:
Ingress
configuration forOpensearch
andDashboards
We were trying to build this chart by following the next logic. We didn't try to template all possible configurations for CRDs, instead, we relied on official parameters supported by specific sections of the CRD, e.g.
Instead of doing this:
We did:
And defined all possible configuration options inside the
.Values.cluster.bootstrap
.Pros of such logic:
values.yaml
file. And even if for some reason it wasn't added, users still could use it;If you apply it, I could help with maintaining this helm chart by fixing bugs, adding new features, etc.
Issues Resolved
Closes #699
Closes #9
Closes #855
Closes #866
Closes #902
Closes #667
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.