Skip to content

Conversation

ruggi99
Copy link
Contributor

@ruggi99 ruggi99 commented Jan 20, 2023

What kind of change does this PR introduce?

Bug fix and feature

What is the current behavior?

Currently healthchecks based on printf return always 0 and are false-positive.

Healthcheck config (the port is wrong in order to test, rest has 3000 open, 4000 is closed):

$ docker inspect -f '{{json .Config.Healthcheck }}' supabase-rest
{"Test":["CMD","printf","\\0",">","/dev/tcp/localhost/4000"],"Interval":5000000000,"Timeout":5000000000,"Retries":3}
$ docker inspect -f '{{json .State.Health.Log }}' supabase-rest
[{"Start":"2023-01-20T17:51:21.136686444Z","End":"2023-01-20T17:51:21.203277311Z","ExitCode":0,"Output":"\u0000printf: warning: ignoring excess arguments, starting with '>'\n"},{"Start":"2023-01-20T17:51:26.211514237Z","End":"2023-01-20T17:51:26.279377587Z","ExitCode":0,"Output":"\u0000printf: warning: ignoring excess arguments, starting with '>'\n"},{"Start":"2023-01-20T17:51:31.286512431Z","End":"2023-01-20T17:51:31.350657173Z","ExitCode":0,"Output":"\u0000printf: warning: ignoring excess arguments, starting with '>'\n"},{"Start":"2023-01-20T17:51:36.362020785Z","End":"2023-01-20T17:51:36.424323443Z","ExitCode":0,"Output":"\u0000printf: warning: ignoring excess arguments, starting with '>'\n"},{"Start":"2023-01-20T17:51:41.43299977Z","End":"2023-01-20T17:51:41.50987106Z","ExitCode":0,"Output":"printf: warning: ignoring excess arguments, starting with '>'\n\u0000"}]

Exit code is always 0

What is the new behavior?

$ docker inspect -f '{{json .Config.Healthcheck }}' supabase-rest
{"Test":["CMD","bash","-c","printf \\0 > /dev/tcp/localhost/4000"],"Interval":5000000000,"Timeout":5000000000,"Retries":3}
$ docker inspect -f '{{json .State.Health.Log }}' supabase-rest
[{"Start":"2023-01-20T18:01:32.979767241Z","End":"2023-01-20T18:01:33.042339059Z","ExitCode":1,"Output":"bash: connect: Cannot assign requested address\nbash: /dev/tcp/localhost/4000: Cannot assign requested address\n"},{"Start":"2023-01-20T18:01:38.04912028Z","End":"2023-01-20T18:01:38.094944501Z","ExitCode":1,"Output":"bash: connect: Cannot assign requested address\nbash: /dev/tcp/localhost/4000: Cannot assign requested address\n"}]

Additional context

Added Healthcheck to PostgREST also

@ruggi99 ruggi99 requested a review from a team as a code owner January 20, 2023 18:02
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3970099738

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 62.39%

Totals Coverage Status
Change from base Build 3965757735: 0.04%
Covered Lines: 3618
Relevant Lines: 5799

💛 - Coveralls

@sweatybridge sweatybridge changed the title Fix healthchecks fix: healthcheck printf returns 0 Jan 20, 2023
@sweatybridge sweatybridge merged commit 4556d03 into supabase:main Jan 20, 2023
@github-actions
Copy link

🎉 This PR is included in version 1.34.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sweatybridge
Copy link
Contributor

sweatybridge commented Jan 20, 2023

Btw I don't think PostgREST exposes a shell so we need to revert that health check

@ruggi99
Copy link
Contributor Author

ruggi99 commented Jan 21, 2023

You're partially right.
PostgREST ships with a shell on ARM64, but not on AMD64.
Sorry about that.

@ruggi99 ruggi99 deleted the fix-healthchecks branch January 21, 2023 06:58
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.

3 participants