opensearch-project / helm-charts

:wheel_of_dharma: A community repository for Helm Charts of OpenSearch Project.
https://opensearch.org/docs/latest/opensearch/install/helm/
Apache License 2.0
176 stars 236 forks source link

[PROPOSAL] In OpenSearch Helm charts repos we should use uniform naming convention for affinity #574

Open vaijosh opened 3 months ago

vaijosh commented 3 months ago

What/Why

What are you proposing?

It observed that the openSearch and OpenSearch-dashboard charts used different conventions for affinity.

OpenSearch chart kept it flat i.e. we can directly specify nodeAffinity, podAffinity and so on. For OpenSeachDashboards we need to use "affinity.nodeAffinity" to do the same thing.

So, we should use consistent naming conventions.

What users have asked for this feature?

What problems are you trying to solve?

We are trying to use some scripts to deploy the charts. We accept nodeAffinity spec from user as a parameter. Due to inconsistent conventions used in chart, we need to use condition logic i.e. if chart is "openSearch" use nodeAffinity as a key, if chart is opensearch-dashboard, we affinity.nodeAffinity.

What is the developer experience going to be?

Helm chart installation install will be simplified if we used consistent naming convention like affinity.nodeAffinity instead of individual keys in openseach charts.

Are there any security considerations?

N/A

Are there any breaking changes to the API

What is the user experience going to be?

Are there breaking changes to the User Experience?

Yes. Existing user might need to follow new naming conventions when deploying helm charts.

Why should it be built? Any reason not to?

What will it take to execute?

Any remaining open questions?

prudhvigodithi commented 3 months ago

[Triage] Hey @vaijosh today for OpenSearch Dashboards we have affinity {} rules, can you please share the drift in naming conventions? Or the as is to onboard to nodeAffinity and podAffinity.

Thank you

vaijosh commented 3 months ago

Hi Prudvi, Thanks for looking into this.

Chart "opensearch" is not following the same convenctions as opensearch-dashboard. In "OpenSearch" chart we don't have affinity but we have it in opensearch-dashboard. For example nodeAffinity can be specified as follows.

OpenSearch:

nodeAffinity: {}

https://github.com/opensearch-project/helm-charts/blob/main/charts/opensearch/values.yaml#L260

OpenSearch-Dashboard: affinity: nodeAffinity: {}

https://github.com/opensearch-project/helm-charts/blob/main/charts/opensearch-dashboards/values.yaml#L215

It seems openSearch-dashboard is using correct hierarchy. Opensearch charts should also follow the same and should be consistent.

Thanks & Regards, Vaibhav