Skip to content

Commit

Permalink
revert leave_on_terminate and autopilot updates from commit #3000 and…
Browse files Browse the repository at this point in the history
… re-apply datadog-integration branch changes
  • Loading branch information
natemollica-nm committed Feb 13, 2024
1 parent 00958ba commit 1da0620
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 91 deletions.
34 changes: 17 additions & 17 deletions charts/consul/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -189,27 +189,24 @@ Expand the name of the chart.
{{- end -}}

{{/*
Calculate max number of server pods that are allowed to be voluntarily disrupted.
When there's 1 server, this is set to 0 because this pod should not be disrupted. This is an edge
case and I'm not sure it makes a difference when there's only one server but that's what the previous config was and
I don't want to change it for this edge case.
Otherwise we've changed this to always be 1 as part of the move to set leave_on_terminate
to true. With leave_on_terminate set to true, whenever a server pod is stopped, the number of peers in raft
is reduced. If the number of servers is odd and the count is reduced by 1, the quorum size doesn't change,
but if it's reduced by more than 1, the quorum size can change so that's why this is now always hardcoded to 1.
Compute the maximum number of unavailable replicas for the PodDisruptionBudget.
This defaults to (n/2)-1 where n is the number of members of the server cluster.
Special case of replica equaling 3 and allowing a minor disruption of 1 otherwise
use the integer value
Add a special case for replicas=1, where it should default to 0 as well.
*/}}
{{- define "consul.server.pdb.maxUnavailable" -}}
{{- define "consul.pdb.maxUnavailable" -}}
{{- if eq (int .Values.server.replicas) 1 -}}
{{ 0 }}
{{- else if .Values.server.disruptionBudget.maxUnavailable -}}
{{ .Values.server.disruptionBudget.maxUnavailable -}}
{{- else -}}
{{ 1 }}
{{- if eq (int .Values.server.replicas) 3 -}}
{{- 1 -}}
{{- else -}}
{{- sub (div (int .Values.server.replicas) 2) 1 -}}
{{- end -}}
{{- end -}}
{{- define "consul.server.autopilotMinQuorum" -}}
{{- add (div (int .Values.server.replicas) 2) 1 -}}
{{- end -}}

{{- define "consul.pdb.connectInject.maxUnavailable" -}}
Expand Down Expand Up @@ -451,10 +448,10 @@ Usage: {{ template "consul.validateTelemetryCollectorCloud" . }}
*/}}
{{- define "consul.validateTelemetryCollectorCloud" -}}
{{- if (and .Values.telemetryCollector.cloud.clientId.secretName (and (not .Values.global.cloud.clientSecret.secretName) (not .Values.telemetryCollector.cloud.clientSecret.secretName))) }}
{{fail "When telemetryCollector.cloud.clientId.secretName is set, telemetryCollector.cloud.clientSecret.secretName must also be set." }}
{{fail "When telemetryCollector.cloud.clientId.secretName is set, telemetryCollector.cloud.clientSecret.secretName must also be set."}}
{{- end }}
{{- if (and .Values.telemetryCollector.cloud.clientSecret.secretName (and (not .Values.global.cloud.clientId.secretName) (not .Values.telemetryCollector.cloud.clientId.secretName))) }}
{{fail "When telemetryCollector.cloud.clientSecret.secretName is set, telemetryCollector.cloud.clientId.secretName must also be set." }}
{{fail "When telemetryCollector.cloud.clientSecret.secretName is set, telemetryCollector.cloud.clientId.secretName must also be set."}}
{{- end }}
{{- end }}

Expand Down Expand Up @@ -514,8 +511,8 @@ Usage: {{ template "consul.validateResourceAPIs" . }}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.global.peering.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, global.peering.enabled is currently unsupported."}}
{{- end }}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) (not (mustHas "v2tenancy" .Values.global.experiments)) .Values.global.adminPartitions.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, global.experiments.v2tenancy must also be set to support global.adminPartitions.enabled."}}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.global.adminPartitions.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, global.adminPartitions.enabled is currently unsupported."}}
{{- end }}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.global.federation.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, global.federation.enabled is currently unsupported."}}
Expand All @@ -532,6 +529,9 @@ Usage: {{ template "consul.validateResourceAPIs" . }}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.syncCatalog.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, syncCatalog.enabled is currently unsupported."}}
{{- end }}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.meshGateway.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, meshGateway.enabled is currently unsupported."}}
{{- end }}
{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.ingressGateways.enabled ) }}
{{fail "When the value global.experiments.resourceAPIs is set, ingressGateways.enabled is currently unsupported."}}
{{- end }}
Expand Down
7 changes: 1 addition & 6 deletions charts/consul/templates/server-config-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,7 @@ data:
"enabled": true
},
{{- end }}
"server": true,
"leave_on_terminate": true,
"autopilot": {
"min_quorum": {{ template "consul.server.autopilotMinQuorum" . }},
"disable_upgrade_migration": true
}
"server": true
}
{{- $vaultConnectCAEnabled := and .Values.global.secretsBackend.vault.connectCA.address .Values.global.secretsBackend.vault.connectCA.rootPKIPath .Values.global.secretsBackend.vault.connectCA.intermediatePKIPath -}}
{{- if and .Values.global.secretsBackend.vault.enabled $vaultConnectCAEnabled }}
Expand Down
2 changes: 1 addition & 1 deletion charts/consul/templates/server-disruptionbudget.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ metadata:
release: {{ .Release.Name }}
component: server
spec:
maxUnavailable: {{ template "consul.server.pdb.maxUnavailable" . }}
maxUnavailable: {{ template "consul.pdb.maxUnavailable" . }}
selector:
matchLabels:
app: {{ template "consul.name" . }}
Expand Down
16 changes: 14 additions & 2 deletions charts/consul/test/unit/helpers.bats
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ load _helpers
[[ "$output" =~ "When the value global.experiments.resourceAPIs is set, global.peering.enabled is currently unsupported." ]]
}

@test "connectInject/Deployment: fails if resource-apis is set, v2tenancy is unset, and admin partitions are enabled" {
@test "connectInject/Deployment: fails if resource-apis is set and admin partitions are enabled" {
cd `chart_dir`
run helm template \
-s templates/tests/test-runner.yaml \
Expand All @@ -359,7 +359,7 @@ load _helpers
--set 'global.adminPartitions.enabled=true' \
.
[ "$status" -eq 1 ]
[[ "$output" =~ "When the value global.experiments.resourceAPIs is set, global.experiments.v2tenancy must also be set to support global.adminPartitions.enabled." ]]
[[ "$output" =~ "When the value global.experiments.resourceAPIs is set, global.adminPartitions.enabled is currently unsupported." ]]
}

@test "connectInject/Deployment: fails if resource-apis is set and federation is enabled" {
Expand Down Expand Up @@ -431,6 +431,18 @@ load _helpers
[[ "$output" =~ "When the value global.experiments.resourceAPIs is set, syncCatalog.enabled is currently unsupported." ]]
}

@test "connectInject/Deployment: fails if resource-apis is set and meshGateway is enabled" {
cd `chart_dir`
run helm template \
-s templates/tests/test-runner.yaml \
--set 'connectInject.enabled=true' \
--set 'global.experiments[0]=resource-apis' \
--set 'ui.enabled=false' \
--set 'meshGateway.enabled=true' .
[ "$status" -eq 1 ]
[[ "$output" =~ "When the value global.experiments.resourceAPIs is set, meshGateway.enabled is currently unsupported." ]]
}

@test "connectInject/Deployment: fails if resource-apis is set and ingressGateways is enabled" {
cd `chart_dir`
run helm template \
Expand Down
60 changes: 1 addition & 59 deletions charts/consul/test/unit/server-config-configmap.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1521,62 +1521,4 @@ load _helpers
yq -r '.data["telemetry-config.json"]' | jq -r .telemetry.enable_host_metrics | tee /dev/stderr)

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

#--------------------------------------------------------------------
# server.autopilot.min_quorum

@test "server/ConfigMap: autopilot.min_quorum=1 when replicas=1" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'server.replicas=1' \
. | tee /dev/stderr |
yq -r '.data["server.json"]' | jq -r .autopilot.min_quorum | tee /dev/stderr)

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

@test "server/ConfigMap: autopilot.min_quorum=2 when replicas=2" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'server.replicas=2' \
. | tee /dev/stderr |
yq -r '.data["server.json"]' | jq -r .autopilot.min_quorum | tee /dev/stderr)

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

@test "server/ConfigMap: autopilot.min_quorum=2 when replicas=3" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'server.replicas=3' \
. | tee /dev/stderr |
yq -r '.data["server.json"]' | jq -r .autopilot.min_quorum | tee /dev/stderr)

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

@test "server/ConfigMap: autopilot.min_quorum=3 when replicas=4" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'server.replicas=4' \
. | tee /dev/stderr |
yq -r '.data["server.json"]' | jq -r .autopilot.min_quorum | tee /dev/stderr)

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

@test "server/ConfigMap: autopilot.min_quorum=3 when replicas=5" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'server.replicas=5' \
. | tee /dev/stderr |
yq -r '.data["server.json"]' | jq -r .autopilot.min_quorum | tee /dev/stderr)

[ "${actual}" = "3" ]
}
}
6 changes: 3 additions & 3 deletions charts/consul/test/unit/server-disruptionbudget.bats
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ load _helpers
--set 'server.replicas=6' \
. | tee /dev/stderr |
yq '.spec.maxUnavailable' | tee /dev/stderr)
[ "${actual}" = "1" ]
[ "${actual}" = "2" ]
}

@test "server/DisruptionBudget: correct maxUnavailable with replicas=7" {
Expand All @@ -107,7 +107,7 @@ load _helpers
--set 'server.replicas=7' \
. | tee /dev/stderr |
yq '.spec.maxUnavailable' | tee /dev/stderr)
[ "${actual}" = "1" ]
[ "${actual}" = "2" ]
}

@test "server/DisruptionBudget: correct maxUnavailable with replicas=8" {
Expand All @@ -117,7 +117,7 @@ load _helpers
--set 'server.replicas=8' \
. | tee /dev/stderr |
yq '.spec.maxUnavailable' | tee /dev/stderr)
[ "${actual}" = "1" ]
[ "${actual}" = "3" ]
}

#--------------------------------------------------------------------
Expand Down
6 changes: 3 additions & 3 deletions charts/consul/test/unit/server-statefulset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ load _helpers
-s templates/server-statefulset.yaml \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
[ "${actual}" = a4771bea366d4a6ee9037572665dc4040519dc22e9b0ff3463a263aab13675b8 ]
[ "${actual}" = 0fb82d8e2e58b3e11c8803a5e6a6575b735b011b373304a724317c299d95cdbb ]
}

@test "server/StatefulSet: adds config-checksum annotation when extraConfig is provided" {
Expand All @@ -1089,7 +1089,7 @@ load _helpers
--set 'server.extraConfig="{\"hello\": \"world\"}"' \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
[ "${actual}" = c6b872933263bf5fe847d61e638035637d2db89edf31ad25d0aaeaa5261649c9 ]
[ "${actual}" = a05ee305cfb15c76587e815d08472098c790ccce6322ab79f1d9702020be5a5e ]
}

@test "server/StatefulSet: adds config-checksum annotation when config is updated" {
Expand All @@ -1099,7 +1099,7 @@ load _helpers
--set 'global.acls.manageSystemACLs=true' \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
[ "${actual}" = 576044232d6181bca69628af87c12f15311ebd3f0ab700e112b3e1dea9225125 ]
[ "${actual}" = 13e6436e0a8e2ee57e35abc358c068eb575737e706dd9e4b3231dff69228d89d ]
}

#--------------------------------------------------------------------
Expand Down

0 comments on commit 1da0620

Please sign in to comment.