Conversation
Walkthrough블루-그린 기반 무중단 배포가 도입되었습니다. CI/CD 워크플로우와 배포 스크립트가 변경되어 스테이징 아티팩트를 새 경로로 배치하고, nginx 업스트림 전환·헬스체크·롤백을 포함한 zero-downtime 배포가 실행됩니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CD as CI/CD
participant Script as zero-downtime-deploy.sh
participant Staging as Staging JAR
participant NewApp as New App (inactive port)
participant Health as Health endpoint
participant Nginx as Nginx
participant OldApp as Old App (active port)
CD->>Script: 배포 트리거
Script->>Script: 현재 활성 포트 확인
Script->>Script: 비활성 포트 결정
Script->>Staging: 스테이징 JAR 복사
Script->>NewApp: 새 앱 시작 (비활성 포트)
Script->>Health: 헬스 체크 (재시도)
alt 헬스 체크 실패
Script->>NewApp: 새 앱 종료 (롤백)
Script->>CD: 배포 실패 알림
else 헬스 체크 성공
Script->>Nginx: 업스트림 포트 전환 (구성 테스트)
Nginx->>Script: 구성 테스트 결과
Script->>Nginx: nginx reload
Script->>Health: 재검증 헬스 체크
alt 재검증 실패
Script->>Nginx: 이전 구성 복구
Script->>NewApp: 새 앱 종료
Script->>CD: 배포 실패 알림
else 성공
Script->>OldApp: 이전 앱 종료 및 정리
Script->>Script: 활성 포트 갱신
Script->>CD: 배포 성공 알림
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 주의할 파일/영역:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
📝 Test Coverage Report
|
Test Results124 files 124 suites 14s ⏱️ Results for commit 37bbfa1. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
nginx/api.prod.debate-timer.com (1)
1-15: Keepalive 설정을 완전하게 구성해주세요.upstream에
keepalive 32를 선언했지만, 이를 활성화하기 위한 추가 설정이 필요합니다.location 블록에 다음 설정을 추가하세요:
location / { proxy_pass http://debate_timer_backend; + proxy_http_version 1.1; + proxy_set_header Connection ""; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; }scripts/dev/zero-downtime-deploy.sh (1)
63-63: 변수 선언과 할당을 분리하세요.Shellcheck SC2155 경고: 변수 선언과 할당을 함께 하면 명령의 반환 코드가 마스킹됩니다.
- local pid=$(lsof -t -i:$port 2>/dev/null) + local pid + pid=$(lsof -t -i:$port 2>/dev/null)동일한 패턴이 Lines 97, 177, 188-190에도 있습니다.
nginx/api.dev.debate-timer.com (1)
1-15: Keepalive 설정을 완전하게 구성해주세요.upstream에
keepalive 32를 선언했지만, 이를 활성화하기 위한 추가 설정이 필요합니다.location 블록에 다음 설정을 추가하세요:
location / { proxy_pass http://debate_timer_backend; + proxy_http_version 1.1; + proxy_set_header Connection ""; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; }scripts/prod/zero-downtime-deploy.sh (1)
63-63: 변수 선언과 할당을 분리하세요.Shellcheck SC2155 경고: 변수 선언과 할당을 함께 하면 명령의 반환 코드가 마스킹됩니다.
- local pid=$(lsof -t -i:$port 2>/dev/null) + local pid + pid=$(lsof -t -i:$port 2>/dev/null)동일한 패턴이 Lines 97, 177, 188-190에도 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/Dev_CD.yml(1 hunks).github/workflows/Prod_CD.yml(1 hunks).gitignore(1 hunks)nginx/api.dev.debate-timer.com(1 hunks)nginx/api.prod.debate-timer.com(1 hunks)scripts/dev/zero-downtime-deploy.sh(1 hunks)scripts/nginx-switch-port.sh(1 hunks)scripts/prod/zero-downtime-deploy.sh(1 hunks)src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/nginx-switch-port.sh (2)
scripts/dev/zero-downtime-deploy.sh (1)
log(17-20)scripts/prod/zero-downtime-deploy.sh (1)
log(17-20)
🪛 Shellcheck (0.11.0)
scripts/nginx-switch-port.sh
[warning] 10-10: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 11-11: Argument mixes string and array. Use * or separate argument.
(SC2145)
scripts/dev/zero-downtime-deploy.sh
[warning] 18-18: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 19-19: Argument mixes string and array. Use * or separate argument.
(SC2145)
[warning] 63-63: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 97-97: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 177-177: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 188-188: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 189-189: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 190-190: Declare and assign separately to avoid masking return values.
(SC2155)
scripts/prod/zero-downtime-deploy.sh
[warning] 18-18: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 19-19: Argument mixes string and array. Use * or separate argument.
(SC2145)
[warning] 63-63: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 97-97: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 177-177: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 188-188: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 189-189: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 190-190: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (6)
.gitignore (1)
44-44:.serena파일의 목적과 생성 출처 확인 필요
.serena파일 추가는 문제없으나, 다음 사항 확인이 필요합니다:
.serena파일이 무엇인지, 그리고 무중단 배포 프로세스에서 어떤 역할을 하는지 명확히 해주세요- 이 파일이 배포 스크립트에서 생성되는 런타임 아티팩트인지 확인해주세요
- 섹션 주석이
### application-local.yml인데,.serena는 배포 관련 파일로 보이므로 섹션 구분이 적절한지 검토해주세요 (예: 별도의### Deployment artifacts섹션 추가 고려)src/main/resources/application.yml (1)
4-8: 무중단 배포를 위한 Graceful Shutdown 설정이 잘 적용되었습니다.30초의 타임아웃은 배포 스크립트의 35초 대기 시간과 잘 조화됩니다. 이를 통해 진행 중인 요청이 완료될 시간을 확보할 수 있습니다.
.github/workflows/Dev_CD.yml (1)
62-74: 무중단 배포 워크플로우로 성공적으로 전환되었습니다.스테이징 경로와 배포 스크립트 실행이 올바르게 구성되었습니다. 기존 헬스 체크 단계를 zero-downtime-deploy.sh 스크립트로 통합한 것은 좋은 개선입니다.
scripts/dev/zero-downtime-deploy.sh (1)
177-181: nginx 헬스 체크 엔드포인트가 혼동될 수 있습니다.Line 177에서
http://localhost/로 헬스 체크를 하고 있는데, 이는 애플리케이션의 모니터 포트가 아닌 nginx의 프록시 경로입니다. 의도가 명확하지 않으며, 실제 애플리케이션 헬스 체크 경로(/monitoring/health)가 nginx를 통해 노출되는지 확인이 필요합니다.다음을 확인해주세요:
- nginx가
/monitoring/health경로를 프록시하는지 확인- 그렇지 않다면 모니터 포트로 직접 헬스 체크하도록 변경:
- local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000") + local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/monitoring/health" 2>/dev/null || echo "000")scripts/prod/zero-downtime-deploy.sh (1)
177-181: nginx 헬스 체크 엔드포인트를 명확히 해주세요.Line 177에서
http://localhost/로 헬스 체크를 하고 있는데, 실제 헬스 체크 엔드포인트(/monitoring/health)가 nginx를 통해 노출되는지 확인이 필요합니다.다음을 확인해주세요:
- nginx가
/monitoring/health경로를 백엔드로 프록시하는지 확인- 프록시한다면 헬스 체크 경로를 명시적으로 지정:
- local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000") + local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/monitoring/health" 2>/dev/null || echo "000").github/workflows/Prod_CD.yml (1)
62-74: 프로덕션 무중단 배포 워크플로우가 올바르게 구성되었습니다.Dev 워크플로우와 일관성 있게 구성되어 있으며, 스테이징 경로와 배포 스크립트 실행이 적절합니다.
| log() { | ||
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "${timestamp} $@" | tee -a "$LOG_FILE" | ||
| } |
There was a problem hiding this comment.
Shellcheck 경고를 수정해주세요.
로그 함수에 SC2155와 SC2145 경고가 있습니다.
다음과 같이 수정하세요:
log() {
- local timestamp=$(date '+%Y-%m-%d %H:%M:%S')
- echo "${timestamp} $@" | tee -a "$LOG_FILE"
+ local timestamp
+ timestamp=$(date '+%Y-%m-%d %H:%M:%S')
+ echo "${timestamp} $*" | tee -a "$LOG_FILE"
}📝 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.
| log() { | |
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | |
| echo "${timestamp} $@" | tee -a "$LOG_FILE" | |
| } | |
| log() { | |
| local timestamp | |
| timestamp=$(date '+%Y-%m-%d %H:%M:%S') | |
| echo "${timestamp} $*" | tee -a "$LOG_FILE" | |
| } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 18-18: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 19-19: Argument mixes string and array. Use * or separate argument.
(SC2145)
🤖 Prompt for AI Agents
In scripts/dev/zero-downtime-deploy.sh around lines 17 to 20, fix ShellCheck
SC2155 and SC2145 by avoiding combined declaration+assignment and by handling
positional parameters safely: declare the timestamp variable first (local
timestamp), assign it on the next line (timestamp=$(date '+%Y-%m-%d %H:%M:%S')),
and use a robust print that preserves arguments (e.g., printf '%s %s\n'
"$timestamp" "$*" or similar) piped to tee -a "$LOG_FILE" so you don't trigger
the warnings.
| sudo cp "$temp_conf" "$nginx_conf" | ||
| if ! sudo nginx -t 2>/dev/null; then | ||
| log "nginx configuration test failed" | ||
| git checkout "$nginx_conf" 2>/dev/null || true |
There was a problem hiding this comment.
git checkout을 롤백 메커니즘으로 사용하지 마세요.
nginx 설정 롤백에 git checkout을 사용하는 것은 부적절합니다:
- 배포 환경에 git 저장소가 없을 수 있습니다
- 백업 파일이 이미 Line 33에서 생성되었습니다
백업 파일을 사용하도록 수정하세요:
if ! sudo nginx -t 2>/dev/null; then
log "nginx configuration test failed"
- git checkout "$nginx_conf" 2>/dev/null || true
+ sudo cp "$BACKUP_CONF" "$nginx_conf" 2>/dev/null || true
return 1
fi그리고 스크립트 상단에 BACKUP_CONF 변수를 추가하세요:
switch_nginx_upstream() {
local new_port=$1
local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com"
+ local backup_conf="/etc/nginx/sites-available/api.dev.debate-timer.com.backup"
local temp_conf="/tmp/api.dev.debate-timer.com.tmp"그리고 백업 생성 단계를 추가하세요:
log "Switching nginx upstream to port $new_port"
+ sudo cp "$nginx_conf" "$backup_conf"
sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$new_port;/" "$nginx_conf" > "$temp_conf"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/dev/zero-downtime-deploy.sh around line 169, don't use "git checkout"
as a rollback; instead use the backup file created earlier. Replace the git
checkout line with logic that restores $BACKUP_CONF to $nginx_conf (ensure
$BACKUP_CONF is defined at the top of the script), add a step near the backup
creation (around line 33) that sets BACKUP_CONF and creates the backup file
(e.g., cp "$nginx_conf" "$BACKUP_CONF" with a timestamp/unique name), and update
the rollback path to check that the backup file exists and restore from it
rather than calling git. Ensure any error handling/logging reflects backup
restore failures.
| NGINX_CONF="/etc/nginx/sites-available/api.dev.debate-timer.com" | ||
| BACKUP_CONF="/etc/nginx/sites-available/api.dev.debate-timer.com.backup" | ||
| TEMP_CONF="/tmp/api.dev.debate-timer.com.tmp" |
There was a problem hiding this comment.
스크립트 이름과 설정이 일치하지 않습니다.
스크립트 이름은 범용적이지만 dev 환경 설정 경로가 하드코딩되어 있습니다. 이는 혼란을 야기할 수 있습니다.
다음 중 하나를 선택하세요:
- 스크립트 이름을
nginx-switch-port-dev.sh로 변경하거나 - 환경 변수나 인자로 설정 경로를 받도록 수정
-NGINX_CONF="/etc/nginx/sites-available/api.dev.debate-timer.com"
-BACKUP_CONF="/etc/nginx/sites-available/api.dev.debate-timer.com.backup"
-TEMP_CONF="/tmp/api.dev.debate-timer.com.tmp"
+NGINX_CONF="${NGINX_CONF:-/etc/nginx/sites-available/api.dev.debate-timer.com}"
+BACKUP_CONF="${BACKUP_CONF}.backup"
+TEMP_CONF="/tmp/$(basename $NGINX_CONF).tmp"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/nginx-switch-port.sh around lines 5 to 7, the script hardcodes the
dev NGINX paths while the filename implies a generic utility; update either the
filename to nginx-switch-port-dev.sh or make the script accept the target config
path via an environment variable or CLI argument (with a sensible default). If
choosing the env/arg approach, add parsing for an optional positional parameter
or NGINX_CONF env var, validate it exists, and use it to compute BACKUP_CONF and
TEMP_CONF; if choosing rename, update the file name and any consumers/CI
references to the new name.
| log() { | ||
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "${timestamp} $@" | ||
| } |
There was a problem hiding this comment.
Shellcheck 경고를 수정해주세요.
로그 함수에 두 가지 문제가 있습니다:
- SC2155: 변수 선언과 할당을 분리하여 반환 값 마스킹 방지
- SC2145:
$@사용 시 문자열과 배열 혼합 문제
다음과 같이 수정하세요:
log() {
- local timestamp=$(date '+%Y-%m-%d %H:%M:%S')
- echo "${timestamp} $@"
+ local timestamp
+ timestamp=$(date '+%Y-%m-%d %H:%M:%S')
+ echo "${timestamp} $*"
}📝 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.
| log() { | |
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | |
| echo "${timestamp} $@" | |
| } | |
| log() { | |
| local timestamp | |
| timestamp=$(date '+%Y-%m-%d %H:%M:%S') | |
| echo "${timestamp} $*" | |
| } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 10-10: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 11-11: Argument mixes string and array. Use * or separate argument.
(SC2145)
🤖 Prompt for AI Agents
In scripts/nginx-switch-port.sh around lines 9 to 12, the log function currently
declares and assigns timestamp on one line and echoes "${timestamp} $@", which
triggers ShellCheck SC2155 and SC2145; fix it by splitting the local declaration
and assignment (local timestamp; timestamp=$(date '+%Y-%m-%d %H:%M:%S')) and
replace echo with a safe printf that combines the timestamp and the joined
arguments (e.g. use printf '%s %s\n' "$timestamp" "$*") so you avoid command
substitution masking and correctly handle multiple arguments.
| sudo cp "$NGINX_CONF" "$BACKUP_CONF" | ||
|
|
||
| log "Updating nginx upstream to port $NEW_PORT" | ||
| sed "s/server localhost:[0-9]\+;/server localhost:$NEW_PORT;/" "$NGINX_CONF" > "$TEMP_CONF" |
There was a problem hiding this comment.
sed 패턴이 nginx 설정과 일치하지 않습니다.
nginx 설정에서는 127.0.0.1:8080을 사용하지만, sed 패턴은 localhost:[0-9]+를 매칭하려고 합니다. 이는 매칭에 실패합니다.
nginx 설정의 실제 형식에 맞춰 수정하세요:
-sed "s/server localhost:[0-9]\+;/server localhost:$NEW_PORT;/" "$NGINX_CONF" > "$TEMP_CONF"
+sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$NEW_PORT;/" "$NGINX_CONF" > "$TEMP_CONF"📝 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.
| sed "s/server localhost:[0-9]\+;/server localhost:$NEW_PORT;/" "$NGINX_CONF" > "$TEMP_CONF" | |
| sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$NEW_PORT;/" "$NGINX_CONF" > "$TEMP_CONF" |
🤖 Prompt for AI Agents
In scripts/nginx-switch-port.sh around line 36, the sed pattern matches
"localhost:[0-9]\+" but the nginx config uses "127.0.0.1:PORT" so the
substitution fails; update the sed match to accept IP addresses (e.g.,
127.0.0.1) by replacing the pattern with one that matches either localhost or
IPv4 host:port (for example use a regex like
'[0-9]\{1,3\}\(\.[0-9]\{1,3\}\)\{3\}:[0-9]\+' or a combined pattern for
localhost|127.0.0.1) and then substitute the port with $NEW_PORT, ensuring the
correct quoting and escaping for sed on the target shell.
| log() { | ||
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "${timestamp} $@" | tee -a "$LOG_FILE" | ||
| } |
There was a problem hiding this comment.
Shellcheck 경고를 수정해주세요.
로그 함수에 SC2155와 SC2145 경고가 있습니다.
다음과 같이 수정하세요:
log() {
- local timestamp=$(date '+%Y-%m-%d %H:%M:%S')
- echo "${timestamp} $@" | tee -a "$LOG_FILE"
+ local timestamp
+ timestamp=$(date '+%Y-%m-%d %H:%M:%S')
+ echo "${timestamp} $*" | tee -a "$LOG_FILE"
}📝 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.
| log() { | |
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | |
| echo "${timestamp} $@" | tee -a "$LOG_FILE" | |
| } | |
| log() { | |
| local timestamp | |
| timestamp=$(date '+%Y-%m-%d %H:%M:%S') | |
| echo "${timestamp} $*" | tee -a "$LOG_FILE" | |
| } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 18-18: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 19-19: Argument mixes string and array. Use * or separate argument.
(SC2145)
🤖 Prompt for AI Agents
In scripts/prod/zero-downtime-deploy.sh around lines 17 to 20, fix ShellCheck
SC2155 and SC2145 by declaring the timestamp local variable and assigning it on
separate lines (avoid combining local with command substitution), and avoid
unquoted $@ by using a quoted form (e.g. "$@" or "$*") or better use printf to
combine timestamp and the message safely; ensure the final output appends to
$LOG_FILE (via tee -a) and preserves argument spacing/quoting.
| sudo cp "$temp_conf" "$nginx_conf" | ||
| if ! sudo nginx -t 2>/dev/null; then | ||
| log "nginx configuration test failed" | ||
| git checkout "$nginx_conf" 2>/dev/null || true |
There was a problem hiding this comment.
git checkout을 롤백 메커니즘으로 사용하지 마세요.
nginx 설정 롤백에 git checkout을 사용하는 것은 부적절합니다. 백업 파일을 사용하도록 수정하세요.
if ! sudo nginx -t 2>/dev/null; then
log "nginx configuration test failed"
- git checkout "$nginx_conf" 2>/dev/null || true
+ sudo cp "$backup_conf" "$nginx_conf" 2>/dev/null || true
return 1
fi그리고 백업 메커니즘을 추가하세요:
switch_nginx_upstream() {
local new_port=$1
local nginx_conf="/etc/nginx/sites-available/api.prod.debate-timer.com"
+ local backup_conf="/etc/nginx/sites-available/api.prod.debate-timer.com.backup"
local temp_conf="/tmp/api.prod.debate-timer.com.tmp"
if [ ! -f "$nginx_conf" ]; then
error_exit "nginx configuration not found at $nginx_conf"
fi
log "Switching nginx upstream to port $new_port"
+ sudo cp "$nginx_conf" "$backup_conf"
sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$new_port;/" "$nginx_conf" > "$temp_conf"🤖 Prompt for AI Agents
In scripts/prod/zero-downtime-deploy.sh around line 169, do not use `git
checkout` as a rollback for the nginx config; instead implement a
backup-and-restore mechanism: before modifying or deploying the new nginx
config, copy the current live config to a timestamped backup (and a consistent
"last-known-good" backup path), verify the copy succeeded, then apply the new
config; on rollback or failure, restore the config by copying the backup back
into place (or moving it atomically), verify ownership and permissions, and
reload nginx; also add checks for the backup file existence and error handling
to abort if restore fails, and log the backup/restore actions for traceability.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
scripts/dev/zero-downtime-deploy.sh (3)
63-63: ShellCheck SC2155: 변수 선언과 할당 분리 권장여러 위치에서
local선언과 명령 치환을 같은 줄에 작성하고 있습니다. ShellCheck 모범 사례에 따라 분리하는 것을 권장합니다.예시 (Line 63):
- local pid=$(lsof -t -i:$port 2>/dev/null) + local pid + pid=$(lsof -t -i:$port 2>/dev/null)같은 패턴이 Lines 97, 177, 188-190에서도 나타납니다. 모두 일관되게 수정해주세요.
Also applies to: 97-97, 177-177, 188-190
17-20: ShellCheck 경고 수정 필요: SC2155, SC2145로그 함수의 변수 선언과 할당이 분리되지 않았으며, 인수 처리도 부적절합니다. 이전 리뷰에서 지적되었던 사항이 아직 해결되지 않았습니다.
log() { - local timestamp=$(date '+%Y-%m-%d %H:%M:%S') - echo "${timestamp} $@" | tee -a "$LOG_FILE" + local timestamp + timestamp=$(date '+%Y-%m-%d %H:%M:%S') + echo "${timestamp} $*" | tee -a "$LOG_FILE" }
153-185: nginx 설정 롤백 메커니즘 개선 필요Line 169에서
git checkout을 롤백 메커니즘으로 사용하고 있습니다. 개발 환경에서도 git 저장소가 배포 서버에 없을 수 있으므로, 백업 파일을 사용하는 방식으로 변경해야 합니다.switch_nginx_upstream() { local new_port=$1 local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com" + local backup_conf="/etc/nginx/sites-available/api.dev.debate-timer.com.backup" local temp_conf="/tmp/api.dev.debate-timer.com.tmp" if [ ! -f "$nginx_conf" ]; then error_exit "nginx configuration not found at $nginx_conf" fi log "Switching nginx upstream to port $new_port" + sudo cp "$nginx_conf" "$backup_conf" sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$new_port;/" "$nginx_conf" > "$temp_conf" sudo cp "$temp_conf" "$nginx_conf" if ! sudo nginx -t 2>/dev/null; then log "nginx configuration test failed" - git checkout "$nginx_conf" 2>/dev/null || true + sudo cp "$backup_conf" "$nginx_conf" 2>/dev/null || true return 1 fiscripts/prod/zero-downtime-deploy.sh (2)
17-20: ShellCheck 경고 수정 필요: SC2155, SC2145로그 함수의 변수 선언과 할당이 분리되지 않았으며, 인수 처리도 부적절합니다. 이전 리뷰에서 지적되었던 사항이 아직 해결되지 않았습니다.
log() { - local timestamp=$(date '+%Y-%m-%d %H:%M:%S') - echo "${timestamp} $@" | tee -a "$LOG_FILE" + local timestamp + timestamp=$(date '+%Y-%m-%d %H:%M:%S') + echo "${timestamp} $*" | tee -a "$LOG_FILE" }
153-185: nginx 설정 롤백 메커니즘 개선 필요Line 169에서
git checkout을 롤백 메커니즘으로 사용하고 있습니다. 프로덕션 환경에서는 git 저장소가 없을 수 있으므로, 백업 파일을 사용하는 방식으로 변경해야 합니다.switch_nginx_upstream() { local new_port=$1 local nginx_conf="/etc/nginx/sites-available/api.prod.debate-timer.com" + local backup_conf="/etc/nginx/sites-available/api.prod.debate-timer.com.backup" local temp_conf="/tmp/api.prod.debate-timer.com.tmp" if [ ! -f "$nginx_conf" ]; then error_exit "nginx configuration not found at $nginx_conf" fi log "Switching nginx upstream to port $new_port" + sudo cp "$nginx_conf" "$backup_conf" sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$new_port;/" "$nginx_conf" > "$temp_conf" sudo cp "$temp_conf" "$nginx_conf" if ! sudo nginx -t 2>/dev/null; then log "nginx configuration test failed" - git checkout "$nginx_conf" 2>/dev/null || true + sudo cp "$backup_conf" "$nginx_conf" 2>/dev/null || true return 1 fi
🧹 Nitpick comments (3)
scripts/prod/zero-downtime-deploy.sh (1)
63-63: ShellCheck SC2155: 변수 선언과 할당 분리 권장여러 위치에서
local선언과 명령 치환을 같은 줄에 작성하고 있습니다. ShellCheck 모범 사례에 따라 분리하는 것을 권장합니다.예시 (Line 63):
- local pid=$(lsof -t -i:$port 2>/dev/null) + local pid + pid=$(lsof -t -i:$port 2>/dev/null)같은 패턴이 Lines 97, 177, 188-190에서도 나타납니다. 모두 일관되게 수정해주세요.
Also applies to: 97-97, 177-177, 188-190
.github/workflows/Dev_CD.yml (1)
8-8: 테스트 브랜치 트리거 제거 권장
chore/#224브랜치를 CI/CD 트리거에 추가했습니다. 개발 완료 후 이 브랜치 트리거를 제거하는 것을 권장합니다.- "develop" - - "chore/#224"scripts/dev/zero-downtime-deploy.sh (1)
1-231: 배포 스크립트 코드 중복 개선 고려dev와 prod 배포 스크립트의 구조가 거의 동일하며, 환경별 설정값만 다릅니다. 향후 유지보수성 개선을 위해 공통 함수를 추출하거나 매개변수화된 단일 스크립트로 통합하는 것을 고려해보세요.
예를 들어, 환경별 설정을 변수로 받는 단일 스크립트 구조로 변경하면:
- 버그 수정 시 한 곳만 수정 가능
- 일관된 로직 유지
- 유지보수 비용 감소
#!/bin/bash ENVIRONMENT=$1 # "dev" 또는 "prod" case $ENVIRONMENT in dev) PROFILE="dev" NGINX_CONF="/etc/nginx/sites-available/api.dev.debate-timer.com" ;; prod) PROFILE="prod" NGINX_CONF="/etc/nginx/sites-available/api.prod.debate-timer.com" ;; esac # 공통 함수들...
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/Dev_CD.yml(2 hunks).github/workflows/Prod_CD.yml(1 hunks)scripts/dev/zero-downtime-deploy.sh(1 hunks)scripts/prod/zero-downtime-deploy.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/Prod_CD.yml
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/dev/zero-downtime-deploy.sh
[warning] 18-18: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 19-19: Argument mixes string and array. Use * or separate argument.
(SC2145)
[warning] 63-63: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 97-97: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 177-177: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 188-188: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 189-189: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 190-190: Declare and assign separately to avoid masking return values.
(SC2155)
scripts/prod/zero-downtime-deploy.sh
[warning] 18-18: Declare and assign separately to avoid masking return values.
(SC2155)
[error] 19-19: Argument mixes string and array. Use * or separate argument.
(SC2145)
[warning] 63-63: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 97-97: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 177-177: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 188-188: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 189-189: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 190-190: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ 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). (2)
- GitHub Check: build-and-push
- GitHub Check: build
🔇 Additional comments (3)
scripts/prod/zero-downtime-deploy.sh (2)
55-59: 포트 확인 로직 문제없음
lsof명령으로 포트 사용 여부를 정확하게 확인하고 있습니다.
143-151: 애플리케이션 시작 후 프로세스 검증 로직 승인프로세스가 정상적으로 시작되었는지 확인하는 로직이 올바르게 구현되었습니다.
kill -0명령으로 프로세스 존재 여부를 안전하게 검사하고 있습니다..github/workflows/Dev_CD.yml (1)
71-81: CI/CD 워크플로우 업데이트 적절로그 디렉토리 설정, 스크립트 실행 권한 설정, 그리고 무중단 배포 실행이 올바르게 구성되었습니다. 워크플로우 변경이 new deployment script와 잘 통합되어 있습니다.
1 participant
🚩 연관 이슈
closed #224
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
새로운 기능
안정성 개선
기타
✏️ Tip: You can customize this high-level summary in your review settings.