From 37098239c6abce13dbf1f2e1ac74edbe6e40e33f Mon Sep 17 00:00:00 2001 From: DanStough Date: Wed, 21 Jun 2023 15:50:13 -0400 Subject: [PATCH 1/4] feat(helm): add configurable server-acl-init and cleanup resource limits --- .changelog/2416.txt | 3 +++ .../server-acl-init-cleanup-job.yaml | 9 +++---- .../consul/templates/server-acl-init-job.yaml | 9 +++---- .../unit/server-acl-init-cleanup-job.bats | 22 +++++++++++++++ .../consul/test/unit/server-acl-init-job.bats | 22 +++++++++++++++ charts/consul/values.yaml | 27 +++++++++++++++++++ 6 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 .changelog/2416.txt diff --git a/.changelog/2416.txt b/.changelog/2416.txt new file mode 100644 index 0000000000..e261758542 --- /dev/null +++ b/.changelog/2416.txt @@ -0,0 +1,3 @@ +```release-note:feature +helm: Adds `acls.resources` field which can be configured to override the `resource` settings for the `server-acl-init` and `server-acl-init-cleanup` Jobs. +``` diff --git a/charts/consul/templates/server-acl-init-cleanup-job.yaml b/charts/consul/templates/server-acl-init-cleanup-job.yaml index 3676144b40..fb48630afd 100644 --- a/charts/consul/templates/server-acl-init-cleanup-job.yaml +++ b/charts/consul/templates/server-acl-init-cleanup-job.yaml @@ -62,13 +62,10 @@ spec: - -log-json={{ .Values.global.logJSON }} - -k8s-namespace={{ .Release.Namespace }} - {{ template "consul.fullname" . }}-server-acl-init + {{- if .Values.acls.resources }} resources: - requests: - memory: "50Mi" - cpu: "50m" - limits: - memory: "50Mi" - cpu: "50m" + {{- toYaml .Values.server.resources | nindent 12 }} + {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index e42a073c42..9cad942fce 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -308,13 +308,10 @@ spec: {{- end }} {{- end }} {{- end }} + {{- if .Values.acls.resources }} resources: - requests: - memory: "50Mi" - cpu: "50m" - limits: - memory: "50Mi" - cpu: "50m" + {{- toYaml .Values.server.resources | nindent 10 }} + {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: {{ tpl .Values.global.acls.tolerations . | indent 8 | trim }} diff --git a/charts/consul/test/unit/server-acl-init-cleanup-job.bats b/charts/consul/test/unit/server-acl-init-cleanup-job.bats index 947cfa9b42..cf9b6795a7 100644 --- a/charts/consul/test/unit/server-acl-init-cleanup-job.bats +++ b/charts/consul/test/unit/server-acl-init-cleanup-job.bats @@ -159,3 +159,25 @@ load _helpers [ "${actualTemplateFoo}" = "bar" ] [ "${actualTemplateBaz}" = "qux" ] } + +#-------------------------------------------------------------------- +# resources + +@test "serverACLInitCleanup/Job: resources defined by default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-acl-init-cleanup-job.yaml \ + . | tee /dev/stderr | + yq -rc '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + [ "${actual}" = '{"limits":{"cpu":"50m","memory":"50Mi"},"requests":{"cpu":"50m","memory":"50Mi"}}' ] +} + +@test "serverACLInitCleanup/Job: resources can be overridden" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-acl-init-cleanup-job.yaml \ + --set 'acls.resources.foo=bar' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr) + [ "${actual}" = "bar" ] +} diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 81064c95eb..40d1bfa960 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -2202,3 +2202,25 @@ load _helpers [ "${actualTemplateFoo}" = "bar" ] [ "${actualTemplateBaz}" = "qux" ] } + +#-------------------------------------------------------------------- +# resources + +@test "serverACLInit/Job: resources defined by default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-acl-init-job.yaml \ + . | tee /dev/stderr | + yq -rc '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + [ "${actual}" = '{"limits":{"cpu":"50m","memory":"50Mi"},"requests":{"cpu":"50m","memory":"50Mi"}}' ] +} + +@test "serverACLInit/Job: resources can be overridden" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-acl-init-job.yaml \ + --set 'acls.resources.foo=bar' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr) + [ "${actual}" = "bar" ] +} diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index faf7f5bcdf..00c69a9934 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -436,6 +436,33 @@ global: # @type: string secretKey: null + # The resource requests (CPU, memory, etc.) for the server-acl-init and server-acl-init-cleanup pods. + # This should be a YAML map corresponding to a Kubernetes + # [`ResourceRequirements``](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#resourcerequirements-v1-core) + # object. + # + # Example: + # + # ```yaml + # resources: + # requests: + # memory: '200Mi' + # cpu: '100m' + # limits: + # memory: '200Mi' + # cpu: '100m' + # ``` + # + # @recurse: false + # @type: map + resources: + requests: + memory: "50Mi" + cpu: "50m" + limits: + memory: "50Mi" + cpu: "50m" + # partitionToken references a Vault secret containing the ACL token to be used in non-default partitions. # This value should only be provided in the default partition and only when setting # the `global.secretsBackend.vault.enabled` value to true. From d50e0560ca6ec60dab368157d838935cf4aaa018 Mon Sep 17 00:00:00 2001 From: Dan Stough Date: Wed, 21 Jun 2023 16:06:05 -0400 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Ashwin Venkatesh --- charts/consul/templates/server-acl-init-cleanup-job.yaml | 2 +- charts/consul/templates/server-acl-init-job.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/consul/templates/server-acl-init-cleanup-job.yaml b/charts/consul/templates/server-acl-init-cleanup-job.yaml index fb48630afd..2b79fd54cd 100644 --- a/charts/consul/templates/server-acl-init-cleanup-job.yaml +++ b/charts/consul/templates/server-acl-init-cleanup-job.yaml @@ -64,7 +64,7 @@ spec: - {{ template "consul.fullname" . }}-server-acl-init {{- if .Values.acls.resources }} resources: - {{- toYaml .Values.server.resources | nindent 12 }} + {{- toYaml .Values.acls.resources | nindent 12 }} {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index 9cad942fce..c47d4600c1 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -310,7 +310,7 @@ spec: {{- end }} {{- if .Values.acls.resources }} resources: - {{- toYaml .Values.server.resources | nindent 10 }} + {{- toYaml .Values.acls.resources | nindent 10 }} {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: From f091ff756e8f7938447227f7782c71ecd6e660ad Mon Sep 17 00:00:00 2001 From: DanStough Date: Wed, 21 Jun 2023 16:27:05 -0400 Subject: [PATCH 3/4] bugfix yaml path --- charts/consul/templates/server-acl-init-cleanup-job.yaml | 4 ++-- charts/consul/templates/server-acl-init-job.yaml | 4 ++-- charts/consul/test/unit/server-acl-init-cleanup-job.bats | 2 +- charts/consul/test/unit/server-acl-init-job.bats | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/charts/consul/templates/server-acl-init-cleanup-job.yaml b/charts/consul/templates/server-acl-init-cleanup-job.yaml index 2b79fd54cd..00a1e294d2 100644 --- a/charts/consul/templates/server-acl-init-cleanup-job.yaml +++ b/charts/consul/templates/server-acl-init-cleanup-job.yaml @@ -62,9 +62,9 @@ spec: - -log-json={{ .Values.global.logJSON }} - -k8s-namespace={{ .Release.Namespace }} - {{ template "consul.fullname" . }}-server-acl-init - {{- if .Values.acls.resources }} + {{- if .Values.global.acls.resources }} resources: - {{- toYaml .Values.acls.resources | nindent 12 }} + {{- toYaml .Values.global.acls.resources | nindent 12 }} {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index c47d4600c1..43f55486a6 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -308,9 +308,9 @@ spec: {{- end }} {{- end }} {{- end }} - {{- if .Values.acls.resources }} + {{- if .Values.global.acls.resources }} resources: - {{- toYaml .Values.acls.resources | nindent 10 }} + {{- toYaml .Values.global.acls.resources | nindent 10 }} {{- end }} {{- if .Values.global.acls.tolerations }} tolerations: diff --git a/charts/consul/test/unit/server-acl-init-cleanup-job.bats b/charts/consul/test/unit/server-acl-init-cleanup-job.bats index cf9b6795a7..a51c2a5abd 100644 --- a/charts/consul/test/unit/server-acl-init-cleanup-job.bats +++ b/charts/consul/test/unit/server-acl-init-cleanup-job.bats @@ -176,7 +176,7 @@ load _helpers cd `chart_dir` local actual=$(helm template \ -s templates/server-acl-init-cleanup-job.yaml \ - --set 'acls.resources.foo=bar' \ + --set 'global.acls.resources.foo=bar' \ . | tee /dev/stderr | yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr) [ "${actual}" = "bar" ] diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 40d1bfa960..f3dedc2835 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -2219,7 +2219,7 @@ load _helpers cd `chart_dir` local actual=$(helm template \ -s templates/server-acl-init-job.yaml \ - --set 'acls.resources.foo=bar' \ + --set 'global.acls.resources.foo=bar' \ . | tee /dev/stderr | yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr) [ "${actual}" = "bar" ] From 9bdb4694c7494492b468f7252dd98488a3858746 Mon Sep 17 00:00:00 2001 From: DanStough Date: Thu, 22 Jun 2023 11:27:08 -0400 Subject: [PATCH 4/4] fix bats test --- charts/consul/test/unit/server-acl-init-cleanup-job.bats | 6 ++++-- charts/consul/test/unit/server-acl-init-job.bats | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/charts/consul/test/unit/server-acl-init-cleanup-job.bats b/charts/consul/test/unit/server-acl-init-cleanup-job.bats index a51c2a5abd..bf6a455d0e 100644 --- a/charts/consul/test/unit/server-acl-init-cleanup-job.bats +++ b/charts/consul/test/unit/server-acl-init-cleanup-job.bats @@ -166,7 +166,8 @@ load _helpers @test "serverACLInitCleanup/Job: resources defined by default" { cd `chart_dir` local actual=$(helm template \ - -s templates/server-acl-init-cleanup-job.yaml \ + -s templates/server-acl-init-cleanup-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | yq -rc '.spec.template.spec.containers[0].resources' | tee /dev/stderr) [ "${actual}" = '{"limits":{"cpu":"50m","memory":"50Mi"},"requests":{"cpu":"50m","memory":"50Mi"}}' ] @@ -175,7 +176,8 @@ load _helpers @test "serverACLInitCleanup/Job: resources can be overridden" { cd `chart_dir` local actual=$(helm template \ - -s templates/server-acl-init-cleanup-job.yaml \ + -s templates/server-acl-init-cleanup-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ --set 'global.acls.resources.foo=bar' \ . | tee /dev/stderr | yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr) diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index f3dedc2835..a0d0950e89 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -2209,7 +2209,8 @@ load _helpers @test "serverACLInit/Job: resources defined by default" { cd `chart_dir` local actual=$(helm template \ - -s templates/server-acl-init-job.yaml \ + -s templates/server-acl-init-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | yq -rc '.spec.template.spec.containers[0].resources' | tee /dev/stderr) [ "${actual}" = '{"limits":{"cpu":"50m","memory":"50Mi"},"requests":{"cpu":"50m","memory":"50Mi"}}' ] @@ -2218,7 +2219,8 @@ load _helpers @test "serverACLInit/Job: resources can be overridden" { cd `chart_dir` local actual=$(helm template \ - -s templates/server-acl-init-job.yaml \ + -s templates/server-acl-init-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ --set 'global.acls.resources.foo=bar' \ . | tee /dev/stderr | yq -r '.spec.template.spec.containers[0].resources.foo' | tee /dev/stderr)