Skip to content

Allow to set node types for elasticsearch #136

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

Merged
merged 48 commits into from
Jun 1, 2023
Merged

Conversation

widhalmt
Copy link
Member

@widhalmt widhalmt commented Mar 29, 2023

fixes #66
fixes #137

You can now set a new variable to set all the node types. If you don't set it, it will stay at the default of "every node has every role".

Since having an odd count of master nodes is important, we built in a check that will assure you have the right count.

@widhalmt widhalmt added the feature New feature or request label Mar 29, 2023
@widhalmt widhalmt added this to the 1.0.0 milestone Mar 29, 2023
@widhalmt widhalmt requested a review from afeefghannam89 March 29, 2023 15:26
@widhalmt widhalmt self-assigned this Mar 29, 2023
@widhalmt widhalmt marked this pull request as draft March 29, 2023 15:26
@widhalmt
Copy link
Member Author

I changed this to draft because something in the calculation of master nodes isn't right. I tried different approaches with temporary files and other humble makeshift solutions. @afeefghannam89 , @Saeid-Abadi and @Donien showed me a nicer approach. Now it only needs to work. ;-)

Note: The current code shouldn't go through the tests. I pushed a draft where the check should actually fail due to a wrong count of master nodes.

@widhalmt
Copy link
Member Author

The new approach with setting some variables and creating groups is all but "sophisticated". But it's simple enough that it should work reliably and even more, it has some nice sideeffects that we might benefit from later. Having groups of master-nodes and data-nodes could be beneficial.

Of course I could just have the user set the variables we're setting. But I like the idea of using a list of roles to configure. We don't know whether there will come new ones or what combinations users might use. Then just setting variables we need for easier handling is a bit cumbersome, but hey, it works.

@widhalmt widhalmt marked this pull request as ready for review March 30, 2023 13:50
@widhalmt widhalmt enabled auto-merge March 30, 2023 13:50
For calculations to go through we would need a 3 node cluster. But that
uses too many ressources. So we introduce another check where only
calculations are tested and Elasticsearch is never started
@widhalmt
Copy link
Member Author

@widhalmt widhalmt disabled auto-merge May 17, 2023 12:48
@widhalmt
Copy link
Member Author

When all the checks pass, I still need to remove all the debug changes.

@afeefghannam89 afeefghannam89 marked this pull request as draft May 30, 2023 19:27
@afeefghannam89 afeefghannam89 marked this pull request as ready for review June 1, 2023 09:12
@afeefghannam89 afeefghannam89 enabled auto-merge June 1, 2023 11:37
@afeefghannam89 afeefghannam89 added this pull request to the merge queue Jun 1, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 1, 2023
@afeefghannam89 afeefghannam89 added this pull request to the merge queue Jun 1, 2023
@afeefghannam89 afeefghannam89 removed this pull request from the merge queue due to a manual request Jun 1, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jun 1, 2023
fixes #66
fixes #137 

You can now set a new variable to set all the node types. If you don't
set it, it will stay at the default of "every node has every role".

Since having an odd count of master nodes is important, we built in a
check that will assure you have the right count.

---------

Co-authored-by: Afeef Ghannam <[email protected]>
Co-authored-by: Afeef Ghannam <[email protected]>
@afeefghannam89 afeefghannam89 merged commit b22cc65 into main Jun 1, 2023
@afeefghannam89 afeefghannam89 deleted the feature/masters-66 branch June 1, 2023 16:41
ivareri pushed a commit to ivareri/ansible-collection-elasticstack that referenced this pull request Jun 17, 2025
* Fix broken keys on some systems

fixes NETWAYS#135

It looks like there are still some hosts where pkcs8 keys for Logstash
Beats input must not be encrypted. The strange thing about it is that it
works on some systems (CentOS 7) and on some it doesn't (so far Ubuntu
20.04). I tried recreating the keys and some other ways but the Ubuntu host didn't like encrypted keys at all. So here's a workaround.

Several other potential issues came up and were fixed, namely:

* There's now a variable to disable encryption of the key used for the
  Beats input in Logstash
* Logstash certificates created with `certutil` get now a suffix to
  their name. This helps when you have different passwords for
  keys in different tools. Otherwise they overwrite each other and you
  don't know which file has which passphrase
* Rename certificate archive
ivareri pushed a commit to ivareri/ansible-collection-elasticstack that referenced this pull request Jun 17, 2025
fixes NETWAYS#66 
fixes NETWAYS#137 

You can now set a new variable to set all the node types. If you don't
set it, it will stay at the default of "every node has every role".

Since having an odd count of master nodes is important, we built in a
check that will assure you have the right count.

---------

Co-authored-by: Afeef Ghannam <[email protected]>
Co-authored-by: Afeef Ghannam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove initial master node after the first cluster initialize Don't use all hosts as potential master hosts
2 participants