Skip to content

Commit 2180608

Browse files
jedcunninghamjedcunningham
authored
Default resultBackendConnection to metadataConnection (apache#15861)
 Instead of requiring anyone using an external db and CeleryExecutor to set their database details twice, let the default for `resultBackendConnection` be the values from `metadataConnection`. Anyone who wants to use a separate backend for results still can.
1 parent 6f646fd commit 2180608

6 files changed

+91
-33
lines changed

‎chart/templates/_helpers.yaml

+5-3
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,12 @@ If release name contains chart name it will be used as a full name.
304304
{{- end }}
305305

306306
{{ define "pgbouncer_config" }}
307+
{{- $resultBackendConnection := .Values.data.resultBackendConnection | default .Values.data.metadataConnection }}
307308
{{- $pgMetadataHost := .Values.data.metadataConnection.host | default (printf "%s-%s.%s" .Release.Name "postgresql" .Release.Namespace) }}
308-
{{- $pgResultBackendHost := .Values.data.resultBackendConnection.host | default (printf "%s-%s.%s" .Release.Name "postgresql" .Release.Namespace) }}
309+
{{- $pgResultBackendHost := $resultBackendConnection.host | default (printf "%s-%s.%s" .Release.Name "postgresql" .Release.Namespace) }}
309310
[databases]
310311
{{ .Release.Name }}-metadata = host={{ $pgMetadataHost }} dbname={{ .Values.data.metadataConnection.db }} port={{ .Values.data.metadataConnection.port }} pool_size={{ .Values.pgbouncer.metadataPoolSize }}
311-
{{ .Release.Name }}-result-backend = host={{ $pgResultBackendHost }} dbname={{ .Values.data.resultBackendConnection.db }} port={{ .Values.data.resultBackendConnection.port }} pool_size={{ .Values.pgbouncer.resultBackendPoolSize }}
312+
{{ .Release.Name }}-result-backend = host={{ $pgResultBackendHost }} dbname={{ $resultBackendConnection.db }} port={{ $resultBackendConnection.port }} pool_size={{ .Values.pgbouncer.resultBackendPoolSize }}
312313

313314
[pgbouncer]
314315
pool_mode = transaction
@@ -339,8 +340,9 @@ server_tls_key_file = /etc/pgbouncer/server.key
339340
{{- end }}
340341

341342
{{ define "pgbouncer_users" }}
343+
{{- $resultBackendConnection := .Values.data.resultBackendConnection | default .Values.data.metadataConnection }}
342344
{{ .Values.data.metadataConnection.user | quote }} {{ .Values.data.metadataConnection.pass | quote }}
343-
{{ .Values.data.resultBackendConnection.user | quote }} {{ .Values.data.resultBackendConnection.pass | quote }}
345+
{{ $resultBackendConnection.user | quote }} {{ $resultBackendConnection.pass | quote }}
344346
{{- end }}
345347

346348
{{ define "airflow_logs" -}}

‎chart/templates/secrets/result-backend-connection-secret.yaml

+6-5
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@
2020
#################################
2121
{{- if not .Values.data.resultBackendSecretName }}
2222
{{- if or (eq .Values.executor "CeleryExecutor") (eq .Values.executor "CeleryKubernetesExecutor") }}
23+
{{- $connection := .Values.data.resultBackendConnection | default .Values.data.metadataConnection }}
2324

24-
{{- $resultBackendHost := .Values.data.resultBackendConnection.host | default (printf "%s-%s" .Release.Name "postgresql") }}
25+
{{- $resultBackendHost := $connection.host | default (printf "%s-%s" .Release.Name "postgresql") }}
2526
{{- $pgbouncerHost := printf "%s-%s" .Release.Name "pgbouncer" }}
2627
{{- $host := ternary $pgbouncerHost $resultBackendHost .Values.pgbouncer.enabled }}
27-
{{- $port := (ternary .Values.ports.pgbouncer .Values.data.resultBackendConnection.port .Values.pgbouncer.enabled) | toString }}
28-
{{- $database := ternary (printf "%s-%s" .Release.Name "result-backend") .Values.data.resultBackendConnection.db .Values.pgbouncer.enabled }}
29-
{{- $extras := ternary (printf "?sslmode=%s" .Values.data.resultBackendConnection.sslmode) "" (eq .Values.data.resultBackendConnection.protocol "postgresql") }}
28+
{{- $port := (ternary .Values.ports.pgbouncer $connection.port .Values.pgbouncer.enabled) | toString }}
29+
{{- $database := ternary (printf "%s-%s" .Release.Name "result-backend") $connection.db .Values.pgbouncer.enabled }}
30+
{{- $extras := ternary (printf "?sslmode=%s" $connection.sslmode) "" (eq $connection.protocol "postgresql") }}
3031
kind: Secret
3132
apiVersion: v1
3233
metadata:
@@ -41,6 +42,6 @@ metadata:
4142
{{- end }}
4243
type: Opaque
4344
data:
44-
connection: {{ (printf "db+%s://%s:%s@%s:%s/%s%s" .Values.data.resultBackendConnection.protocol .Values.data.resultBackendConnection.user .Values.data.resultBackendConnection.pass $host $port $database $extras) | b64enc | quote }}
45+
connection: {{ (printf "db+%s://%s:%s@%s:%s/%s%s" $connection.protocol $connection.user $connection.pass $host $port $database $extras) | b64enc | quote }}
4546
{{- end }}
4647
{{- end }}

‎chart/tests/test_pgbouncer.py

+9-1
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,15 @@ def test_databases_override(self):
230230
"pgbouncer": {"enabled": True, "metadataPoolSize": 12, "resultBackendPoolSize": 7},
231231
"data": {
232232
"metadataConnection": {"host": "meta_host", "db": "meta_db", "port": 1111},
233-
"resultBackendConnection": {"host": "rb_host", "db": "rb_db", "port": 2222},
233+
"resultBackendConnection": {
234+
"protocol": "postgresql",
235+
"host": "rb_host",
236+
"user": "someuser",
237+
"pass": "someuser",
238+
"db": "rb_db",
239+
"port": 2222,
240+
"sslmode": "disabled",
241+
},
234242
},
235243
}
236244
ini = self._get_pgbouncer_ini(values)

‎chart/tests/test_result_backend_connection_secret.py

+39-8
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ class ResultBackendConnectionSecretTest(unittest.TestCase):
3030
"user": "someuser",
3131
"pass": "somepass",
3232
"host": "somehost",
33+
"protocol": "postgresql",
3334
"port": 7777,
3435
"db": "somedb",
36+
"sslmode": "allow",
3537
}
3638

3739
def test_should_not_generate_a_document_if_using_existing_secret(self):
@@ -73,6 +75,19 @@ def test_default_connection(self):
7375
== connection
7476
)
7577

78+
def test_should_default_to_custom_metadata_db_connection_with_pgbouncer_overrides(self):
79+
values = {
80+
"pgbouncer": {"enabled": True},
81+
"data": {"metadataConnection": {**self.non_chart_database_values}},
82+
}
83+
connection = self._get_connection(values)
84+
85+
# host, port, dbname still get overridden
86+
assert (
87+
"db+postgresql://someuser:somepass@RELEASE-NAME-pgbouncer:6543"
88+
"/RELEASE-NAME-result-backend?sslmode=allow" == connection
89+
)
90+
7691
def test_should_set_pgbouncer_overrides_when_enabled(self):
7792
values = {"pgbouncer": {"enabled": True}}
7893
connection = self._get_connection(values)
@@ -93,32 +108,48 @@ def test_should_set_pgbouncer_overrides_with_non_chart_database_when_enabled(sel
93108
# host, port, dbname still get overridden even with an non-chart db
94109
assert (
95110
"db+postgresql://someuser:somepass@RELEASE-NAME-pgbouncer:6543"
96-
"/RELEASE-NAME-result-backend?sslmode=disable" == connection
111+
"/RELEASE-NAME-result-backend?sslmode=allow" == connection
97112
)
98113

114+
def test_should_default_to_custom_metadata_db_connection(self):
115+
values = {
116+
"data": {"metadataConnection": {**self.non_chart_database_values}},
117+
}
118+
connection = self._get_connection(values)
119+
120+
assert "db+postgresql://someuser:somepass@somehost:7777/somedb?sslmode=allow" == connection
121+
99122
def test_should_correctly_use_non_chart_database(self):
123+
values = {"data": {"resultBackendConnection": {**self.non_chart_database_values}}}
124+
connection = self._get_connection(values)
125+
126+
assert "db+postgresql://someuser:somepass@somehost:7777/somedb?sslmode=allow" == connection
127+
128+
def test_should_support_non_postgres_db(self):
100129
values = {
101130
"data": {
102131
"resultBackendConnection": {
103132
**self.non_chart_database_values,
104-
"sslmode": "require",
133+
"protocol": "mysql",
105134
}
106135
}
107136
}
108137
connection = self._get_connection(values)
109138

110-
assert "db+postgresql://someuser:somepass@somehost:7777/somedb?sslmode=require" == connection
139+
# sslmode is only added for postgresql
140+
assert "db+mysql://someuser:somepass@somehost:7777/somedb" == connection
111141

112-
def test_should_support_non_postgres_db(self):
142+
def test_should_correctly_use_non_chart_database_when_both_db_are_external(self):
113143
values = {
114144
"data": {
145+
"metadataConnection": {**self.non_chart_database_values},
115146
"resultBackendConnection": {
116147
**self.non_chart_database_values,
117-
"protocol": "mysql",
118-
}
148+
"user": "anotheruser",
149+
"pass": "anotherpass",
150+
},
119151
}
120152
}
121153
connection = self._get_connection(values)
122154

123-
# sslmode is only added for postgresql
124-
assert "db+mysql://someuser:somepass@somehost:7777/somedb" == connection
155+
assert "db+postgresql://anotheruser:anotherpass@somehost:7777/somedb?sslmode=allow" == connection

‎chart/values.schema.json

+21-8
Original file line numberDiff line numberDiff line change
@@ -690,23 +690,27 @@
690690
},
691691
"resultBackendConnection": {
692692
"description": "Result backend connection configuration.",
693-
"type": "object",
693+
"type": [
694+
"object",
695+
"null"
696+
],
697+
"default": null,
694698
"additionalProperties": false,
695699
"properties": {
696700
"user": {
697701
"description": "The database user.",
698702
"type": "string",
699-
"default": "postgres"
703+
"default": null
700704
},
701705
"pass": {
702706
"description": "The database password.",
703707
"type": "string",
704-
"default": "postgres"
708+
"default": null
705709
},
706710
"protocol": {
707711
"description": "The database protocol.",
708712
"type": "string",
709-
"default": "postgresql"
713+
"default": null
710714
},
711715
"host": {
712716
"description": "The database host.",
@@ -719,19 +723,28 @@
719723
"port": {
720724
"description": "The database port.",
721725
"type": "integer",
722-
"default": 5432
726+
"default": null
723727
},
724728
"db": {
725729
"description": "The name of the database.",
726730
"type": "string",
727-
"default": "postgres"
731+
"default": null
728732
},
729733
"sslmode": {
730734
"description": "The database SSL parameter.",
731735
"type": "string",
732-
"default": "disable"
736+
"default": null
733737
}
734-
}
738+
},
739+
"required": [
740+
"user",
741+
"pass",
742+
"protocol",
743+
"host",
744+
"port",
745+
"db",
746+
"sslmode"
747+
]
735748
},
736749
"brokerUrl": {
737750
"description": "Direct url to the redis broker (when using an external redis instance).",

‎chart/values.yaml

+11-8
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,17 @@ data:
249249
port: 5432
250250
db: postgres
251251
sslmode: disable
252-
resultBackendConnection:
253-
user: postgres
254-
pass: postgres
255-
protocol: postgresql
256-
host: ~
257-
port: 5432
258-
db: postgres
259-
sslmode: disable
252+
# resultBackendConnection defaults to the same database as metadataConnection
253+
resultBackendConnection: ~
254+
# or, you can use a different database
255+
# resultBackendConnection:
256+
# user: postgres
257+
# pass: postgres
258+
# protocol: postgresql
259+
# host: ~
260+
# port: 5432
261+
# db: postgres
262+
# sslmode: disable
260263
brokerUrl: ~
261264

262265
# Fernet key settings

0 commit comments

Comments
 (0)