Skip to content

feat(relay): Exit container on healthcheck failure#5754

Open
aminvakil wants to merge 6 commits intogetsentry:masterfrom
aminvakil:healthcheck_exit_on_failure
Open

feat(relay): Exit container on healthcheck failure#5754
aminvakil wants to merge 6 commits intogetsentry:masterfrom
aminvakil:healthcheck_exit_on_failure

Conversation

@aminvakil
Copy link

@aminvakil aminvakil commented Mar 20, 2026

Another attempt of fixing getsentry/sentry#109002 after #5750.

docker-compose does not restart services if they become unhealthy for any reason whether it's web container not accessible and returning NXDOMAIN or any another reason, this PR adds a kill signal to PID 1 to healthcheck failures which can be enabled using kill-on-fail argument, effectively stopping container, and as restart_policy is unless-stopped in self-hosted, it should restart itself.

This may rises a problem of getting relay in a restart loop if it does not get healthy, which is fine IMO.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@aminvakil aminvakil requested a review from a team as a code owner March 20, 2026 19:01
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@aminvakil aminvakil marked this pull request as draft March 20, 2026 19:35
@aminvakil
Copy link
Author

There is a failure on building it, I should add deps to it, will make ready for review afterwards.

This was necessary, to have minimum changes
@aminvakil aminvakil marked this pull request as ready for review March 20, 2026 20:30
@aminvakil
Copy link
Author

aminvakil commented Mar 20, 2026

It's working correctly as expected , but what about having flush-dns-on-fail instead of kill-on-fail?

This is better I think, it seems relay handles SIGTERM perfectly well.

@aminvakil
Copy link
Author

cc @aldy505

Copy link
Collaborator

@aldy505 aldy505 left a comment

Choose a reason for hiding this comment

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

Makes sense to me

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.

2 participants