-
Notifications
You must be signed in to change notification settings - Fork 3k
fix: Add curl in Space image and fix route used in HEALTHCHECK that was causing unhealthy status #8393
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
base: preview
Are you sure you want to change the base?
fix: Add curl in Space image and fix route used in HEALTHCHECK that was causing unhealthy status #8393
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdated the Space service Dockerfile to install the missing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the space app container health status issue by adding the missing curl package required by the HEALTHCHECK command and correcting the health check endpoint from / to /spaces/ to match the application's configured base path.
Key changes:
- Added
curlandlibc6-compatpackages to the Alpine base image - Updated HEALTHCHECK command to use
/spaces/endpoint instead of root path
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ | ||
| CMD curl -fsS http://127.0.0.1:3000/ >/dev/null || exit 1 | ||
| CMD curl -fsS http://127.0.0.1:3000/spaces/ >/dev/null || exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Hardcoded healthcheck path ignores configurable base path
The healthcheck path /spaces/ is hardcoded, but VITE_SPACE_BASE_PATH is a configurable build argument (default /spaces) that determines where the application serves content. If someone builds with a custom --build-arg VITE_SPACE_BASE_PATH="/custom-path", the application would be built to serve at /custom-path/ while the healthcheck still checks /spaces/, causing the container to be incorrectly marked as unhealthy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/space/Dockerfile.space(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: check:types
- GitHub Check: check:lint
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/space/Dockerfile.space (2)
81-82: LGTM! curl installation is correctly placed in the runner stage.The curl package is appropriately installed in the runner stage where it's needed for the HEALTHCHECK, avoiding unnecessary bloat in the intermediate build stages. Using
--no-cachekeeps the image size minimal.Note: The PR description mentions adding curl to the "base stage," but the implementation correctly places it in the "runner stage," which addresses the concern raised in the previous review.
92-92: LGTM! HEALTHCHECK path correctly updated to match the app's base path.The health check now queries
/spaces/which aligns withVITE_SPACE_BASE_PATH="/spaces"(line 44), fixing the 404 errors that were marking the container as unhealthy. The use of127.0.0.1and curl flags-fsSare appropriate for health checking.
| RUN apk add --no-cache curl | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other Plane Dockerfiles use libc6-compat and where they install it
echo "=== Checking libc6-compat usage in other Dockerfiles ==="
fd -name 'Dockerfile*' -type f | xargs rg -n 'libc6-compat' || echo "No libc6-compat found"
echo ""
echo "=== Checking Dockerfile.space content ==="
cat apps/space/Dockerfile.space | head -100
echo ""
echo "=== Checking for package.json in space app ==="
fd -path '**/space/package.json' -type fRepository: makeplane/plane
Length of output: 4098
🏁 Script executed:
#!/bin/bash
# Find and check space app package.json
echo "=== Finding space package.json ==="
find . -path '*/apps/space/package.json' -type f
echo ""
echo "=== Checking space package.json dependencies ==="
if [ -f "apps/space/package.json" ]; then
cat apps/space/package.json
else
echo "File not found at expected location"
fiRepository: makeplane/plane
Length of output: 2549
Add libc6-compat to the base stage for consistency with other production Dockerfiles.
libc6-compat is missing from Dockerfile.space but present in all other production Dockerfiles (web, live). This is also already included in Dockerfile.dev (line 3). Add it to the base stage after line 8 (RUN corepack enable pnpm) to ensure consistency across the codebase and follow Alpine + Node.js best practices.
🤖 Prompt for AI Agents
In apps/space/Dockerfile.space (around lines 8 and 81-82), the base stage is
missing libc6-compat which other production Dockerfiles include; add
libc6-compat to the base stage by inserting a RUN apk add --no-cache
libc6-compat (or add it to the existing RUN apk add --no-cache curl line)
immediately after the RUN corepack enable pnpm line to match the project
standard and Alpine+Node best practices.
Description
Issue in #8392
The space app container was consistently marked as unhealthy because:
curl: The HEALTHCHECK command relies on curl, which was not installed in the Alpine-based image.Changes
Type of Change
Test Scenarios
Note
Installs curl in the Space runner image and updates the HEALTHCHECK to target /spaces/ instead of /.
curlin therunnerstage.HEALTHCHECKtohttp://127.0.0.1:3000/spaces/(was/).Written by Cursor Bugbot for commit 0b3a29f. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.