You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/04/09 00:36:34 UTC
[GitHub] [airflow] DerekHeldtWerle opened a new pull request #15296: adds pgbouncerConfigSecretName to allow the setting of an existing secret for pgbouncers config
DerekHeldtWerle opened a new pull request #15296:
URL: https://github.com/apache/airflow/pull/15296
Previously, if a user wanted to supply the username and password to the `users.txt` secret for use by pgbouncer, they had to be set directly in the `values.yaml` file. This change allows users to create this secret out of band (with the `pgbouncer.ini`) and avoid supplying secrets directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #15296: adds pgbouncerConfigSecretName to allow the setting of an existing secret for pgbouncers config
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15296:
URL: https://github.com/apache/airflow/pull/15296#issuecomment-816955407
Can you rebase on latest master and fix the conflicting files please @DerekHeldtWerle
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #15296: adds pgbouncerConfigSecretName to allow the setting of an existing secret for pgbouncers config
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15296:
URL: https://github.com/apache/airflow/pull/15296#discussion_r610923935
##########
File path: chart/values.schema.json
##########
@@ -1049,6 +1049,13 @@
"description": "Maximum clients that can connect to pgbouncer (higher = more file descriptors).",
"type": "integer"
},
+ "configSecretName": {
+ "description": "The pgbouncer config secret name.",
Review comment:
```suggestion
"description": "The PgBouncer config secret name.",
```
##########
File path: docs/helm-chart/parameters-ref.rst
##########
@@ -414,6 +414,9 @@ The following tables lists the configurable parameters of the Airflow chart and
* - ``pgbouncer.tolerations``
- Toleration labels for pod assignment
- ``1``
+ * - ``pgbouncer.configSecretName``
+ - Name of existing pgbouncer config secret
Review comment:
```suggestion
- Name of existing PgBouncer config secret
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #15296: Chart: Allow setting an existing secret for PgBouncer config
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #15296:
URL: https://github.com/apache/airflow/pull/15296
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] DerekHeldtWerle commented on pull request #15296: adds pgbouncerConfigSecretName to allow the setting of an existing secret for pgbouncers config
Posted by GitBox <gi...@apache.org>.
DerekHeldtWerle commented on pull request #15296:
URL: https://github.com/apache/airflow/pull/15296#issuecomment-816982547
@kaxil Done 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #15296: Chart: Allow setting an existing secret for PgBouncer config
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15296:
URL: https://github.com/apache/airflow/pull/15296#discussion_r610952261
##########
File path: chart/tests/test_pgbouncer.py
##########
@@ -77,3 +77,40 @@ def test_should_create_valid_affinity_tolerations_and_node_selector(self):
"spec.template.spec.tolerations[0].key",
docs[0],
)
+
+ def test_no_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-CONFIG",
+ values={
+ "pgbouncer": {
+ "enabled": True
+ },
Review comment:
```suggestion
"pgbouncer": {"enabled": True},
```
##########
File path: chart/tests/test_pgbouncer.py
##########
@@ -77,3 +77,40 @@ def test_should_create_valid_affinity_tolerations_and_node_selector(self):
"spec.template.spec.tolerations[0].key",
docs[0],
)
+
+ def test_no_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-CONFIG",
+ values={
+ "pgbouncer": {
+ "enabled": True
+ },
+ },
+ show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"],
+ )
+
+ assert {
+ "name": "pgbouncer-config",
+ "secret": {
+ "secretName": "TEST-PGBOUNCER-CONFIG-pgbouncer-config"
+ }
Review comment:
```suggestion
"secret": {"secretName": "TEST-PGBOUNCER-CONFIG-pgbouncer-config"},
```
##########
File path: chart/tests/test_pgbouncer.py
##########
@@ -77,3 +77,40 @@ def test_should_create_valid_affinity_tolerations_and_node_selector(self):
"spec.template.spec.tolerations[0].key",
docs[0],
)
+
+ def test_no_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-CONFIG",
+ values={
+ "pgbouncer": {
+ "enabled": True
+ },
+ },
+ show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"],
+ )
+
+ assert {
+ "name": "pgbouncer-config",
+ "secret": {
+ "secretName": "TEST-PGBOUNCER-CONFIG-pgbouncer-config"
+ }
+ } == jmespath.search("spec.template.spec.volumes[0]", docs[0])
+
+ def test_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-CONFIG",
+ values={
+ "pgbouncer": {
+ "enabled": True,
+ "configSecretName": "pgbouncer-config-secret"
+ },
Review comment:
```suggestion
"pgbouncer": {"enabled": True, "configSecretName": "pgbouncer-config-secret"},
```
##########
File path: chart/tests/test_pgbouncer.py
##########
@@ -77,3 +77,40 @@ def test_should_create_valid_affinity_tolerations_and_node_selector(self):
"spec.template.spec.tolerations[0].key",
docs[0],
)
+
+ def test_no_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-CONFIG",
+ values={
+ "pgbouncer": {
+ "enabled": True
+ },
+ },
+ show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"],
+ )
+
+ assert {
+ "name": "pgbouncer-config",
+ "secret": {
+ "secretName": "TEST-PGBOUNCER-CONFIG-pgbouncer-config"
+ }
+ } == jmespath.search("spec.template.spec.volumes[0]", docs[0])
+
+ def test_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-CONFIG",
+ values={
+ "pgbouncer": {
+ "enabled": True,
+ "configSecretName": "pgbouncer-config-secret"
+ },
+ },
+ show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"],
+ )
+
+ assert {
+ "name": "pgbouncer-config",
+ "secret": {
+ "secretName": "pgbouncer-config-secret"
+ }
Review comment:
```suggestion
"secret": {"secretName": "pgbouncer-config-secret"},
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #15296: adds pgbouncerConfigSecretName to allow the setting of an existing secret for pgbouncers config
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #15296:
URL: https://github.com/apache/airflow/pull/15296#discussion_r610242636
##########
File path: chart/values.yaml
##########
@@ -567,6 +567,9 @@ pgbouncer:
# Maximum clients that can connect to pgbouncer (higher = more file descriptors)
maxClientConn: 100
+ # supply name of existing secret with pgbouncer.ini and users.txt defined
+ pgbouncerConfigSecretName: ~
Review comment:
```suggestion
configSecretName: ~
```
Since it is already in `pgbouncer` key -- adding `pgbouncer` is a bit redundant. Can you also give an example of an existing config with `pgbouncer.ini` and `users.txt`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org