From 5a864f7cbb59e9147c34484f50cd0a4168600d51 Mon Sep 17 00:00:00 2001 From: Theron Voran Date: Wed, 15 Sep 2021 18:43:04 -0700 Subject: [PATCH] Adding support for the old leader-elector (#607) Adds the leader-elector container support that was removed in PR #568. The new vault-k8s uses an internal mechanism for leader determination, so this is just for backwards compatibility, and can be removed in the near future. * mark the endpoint as deprecated * add a new useContainer option for leaderElector Default to not deploying the old leader-elector container, unless injector.leaderElector.useContainer is `true`. --- templates/injector-certs-secret.yaml | 2 +- templates/injector-deployment.yaml | 29 +++++ templates/injector-leader-endpoint.yaml | 14 +++ templates/injector-role.yaml | 4 +- templates/injector-rolebinding.yaml | 2 +- test/acceptance/injector-leader-elector.bats | 13 ++- test/unit/injector-leader-elector.bats | 107 ++++++++++++++++++- values.schema.json | 17 +++ values.yaml | 10 ++ 9 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 templates/injector-leader-endpoint.yaml diff --git a/templates/injector-certs-secret.yaml b/templates/injector-certs-secret.yaml index aec8021..78363be 100644 --- a/templates/injector-certs-secret.yaml +++ b/templates/injector-certs-secret.yaml @@ -7,4 +7,4 @@ metadata: app.kubernetes.io/name: {{ include "vault.name" . }}-agent-injector app.kubernetes.io/instance: {{ .Release.Name }} app.kubernetes.io/managed-by: {{ .Release.Service }} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/templates/injector-deployment.yaml b/templates/injector-deployment.yaml index e753c9c..f4a796b 100644 --- a/templates/injector-deployment.yaml +++ b/templates/injector-deployment.yaml @@ -137,6 +137,35 @@ spec: periodSeconds: 2 successThreshold: 1 timeoutSeconds: 5 + {{- if and (eq (.Values.injector.leaderElector.enabled | toString) "true") (gt (.Values.injector.replicas | int) 1) (eq (.Values.injector.leaderElector.useContainer | toString) "true") }} + - name: leader-elector + image: {{ .Values.injector.leaderElector.image.repository }}:{{ .Values.injector.leaderElector.image.tag }} + args: + - --election={{ template "vault.fullname" . }}-agent-injector-leader + - --election-namespace={{ .Release.Namespace }} + - --http=0.0.0.0:4040 + - --ttl={{ .Values.injector.leaderElector.ttl }} + livenessProbe: + httpGet: + path: / + port: 4040 + scheme: HTTP + failureThreshold: 2 + initialDelaySeconds: 5 + periodSeconds: 2 + successThreshold: 1 + timeoutSeconds: 5 + readinessProbe: + httpGet: + path: / + port: 4040 + scheme: HTTP + failureThreshold: 2 + initialDelaySeconds: 5 + periodSeconds: 2 + successThreshold: 1 + timeoutSeconds: 5 + {{- end }} {{- if .Values.injector.certs.secretName }} volumeMounts: - name: webhook-certs diff --git a/templates/injector-leader-endpoint.yaml b/templates/injector-leader-endpoint.yaml new file mode 100644 index 0000000..42c4c0a --- /dev/null +++ b/templates/injector-leader-endpoint.yaml @@ -0,0 +1,14 @@ +{{- if and (eq (.Values.injector.enabled | toString) "true" ) (eq (.Values.global.enabled | toString) "true") (eq (.Values.injector.leaderElector.enabled | toString) "true") (gt (.Values.injector.replicas | int) 1) (eq (.Values.injector.leaderElector.useContainer | toString) "true")}} +# This is created here so it can be cleaned up easily, since if +# the endpoint is left around the leader won't expire for about a minute. +apiVersion: v1 +kind: Endpoints +metadata: + name: {{ template "vault.fullname" . }}-agent-injector-leader + annotations: + deprecated: "true" + labels: + app.kubernetes.io/name: {{ include "vault.name" . }}-agent-injector + app.kubernetes.io/instance: {{ .Release.Name }} + app.kubernetes.io/managed-by: {{ .Release.Service }} +{{- end }} diff --git a/templates/injector-role.yaml b/templates/injector-role.yaml index 88fe53f..446efaf 100644 --- a/templates/injector-role.yaml +++ b/templates/injector-role.yaml @@ -9,7 +9,7 @@ metadata: app.kubernetes.io/managed-by: {{ .Release.Service }} rules: - apiGroups: [""] - resources: ["secrets", "configmaps"] + resources: ["secrets", "configmaps", "endpoints"] verbs: - "create" - "get" @@ -22,4 +22,4 @@ rules: - "get" - "patch" - "delete" -{{- end }} \ No newline at end of file +{{- end }} diff --git a/templates/injector-rolebinding.yaml b/templates/injector-rolebinding.yaml index e06d242..aa81794 100644 --- a/templates/injector-rolebinding.yaml +++ b/templates/injector-rolebinding.yaml @@ -15,4 +15,4 @@ subjects: - kind: ServiceAccount name: {{ template "vault.fullname" . }}-agent-injector namespace: {{ .Release.Namespace }} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/test/acceptance/injector-leader-elector.bats b/test/acceptance/injector-leader-elector.bats index 4c7154c..6f9f0b4 100644 --- a/test/acceptance/injector-leader-elector.bats +++ b/test/acceptance/injector-leader-elector.bats @@ -12,7 +12,8 @@ load _helpers helm install "$(name_prefix)" \ --wait \ --timeout=5m \ - --set="injector.replicas=3" . + --set="injector.replicas=3" \ + --set="injector.leaderElector.useContainer=true" . kubectl wait --for condition=Ready pod -l app.kubernetes.io/name=vault-agent-injector --timeout=5m pods=($(kubectl get pods -l app.kubernetes.io/name=vault-agent-injector -o json | jq -r '.items[] | .metadata.name')) @@ -22,15 +23,21 @@ load _helpers tries=0 until [ $tries -ge 60 ] do + ## The new internal leader mechanism uses a ConfigMap owner=$(kubectl get configmaps vault-k8s-leader -o json | jq -r .metadata.ownerReferences\[0\].name) leader=$(kubectl get pods $owner -o json | jq -r .metadata.name) [ -n "${leader}" ] && [ "${leader}" != "null" ] && break - let "tries=tries+1" + + ## Also check the old leader-elector container + old_leader="$(echo "$(kubectl exec ${pods[0]} -c sidecar-injector -- wget --quiet --output-document - localhost:4040)" | jq -r .name)" + [ -n "${old_leader}" ] && break + + ((++tries)) sleep .5 done # Check the leader name is valid - i.e. one of the 3 pods - [[ " ${pods[@]} " =~ " ${leader} " ]] + [[ " ${pods[@]} " =~ " ${leader} " || " ${pods[@]} " =~ " ${old_leader} " ]] } diff --git a/test/unit/injector-leader-elector.bats b/test/unit/injector-leader-elector.bats index a1b27a4..75ab298 100644 --- a/test/unit/injector-leader-elector.bats +++ b/test/unit/injector-leader-elector.bats @@ -165,4 +165,109 @@ load _helpers . || echo "---") | tee /dev/stderr | yq 'length > 0' | tee /dev/stderr) [ "${actual}" = "true" ] -} \ No newline at end of file +} + +#-------------------------------------------------------------------- +# Old leader-elector container support +# Note: deprecated and will be removed soon + +@test "injector/deployment: leader elector - sidecar is created only when enabled" { + cd `chart_dir` + local actual=$(helm template \ + --show-only templates/injector-deployment.yaml \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers | length' | tee /dev/stderr) + [ "${actual}" = "1" ] + + local actual=$(helm template \ + --show-only templates/injector-deployment.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.enabled=false" \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers | length' | tee /dev/stderr) + [ "${actual}" = "1" ] + + local actual=$(helm template \ + --show-only templates/injector-deployment.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.useContainer=true" \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers | length' | tee /dev/stderr) + [ "${actual}" = "2" ] +} + +@test "injector/deployment: leader elector image name is configurable" { + cd `chart_dir` + local actual=$(helm template \ + --show-only templates/injector-deployment.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.useContainer=true" \ + --set "injector.leaderElector.image.repository=SomeOtherImage" \ + --set "injector.leaderElector.image.tag=SomeOtherTag" \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[1].image' | tee /dev/stderr) + [ "${actual}" = "SomeOtherImage:SomeOtherTag" ] +} + +@test "injector/deployment: leader elector TTL is configurable" { + cd `chart_dir` + # Default value 60s + local actual=$(helm template \ + --show-only templates/injector-deployment.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.useContainer=true" \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[1].args[3]' | tee /dev/stderr) + [ "${actual}" = "--ttl=60s" ] + + # Configured to 30s + local actual=$(helm template \ + --show-only templates/injector-deployment.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.useContainer=true" \ + --set "injector.leaderElector.ttl=30s" \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[1].args[3]' | tee /dev/stderr) + [ "${actual}" = "--ttl=30s" ] +} + +@test "injector/leader-endpoint: created/skipped as appropriate" { + cd `chart_dir` + local actual=$( (helm template \ + --show-only templates/injector-leader-endpoint.yaml \ + . || echo "---") | tee /dev/stderr | + yq 'length > 0' | tee /dev/stderr) + [ "${actual}" = "false" ] + + local actual=$( (helm template \ + --show-only templates/injector-leader-endpoint.yaml \ + --set "injector.replicas=2" \ + --set "global.enabled=false" \ + . || echo "---") | tee /dev/stderr | + yq 'length > 0' | tee /dev/stderr) + [ "${actual}" = "false" ] + + local actual=$( (helm template \ + --show-only templates/injector-leader-endpoint.yaml \ + --set "injector.replicas=2" \ + --set "injector.enabled=false" \ + . || echo "---") | tee /dev/stderr | + yq 'length > 0' | tee /dev/stderr) + [ "${actual}" = "false" ] + + local actual=$( (helm template \ + --show-only templates/injector-leader-endpoint.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.enabled=false" \ + . || echo "---") | tee /dev/stderr | + yq 'length > 0' | tee /dev/stderr) + [ "${actual}" = "false" ] + + local actual=$( (helm template \ + --show-only templates/injector-leader-endpoint.yaml \ + --set "injector.replicas=2" \ + --set "injector.leaderElector.useContainer=true" \ + . || echo "---") | tee /dev/stderr | + yq 'length > 0' | tee /dev/stderr) + [ "${actual}" = "true" ] +} diff --git a/values.schema.json b/values.schema.json index 76ecb59..4c0a004 100644 --- a/values.schema.json +++ b/values.schema.json @@ -287,6 +287,23 @@ "properties": { "enabled": { "type": "boolean" + }, + "image": { + "type": "object", + "properties": { + "repository": { + "type": "string" + }, + "tag": { + "type": "string" + } + } + }, + "ttl": { + "type": "string" + }, + "useContainer": { + "type": "boolean" } } }, diff --git a/values.yaml b/values.yaml index fe998f5..d71460a 100644 --- a/values.yaml +++ b/values.yaml @@ -37,6 +37,16 @@ injector: # so that only one injector attempts to create TLS certificates. leaderElector: enabled: true + # Note: The deployment of the leader-elector container will soon be removed + # from this chart since vault-k8s now uses an internal mechanism to + # determine leadership. + # To enable the deployment of the leader-elector container for use with + # vault-k8s 0.12.0 and earlier, set `useContainer=true` + useContainer: false + image: + repository: "gcr.io/google_containers/leader-elector" + tag: "0.4" + ttl: 60s # If true, will enable a node exporter metrics endpoint at /metrics. metrics: