From ad36ca047d31a29a482b840ba1bb7e091947242a Mon Sep 17 00:00:00 2001 From: Rebecca Zanzig Date: Mon, 12 Nov 2018 14:16:24 -0800 Subject: [PATCH 1/2] Pass server resource limits on to the pods in the stateful set The values.yaml allowed users to define resource limitations for the server pods, but was not applying these to the pods in the server-statefulset.yaml. This fixes that oversight. --- templates/server-statefulset.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/templates/server-statefulset.yaml b/templates/server-statefulset.yaml index fd022024405b..6e6aea8a8336 100644 --- a/templates/server-statefulset.yaml +++ b/templates/server-statefulset.yaml @@ -146,6 +146,8 @@ spec: periodSeconds: 3 successThreshold: 1 timeoutSeconds: 5 + resources: +{{ toYaml .Values.server.resources | indent 12 }} volumeClaimTemplates: - metadata: name: data From 2bfcb1c1bf135699e6b880f7512c7c5b029be997 Mon Sep 17 00:00:00 2001 From: Rebecca Zanzig Date: Tue, 20 Nov 2018 09:39:25 -0800 Subject: [PATCH 2/2] Switch resource definitions to multi-line strings These values now default to null and are optional in the template files. Adds tests for the new values. Also changes the file extension on several of the test files from .yaml to .bats. --- templates/client-daemonset.yaml | 4 +- templates/server-statefulset.yaml | 4 +- ...t-daemonset.yaml => client-daemonset.bats} | 22 +++++++ ...ml => connect-inject-mutatingwebhook.bats} | 0 ...rvice.yaml => connect-inject-service.bats} | 0 test/unit/server-statefulset.bats | 61 +++++++++++++------ values.yaml | 14 +++-- 7 files changed, 78 insertions(+), 27 deletions(-) rename test/unit/{client-daemonset.yaml => client-daemonset.bats} (90%) rename test/unit/{connect-inject-mutatingwebhook.yaml => connect-inject-mutatingwebhook.bats} (100%) rename test/unit/{connect-inject-service.yaml => connect-inject-service.bats} (100%) diff --git a/templates/client-daemonset.yaml b/templates/client-daemonset.yaml index 754183278d96..9d0dddb1bdbb 100644 --- a/templates/client-daemonset.yaml +++ b/templates/client-daemonset.yaml @@ -145,6 +145,8 @@ spec: - | curl http://127.0.0.1:8500/v1/status/leader 2>/dev/null | \ grep -E '".+"' + {{- if .Values.client.resources }} resources: -{{ toYaml .Values.client.resources | indent 12 }} + {{ tpl .Values.client.resources . | nindent 12 | trim }} + {{- end }} {{- end }} diff --git a/templates/server-statefulset.yaml b/templates/server-statefulset.yaml index 6e6aea8a8336..5427cbd52478 100644 --- a/templates/server-statefulset.yaml +++ b/templates/server-statefulset.yaml @@ -146,8 +146,10 @@ spec: periodSeconds: 3 successThreshold: 1 timeoutSeconds: 5 + {{- if .Values.server.resources }} resources: -{{ toYaml .Values.server.resources | indent 12 }} + {{ tpl .Values.server.resources . | nindent 12 | trim }} + {{- end }} volumeClaimTemplates: - metadata: name: data diff --git a/test/unit/client-daemonset.yaml b/test/unit/client-daemonset.bats similarity index 90% rename from test/unit/client-daemonset.yaml rename to test/unit/client-daemonset.bats index 2b9dbdc53e93..9544d55ff348 100755 --- a/test/unit/client-daemonset.yaml +++ b/test/unit/client-daemonset.bats @@ -94,6 +94,28 @@ load _helpers [ "${actual}" = "true" ] } +#-------------------------------------------------------------------- +# resources + +@test "client/DaemonSet: no resources defined by default" { + cd `chart_dir` + local actual=$(helm template \ + -x templates/client-daemonset.yaml \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + [ "${actual}" = "null" ] +} + +@test "client/DaemonSet: resources can be set" { + cd `chart_dir` + local actual=$(helm template \ + -x templates/client-daemonset.yaml \ + --set 'client.resources=foo' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + [ "${actual}" = "foo" ] +} + #-------------------------------------------------------------------- # extraVolumes diff --git a/test/unit/connect-inject-mutatingwebhook.yaml b/test/unit/connect-inject-mutatingwebhook.bats similarity index 100% rename from test/unit/connect-inject-mutatingwebhook.yaml rename to test/unit/connect-inject-mutatingwebhook.bats diff --git a/test/unit/connect-inject-service.yaml b/test/unit/connect-inject-service.bats similarity index 100% rename from test/unit/connect-inject-service.yaml rename to test/unit/connect-inject-service.bats diff --git a/test/unit/server-statefulset.bats b/test/unit/server-statefulset.bats index f1bc2c9d1d71..db1a31cbcdc2 100755 --- a/test/unit/server-statefulset.bats +++ b/test/unit/server-statefulset.bats @@ -42,6 +42,9 @@ load _helpers [ "${actual}" = "false" ] } +#-------------------------------------------------------------------- +# image + @test "server/StatefulSet: image defaults to global.image" { cd `chart_dir` local actual=$(helm template \ @@ -64,7 +67,29 @@ load _helpers } #-------------------------------------------------------------------- -# updateStrategy +# resources + +@test "server/StatefulSet: no resources defined by default" { + cd `chart_dir` + local actual=$(helm template \ + -x templates/server-statefulset.yaml \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + [ "${actual}" = "null" ] +} + +@test "server/StatefulSet: resources can be set" { + cd `chart_dir` + local actual=$(helm template \ + -x templates/server-statefulset.yaml \ + --set 'server.resources=foo' \ + . | tee /dev/stderr | + yq -r '.spec.template.spec.containers[0].resources' | tee /dev/stderr) + [ "${actual}" = "foo" ] +} + +#-------------------------------------------------------------------- +# updateStrategy (derived from updatePartition) @test "server/StatefulSet: no updateStrategy when not updating" { cd `chart_dir` @@ -93,25 +118,25 @@ load _helpers } #-------------------------------------------------------------------- -# affinity +# storageClass -@test "server/StatefulSet: affinity not set with server.affinity" { +@test "server/StatefulSet: no storageClass on claim by default" { cd `chart_dir` local actual=$(helm template \ -x templates/server-statefulset.yaml \ - --set 'server.affinity=null' \ . | tee /dev/stderr | - yq '.spec.template.spec | .affinity? == null' | tee /dev/stderr) - [ "${actual}" = "true" ] + yq -r '.spec.volumeClaimTemplates[0].spec.storageClassName' | tee /dev/stderr) + [ "${actual}" = "null" ] } -@test "server/StatefulSet: affinity set by default" { +@test "server/StatefulSet: can set storageClass" { cd `chart_dir` local actual=$(helm template \ -x templates/server-statefulset.yaml \ + --set 'server.storageClass=foo' \ . | tee /dev/stderr | - yq '.spec.template.spec.affinity | .podAntiAffinity? != null' | tee /dev/stderr) - [ "${actual}" = "true" ] + yq -r '.spec.volumeClaimTemplates[0].spec.storageClassName' | tee /dev/stderr) + [ "${actual}" = "foo" ] } #-------------------------------------------------------------------- @@ -220,25 +245,23 @@ load _helpers } #-------------------------------------------------------------------- -# updateStrategy +# affinity -@test "server/StatefulSet: no storageClass on claim by default" { +@test "server/StatefulSet: affinity not set with server.affinity=null" { cd `chart_dir` local actual=$(helm template \ -x templates/server-statefulset.yaml \ + --set 'server.affinity=null' \ . | tee /dev/stderr | - yq -r '.spec.volumeClaimTemplates[0].spec.storageClassName' | tee /dev/stderr) - [ "${actual}" = "null" ] + yq '.spec.template.spec | .affinity? == null' | tee /dev/stderr) + [ "${actual}" = "true" ] } - -@test "server/StatefulSet: can set storageClass" { +@test "server/StatefulSet: affinity set by default" { cd `chart_dir` local actual=$(helm template \ -x templates/server-statefulset.yaml \ - --set 'server.storageClass=foo' \ . | tee /dev/stderr | - yq -r '.spec.volumeClaimTemplates[0].spec.storageClassName' | tee /dev/stderr) - [ "${actual}" = "foo" ] + yq '.spec.template.spec.affinity | .podAntiAffinity? != null' | tee /dev/stderr) + [ "${actual}" = "true" ] } - diff --git a/values.yaml b/values.yaml index f18f4d164006..29cb44fe8fe5 100644 --- a/values.yaml +++ b/values.yaml @@ -48,9 +48,10 @@ server: connect: true # Resource requests, limits, etc. for the server cluster placement. This - # should map directly to the value of the resources field for a PodSpec. - # By default no direct resource request is made. - resources: {} + # should map directly to the value of the resources field for a PodSpec, + # formatted as a multi-line string. By default no direct resource request + # is made. + resources: null # updatePartition is used to control a careful rolling update of Consul # servers. This should be done particularly when changing the version @@ -105,9 +106,10 @@ client: grpc: false # Resource requests, limits, etc. for the client cluster placement. This - # should map directly to the value of the resources field for a PodSpec. - # By default no direct resource request is made. - resources: {} + # should map directly to the value of the resources field for a PodSpec, + # formatted as a multi-line string. By default no direct resource request + # is made. + resources: null # extraConfig is a raw string of extra configuration to set with the # server. This should be JSON.