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 2022/02/16 09:15:37 UTC
[GitHub] [airflow] riccardo-salamanna opened a new pull request #21606: Feaure/stats secret pgbouncer
riccardo-salamanna opened a new pull request #21606:
URL: https://github.com/apache/airflow/pull/21606
I noticed that when deploying airflow with pgbouncer using resultBackendSecretName and metadataSecretName, the pgbouncer-stats-secrets defaults to the values predefined credentials expressed in data.metadataConnection.user and data.metadataConnection.password, breaking the deployment
```
data:
connection: {{ urlJoin (dict "scheme" "postgresql" "userinfo" (printf "%s:%s" (.Values.data.metadataConnection.user | urlquery) (.Values.data.metadataConnection.pass | urlquery) ) "host" (printf "127.0.0.1:%s" (.Values.ports.pgbouncer | toString)) "path" "/pgbouncer" "query" (printf "sslmode=%s" (.Values.pgbouncer.metricsExporterSidecar.sslmode | toString ))) | b64enc | quote }}
{{- end }}
```
i added the possibility to use an external stats secret for pgbouncer exporter and it does worj, i don't know if it is the preferred solution but i mainly wanted to raise the issue. thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna commented on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna commented on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1054023239
ok then there must be a difference between the path variable python has and the one in my shell, because i do have helm installed 👍 thanks for pointing me in this direction
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1053638536
> regardless, thanks @jedcunningham for your support, i tried to run the testing for helm locally both with breeze and virtualenv, and we were not able to get them running (`subprocess.CalledProcessError: Command '['helm', 'dep', 'update', '/opt/airflow/chart']' returned non-zero exit status 1`)
I think you tried to run the tests in `breeze` but it is not needed. What is simply needed is to have `pytest` installed and `helm` in your path. Breeze does not have `helm` installed by default that's why it failed for you. You can also run the tests inside breeze but you need to install helm after entering Breeze.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna commented on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna commented on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1054112136
> > regardless, thanks @jedcunningham for your support, i tried to run the testing for helm locally both with breeze and virtualenv, and we were not able to get them running (`subprocess.CalledProcessError: Command '['helm', 'dep', 'update', '/opt/airflow/chart']' returned non-zero exit status 1`)
>
> I think you tried to run the tests in `breeze` but it is not needed. What is simply needed is to have `pytest` installed and `helm` in your path. Breeze does not have `helm` installed by default that's why it failed for you. You can also run the tests inside breeze but you need to install helm after entering Breeze.
i tried breeze because my first attempt at running pytest in my virtualenv failed. i actually have the same error in both. but i can assure you, helm is the path of my shell. i will try to verify what is going on, thank you agian
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna commented on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna commented on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1048606054
Yeah sorry i don't understand how the testing works, i'm not a strong python developer. Can anybody help me learn? maybe point me in the directions of some documentation? many thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna commented on a change in pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna commented on a change in pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#discussion_r813127390
##########
File path: chart/tests/test_pgbouncer.py
##########
@@ -168,6 +168,34 @@ def test_existing_secret(self):
"secret": {"secretName": "pgbouncer-config-secret"},
} == jmespath.search("spec.template.spec.volumes[0]", docs[0])
+ def test_stats_no_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-STATS",
+ values={
+ "pgbouncer": {"enabled": True},
+ },
+ show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"],
+ )
+
+ assert {
+ "name": "pgbouncer-stats",
+ "secret": {"secretName": "TEST-PGBOUNCER-STATS-pgbouncer-stats"},
+ } == jmespath.search("spec.template.spec.containers[1].env", docs[0])
Review comment:
you have a lot of patience 🙏 thank you - i'll try tonight 😄
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna removed a comment on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna removed a comment on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1054023239
ok then there must be a difference between the path variable python has and the one in my shell, because i do have helm installed 👍 thanks for pointing me in this direction
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a change in pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#discussion_r813060348
##########
File path: chart/tests/test_pgbouncer.py
##########
@@ -168,6 +168,34 @@ def test_existing_secret(self):
"secret": {"secretName": "pgbouncer-config-secret"},
} == jmespath.search("spec.template.spec.volumes[0]", docs[0])
+ def test_stats_no_existing_secret(self):
+ docs = render_chart(
+ "TEST-PGBOUNCER-STATS",
+ values={
+ "pgbouncer": {"enabled": True},
+ },
+ show_only=["templates/pgbouncer/pgbouncer-deployment.yaml"],
+ )
+
+ assert {
+ "name": "pgbouncer-stats",
+ "secret": {"secretName": "TEST-PGBOUNCER-STATS-pgbouncer-stats"},
+ } == jmespath.search("spec.template.spec.containers[1].env", docs[0])
Review comment:
Basically `env` is a list and you are comparing it to a `dict`. Try this:
```suggestion
assert [{
"name": "pgbouncer-stats",
"secret": {"secretName": "TEST-PGBOUNCER-STATS-pgbouncer-stats"},
}] == jmespath.search("spec.template.spec.containers[1].env", docs[0])
```
or, a little more resilient and how I'd go:
```suggestion
assert {
"name": "pgbouncer-stats",
"secret": {"secretName": "TEST-PGBOUNCER-STATS-pgbouncer-stats"},
} in jmespath.search("spec.template.spec.containers[1].env", docs[0])
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna commented on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna commented on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1043006629
i managed to setup my local virtualenv and running the test locally, i **hope** to be able to write a meaningful test... 😄
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] riccardo-salamanna commented on pull request #21606: Feaure/stats secret pgbouncer
Posted by GitBox <gi...@apache.org>.
riccardo-salamanna commented on pull request #21606:
URL: https://github.com/apache/airflow/pull/21606#issuecomment-1049692233
i removed the test cause i'm not thinking it might not be needed, since all i'm doing is adding a condition to the creation of the pgbouncer stats secret if i provide an external one, and there's already a test on the env for the stats....
regardless, thanks @jedcunningham for your support, i tried to run the testing for helm locally both with breeze and virtualenv, and we were not able to get them running (```subprocess.CalledProcessError: Command '['helm', 'dep', 'update', '/opt/airflow/chart']' returned non-zero exit status 1```)
If you have any other instructions let me know, i would be interested in contributing to the helm chart more. thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org