Skip to content

fix(vroom): Run the healthcheck without a shell - #4534

Merged
oioki merged 3 commits into
masterfrom
alextarasov/vroom-shell-free
Oct 7, 2026
Merged

oioki merged 3 commits into
masterfrom
alextarasov/vroom-shell-free

Conversation

@oioki

@oioki oioki commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Switch the vroom healthcheck from a bash /dev/tcp probe to ["CMD", "/bin/vroom", "healthcheck"], so it keeps working once the vroom image has no shell.

vroom healthcheck (getsentry/vroom#679) checks /health on $PORT (default 8085) and is already in ghcr.io/getsentry/vroom:nightly, so this can merge any time.

The other places that run a shell in vroom (ensure-correct-permissions-profiles-dir.sh and the volume migration in bootstrap-s3-profiles.sh) are both past the 26.5.0 hard stop and will be removed in separate PRs.

Refs SEC-1098

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Coverage Results 📊

✅ 22 passed | Total: 22 | Pass Rate: 100% | Execution Time: 11m 2s

📊 Comparison with Base Branch

Metric Change
Total Tests —
Passed Tests —
Failed Tests —
Skipped Tests —

✨ Test counts unchanged from base.

All tests are passing successfully.

✅ Patch coverage is 100.00% (no changed executable lines found; target 50%).
Project statement coverage is 95.52% (unchanged from base (7b07b5e) to head (9d3db1a)).

Coverage diff
@@            Coverage Diff             @@
##        master     #4534       +/-##
==========================================
  Coverage    95.52%    95.52%        —%
==========================================
  Files            5         5         —
  Tracked lines       335       335         —
  Branches         0         0         —
==========================================
  Hits           320       320         —
  Misses          15        15         —
  Partials         0         0         —

Generated by Coverage Action

@BYK
BYK requested a review from aldy505 September 28, 2026 21:27

@aldy505 aldy505 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure on whether to approve this.


# Generate some random files on `sentry-vroom` volume for testing
$dc run --rm --no-deps -v sentry-vroom:/var/vroom/sentry-profiles --entrypoint /bin/bash vroom -c '
$dc run --rm --no-deps -v "${COMPOSE_PROJECT_NAME}_sentry-vroom:/var/vroom/sentry-profiles" --entrypoint /bin/sh seaweedfs -c '

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This would probably breaks users with custom/named volume.

@aldy505
aldy505 requested a review from aminvakil September 29, 2026 09:48

@aminvakil aminvakil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Current code does not handle instances which have specified proxy, and as @aldy505 suggested, custom volumes as well.

I was waiting for PR to get ready and not be draft and review it afterwards.

@linear-code

linear-code Bot commented Oct 6, 2026

Copy link
Copy Markdown

SEC-1098

Use the `vroom healthcheck` subcommand (getsentry/vroom#679) instead of
a bash /dev/tcp probe, so the healthcheck keeps working once the vroom
image has no shell.

Co-Authored-By: Claude <noreply@anthropic.com>
@oioki
oioki force-pushed the alextarasov/vroom-shell-free branch from bb9d187 to 6dfe934 Compare October 6, 2026 21:31
@oioki oioki changed the title fix(vroom): Stop running shell commands inside the vroom container fix(vroom): Run the healthcheck without a shell Oct 6, 2026
@oioki
oioki marked this pull request as ready for review October 6, 2026 23:03
@oioki
oioki requested review from aldy505 and aminvakil October 6, 2026 23:03
@oioki

oioki commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

@aldy505 @aminvakil This PR only changes the healthcheck. The rest of concerns will be addressed in follow-up PRs.

Comment thread docker-compose.yml
The vroom hunk used the old bash healthcheck as context, so it no longer
applied after switching to `vroom healthcheck`. Merge the two vroom hunks
into one with the new one-line healthcheck as context.

Co-Authored-By: Claude <noreply@anthropic.com>
@oioki
oioki merged commit 3f77a67 into master Oct 7, 2026
23 checks passed
@oioki
oioki deleted the alextarasov/vroom-shell-free branch October 7, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants