Remove multi instances support #566
Conversation
The goal is to stop supporting installation of more than one node in the same host. This commit update the Ansible role README documentation and remove the multi instances kitchen test.
As we no more need to support more than one node on the same host, we no more need to override init files provided by elasticsearch official packages.
File scripts have been removed since elasticsearch 6.0 (https://www.elastic.co/guide/en/elasticsearch/reference/6.0/breaking_60_scripting_changes.html#_file_scripts_removed)
…ta_dirs variables
ES_USER and ES_GROUP settings are no longer supported (https://www.elastic.co/guide/en/elasticsearch/reference/6.0/breaking_60_packaging_changes.html#_configuring_custom_user_and_group_for_package_is_no_longer_allowed)
…mlrt/ansible-elasticsearch into jmlrt-remove-multi-instances-support
…-instances-support
for example: es_conf_dir: "/etc/elasticsearch/node1"
README.md
Outdated
|
|
||
| The parameter `es_xpack_features` by default enables all features i.e. it defaults to ["alerting","monitoring","graph","security","ml"] | ||
| The parameter `es_xpack_features` allows to list xpack features to install (example: `["alerting","monitoring","graph","security","ml"]`). | ||
| When the list is empty, it install all features available with the current licence. |
There was a problem hiding this comment.
This variable doesn't actually install anything. The only time es_xpack_features is actually referred to is this line:
ansible-elasticsearch/tasks/elasticsearch-parameters.yml
Lines 18 to 21 in 0e61dba
Now that it is empty by default I'm not sure if this adds any value at all. Instead I think we can just drop this whole variable and make sure that we explain that you need to set the username and password if you are using security.
There was a problem hiding this comment.
It's strange that you saw a change in this PR because this is a change from https://github.com/elastic/ansible-elasticsearch/pull/560/files which is already merged
| end | ||
| end | ||
| else | ||
| features.each do |feature, status| |
There was a problem hiding this comment.
Whats the idea of wanting to test this? If this were to start failing it would be because of a changed default in an Elasticsearch version. If that were to happen we would just be updating the defaults in this test file right?
There was a problem hiding this comment.
The idea was to still have a test when es_xpack_features variable is empty but I can remove it if you think it's not relevant.
There was a problem hiding this comment.
It's strange that you saw a change in this PR because this is a change from https://github.com/elastic/ansible-elasticsearch/pull/560/files which is already merged
This PR should remove multi-instance support of ansible-elasticsearch role.
As mentionned in #554 (comment), with Elasticsearch 7.0 installing more than one node on the same host is no more supported.
Note that my IDE cleaned a lot of end of line whitespaces, so I highly recommend selecting GitHub Hide whitespace changes checkbox during review :).