Skip to content

Fix DATABASE_PASSWORD env for airbyte-metrics helm chart #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

octavia-squidington-iii
Copy link
Collaborator

@octavia-squidington-iii octavia-squidington-iii commented Feb 21, 2023

What

Resolves airbytehq/airbyte#23226

Currently the airbyte-metrics pod does not start up due to being unable to find the env. More info in the issue linked above, but I was able to get it to start normally with the change included in this PR.

Maybe this change was meant to be included in airbytehq/airbyte#16131?

How

This PR brings the airbyte-metrics deployment.yaml template in conformance with other charts pulling this secret.

a couple examples of the pattern used elsewhere that I've copied:

- name: DATABASE_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Values.global.database.secretName | default (printf "%s-airbyte-secrets" .Release.Name ) }}
key: {{ .Values.global.database.secretValue | default "DATABASE_PASSWORD" }}

- name: POSTGRES_PWD
valueFrom:
secretKeyRef:
name: {{ .Values.global.database.secretName | default (printf "%s-airbyte-secrets" .Release.Name ) }}
key: {{ .Values.global.database.secretValue | default "DATABASE_PASSWORD" }}

source: airbytehq/airbyte#23230

@octavia-squidington-iii
Copy link
Collaborator Author

@xpuska513 since you have added this change to other templates, I would appreciate your input and review 🙏🏽

source: airbytehq/airbyte#23230 (comment)

@github-actions
Copy link
Contributor

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage 26.89% 🍏

@sh4sh
Copy link
Contributor

sh4sh commented Feb 21, 2023

Also @davinchia would appreciate your review!

@davinchia
Copy link
Contributor

Thanks @sh4sh ! This looks right to me.

I also want @xpuska513 to double check before we merge this in.

Copy link
Contributor

@xpuska513 xpuska513 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, AC tests passed so I think we can merge this in

@davinchia davinchia merged commit 4911d65 into main Feb 23, 2023
@davinchia davinchia deleted the airbytehq/airbyte/23226/helm-metrics-dbpassword-secret-fix branch February 23, 2023 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

airbyte-metrics helm chart errors on deploy, can't find DATABASE_PASSWORD
4 participants