fix: 배포 헬스체크를 로그 기반 완료 신호 감지 방식으로 변경#196
Conversation
기존 방식(15초 대기 + 10회x10초 폴링)은 OTel agent 계측으로 부팅 시간이
늘어나면서(3s→20~30s) 정상 배포도 실패로 오판하는 문제가 있었음.
이제 프로세스 생존 여부로 진짜 실패(크래시)를 즉시 감지하고, 살아있는 동안은
Spring Boot가 실제로 찍는 완료 로그("Started ServerApplication")를 기다림.
임의의 타임아웃 추측 대신 실제 이벤트 기반 판단이라 부팅 시간 변동에 안전하고,
5분 상한선은 진짜 행(hang) 상황에 대한 안전장치로만 남김.
📝 WalkthroughWalkthroughChangesDeployment readiness
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/deploy.sh (1)
33-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnbounded log growth from switching to append mode.
LOG_FILEis now appended (>> "$LOG_FILE") instead of truncated, needed to trackSTART_LINEfor readiness detection. Over many deploys with no rotation,nohup.outwill grow indefinitely (worse with OTel agent output) and can eventually fill disk on the instance.Also,
LOG_FILEre-hardcodes/home/ec2-user/app/nohup.outeven thoughAPP_DIR=/home/ec2-user/appis already defined and the scriptcds into it — considerLOG_FILE="$APP_DIR/nohup.out"for consistency.♻️ Suggested tweaks
-LOG_FILE=/home/ec2-user/app/nohup.out +LOG_FILE="$APP_DIR/nohup.out" START_LINE=$(wc -l < "$LOG_FILE" 2>/dev/null || echo 0)Add basic rotation (e.g., via
logrotateconfig fornohup.out, or truncate/archive it on each deploy onceSTART_LINEbookkeeping is no longer needed for the previous run).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/deploy.sh` around lines 33 - 41, Update the deployment script’s LOG_FILE to derive from APP_DIR instead of hard-coding the application path, and add basic nohup.out rotation or archival before appending new output. Preserve START_LINE-based readiness detection by rotating only when the previous run’s log bookkeeping is no longer needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/deploy.sh`:
- Around line 75-77: Update the health-check curl invocation in the deployment
script to include an explicit request timeout, ensuring an unavailable or
unresponsive actuator endpoint cannot block indefinitely. Keep the existing
health response logging behavior and do not change deployment success gating.
- Around line 43-73: Reduce MAX_WAIT in the readiness loop to leave sufficient
time below the AppSpec hook’s 300-second timeout for the earlier shutdown delay,
health check, and cleanup diagnostics; use a margin such as 240–270 seconds
while preserving the existing timeout failure path and 5-minute overall
hang-protection intent.
---
Nitpick comments:
In `@scripts/deploy.sh`:
- Around line 33-41: Update the deployment script’s LOG_FILE to derive from
APP_DIR instead of hard-coding the application path, and add basic nohup.out
rotation or archival before appending new output. Preserve START_LINE-based
readiness detection by rotating only when the previous run’s log bookkeeping is
no longer needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| echo "> 앱 준비 신호 대기 중 (최대 300초, 프로세스 생존 여부로 실패 판단)" | ||
| MAX_WAIT=300 | ||
| ELAPSED=0 | ||
| READY=false | ||
|
|
||
| for i in {1..10}; do | ||
| RESPONSE=$(curl -s http://localhost:8080/actuator/health || true) | ||
| if echo "$RESPONSE" | grep -q '"status":"UP"'; then | ||
| echo "> 헬스체크 성공" | ||
| break | ||
| fi | ||
| echo "> 헬스체크 실패($i/10): $RESPONSE" | ||
| if [ $i -eq 10 ]; then | ||
| while [ $ELAPSED -lt $MAX_WAIT ]; do | ||
| if ! kill -0 $APP_PID 2>/dev/null; then | ||
| echo "> 프로세스가 예기치 않게 종료됨 (크래시)" | ||
| echo "> 최근 로그:" | ||
| tail -n 40 "$LOG_FILE" | ||
| echo "> 배포 실패" | ||
| exit 1 | ||
| fi | ||
| sleep 10 | ||
|
|
||
| if tail -n +"$((START_LINE + 1))" "$LOG_FILE" | grep -q "Started ServerApplication"; then | ||
| echo "> 앱 준비 완료 신호 감지 (${ELAPSED}초 소요)" | ||
| READY=true | ||
| break | ||
| fi | ||
|
|
||
| sleep 2 | ||
| ELAPSED=$((ELAPSED + 2)) | ||
| done | ||
|
|
||
| if [ "$READY" != "true" ]; then | ||
| echo "> ${MAX_WAIT}초 내에 준비 신호 없음 — 안전장치 발동" | ||
| echo "> 최근 로그:" | ||
| tail -n 40 "$LOG_FILE" | ||
| echo "> 배포 실패" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
MAX_WAIT=300 matches the AppSpec hook's own 300s timeout, leaving no margin.
appspec.yml runs this script with timeout: 300 for the whole AfterInstall hook. This script's own readiness loop can consume the full 300s (line 44), on top of the earlier sleep 5 (line 14) for killing the previous process and the subsequent health curl (line 75) and nginx start. If the app is slow enough to approach MAX_WAIT, CodeDeploy will kill the entire script via its outer timeout before — or right around when — this loop's own graceful timeout branch (lines 67-73) would fire, so the intended "print log tail, exit 1 cleanly" diagnostics may never actually run. The PR's stated intent to keep the "5-minute limit for hang situations" while leaving CI/CD unchanged means this exact overlap should be accounted for.
Reduce MAX_WAIT to leave real margin under the 300s hook budget (e.g., 240-270s) so the script's own failure path reliably completes before CodeDeploy's outer timeout intervenes.
🛡️ Suggested fix
-MAX_WAIT=300
+MAX_WAIT=240As per PR objectives, the 5-minute (300s) limit is intentionally retained for hang protection, but this needs coordination with the AppSpec hook's own 300s budget rather than an exact 1:1 match.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "> 앱 준비 신호 대기 중 (최대 300초, 프로세스 생존 여부로 실패 판단)" | |
| MAX_WAIT=300 | |
| ELAPSED=0 | |
| READY=false | |
| for i in {1..10}; do | |
| RESPONSE=$(curl -s http://localhost:8080/actuator/health || true) | |
| if echo "$RESPONSE" | grep -q '"status":"UP"'; then | |
| echo "> 헬스체크 성공" | |
| break | |
| fi | |
| echo "> 헬스체크 실패($i/10): $RESPONSE" | |
| if [ $i -eq 10 ]; then | |
| while [ $ELAPSED -lt $MAX_WAIT ]; do | |
| if ! kill -0 $APP_PID 2>/dev/null; then | |
| echo "> 프로세스가 예기치 않게 종료됨 (크래시)" | |
| echo "> 최근 로그:" | |
| tail -n 40 "$LOG_FILE" | |
| echo "> 배포 실패" | |
| exit 1 | |
| fi | |
| sleep 10 | |
| if tail -n +"$((START_LINE + 1))" "$LOG_FILE" | grep -q "Started ServerApplication"; then | |
| echo "> 앱 준비 완료 신호 감지 (${ELAPSED}초 소요)" | |
| READY=true | |
| break | |
| fi | |
| sleep 2 | |
| ELAPSED=$((ELAPSED + 2)) | |
| done | |
| if [ "$READY" != "true" ]; then | |
| echo "> ${MAX_WAIT}초 내에 준비 신호 없음 — 안전장치 발동" | |
| echo "> 최근 로그:" | |
| tail -n 40 "$LOG_FILE" | |
| echo "> 배포 실패" | |
| exit 1 | |
| fi | |
| echo "> 앱 준비 신호 대기 중 (최대 300초, 프로세스 생존 여부로 실패 판단)" | |
| MAX_WAIT=240 | |
| ELAPSED=0 | |
| READY=false | |
| while [ $ELAPSED -lt $MAX_WAIT ]; do | |
| if ! kill -0 $APP_PID 2>/dev/null; then | |
| echo "> 프로세스가 예기치 않게 종료됨 (크래시)" | |
| echo "> 최근 로그:" | |
| tail -n 40 "$LOG_FILE" | |
| echo "> 배포 실패" | |
| exit 1 | |
| fi | |
| if tail -n +"$((START_LINE + 1))" "$LOG_FILE" | grep -q "Started ServerApplication"; then | |
| echo "> 앱 준비 완료 신호 감지 (${ELAPSED}초 소요)" | |
| READY=true | |
| break | |
| fi | |
| sleep 2 | |
| ELAPSED=$((ELAPSED + 2)) | |
| done | |
| if [ "$READY" != "true" ]; then | |
| echo "> ${MAX_WAIT}초 내에 준비 신호 없음 — 안전장치 발동" | |
| echo "> 최근 로그:" | |
| tail -n 40 "$LOG_FILE" | |
| echo "> 배포 실패" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/deploy.sh` around lines 43 - 73, Reduce MAX_WAIT in the readiness
loop to leave sufficient time below the AppSpec hook’s 300-second timeout for
the earlier shutdown delay, health check, and cleanup diagnostics; use a margin
such as 240–270 seconds while preserving the existing timeout failure path and
5-minute overall hang-protection intent.
| RESPONSE=$(curl -s http://localhost:8080/actuator/health || true) | ||
| echo "> 헬스체크 확인: $RESPONSE" | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Health curl has no timeout and its result doesn't gate success.
curl -s http://localhost:8080/actuator/health || true can hang indefinitely if the port isn't accepting connections yet, further eating into the already-tight 300s hook budget (see readiness-loop comment above). It also never fails the deploy even if the response is empty or reports DOWN, since only the raw string is echoed.
Add a request timeout so this can't block indefinitely, regardless of whether you decide to gate success on the response.
🛡️ Suggested fix
-RESPONSE=$(curl -s http://localhost:8080/actuator/health || true)
+RESPONSE=$(curl -s --max-time 10 http://localhost:8080/actuator/health || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RESPONSE=$(curl -s http://localhost:8080/actuator/health || true) | |
| echo "> 헬스체크 확인: $RESPONSE" | |
| RESPONSE=$(curl -s --max-time 10 http://localhost:8080/actuator/health || true) | |
| echo "> 헬스체크 확인: $RESPONSE" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/deploy.sh` around lines 75 - 77, Update the health-check curl
invocation in the deployment script to include an explicit request timeout,
ensuring an unavailable or unresponsive actuator endpoint cannot block
indefinitely. Keep the existing health response logging behavior and do not
change deployment success gating.
2 participants
작업 배경
변경 사항
scripts/deploy.shStarted ServerApplication) 감지 방식으로 교체영향 범위
Test Plan
bash -n scripts/deploy.sh문법 검증🤖 Generated with Claude Code
Summary by CodeRabbit