From 533964d4c2d60930c25d141dd11bce4e53d776cb Mon Sep 17 00:00:00 2001 From: Lord-Y Date: Tue, 21 Dec 2021 15:16:16 +0100 Subject: [PATCH 1/3] [charts/consul] - Enable terminationGracePeriodSeconds customization Signed-off-by: Lord-Y --- .../ingress-gateways-deployment.yaml | 2 +- .../unit/ingress-gateways-deployment.bats | 28 ++++++++++++++++++- charts/consul/values.yaml | 3 ++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml index 5b8c8dc4b7..8d0ba09cdb 100644 --- a/charts/consul/templates/ingress-gateways-deployment.yaml +++ b/charts/consul/templates/ingress-gateways-deployment.yaml @@ -88,7 +88,7 @@ spec: tolerations: {{ tpl (default $defaults.tolerations .tolerations) $root | nindent 8 | trim }} {{- end }} - terminationGracePeriodSeconds: 10 + terminationGracePeriodSeconds: {{ default $defaults.terminationGracePeriodSeconds .terminationGracePeriodSeconds }} serviceAccountName: {{ template "consul.fullname" $root }}-{{ .name }} volumes: - name: consul-bin diff --git a/charts/consul/test/unit/ingress-gateways-deployment.bats b/charts/consul/test/unit/ingress-gateways-deployment.bats index c22e325771..1a4d184bfc 100644 --- a/charts/consul/test/unit/ingress-gateways-deployment.bats +++ b/charts/consul/test/unit/ingress-gateways-deployment.bats @@ -1609,4 +1609,30 @@ EOF [ "${actual}" = "ca" ] local actual=$(echo $object | yq -r '.metadata.annotations."vault.hashicorp.com/ca-cert"') [ "${actual}" = "/vault/custom/tls.crt" ] -} \ No newline at end of file +} + +#-------------------------------------------------------------------- +# terminationGracePeriodSeconds + +@test "ingressGateways/Deployment: terminationGracePeriodSeconds defaults to 10" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/ingress-gateways-deployment.yaml \ + --set 'ingressGateways.enabled=true' \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq -s -r '.[0].spec.template.spec.terminationGracePeriodSeconds' | tee /dev/stderr) + [ "${actual}" = "10" ] +} + +@test "ingressGateways/Deployment: terminationGracePeriodSeconds set defaults to 30" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/ingress-gateways-deployment.yaml \ + --set 'ingressGateways.enabled=true' \ + --set 'connectInject.enabled=true' \ + --set 'ingressGateways.defaults.terminationGracePeriodSeconds=30' \ + . | tee /dev/stderr | + yq -s -r '.[0].spec.template.spec.terminationGracePeriodSeconds' | tee /dev/stderr) + [ "${actual}" = "30" ] +} diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 800c640528..ffb3e2247d 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2304,6 +2304,9 @@ ingressGateways: # Optional priorityClassName. priorityClassName: "" + # Amount of seconds to wait before killing the pod + terminationGracePeriodSeconds: 10 + # Annotations to apply to the ingress gateway deployment. Annotations defined # here will be applied to all ingress gateway deployments in addition to any # annotations defined for a specific gateway in `ingressGateways.gateways`. From 190e87c268b26865fd88c0d1faff907f39f61b46 Mon Sep 17 00:00:00 2001 From: Lord-Y Date: Wed, 22 Dec 2021 10:57:39 +0100 Subject: [PATCH 2/3] [charts/consul] - Update terminationGracePeriodSeconds test Signed-off-by: Lord-Y --- charts/consul/test/unit/ingress-gateways-deployment.bats | 7 +++++-- charts/consul/values.yaml | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/charts/consul/test/unit/ingress-gateways-deployment.bats b/charts/consul/test/unit/ingress-gateways-deployment.bats index 1a4d184bfc..2fd08a35f3 100644 --- a/charts/consul/test/unit/ingress-gateways-deployment.bats +++ b/charts/consul/test/unit/ingress-gateways-deployment.bats @@ -1614,7 +1614,7 @@ EOF #-------------------------------------------------------------------- # terminationGracePeriodSeconds -@test "ingressGateways/Deployment: terminationGracePeriodSeconds defaults to 10" { +@test "ingressGateways/Deployment: can set terminationGracePeriodSeconds through defaults" { cd `chart_dir` local actual=$(helm template \ -s templates/ingress-gateways-deployment.yaml \ @@ -1625,12 +1625,15 @@ EOF [ "${actual}" = "10" ] } -@test "ingressGateways/Deployment: terminationGracePeriodSeconds set defaults to 30" { +@test "ingressGateways/Deployment: can set terminationGracePeriodSeconds through specific gateway overriding defaults" { cd `chart_dir` local actual=$(helm template \ -s templates/ingress-gateways-deployment.yaml \ --set 'ingressGateways.enabled=true' \ --set 'connectInject.enabled=true' \ + --set 'ingressGateways.defaults.replicas=3' \ + --set 'ingressGateways.gateways[0].name=gateway1' \ + --set 'ingressGateways.gateways[0].replicas=12' \ --set 'ingressGateways.defaults.terminationGracePeriodSeconds=30' \ . | tee /dev/stderr | yq -s -r '.[0].spec.template.spec.terminationGracePeriodSeconds' | tee /dev/stderr) diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index ffb3e2247d..c97aef15ce 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2304,7 +2304,7 @@ ingressGateways: # Optional priorityClassName. priorityClassName: "" - # Amount of seconds to wait before killing the pod + # Amount of seconds to wait for graceful termination before killing the pod. terminationGracePeriodSeconds: 10 # Annotations to apply to the ingress gateway deployment. Annotations defined From c7a21789bb940a55017463ad017070d46c8a2189 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Tue, 4 Jan 2022 09:43:05 -0800 Subject: [PATCH 3/3] charts/consul: update tests and changelog for #938 by @Lord-Y --- CHANGELOG.md | 3 +++ .../unit/ingress-gateways-deployment.bats | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66de91324f..17f3b4d878 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ ## UNRELEASED +IMPROVEMENTS: +* Helm + * Allow customization of `terminationGracePeriodSeconds` on the ingress gateways. [[GH-947](https://github.com/hashicorp/consul-k8s/pull/947)] ## 0.39.0 (December 15, 2021) diff --git a/charts/consul/test/unit/ingress-gateways-deployment.bats b/charts/consul/test/unit/ingress-gateways-deployment.bats index 2fd08a35f3..b72819c8ff 100644 --- a/charts/consul/test/unit/ingress-gateways-deployment.bats +++ b/charts/consul/test/unit/ingress-gateways-deployment.bats @@ -1614,7 +1614,7 @@ EOF #-------------------------------------------------------------------- # terminationGracePeriodSeconds -@test "ingressGateways/Deployment: can set terminationGracePeriodSeconds through defaults" { +@test "ingressGateways/Deployment: terminationGracePeriodSeconds defaults to 10" { cd `chart_dir` local actual=$(helm template \ -s templates/ingress-gateways-deployment.yaml \ @@ -1625,16 +1625,27 @@ EOF [ "${actual}" = "10" ] } +@test "ingressGateways/Deployment: terminationGracePeriodSeconds can be set through defaults" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/ingress-gateways-deployment.yaml \ + --set 'ingressGateways.enabled=true' \ + --set 'connectInject.enabled=true' \ + --set 'ingressGateways.defaults.terminationGracePeriodSeconds=5' \ + . | tee /dev/stderr | + yq -s -r '.[0].spec.template.spec.terminationGracePeriodSeconds' | tee /dev/stderr) + [ "${actual}" = "5" ] +} + @test "ingressGateways/Deployment: can set terminationGracePeriodSeconds through specific gateway overriding defaults" { cd `chart_dir` local actual=$(helm template \ -s templates/ingress-gateways-deployment.yaml \ --set 'ingressGateways.enabled=true' \ --set 'connectInject.enabled=true' \ - --set 'ingressGateways.defaults.replicas=3' \ + --set 'ingressGateways.defaults.terminationGracePeriodSeconds=5' \ --set 'ingressGateways.gateways[0].name=gateway1' \ - --set 'ingressGateways.gateways[0].replicas=12' \ - --set 'ingressGateways.defaults.terminationGracePeriodSeconds=30' \ + --set 'ingressGateways.gateways[0].terminationGracePeriodSeconds=30' \ . | tee /dev/stderr | yq -s -r '.[0].spec.template.spec.terminationGracePeriodSeconds' | tee /dev/stderr) [ "${actual}" = "30" ]