Refactor node configuration and add role validation #3694
Conversation
| masterRequiredMsg = "Elasticsearch needs to have at least one master node" | ||
| parseVersionErrMsg = "Cannot parse Elasticsearch version. String format must be {major}.{minor}.{patch}[-{label}]" | ||
| mixedRoleConfigMsg = "Detected a combination of node.roles and %s. Use only node.roles" |
There was a problem hiding this comment.
Should we just suggest to pick one?
There was a problem hiding this comment.
Because there's no good term to describe node.{role} config settings, I chose to just list them so that the user understands what needs to be removed. Do you think it's not necessary?
There was a problem hiding this comment.
Ah, that's fine. I was referring to "Use only node.roles", I thought it might be better to just say that either one approach or the other should be used instead of pointing users to use node.roles specifically.
There was a problem hiding this comment.
Ah I see. Presumably node.roles is the preferred method of defining roles going forward. I don't even see a mention of node.{role} settings in the 7.9 docs so I assume they'll be deprecated in the future.
There was a problem hiding this comment.
If it is preferred then I'm fine leaving it as it is.
david-kow
changed the title
Adds validation of node roles. The checks are:
node.rolesare not supported before 7.7.0node.rolesandnode.{role}settings should not be mixedA couple of issues were discovered during this work:
voting_onlyrolenode.transformwas set tofalseby the user or by Go's zero value rules)This includes a refactoring to address the above issues as well.
Fixes #3409