Use elasticsearch_network_host for connection tests if defined

[ivareri]

When elasticsearch_network_host is set to something not including localhost, all connection tests fails.

This uses elasticsearch_network_host for connections if it is defined, and falls back to localhost if not.

(There might be more places this change should be made, but the role works for me with this fix)

[widhalmt]

Thanks for your contribution! The more we can configure, the better. :-)

[widhalmt]

Clean and neat. Thank you.

elasticsearch_network_host is a string consisting of different entries in a single string. So this will fail if you set more than one entry for it. I'm afraid we need to rework the logic of this variable so we can still have several values and have your change work.

[widhalmt]

If you want, I can propose a change so the variable will work as a list instead of a string. Then you can fix your PR and we can merge it.

[widhalmt]

When looking at the code, this seems to be even more complicated than I thought. What if someone uses macros like _local_ or _site_ in their elasticsearch_network_host configuration? curl and uri can't use that. Any idea?

[widhalmt]

I opened #317 to work on a larger scale on this.

For now I guess the easiest solution would be if you changed your change request and introduced a new variable like elasticsearch_api_host. Set it to localhost in https://github.com/NETWAYS/ansible-collection-elasticstack/tree/main/roles/elasticsearch/defaults and add a line of documentation to https://github.com/NETWAYS/ansible-collection-elasticstack/blob/main/docs/role-elasticsearch.md .

Maybe we can come up with a way to automatically fill it depending on elasticsearch_network_host automatically.

I really don't like it but if you have a look at #317 I guess you see why I'm struggling with a clean solution.

The reason why I proposed api instead of http or bind_host is that these can also take arrays so we would merely be shifting the problem.

[ivareri]

Well duh, I had a feeling this was too neat of a fix.

I agree, elasticsearch_api_host is ugly, but probably the best solution. I'll update the PR.

[ivareri]

Note: I have not had a chance to do any extensive testing on this.

I changed all the places I found that does connection checks, so it should hopefully be consistent across the role.

[widhalmt]

Thank you for your work. Even if you missed an occurrence, that's not a problem. Especially since there's more to come. Some connections will be replaced with a module and others should have their own variable (e.g. other roles).

So, approved. Thank you so much.