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

Update healthchecks for all containers #27316

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

encima
Copy link
Member

@encima encima commented Jun 17, 2024

I have read the CONTRIBUTING.md file.

YES

What kind of change does this PR introduce?

Fix/Feature

What is the current behavior?

Current healthchecks are run through localhost or not using docker's networking which can result in failing checks

Studio, in particular, fails healthchecks by default as it does not expose the running app over localhost.

A few issues exist regarding this, one recent example is #27312

What is the new behavior?

This PR adds a healthcheck that retrieves the IP of the container (as curl and wget are not available so the container name cannot be used).

For all other containers, their internal name is used for Docker to resolve their respective addresses.

Additional context

The current image for studio would need to be updated as this introduces a docker-healthcheck.js file that will only be included in subsequent builds.

@encima encima requested review from a team as code owners June 17, 2024 10:57
Copy link

vercel bot commented Jun 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
studio-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 8:15am
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
design-system ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2024 8:15am
docs ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2024 8:15am
studio ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2024 8:15am
studio-self-hosted ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2024 8:15am
zone-www-dot-com ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2024 8:15am

Copy link

supabase bot commented Jun 17, 2024

No changes detected in supabase directory.
This pull request has been ignored for the connected project xguihxuzqibwxjnimxev due to its connection settings.
Go to Project Integrations Settings ↗︎ in order to change this behavior.


Branching Preview Branches by Supabase.
Learn more about Supabase for Git ↗︎.

@cmantsch
Copy link

Was about to also create a PR, I guess you were faster 😄

Main issue seemed to be the localhost, the service does not bind to 127.0.0.1 (however for storage it fails on linux amd64, but works fine on apple arm64).

The healthcheck for studio can also be fixed without the healthcheck.js file by adding quotes around the JS command (needs to be escaped):

healthcheck:
      test:
        [
          "CMD",
          "node",
          "-e",
          "\"require('http').get('http://studio:3000/api/profile', (r) => {if (r.statusCode !== 200) throw new Error(r.statusCode)})\""
        ]
      timeout: 5s
      interval: 5s
      retries: 3

@encima
Copy link
Member Author

encima commented Jun 18, 2024

@cmantsch nice! that seems better to incorporate

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