Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Set default for bootstrapExpect to replicas #721

Merged
merged 1 commit into from
Dec 10, 2020
Merged

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Dec 1, 2020

In almost all cases, users want to set bootstrapExpect to the number of
server replicas. This change defaults it to null in values.yaml and then
in the template if it's left as null, then we set the -bootstrap-expect
flag to the number of server replicas.

This is backwards compatible, if users have been setting this it will
continue to be set.

Also error out if bootstrapExpect is less than the number of replicas
because this is definitely a misconfiguration as the servers won't wait
until the proper number have started before electing a leader.

Before:

server:
  replicas: 1
  bootstrapExpect: 1

After:

server:
  replicas: 1

Closes #573

[ "${actual}" = "true" ]
}

@test "server/StatefulSet: errors if bootstrapExpect < replicas" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change backwards compatible? Previously, we would allow bootstrap expect to be <= replicas but this change would cause such a setting to change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, you're right! Will add to changelog. I could also remove this but I think this is an invalid configuration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I mean, your comment about bootstrapExpect made me realize that my understanding of it was incorrect. I thought it was the min number of servers that needed to be up and running for consensus, which seemed odd but 🤷 . This makes more sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent inconsistencies and split-brain (clusters where multiple servers consider themselves leader) situations, you should either specify the same value for -bootstrap-expect or specify no value at all on all the servers.

From https://www.consul.io/docs/install/bootstrapping

@lkysow lkysow requested review from a team, ndhanushkodi and kschoche and removed request for a team and kschoche December 10, 2020 18:35
Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent change.

In almost all cases, users want to set bootstrapExpect to the number of
server replicas. This change defaults it to null in values.yaml and then
in the template if it's left as null, then we set the -bootstrap-expect
flag to the number of server replicas.

This is backwards compatible, if users have been setting this it will
continue to be set.

Also error out if bootstrapExpect is less than the number of replicas
because this is definitely a misconfiguration as the servers won't wait
until the proper number have started before electing a leader.
@lkysow lkysow merged commit abedb03 into master Dec 10, 2020
@lkysow lkysow deleted the bootstrap-expect branch December 10, 2020 18:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove bootstrap_expect from values.yml
3 participants