-
Notifications
You must be signed in to change notification settings - Fork 708
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
Do not set initial_master_nodes if cluster has been bootstrapped #1272
Conversation
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.
Looks pretty good, only concern is the isBootstrapped
method.
operators/pkg/controller/elasticsearch/version/version7/initial_master_nodes.go
Outdated
Show resolved
Hide resolved
} | ||
|
||
// no annotation, let see if the cluster has been bootstrapped by looking at it's UUID | ||
if clusterState.ClusterState != nil && len(clusterState.ClusterState.ClusterUUID) > 0 { |
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.
I don't like that isBootstrapped
has the side effect of actually storing the cluster UUID on the ES resource.
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.
Since it is rather small I have inlined the function and make it explicit in the ClusterInitialMasterNodesEnforcer
documentation.
…stic#1272) * Do not set initial_master_nodes if cluster has been bootstrapped
* Do not set initial_master_nodes if cluster has been bootstrapped * Backport of #1272
This is a port of elastic#1272 in the new code structure.
This is a port of elastic#1272 in the new code structure.
This is a port of elastic#1272 in the new code structure.
This PR ensures that
initial_master_nodes
is not set once the cluster has been bootstrapped.It is a partial fix for #1201