fix: deploy.sh를 2프로세스 롤링 재시작 방식으로 변경 - #201
Conversation
prod가 nginx+2프로세스(8080/8081) 구조로 전환됨에 따라, 배포 스크립트도 프로세스를 한 번에 재시작하지 않고 포트 하나씩 순서대로 종료→재기동하도록 수정. 재시작 중에도 nginx가 다른 포트로 트래픽을 계속 보내 무중단 배포가 유지된다.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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)
82-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHealth check substring match can false-positive on nested component status.
grep -q '"status":"UP"'matches anywhere in the response body. Withshow-details/show-componentsenabled, Spring Boot Actuator can return an aggregate"status":"DOWN"while individual sub-components still report"status":"UP"(e.g.{"status":"DOWN","details":{"serviceA":{"status":"UP"...), which would still satisfy this grep and be treated as healthy. Whether this app enables detailed health output isn't visible in this diff, but the check should validate the top-levelstatusfield specifically rather than any substring match.♻️ Proposed fix — anchor the match to the top-level field
- if ! echo "$RESPONSE" | grep -q '"status":"UP"'; then + if ! echo "$RESPONSE" | grep -Eq '^\{"status":"UP"'; thenOr, if
jqis available in the deployment environment, prefer[ "$(echo "$RESPONSE" | jq -r '.status')" = "UP" ]for a robust parse instead of text matching.🤖 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 82 - 88, Update the health validation in the deployment check around the RESPONSE and grep logic to verify the top-level JSON status field specifically, not any nested component status. Prefer parsing .status with jq when available; otherwise anchor the text match to the response’s top-level status while preserving the existing failure return behavior.
🤖 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 23-30: Update the shutdown flow around OLD_PID in
scripts/deploy.sh to poll until the identified process exits instead of relying
only on a fixed 5-second sleep. After sending SIGTERM with kill -15, wait for
the process to disappear, and add an escalation fallback such as SIGKILL when
the graceful-shutdown timeout is exceeded, before starting the replacement
process.
- Around line 45-51: Reorder the Java command in the deployment startup block so
-Duser.timezone=Asia/Seoul appears before -jar, while keeping $JAR_PATH and the
existing application arguments unchanged.
---
Nitpick comments:
In `@scripts/deploy.sh`:
- Around line 82-88: Update the health validation in the deployment check around
the RESPONSE and grep logic to verify the top-level JSON status field
specifically, not any nested component status. Prefer parsing .status with jq
when available; otherwise anchor the text match to the response’s top-level
status while preserving the existing failure return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| OLD_PID=$(pgrep -f "server.port=$PORT" || true) | ||
| if [ -n "$OLD_PID" ]; then | ||
| echo "> [$PORT] 종료: $OLD_PID" | ||
| kill -15 $OLD_PID | ||
| sleep 5 | ||
| else | ||
| echo "> [$PORT] 실행 중인 프로세스 없음" | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fixed 5s sleep after SIGTERM doesn't guarantee the port is free.
After kill -15, the script blindly sleeps 5 seconds with no check that the old process actually exited before starting a new process bound to the same port. If graceful shutdown (e.g., in-flight connection draining) takes longer than 5s, the new java process will fail to bind the port; this is only caught indirectly via the crash-detection loop, causing an avoidable deploy failure for that port.
♻️ Proposed fix — poll for exit with an escalation fallback
if [ -n "$OLD_PID" ]; then
echo "> [$PORT] 종료: $OLD_PID"
kill -15 $OLD_PID
- sleep 5
+ for i in $(seq 1 10); do
+ kill -0 $OLD_PID 2>/dev/null || break
+ sleep 1
+ done
+ kill -0 $OLD_PID 2>/dev/null && kill -9 $OLD_PID
else📝 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.
| OLD_PID=$(pgrep -f "server.port=$PORT" || true) | |
| if [ -n "$OLD_PID" ]; then | |
| echo "> [$PORT] 종료: $OLD_PID" | |
| kill -15 $OLD_PID | |
| sleep 5 | |
| else | |
| echo "> [$PORT] 실행 중인 프로세스 없음" | |
| fi | |
| OLD_PID=$(pgrep -f "server.port=$PORT" || true) | |
| if [ -n "$OLD_PID" ]; then | |
| echo "> [$PORT] 종료: $OLD_PID" | |
| kill -15 $OLD_PID | |
| for i in $(seq 1 10); do | |
| kill -0 $OLD_PID 2>/dev/null || break | |
| sleep 1 | |
| done | |
| kill -0 $OLD_PID 2>/dev/null && kill -9 $OLD_PID | |
| else | |
| echo "> [$PORT] 실행 중인 프로세스 없음" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 26-26: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 23 - 30, Update the shutdown flow around
OLD_PID in scripts/deploy.sh to poll until the identified process exits instead
of relying only on a fixed 5-second sleep. After sending SIGTERM with kill -15,
wait for the process to disappear, and add an escalation fallback such as
SIGKILL when the graceful-shutdown timeout is exceeded, before starting the
replacement process.
| nohup java $JAVA_AGENT_OPTS -jar \ | ||
| -Duser.timezone=Asia/Seoul \ | ||
| $JAR_PATH \ | ||
| --server.port=$PORT \ | ||
| >> "$LOG_FILE" 2>&1 & | ||
| APP_PID=$! | ||
| echo "> [$PORT] 프로세스 시작됨 (PID: $APP_PID)" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== deploy.sh excerpt =="
cat -n scripts/deploy.sh | sed -n '1,130p'
echo
echo "== available java version =="
if command -v java >/dev/null 2>&1; then
java -version 2>&1 || true
else
echo "java not installed"
fi
echo
echo "== Java -jar option semantics probe =="
if command -v java >/dev/null 2>&1; then
mkdir -p /tmp/java-jar-probe
echo 'public class App { public static void main(String... args) { for(String a: args) System.out.println(a); } }' > /tmp/java-jar-probe/App.java
javac /tmp/java-jar-probe/App.java
java -jar -Duser.timezone=Asia/Seoul /tmp/java-jar-probe/App.jar 2>&1 || true
java -cp /tmp/java-jar-probe -Duser.timezone=Asia/Seoul App 2>&1 || true
rm -f /tmp/java-jar-probe/App.java /tmp/java-jar-probe/App.class
fiRepository: Runnect/Runnect-Spring-Boot-Server
Length of output: 1957
🌐 Web query:
Java -jar option JVM options must precede -jar man page
💡 Result:
When launching a Java application using the java command, options intended for the Java Virtual Machine (JVM)—such as -Xmx or -Dproperty=value—must precede the name of the class or the JAR file [1]. The general syntax for the java command is: java [options] classname [args] or java [options] -jar jarfile [args] Everything provided before the class name or JAR file is treated as an option for the JVM, while everything provided after is treated as an argument for the application's main method [1][2]. Regarding specific tools or legacy commands: 1. javaws (Java Web Start): Documentation has explicitly stated that all run-options must precede the JNLP file argument [3]. 2. javadoc: Certain options, such as -locale, must be placed before other options to function correctly [4]. 3. jcmd: Options and arguments for this diagnostic tool have specific syntax requirements, often requiring key or key=value syntax [5]. For most standard Java invocations, the requirement that JVM options precede the application entry point (class or JAR) is a fundamental rule of the Java launcher's command-line parsing [1].
Citations:
- 1: https://www.baeldung.com/java-command-line-arguments
- 2: https://docs.oracle.com/en/java/javase/26/docs/specs/jls/jls-12.html
- 3: https://commandlinux.com/man-page/man1/javaws/
- 4: https://man.cx/javadoc(1)
- 5: https://docs.oracle.com/en/java/javase/26/docs/specs/man/jcmd.html
🌐 Web query:
Oracle Java man java "If a JAR file is being used, the options must precede the -jar"
💡 Result:
Yes, when using the -jar option with the java command, all command-line options intended for the Java Virtual Machine (JVM)—such as system properties defined with -D—must precede the -jar flag [1][2]. The standard syntax for the java command is [3][4][5]: java [options] -jar jarfile [args...] In this structure: 1. [options]: These are JVM options (e.g., -Dproperty=value, -Xmx, etc.) that must be placed before the -jar argument to be recognized as configuration for the JVM [1][2]. 2. -jar jarfile: This specifies the JAR file to execute. When this flag is used, the JAR file becomes the source of all user classes, and other classpath settings are ignored [3][4][5]. 3. [args...]: Any arguments placed after the JAR file name are treated as application arguments and are passed directly to the main method of the application's entry point class, not to the JVM [3][4][5]. If JVM options (like -D) are placed after the -jar flag, the JVM will typically interpret them as application arguments rather than system settings, which often causes the application to fail to receive the expected configuration [1][2].
Citations:
- 1: https://learn-it-university.com/mastering-the-proper-usage-of-java-d-command-line-parameters/
- 2: https://codingtechroom.com/question/how-to-pass-system-properties-to-a-jar-file-in-java
- 3: https://docs.oracle.com/en/java/javase/24/docs/specs/man/java.html
- 4: https://docs.oracle.com/en/java/javase/17/docs/specs/man/java.html
- 5: https://docs.oracle.com/javase/8/docs/technotes/tools/windows/java.html
Critical: Move -Duser.timezone before -jar.
JVM options must precede -jar; with the current order, -jar consumes -Duser.timezone=Asia/Seoul as the jar file name, java fails to launch, and $JAR_PATH becomes an application argument.
Proposed fix
nohup java $JAVA_AGENT_OPTS -jar \
- -Duser.timezone=Asia/Seoul \
- $JAR_PATH \
+ -Duser.timezone=Asia/Seoul \
+ -jar $JAR_PATH \
--server.port=$PORT \
>> "$LOG_FILE" 2>&1 &🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 47-47: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 48-48: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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 45 - 51, Reorder the Java command in the
deployment startup block so -Duser.timezone=Asia/Seoul appears before -jar,
while keeping $JAR_PATH and the existing application arguments unchanged.
2 participants
배경
prod가 단일 인스턴스+단일 프로세스 구조에서, nginx + 2프로세스(8080/8081) 구조로 무중단 전환됨 (DNS 이중등록 기반 블루-그린 전환, 새 인스턴스
runnect-prod로 태그 교체 완료).기존
scripts/deploy.sh는 실행 중인 jar 프로세스를 전부 찾아 한 번에 종료하고 새 프로세스 1개만 재기동하는 방식이라, 2프로세스 구조에 그대로 쓰면:변경 내용
포트(8080, 8081)를 하나씩 순서대로 종료 → 재기동 → 헬스체크 확인하도록 변경 (롤링 재시작). 한쪽이 재시작되는 동안 nginx가 다른 한쪽으로 계속 트래픽을 보내므로 배포 중에도 무중단 유지.
/actuator/healthUP 여부까지 확인, 실패 시 해당 포트만 실패 처리(다른 포트는 서비스 유지)테스트
Name=runnect-prod)를 새 인스턴스로 교체 완료, CodeDeploy 에이전트 설치 및 실행 확인 완료Summary by CodeRabbit
Reliability
Operations