From 0df999d546321978dda058cd01979caed6f81459 Mon Sep 17 00:00:00 2001 From: antruigon Date: Thu, 6 Aug 2026 14:06:38 +0200 Subject: [PATCH 1/2] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart The chart ships a default PgBouncer preStop hook that drains client connections for up to 120 seconds (killall -INT pgbouncer && sleep 120), but the Deployment never sets terminationGracePeriodSeconds, so the Kubernetes default of 30s SIGKILLs the pod mid-drain on any node drain or eviction, cutting in-flight client connections. Every other long-running component in the chart (scheduler, workers, triggerer, dag-processor, statsd, redis, otel-collector) already exposes this value; PgBouncer was the only one missing it. Defaults to 120 to match the drain window of the default preStop hook. Since PgBouncer exits as soon as the last client connection is released, the full 120s is an upper bound, not a fixed wait. Co-Authored-By: Claude Fable 5 --- .../templates/pgbouncer/pgbouncer-deployment.yaml | 1 + chart/tests/helm_tests/other/test_pgbouncer.py | 14 ++++++++++++++ chart/values.schema.json | 6 ++++++ chart/values.yaml | 6 ++++++ 4 files changed, 27 insertions(+) diff --git a/chart/templates/pgbouncer/pgbouncer-deployment.yaml b/chart/templates/pgbouncer/pgbouncer-deployment.yaml index b1aad5e1d0e08..97470b7910842 100644 --- a/chart/templates/pgbouncer/pgbouncer-deployment.yaml +++ b/chart/templates/pgbouncer/pgbouncer-deployment.yaml @@ -87,6 +87,7 @@ spec: {{- end }} tolerations: {{- toYaml $tolerations | nindent 8 }} topologySpreadConstraints: {{- toYaml $topologySpreadConstraints | nindent 8 }} + terminationGracePeriodSeconds: {{ .Values.pgbouncer.terminationGracePeriodSeconds }} serviceAccountName: {{ include "pgbouncer.serviceAccountName" . }} enableServiceLinks: {{ .Values.enableServiceLinks }} securityContext: {{ $securityContext | nindent 8 }} diff --git a/chart/tests/helm_tests/other/test_pgbouncer.py b/chart/tests/helm_tests/other/test_pgbouncer.py index cf69a39cab2f6..93b0f70ebc44e 100644 --- a/chart/tests/helm_tests/other/test_pgbouncer.py +++ b/chart/tests/helm_tests/other/test_pgbouncer.py @@ -453,6 +453,20 @@ def test_should_add_component_specific_labels(self): assert "labels" in jmespath.search("spec.template.metadata", docs[0]) assert jmespath.search("spec.template.metadata.labels", docs[0])["test_label"] == "test_label_value" + @pytest.mark.parametrize( + ("pgbouncer_values", "expected"), + [ + ({"enabled": True}, 120), + ({"enabled": True, "terminationGracePeriodSeconds": 30}, 30), + ], + ) + def test_pgbouncer_termination_grace_period_seconds(self, pgbouncer_values, expected): + docs = render_chart( + values={"pgbouncer": pgbouncer_values}, + show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"], + ) + assert expected == jmespath.search("spec.template.spec.terminationGracePeriodSeconds", docs[0]) + class TestPgbouncerConfig: """Tests PgBouncer config.""" diff --git a/chart/values.schema.json b/chart/values.schema.json index 7402d3b48f7d6..8999b7d86ca3c 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -7801,6 +7801,12 @@ } ] }, + "terminationGracePeriodSeconds": { + "description": "Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes. The default matches the default preStop hook, which needs up to 120 seconds to drain client connections.", + "type": "integer", + "default": 120, + "x-docsSection": "Kubernetes" + }, "securityContexts": { "description": "Security context definition for the PgBouncer.", "type": "object", diff --git a/chart/values.yaml b/chart/values.yaml index 4551250ee0ac4..6ea10584962b0 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -2918,6 +2918,12 @@ pgbouncer: # Allow existing queries clients to complete within 120 seconds command: ["/bin/sh", "-c", "killall -INT pgbouncer && sleep 120"] + # Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes. + # Matches the default preStop hook above, which needs up to 120 seconds to + # drain client connections; with a shorter grace period the pod is killed + # mid-drain and in-flight connections are cut. + terminationGracePeriodSeconds: 120 + metricsExporterSidecar: resources: {} # limits: From 89ef4d3b63eed4c56b50f2390f76a665de6b3a16 Mon Sep 17 00:00:00 2001 From: antruigon Date: Thu, 6 Aug 2026 14:07:19 +0200 Subject: [PATCH 2/2] Add newsfragment for pgbouncer terminationGracePeriodSeconds Co-Authored-By: Claude Fable 5 --- chart/newsfragments/71237.significant.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 chart/newsfragments/71237.significant.rst diff --git a/chart/newsfragments/71237.significant.rst b/chart/newsfragments/71237.significant.rst new file mode 100644 index 0000000000000..bf678fdb9ac99 --- /dev/null +++ b/chart/newsfragments/71237.significant.rst @@ -0,0 +1,5 @@ +PgBouncer Deployment now sets ``terminationGracePeriodSeconds`` (default ``120``) + +The PgBouncer Deployment previously relied on the Kubernetes default termination grace period of 30 seconds, which SIGKILLed the pod mid-drain: the chart's default preStop hook (``killall -INT pgbouncer && sleep 120``) needs up to 120 seconds to drain client connections, so on a node drain or eviction in-flight connections were cut after 30 seconds instead of being released. + +The new ``pgbouncer.terminationGracePeriodSeconds`` value defaults to ``120`` to match the preStop drain window. PgBouncer exits as soon as the last client connection is released, so the full 120 seconds is an upper bound, not a fixed wait. Set the value to ``30`` to restore the previous behaviour.