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

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

Open
vaijosh opened this issue Aug 16, 2024 · 2 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@vaijosh
Copy link

vaijosh commented Aug 16, 2024

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?

@github-actions github-actions bot added the untriaged Issues that have not yet been triaged label Aug 16, 2024
@dblock dblock transferred this issue from opensearch-project/.github Aug 16, 2024
@prudhvigodithi
Copy link
Member

[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

@prudhvigodithi prudhvigodithi added enhancement New feature or request question Further information is requested and removed untriaged Issues that have not yet been triaged labels Aug 29, 2024
@vaijosh
Copy link
Author

vaijosh commented Aug 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
Status: 📦 Backlog
Development

No branches or pull requests

2 participants