Skip to content

Commit

Permalink
Set default for bootstrapExpect to replicas (hashicorp#721)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lkysow authored Dec 10, 2020
1 parent 0e7dae4 commit abedb03
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 2 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
BUG FIXES:
* Fix pod security policy when running mesh gateways in `hostNetwork` mode. [[GH-605](https://github.com/hashicorp/consul-helm/issues/605)]

IMPROVEMENTS:
* Make `server.bootstrapExpect` optional. If not set, will now default to `server.replicas`.
If you're currently setting `server.replicas`, there is no effect. [[GH-721](https://github.com/hashicorp/consul-helm/pull/721)]

BREAKING CHANGES:
* Setting `server.bootstrapExpect` to a value less than `server.replicas` will now
give an error. This was a misconfiguration as the servers wouldn't wait
until the proper number have started before electing a leader. [[GH-721](https://github.com/hashicorp/consul-helm/pull/721)]

## 0.27.0 (Nov 25, 2020)

IMPROVEMENTS:
Expand Down
3 changes: 2 additions & 1 deletion templates/server-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{{- if and .Values.global.federation.enabled (not .Values.global.tls.enabled) }}{{ fail "If global.federation.enabled is true, global.tls.enabled must be true because federation is only supported with TLS enabled" }}{{ end }}
{{- if and .Values.global.federation.enabled (not .Values.meshGateway.enabled) }}{{ fail "If global.federation.enabled is true, meshGateway.enabled must be true because mesh gateways are required for federation" }}{{ end }}
{{- if .Values.server.disableFsGroupSecurityContext }}{{ fail "server.disableFsGroupSecurityContext has been removed. Please use global.openshift.enabled instead." }}{{ end }}
{{- if .Values.server.bootstrapExpect }}{{ if lt (int .Values.server.bootstrapExpect) (int .Values.server.replicas) }}{{ fail "server.bootstrapExpect cannot be less than server.replicas" }}{{ end }}{{ end }}
# StatefulSet to run the actual Consul server cluster.
apiVersion: apps/v1
kind: StatefulSet
Expand Down Expand Up @@ -143,7 +144,7 @@ spec:
exec /bin/consul agent \
-advertise="${POD_IP}" \
-bind=0.0.0.0 \
-bootstrap-expect={{ .Values.server.bootstrapExpect }} \
-bootstrap-expect={{ if .Values.server.bootstrapExpect }}{{ .Values.server.bootstrapExpect }}{{ else }}{{ .Values.server.replicas }}{{ end }} \
{{- if .Values.global.tls.enabled }}
-hcl='ca_file = "/consul/tls/ca/tls.crt"' \
-hcl='cert_file = "/consul/tls/server/tls.crt"' \
Expand Down
31 changes: 31 additions & 0 deletions test/unit/server-statefulset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -916,3 +916,34 @@ load _helpers
yq '.spec.template.spec.containers[0].command | join(" ") | contains("auto_encrypt = {allow_tls = true}")' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# -bootstrap-expect

@test "server/StatefulSet: -bootstrap-expect defaults to replicas" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-statefulset.yaml \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | join(" ") | contains("-bootstrap-expect=3")' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "server/StatefulSet: -bootstrap-expect can be set by server.bootstrapExpect" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-statefulset.yaml \
--set 'server.bootstrapExpect=5' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | join(" ") | contains("-bootstrap-expect=5")' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "server/StatefulSet: errors if bootstrapExpect < replicas" {
cd `chart_dir`
run helm template \
-s templates/server-statefulset.yaml \
--set 'server.bootstrapExpect=1' .
[ "$status" -eq 1 ]
[[ "$output" =~ "server.bootstrapExpect cannot be less than server.replicas" ]]
}
10 changes: 9 additions & 1 deletion values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,15 @@ server:
enabled: "-"
image: null
replicas: 3
bootstrapExpect: 3 # Should <= replicas count

# bootstrapExpect is the number of servers that are expected to be running.
# It defaults to server.replicas.
# In most cases the default should be used, however if there are more
# servers in this datacenter than server.replicas it might make sense
# to override the default. This would be the case if two kube clusters
# were joined into the same datacenter and each cluster ran a certain number
# of servers.
bootstrapExpect: null

# enterpriseLicense refers to a Kubernetes secret that you have created that
# contains your enterprise license. It is required if you are using an
Expand Down

0 comments on commit abedb03

Please sign in to comment.