Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support vault-k8s internal leader election #568

Merged
merged 7 commits into from
Aug 31, 2021
Merged

Support vault-k8s internal leader election #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b3647d1
working on internal-leader
tvoran Jul 1, 2021
8e51a51
extra rbac and cleanup
tvoran Jul 10, 2021
04eeabb
Merge remote-tracking branch 'origin/master' into VAULT-2403/internal…
tvoran Jul 22, 2021
3746d7a
Updating acceptance test
tvoran Jul 22, 2021
78de5e6
fix unit tests, values, remove endpoint
tvoran Jul 22, 2021
17750ec
add pod delete to injector rbac
tvoran Aug 31, 2021
4ae80c4
Merge remote-tracking branch 'origin/master' into VAULT-2403/internal…
tvoran Aug 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 4 additions & 29 deletions templates/injector-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ spec:
- name: AGENT_INJECT_TEMPLATE_CONFIG_EXIT_ON_RETRY_FAILURE
value: "{{ .Values.injector.agentDefaults.templateConfig.exitOnRetryFailure }}"
{{- include "vault.extraEnvironmentVars" .Values.injector | nindent 12 }}
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
args:
- agent-inject
- 2>&1
Expand Down Expand Up @@ -139,35 +143,6 @@ spec:
mountPath: /etc/webhook/certs
readOnly: true
{{- end }}
{{- if and (eq (.Values.injector.leaderElector.enabled | toString) "true") (gt (.Values.injector.replicas | int) 1) }}
- 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 }}
volumes:
- name: webhook-certs
Expand Down
12 changes: 0 additions & 12 deletions templates/injector-leader-endpoint.yaml
View file
Open in desktop

This file was deleted.

8 changes: 7 additions & 1 deletion templates/injector-role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ metadata:
app.kubernetes.io/managed-by: {{ .Release.Service }}
rules:
- apiGroups: [""]
resources: ["endpoints", "secrets"]
resources: ["secrets", "configmaps"]
verbs:
- "create"
- "get"
- "watch"
- "list"
- "update"
- apiGroups: [""]
resources: ["pods"]
verbs:
- "get"
- "patch"
- "delete"
{{- end }}
13 changes: 4 additions & 9 deletions test/acceptance/injector-leader-elector.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,16 @@ load _helpers
tries=0
until [ $tries -ge 60 ]
do
leader="$(echo "$(kubectl exec ${pods[0]} -c sidecar-injector -- wget --quiet --output-document - localhost:4040)" | jq -r .name)"
[ -n "${leader}" ] && break
((tries++))
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"
Copy link
Contributor

Choose a reason for hiding this comment

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

You prefer to use let instead of the ++ operator?

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer whatever works, and this works 😄

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 explain more, ((tries++)) seems to always throw an error for me under bats.)

sleep .5
done

# Check the leader name is valid - i.e. one of the 3 pods
[[ " ${pods[@]} " =~ " ${leader} " ]]

# Check every pod agrees on who the leader is
for pod in "${pods[@]}"
do
pod_leader="$(echo "$(kubectl exec $pod -c sidecar-injector -- wget --quiet --output-document - localhost:4040)" | jq -r .name)"
[ "${pod_leader}" == "${leader}" ]
done
}

setup() {
Expand Down
98 changes: 1 addition & 97 deletions test/unit/injector-leader-elector.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ load _helpers
#--------------------------------------------------------------------
# Deployment

@test "injector/deployment: leader elector replica count" {
@test "injector/deployment: replica count" {
cd `chart_dir`
local actual=$(helm template \
--show-only templates/injector-deployment.yaml \
Expand All @@ -15,42 +15,6 @@ load _helpers
[ "${actual}" = "2" ]
}

@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" \
. | 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.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 configuration for sidecar-injector" {
cd `chart_dir`
local actual=$(helm template \
Expand Down Expand Up @@ -80,26 +44,6 @@ load _helpers
[ "${actual}" = "metadata.namespace" ]
}

@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" \
. | 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.ttl=30s" \
. | tee /dev/stderr |
yq -r '.spec.template.spec.containers[1].args[3]' | tee /dev/stderr)
[ "${actual}" = "--ttl=30s" ]
}

#--------------------------------------------------------------------
# Resource creation

Expand Down Expand Up @@ -143,46 +87,6 @@ load _helpers
[ "${actual}" = "true" ]
}

@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" \
. || echo "---") | tee /dev/stderr |
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "injector/role: created/skipped as appropriate" {
cd `chart_dir`
local actual=$( (helm template \
Expand Down
14 changes: 0 additions & 14 deletions values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,20 +281,6 @@
"properties": {
"enabled": {
"type": "boolean"
},
"image": {
"type": "object",
"properties": {
"repository": {
"type": "string"
},
"tag": {
"type": "string"
}
}
},
"ttl": {
"type": "string"
}
}
},
Expand Down
8 changes: 2 additions & 6 deletions values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,10 @@ injector:
# Configures the port the injector should listen on
port: 8080

# If multiple replicas are specified, by default a leader-elector side-car
# will be created so that only one injector attempts to create TLS certificates.
# If multiple replicas are specified, by default a leader will be determined
# so that only one injector attempts to create TLS certificates.
leaderElector:
enabled: true
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:
Expand Down