From 61aae0b82cd197f9825076cbdf3f2290883a8228 Mon Sep 17 00:00:00 2001 From: natemollica-dev Date: Tue, 13 Feb 2024 15:20:08 -0800 Subject: [PATCH] datadog-integration: manual cherry-pick --- charts/consul/templates/_helpers.tpl | 80 +++---------------- .../templates/server-disruptionbudget.yaml | 2 +- .../consul/test/unit/server-acl-init-job.bats | 40 +--------- .../test/unit/server-config-configmap.bats | 60 +------------- .../consul/test/unit/server-statefulset.bats | 34 +------- 5 files changed, 19 insertions(+), 197 deletions(-) diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index ca87485a78..174e8c18c8 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -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" -}} @@ -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 }} @@ -492,57 +489,6 @@ Usage: {{ template "consul.validateTelemetryCollectorResourceId" . }} {{- end }} {{- end }} -{{/**/}} - -{{/* -Fails if global.experiments.resourceAPIs is set along with any of these unsupported features. -- global.peering.enabled -- global.federation.enabled -- global.cloud.enabled -- client.enabled -- ui.enabled -- syncCatalog.enabled -- meshGateway.enabled -- ingressGateways.enabled -- terminatingGateways.enabled -- apiGateway.enabled - -Usage: {{ template "consul.validateResourceAPIs" . }} - -*/}} -{{- define "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."}} -{{- 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."}} -{{- end }} -{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.global.cloud.enabled ) }} -{{fail "When the value global.experiments.resourceAPIs is set, global.cloud.enabled is currently unsupported."}} -{{- end }} -{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.client.enabled ) }} -{{fail "When the value global.experiments.resourceAPIs is set, client.enabled is currently unsupported."}} -{{- end }} -{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.ui.enabled ) }} -{{fail "When the value global.experiments.resourceAPIs is set, ui.enabled is currently unsupported."}} -{{- end }} -{{- 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.ingressGateways.enabled ) }} -{{fail "When the value global.experiments.resourceAPIs is set, ingressGateways.enabled is currently unsupported."}} -{{- end }} -{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.terminatingGateways.enabled ) }} -{{fail "When the value global.experiments.resourceAPIs is set, terminatingGateways.enabled is currently unsupported."}} -{{- end }} -{{- if (and (mustHas "resource-apis" .Values.global.experiments) .Values.apiGateway.enabled ) }} -{{fail "When the value global.experiments.resourceAPIs is set, apiGateway.enabled is currently unsupported."}} -{{- end }} -{{- end }} - {{/* Validation for Consul Metrics configuration: @@ -685,4 +631,4 @@ Usage: {{ template "consul.versionInfo" }} {{- $sanitizedVersion = $versionInfo }} {{- end -}} {{- printf "%s" $sanitizedVersion | quote }} -{{- end -}} \ No newline at end of file +{{- end -}} diff --git a/charts/consul/templates/server-disruptionbudget.yaml b/charts/consul/templates/server-disruptionbudget.yaml index 56805edc2a..edf9c1c57f 100644 --- a/charts/consul/templates/server-disruptionbudget.yaml +++ b/charts/consul/templates/server-disruptionbudget.yaml @@ -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" . }} diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 99fc6b9a9e..369c80333b 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -1081,7 +1081,6 @@ load _helpers local expected=$(echo '{ "consul.hashicorp.com/connect-inject": "false", - "consul.hashicorp.com/mesh-inject": "false", "vault.hashicorp.com/agent-inject": "true", "vault.hashicorp.com/agent-pre-populate": "true", "vault.hashicorp.com/agent-pre-populate-only": "false", @@ -2357,11 +2356,7 @@ load _helpers -s templates/server-acl-init-job.yaml \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq -r '.spec.template.metadata.annotations | - del(."consul.hashicorp.com/connect-inject") | - del(."consul.hashicorp.com/mesh-inject") | - del(."consul.hashicorp.com/config-checksum")' | - tee /dev/stderr) + yq -r '.spec.template.metadata.annotations | del(."consul.hashicorp.com/connect-inject") | del(."consul.hashicorp.com/config-checksum")' | tee /dev/stderr) [ "${actual}" = "{}" ] } @@ -2412,39 +2407,6 @@ load _helpers [ "${actual}" = null ] } -#-------------------------------------------------------------------- -# resource-apis - -@test "serverACLInit/Job: resource-apis is not set by default" { - cd `chart_dir` - local object=$(helm template \ - -s templates/server-acl-init-job.yaml \ - --set 'global.acls.manageSystemACLs=true' \ - . | tee /dev/stderr | - yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) - - local actual=$(echo $object | - yq 'any(contains("-enable-resource-apis"))' | tee /dev/stderr) - [ "${actual}" = "false" ] -} - -@test "serverACLInit/Job: -enable-resource-apis=true is set when global.experiments contains [\"resource-apis\"] " { - cd `chart_dir` - local object=$(helm template \ - -s templates/server-acl-init-job.yaml \ - --set 'global.acls.manageSystemACLs=true' \ - --set 'global.tls.enabled=true' \ - --set 'connectInject.enabled=true' \ - --set 'global.experiments[0]=resource-apis' \ - --set 'ui.enabled=false' \ - . | tee /dev/stderr | - yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) - - local actual=$(echo $object | - yq 'any(contains("-enable-resource-apis=true"))' | tee /dev/stderr) - [ "${actual}" = "true" ] -} - #-------------------------------------------------------------------- # global.metrics.datadog diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats index d75e2fd799..36e049a803 100755 --- a/charts/consul/test/unit/server-config-configmap.bats +++ b/charts/consul/test/unit/server-config-configmap.bats @@ -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" ] -} +} \ No newline at end of file diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats index 8fceb5c474..247d854b3b 100755 --- a/charts/consul/test/unit/server-statefulset.bats +++ b/charts/consul/test/unit/server-statefulset.bats @@ -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" { @@ -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" { @@ -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 ] } #-------------------------------------------------------------------- @@ -3450,32 +3450,4 @@ MIICFjCCAZsCCQCdwLtdjbzlYzAKBggqhkjOPQQDAjB0MQswCQYDVQQGEwJDQTEL' \ . | tee /dev/stderr | yq -r '.spec.template.spec.containers[1].command[2] | contains("-interval=10h34m5s")' | tee /dev/stderr) [ "${actual}" = "true" ] -} - -#-------------------------------------------------------------------- -# global.experiments=["resource-apis"] - -@test "server/StatefulSet: experiments=[\"resource-apis\"] is not set in command when global.experiments is empty" { - cd `chart_dir` - local object=$(helm template \ - -s templates/server-statefulset.yaml \ - . | tee /dev/stderr) - - # Test the flag is set. - local actual=$(echo "$object" | - yq '.spec.template.spec.containers[] | select(.name == "consul") | .command | any(contains("-hcl=\"experiments=[\\\"resource-apis\\\"]\""))' | tee /dev/stderr) - [ "${actual}" = "false" ] -} - -@test "server/StatefulSet: experiments=[\"resource-apis\"] is set in command when global.experiments contains \"resource-apis\"" { - cd `chart_dir` - local object=$(helm template \ - -s templates/server-statefulset.yaml \ - --set 'global.experiments[0]=resource-apis' \ - --set 'ui.enabled=false' \ - . | tee /dev/stderr) - - local actual=$(echo "$object" | - yq '.spec.template.spec.containers[] | select(.name == "consul") | .command | any(contains("-hcl=\"experiments=[\\\"resource-apis\\\"]\""))' | tee /dev/stderr) - [ "${actual}" = "true" ] } \ No newline at end of file