Skip to content
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

chore: remove pgbouncer from AIO image except pg role [GEN-8039] #920

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

bmpandrade
Copy link
Contributor

Same changes as previous PR #907 , except this will keep the .sql that creates the pgbouncer role. This will be kept for now until the full pgbouncer cleanup happens, since there were migrations failing due to the lack of pgbouncer role present, when creating new projects in Fly, during tests in Staging.

Before opening the PR, this changes were tested with aio-15.1.1.31-no-pgbouncer-docker image in staging, which went through, creating a complete project with no issues and no pgbouncer presence, except for the role, as mentioned before.

@bmpandrade bmpandrade requested a review from a team as a code owner March 22, 2024 16:42
Copy link

@@ -52,5 +50,4 @@ sed -i "s|{{ .ApiKey }}|$LOGFLARE_API_KEY|g" $VECTOR_CONF
sed -i "s|{{ .DbSource }}|$LOGFLARE_DB_SOURCE|g" $VECTOR_CONF
sed -i "s|{{ .GotrueSource }}|$LOGFLARE_GOTRUE_SOURCE|g" $VECTOR_CONF
sed -i "s|{{ .PostgrestSource }}|$LOGFLARE_POSTGREST_SOURCE|g" $VECTOR_CONF
sed -i "s|{{ .PgbouncerSource }}|$LOGFLARE_PGBOUNCER_SOURCE|g" $VECTOR_CONF
sed -i "s|{{ .PitrErrorsSource }}|$LOGFLARE_PITR_ERRORS_SOURCE|g" $VECTOR_CONF
sed -i "s|{{ .PitrErrorsSource }}|$LOGFLARE_PITR_ERRORS_SOURCE|g" $VECTOR_CONF
Copy link
Member

Choose a reason for hiding this comment

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

New line here 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😱 need to update my formatters to do it automatically 🤖

\set auth_pass `echo "${SUPABASE_AUTH_ADMIN_PASSWORD:-$POSTGRES_PASSWORD}"`
\set storage_pass `echo "${SUPABASE_STORAGE_ADMIN_PASSWORD:-$POSTGRES_PASSWORD}"`
\set replication_pass `echo "${SUPABASE_REPLICATION_ADMIN_PASSWORD:-$POSTGRES_PASSWORD}"`
\set read_only_pass `echo "${SUPABASE_READ_ONLY_USER_PASSWORD:-$POSTGRES_PASSWORD}"`

ALTER USER supabase_admin WITH PASSWORD :'admin_pass';
ALTER USER authenticator WITH PASSWORD :'pgrst_pass';
ALTER USER pgbouncer WITH PASSWORD :'pgbouncer_pass';
Copy link
Member

Choose a reason for hiding this comment

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

This would need to be kept around as supavisor is still using the user

Copy link
Member

Choose a reason for hiding this comment

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

There might still be processes which are using this :saddog: namely config updates. We should gut them out of the worker first

@@ -2,7 +2,6 @@ Cmnd_Alias ENVOY = /usr/bin/supervisorctl start services\:envoy, /usr/bin/superv
Cmnd_Alias KONG = /usr/bin/supervisorctl start services\:kong, /usr/bin/supervisorctl stop services\:kong, /usr/bin/supervisorctl restart services\:kong, /usr/bin/supervisorctl status services\:kong
Cmnd_Alias POSTGREST = /usr/bin/supervisorctl start services\:postgrest, /usr/bin/supervisorctl stop services\:postgrest, /usr/bin/supervisorctl restart services\:postgrest, /usr/bin/supervisorctl status services\:postgrest
Cmnd_Alias GOTRUE = /usr/bin/supervisorctl start services\:gotrue, /usr/bin/supervisorctl stop services\:gotrue, /usr/bin/supervisorctl restart services\:gotrue, /usr/bin/supervisorctl status services\:gotrue
Cmnd_Alias PGBOUNCER = /usr/bin/supervisorctl start pgbouncer, /usr/bin/supervisorctl stop pgbouncer, /usr/bin/supervisorctl restart pgbouncer, /usr/bin/supervisorctl status pgbouncer
Copy link
Member

Choose a reason for hiding this comment

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

Wound still need this to allow perms for adminapi to run lifecycle commands (restarts, reloads etc)

@bmpandrade bmpandrade marked this pull request as draft May 14, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants