search

opendistro-for-elasticsearch / opendistro-build

🧰 Open Distro Build Scripts
https://opendistro.github.io/
Apache License 2.0
344 stars 175 forks source link

allow defining service topology #650

Closed tareksha closed 3 years ago

tareksha commented 3 years ago

Issue #, if available:

Description of changes:

Allow setting topologyKeys in services to define a service topology https://kubernetes.io/docs/concepts/services-networking/service-topology/

Test Results:

Verified manually.

Note: If this PR is related to Helm, please also update the README for related documentation changes. Thanks. https://github.com/opendistro-for-elasticsearch/opendistro-build/blob/master/helm/README.md

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

peterzhuamazon commented 3 years ago

Hi @tareqhs this helm setup was initially written for kube 1.15 and this topology feature seems only introduced after 1.17. I assume this could cause compatibility issues that it wont run on 1.15 anymore?

Thanks.

tareksha commented 3 years ago

Hi @peterzhuamazon, this is intentionally off by default. Users are free to enable it with custom topology keys if they want.

peterzhuamazon commented 3 years ago

Could you add a comment / update readme mention this is only working for Kube 1.17+? This is to prevent people mistakenly enable it and expect it to work on unsupported versions.

Thanks @tareqhs.

peterzhuamazon commented 3 years ago

Hi @tareqhs it has been 11 days and we have not hear back from you. Therefore, we will close this PR for now. Please feel free to reopen this PR. Thanks.

tareksha commented 3 years ago

Hi @peterzhuamazon , sorry for delayed response. I've added a dedicated note in the README. Please reopen this PR.

peterzhuamazon commented 3 years ago

Hi @tareqhs seems like I cannot reopen the PR due to it is being forced-pushed. Could you open another PR on this? Sorry for the back and forth. Thanks.