-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Fix roles parsing in client nodes sniffer #52888
Fix roles parsing in client nodes sniffer #52888
Conversation
We mades roles pluggable, but never updated the client to account for this. This means that when speaking to a modern cluster, application logs are spammed with warning messages around unrecognized roles. This commit addresses this by accounting for the fact that roles can extend beyond master/data/ingest now.
Pinging @elastic/es-core-features (:Core/Features/Java Low Level REST Client) |
@elasticmachine update branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left one question regarding optional future work, but nothing this PR depends on.
} | ||
|
||
/** | ||
* Teturns whether or not the node <strong>could</strong> be elected master. | ||
*/ | ||
public boolean isMasterEligible() { | ||
return masterEligible; | ||
return roles.contains("master"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, should we make these constants since they're hardcoded as strings all over the place? (in separate work, not this PR)
We mades roles pluggable, but never updated the client to account for this. This means that when speaking to a modern cluster, application logs are spammed with warning messages around unrecognized roles. This commit addresses this by accounting for the fact that roles can extend beyond master/data/ingest now.
We mades roles pluggable, but never updated the client to account for this. This means that when speaking to a modern cluster, application logs are spammed with warning messages around unrecognized roles. This commit addresses this by accounting for the fact that roles can extend beyond master/data/ingest now.
We mades roles pluggable, but never updated the client to account for this. This means that when speaking to a modern cluster, application logs are spammed with warning messages around unrecognized roles. This commit addresses this by accounting for the fact that roles can extend beyond master/data/ingest now.
Closes #52864