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

fix: gotrue config persistence for AIO #961

Merged
merged 7 commits into from May 14, 2024
Merged

fix: gotrue config persistence for AIO #961

merged 7 commits into from May 14, 2024

Conversation

pcnc
Copy link
Member

@pcnc pcnc commented May 1, 2024

  • Fixes GoTrue losing its config on subsequent restarts
  • Replaces gotrue's and fail2ban's commands to ensure external processes don't restart the service's process itself
  • Adds flag to disable pgbouncer

@pcnc pcnc requested a review from a team as a code owner May 1, 2024 10:08
@@ -80,10 +80,23 @@ function enable_lsn_checkpoint_push {
}

function disable_fail2ban {
sed -i "s/command=.*/command=sleep 1/" /etc/supervisor/services/fail2ban.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a danger here that this sleep isn't long enough.
By default if a program doesn't stay up for 1 second, it is considered a failure and it will restart
I would set this to 2, or set startsecs to 0 - but startsecs isn't in the config file rn

from the manual

startsecs

The total number of seconds which the program needs to stay running after a startup to consider the start successful (moving the process from the STARTING state to the RUNNING state). Set to 0 to indicate that the program needn’t stay running for any particular amount of time.

Note

Even if a process exits with an “expected” exit code (see exitcodes), the start will still be considered a failure if the process exits quicker than startsecs.

Default: 1

Required: No.

Introduced: 3.0

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooh, nice catch. Will set it to 5 just to make sure it doesn't have any side effects

@pcnc pcnc changed the title fix: gotrue config persistenc for AIO fix: gotrue config persistence for AIO May 1, 2024
@pcnc pcnc requested review from tomashley and a team May 1, 2024 10:20
@delgado3d
Copy link
Contributor

may need to update testinfra tests

@bmpandrade
Copy link
Contributor

@pcnc , related to disabled PGbouncer, had this stale PR for quite a while now, which removes it completely from AIO: #920

Do you prefer to merge yours and than update my PR with gotrue/fail2ban changes that went in?

@pcnc pcnc requested a review from a team as a code owner May 14, 2024 08:51
@pcnc pcnc merged commit 77b2979 into develop May 14, 2024
9 of 10 checks passed
@pcnc pcnc deleted the pcnc/gotrue-fix branch May 14, 2024 09:59
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

5 participants