Conversation
|
Warning Rate limit exceeded@unifolio0 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
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. Walkthrough무중단(Blue‑Green) 배포 워크플로우가 CI/CD 파이프라인, 배포 스크립트, Nginx 구성 및 애플리케이션 우아한 종료 설정으로 추가/대체되었고, 로그 디렉토리 생성과 스테이징 아티팩트 경로 변경이 포함됩니다. (요약 50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI/CD Pipeline
participant FS as File System (~/app/staging)
participant NewApp as New App (inactive 포트)
participant HC as Health Check (monitor endpoint)
participant Nginx as Nginx (upstream)
participant OldApp as Old App (active 포트)
CI->>FS: 아티팩트 배치 (~/app/staging), 로그 디렉토리 생성
CI->>FS: 스테이징 JAR을 버전화된 JAR로 복사
CI->>NewApp: 새 버전 시작 (inactive 포트, 관리/모니터 포트 설정)
loop 헬스체크 재시도
HC->>NewApp: /monitoring/health 요청
NewApp-->>HC: 상태 코드(200/비정상)
end
HC-->>CI: 헬스체크 성공
CI->>Nginx: `scripts/nginx-switch-port.sh`로 업스트림 포트 전환
Nginx->>Nginx: config 검사 (`nginx -t`) 및 reload
Nginx-->>CI: post-reload 헬스체크 결과
CI->>OldApp: 이전 인스턴스 graceful shutdown (최대 60s)
CI->>FS: 이전 JAR 정리 및 active 포트 파일 업데이트
CI-->>CI: 배포 완료 로그 기록
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 검토 시 주의할 항목:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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 Results124 files 124 suites 14s ⏱️ Results for commit 4bdbc4d. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
.gitignore (1)
44-44:.serena패턴에 대한 설명이 필요합니다.
.serena파일/디렉토리가 무엇인지, 왜 무시해야 하는지에 대한 컨텍스트가 없습니다. 프로젝트에서 이 패턴이 생성되는 이유를 주석으로 추가하거나 팀과 공유해 주세요.scripts/nginx-switch-port.sh (1)
9-12: Shellcheck 경고 수정 권장변수 선언과 할당을 분리하고
$@올바르게 인용하면 에러 코드 마스킹을 방지할 수 있습니다.다음과 같이 수정하세요:
log() { - local timestamp=$(date '+%Y-%m-%d %H:%M:%S') - echo "${timestamp} $@" + local timestamp + timestamp=$(date '+%Y-%m-%d %H:%M:%S') + echo "${timestamp} $*" }scripts/dev/zero-downtime-deploy.sh (1)
17-20: Shellcheck 경고 수정 권장변수 선언과 할당을 분리하고
$@를$*로 변경하면 에러 코드 마스킹을 방지할 수 있습니다.다음과 같이 수정하세요:
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" }scripts/prod/zero-downtime-deploy.sh (1)
17-20: Shellcheck 경고 수정 권장변수 선언과 할당을 분리하고
$@를$*로 변경하면 에러 코드 마스킹을 방지할 수 있습니다. 이는 dev 스크립트와 동일한 이슈입니다.다음과 같이 수정하세요:
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" }nginx/api.dev.debate-timer.com (2)
6-23: 프록시/SSL 설정 전반적으로 적절하나, keepalive 활용을 위한 소소한 보완 제안현재 설정만으로도 정상 동작에는 문제가 없어 보입니다. 다만 업스트림에
keepalive를 사용하고 있으므로, 프록시 단에서도 HTTP/1.1 명시 및 커넥션 헤더를 정리해 두면 장기적으로 더 안정적인 커넥션 재사용이 가능합니다.예를 들어 다음과 같이 보완을 고려해볼 수 있습니다.
location / { proxy_pass http://debate_timer_backend; 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; + proxy_http_version 1.1; + proxy_set_header Connection ""; }선택 사항이지만, 무중단 배포 시 트래픽이 몰리는 상황에서도 커넥션 재활용에 조금 더 도움이 될 수 있습니다.
25-34: HTTP→HTTPS 리다이렉트 동작은 적절하며,if없이 단순화 여지도 있음현재 설정(
if ($host = api.dev-debate-timer.com) { return 308 ... }+ 기본 404)은 Certbot 기본 패턴으로, 원하는 호스트만 308 HTTPS 리다이렉트하고 나머지는 404로 막는 목적에 부합합니다.구성을 더 단순하게 가져가고 싶다면, 호스트 매칭은
server_name에만 맡기고 블록 전체를 고정 308 리다이렉트로 처리하는 방식도 고려할 수 있습니다.server { listen 80; listen [::]:80; server_name api.dev.debate-timer.com; return 308 https://$host$request_uri; }지금 방식도 문제는 없으므로, 팀 스타일에 따라 선택하시면 되겠습니다.
📜 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/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] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
[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)
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)
⏰ 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). (1)
- GitHub Check: build-and-push
🔇 Additional comments (19)
.github/workflows/Dev_CD.yml (3)
62-62: LGTM: 스테이징 경로로 아티팩트 다운로드 경로 변경아티팩트 경로를
~/app/staging으로 변경하여 무중단 배포 스크립트(zero-downtime-deploy.sh)의 스테이징 JAR 경로와 일치시켰습니다.
70-74: LGTM: 로그 디렉토리 설정로그 디렉토리를 생성하고 적절한 권한과 소유권을 설정했습니다.
76-80: LGTM: 무중단 배포 단계 추가기존의 수동 배포 및 헬스체크 단계를 zero-downtime-deploy.sh 스크립트 실행으로 대체하여 블루-그린 배포를 구현했습니다.
nginx/api.prod.debate-timer.com (3)
1-4: LGTM: Upstream 설정백엔드 upstream 설정이 적절하며, keepalive 32는 연결 재사용을 위한 합리적인 값입니다.
6-23: LGTM: HTTPS 서버 블록 및 프록시 헤더SSL 설정이 Certbot으로 관리되며, 필수 프록시 헤더(Host, X-Real-IP, X-Forwarded-For, X-Forwarded-Proto)가 모두 포함되어 있습니다.
25-34: LGTM: HTTP to HTTPS 리다이렉트308 영구 리다이렉트를 사용하여 HTTP 요청을 HTTPS로 안전하게 리다이렉트합니다.
scripts/nginx-switch-port.sh (2)
43-49: LGTM: nginx 설정 테스트 및 롤백설정 테스트가 실패하면 백업 설정으로 롤백하는 로직이 적절합니다.
54-66: LGTM: 헬스체크 수행nginx 리로드 후 헬스체크를 통해 백엔드 라우팅을 검증하고, 실패 시 적절한 에러 코드로 종료합니다.
scripts/dev/zero-downtime-deploy.sh (5)
8-15: LGTM: 포트 및 구성 상수 정의블루-그린 배포를 위한 포트 쌍 및 헬스체크 설정이 적절합니다. 헬스체크 재시도 60회 × 2초 = 120초는 애플리케이션 시작 시간을 고려할 때 합리적입니다.
134-141: 주의: sudo가 리다이렉션에 영향을 주지 않습니다.
sudo nohup java ... > file.log 2>&1 &에서 sudo는 리다이렉션에 영향을 주지 않으므로, 로그 파일이 현재 사용자 권한으로 생성됩니다. 이것이 의도된 동작인지 확인하세요. 만약 sudo 권한으로 로그를 작성해야 한다면 다음과 같이 수정하세요.- sudo nohup java \ + sudo sh -c "nohup java \ -Dspring.profiles.active=$PROFILE,monitor \ -Duser.timezone=$TIMEZONE \ -Dserver.port=$port \ -Dmanagement.server.port=$monitor_port \ -Ddd.service=debate-timer \ -Ddd.env=$PROFILE \ - -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & + -jar '$jar_file' > '$APP_DIR/app-$port.log' 2>&1 &"현재 구현이 의도된 동작인 경우 이 제안을 무시하세요.
73-77: LGTM: Graceful shutdown 대기 시간65초 대기 시간은
application.yml의timeout-per-shutdown-phase: 60s설정과 잘 맞으며, 진행 중인 요청을 완료할 충분한 시간을 제공합니다.
164-164: LGTM: sed 패턴이 nginx 설정과 일치sed 패턴이
server 127.0.0.1:[port]형식을 올바르게 매칭하며, nginx 설정 파일의 실제 형식과 일치합니다.
187-229: LGTM: 무중단 배포 메인 플로우블루-그린 배포의 전체 흐름이 잘 구성되어 있습니다:
- 새 버전을 비활성 포트에서 시작
- 헬스체크 수행 (실패 시 롤백)
- nginx를 새 포트로 전환 (실패 시 롤백)
- 이전 버전 중지 및 정리
scripts/prod/zero-downtime-deploy.sh (1)
187-229: LGTM: 무중단 배포 메인 플로우전체 블루-그린 배포 로직이 잘 구성되어 있으며, 헬스체크 실패 및 nginx 전환 실패 시 적절한 롤백을 수행합니다.
.github/workflows/Prod_CD.yml (3)
62-62: LGTM: 스테이징 경로로 아티팩트 다운로드 경로 변경Dev_CD.yml과 일관되게 아티팩트 경로를
~/app/staging으로 변경하여 무중단 배포 스크립트와 일치시켰습니다.
70-74: LGTM: 로그 디렉토리 설정로그 디렉토리 생성 및 권한 설정이 적절합니다.
76-80: LGTM: 무중단 배포 단계 추가프로덕션 환경에 zero-downtime-deploy.sh 스크립트를 적용하여 블루-그린 배포를 구현했습니다.
src/main/resources/application.yml (1)
4-8: LGTM: Graceful shutdown 설정 추가무중단 배포를 지원하기 위한 graceful shutdown 설정이 적절합니다. 60초 타임아웃은 진행 중인 요청을 완료할 충분한 시간을 제공하며, 배포 스크립트의 대기 시간과 조화를 이룹니다.
참고: prod 배포 스크립트의 35초 대기 시간이 이 설정보다 짧아서 문제가 될 수 있으니
scripts/prod/zero-downtime-deploy.sh의 리뷰 코멘트를 확인하세요.nginx/api.dev.debate-timer.com (1)
1-4: 업스트림 정의 구조 적절함
debate_timer_backend업스트림이 단일 포트(127.0.0.1:8080)로 정의되어 있고keepalive 32도 설정되어 있어, 배포 스크립트에서 포트 스위칭(blue/green)만 잘 맞춰주면 무중단 배포 시에도 무난히 동작할 구성입니다.
현재 상태 그대로 사용해도 무방해 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
scripts/prod/zero-downtime-deploy.sh (6)
17-20: ** Shellcheck 경고: 변수 선언 및 출력 리다이렉션 개선**dev 스크립트와 동일한 이슈입니다.
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" }
61-86: ** SC2155 경고: 변수 선언과 할당 분리**dev 스크립트와 동일하게 개선하세요.
kill_process_on_port() { local port=$1 - local pid=$(sudo lsof -t -i:$port 2>/dev/null) + local pid + pid=$(sudo lsof -t -i:$port 2>/dev/null)
88-112: ** SC2155 경고: 변수 할당 분리 (낮은 우선순위)**dev 스크립트와 동일합니다.
187-190: ** SC2155 경고: main 함수의 변수 할당**dev 스크립트와 동일합니다.
- local current_port=$(get_current_port) - local new_port=$(get_inactive_port "$current_port") - local new_monitor_port=$(get_monitor_port "$new_port") + local current_port + local new_port + local new_monitor_port + current_port=$(get_current_port) + new_port=$(get_inactive_port "$current_port") + new_monitor_port=$(get_monitor_port "$new_port")
134-141: ** 심각: sudo와 리다이렉션 상호작용 문제 (SC2024)**dev 스크립트와 동일한 중요한 문제입니다.
sudo nohup java \ -Dspring.profiles.active=$PROFILE,monitor \ -Duser.timezone=$TIMEZONE \ -Dserver.port=$port \ -Dmanagement.server.port=$monitor_port \ -Ddd.service=debate-timer \ -Ddd.env=$PROFILE \ - -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & + -jar "$jar_file" > /dev/null 2>&1 &
153-185: ** 심각: git checkout 롤백 메커니즘 신뢰성 부족**dev 스크립트와 동일한 문제입니다. 백업 파일 기반 롤백으로 변경하세요.
+ local backup_conf="${nginx_conf}.backup" + if [ ! -f "$nginx_conf" ]; then error_exit "nginx configuration not found at $nginx_conf" fi + + log "Backing up current nginx configuration" + sudo cp "$nginx_conf" "$backup_conf" log "Switching nginx upstream to port $new_port" 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 + if [ -f "$backup_conf" ]; then + log "Restoring from backup configuration" + sudo cp "$backup_conf" "$nginx_conf" + fi return 1 fiscripts/dev/zero-downtime-deploy.sh (1)
153-185: ** 심각: git checkout 롤백 메커니즘 신뢰성 부족**배포 호스트에 git 저장소가 없거나 nginx 설정이 버전 관리 외부에 있을 경우
git checkout이 실패합니다. nginx 설정 백업은 빠른 롤백을 위해 중요합니다.switch_nginx_upstream() { local new_port=$1 local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com" local temp_conf="/tmp/api.dev.debate-timer.com.tmp" + local backup_conf="${nginx_conf}.backup" if [ ! -f "$nginx_conf" ]; then error_exit "nginx configuration not found at $nginx_conf" fi + + log "Backing up current nginx configuration" + sudo cp "$nginx_conf" "$backup_conf" log "Switching nginx upstream to port $new_port" 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 + if [ -f "$backup_conf" ]; then + log "Restoring from backup configuration" + sudo cp "$backup_conf" "$nginx_conf" + else + log "ERROR: Backup configuration not found at $backup_conf" + return 1 + fi return 1 fi
🧹 Nitpick comments (4)
scripts/dev/zero-downtime-deploy.sh (4)
17-20: Shellcheck 경고 해결: 변수 선언 및 출력 리다이렉션 개선리다이렉션을 sudo 명령에 적용하면 내부 명령에 적용되지 않습니다. 또한 선언과 할당을 분리하여 반환값을 보호해야 합니다.
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" }
61-86: SC2155 경고: 변수 선언과 할당 분리
$!캡처는 신뢰할 수 있으나, 변수 선언과 할당을 분리하여 반환값을 보호하세요.kill_process_on_port() { local port=$1 - local pid=$(sudo lsof -t -i:$port 2>/dev/null) + local pid + pid=$(sudo lsof -t -i:$port 2>/dev/null) if [ -z "$pid" ]; then log "No process running on port $port" return 0 fi
88-112: SC2155 경고: 변수 할당 분리 (낮은 우선순위)비파괴적 명령이지만 일관성을 위해 분리를 권장합니다.
- local status=$(curl -s -o /dev/null -w "%{http_code}" "$health_url" 2>/dev/null || echo "000") + local status + status=$(curl -s -o /dev/null -w "%{http_code}" "$health_url" 2>/dev/null || echo "000")
187-190: SC2155 경고: main 함수의 변수 할당- local current_port=$(get_current_port) - local new_port=$(get_inactive_port "$current_port") - local new_monitor_port=$(get_monitor_port "$new_port") + local current_port + local new_port + local new_monitor_port + current_port=$(get_current_port) + new_port=$(get_inactive_port "$current_port") + new_monitor_port=$(get_monitor_port "$new_port")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/dev/zero-downtime-deploy.sh(1 hunks)scripts/prod/zero-downtime-deploy.sh(1 hunks)
🧰 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] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
[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] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
[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). (1)
- GitHub Check: build-and-push
🔇 Additional comments (2)
scripts/dev/zero-downtime-deploy.sh (1)
27-53: 포트 헬퍼 함수들이 올바르게 구현됨논리가 명확하고 포트 관리 및 초기화가 안전하게 처리됩니다.
scripts/prod/zero-downtime-deploy.sh (1)
27-53: 포트 헬퍼 함수들이 올바르게 구현됨
| sudo nohup java \ | ||
| -Dspring.profiles.active=$PROFILE,monitor \ | ||
| -Duser.timezone=$TIMEZONE \ | ||
| -Dserver.port=$port \ | ||
| -Dmanagement.server.port=$monitor_port \ | ||
| -Ddd.service=debate-timer \ | ||
| -Ddd.env=$PROFILE \ | ||
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if file exists and read lines 134-141
if [ -f "scripts/dev/zero-downtime-deploy.sh" ]; then
echo "=== Lines 130-145 of zero-downtime-deploy.sh ==="
sed -n '130,145p' scripts/dev/zero-downtime-deploy.sh | cat -n
else
echo "File not found"
fiRepository: debate-timer/debate-timer-be
Length of output: 704
🌐 Web query:
sudo output redirection shell behavior does redirection happen inside or outside sudo command
💡 Result:
The shell (your unprivileged shell) performs the redirection before sudo runs the command — so sudo applies to the command being run, not to the > or >> open that writes the file. [1][2][3]
Common fixes:
- Run the whole thing under a root shell: sudo bash -c 'echo "text" > /path/file'. [1]
- Use tee so the writing is done by a sudo'd process: echo "text" | sudo tee /path/file >/dev/null (use -a to append). [2][3]
Sources: [1] Stack Overflow, [2] Baeldung, [3] ShellHacks.
🏁 Script executed:
# Also check for application logging configuration
echo "=== Check for logging in application configuration files ==="
find . -type f \( -name "application*.yml" -o -name "application*.properties" \) 2>/dev/null | head -5
echo -e "\n=== Check surrounding context of the deploy script ==="
if [ -f "scripts/dev/zero-downtime-deploy.sh" ]; then
wc -l scripts/dev/zero-downtime-deploy.sh
fiRepository: debate-timer/debate-timer-be
Length of output: 437
Critical: sudo does not apply to output redirection (SC2024 logic issue)
The redirection > "$APP_DIR/app-$port.log" 2>&1 is evaluated by the shell before sudo executes, so the log file will be written with unprivileged permissions and likely fail. Wrap the command in a shell invocation to apply redirection inside the sudo context:
- sudo nohup java \
+ sudo sh -c 'nohup java \
-Dspring.profiles.active=$PROFILE,monitor \
-Duser.timezone=$TIMEZONE \
-Dserver.port=$port \
-Dmanagement.server.port=$monitor_port \
-Ddd.service=debate-timer \
-Ddd.env=$PROFILE \
- -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &
+ -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &'Alternatively, verify that Java logging is properly configured via logging.file.name in your application.yml.
📝 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.
| sudo nohup java \ | |
| -Dspring.profiles.active=$PROFILE,monitor \ | |
| -Duser.timezone=$TIMEZONE \ | |
| -Dserver.port=$port \ | |
| -Dmanagement.server.port=$monitor_port \ | |
| -Ddd.service=debate-timer \ | |
| -Ddd.env=$PROFILE \ | |
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & | |
| sudo sh -c 'nohup java \ | |
| -Dspring.profiles.active=$PROFILE,monitor \ | |
| -Duser.timezone=$TIMEZONE \ | |
| -Dserver.port=$port \ | |
| -Dmanagement.server.port=$monitor_port \ | |
| -Ddd.service=debate-timer \ | |
| -Ddd.env=$PROFILE \ | |
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &' |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
🤖 Prompt for AI Agents
In scripts/dev/zero-downtime-deploy.sh around lines 134-141 the shell
redirection is evaluated before sudo runs so the log file is created with
unprivileged permissions; fix by running the entire java invocation (including
its redirection) inside a root shell invoked by sudo so the redirection is
performed as root, or alternatively pipe the process output through a root-owned
writer (e.g., sudo tee) or configure Java logging via logging.file.name in
application.yml so the app writes logs itself.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/nginx-switch-port.sh (3)
9-12: 로깅 함수를 다른 배포 스크립트와 일관되게 수정
log()함수가scripts/dev/zero-downtime-deploy.sh와scripts/prod/zero-downtime-deploy.sh의 로깅 패턴과 일치하지 않습니다. 배포 스크립트들은tee -a "$LOG_FILE"을 사용하여 로그를 파일에 저장하지만, 이 스크립트는 stdout으로만 출력합니다. 무중단 배포 실패 디버깅을 위해 로그를 파일에 저장하는 것이 중요합니다.LOG_FILE 변수를 정의하고 로그를 파일에 저장하도록 일관되게 수정하세요.
#!/bin/bash set -e 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" +LOG_DIR="/var/log/debate-timer" +LOG_FILE="$LOG_DIR/nginx-switch-port.log" + +mkdir -p "$LOG_DIR" log() { local timestamp timestamp=$(date '+%Y-%m-%d %H:%M:%S') - echo "${timestamp}" "$@" + echo "${timestamp}" "$@" | tee -a "$LOG_FILE" }
5-7: 설정 경로 파라미터화 고려nginx 설정 경로가
api.dev.debate-timer.com으로 하드코딩되어 있습니다. 프로덕션 환경 배포 시에도 이 스크립트가 사용된다면, 경로를 매개변수로 받거나 호출 스크립트에서 환경 변수를 통해 전달받는 것이 좋습니다.현재 dev 환경 전용으로 의도된 것이라면 이 주석을 무시해도 됩니다.
63-65: 상태 확인 실패 시 롤백 추가 고려상태 확인이 실패할 경우 (라인 63-65), 스크립트가 오류를 보고하지만 이전 nginx 설정으로 자동 롤백하지 않습니다. 상태 확인 실패는 upstream 포트 전환이 문제가 있음을 의미하므로, 안전성을 위해 자동 롤백을 추가하는 것이 좋습니다.
else log "Health check failed after nginx reload (status: $HEALTH_STATUS)" log "nginx may not be routing to the correct backend" + log "Rolling back to previous configuration" + sudo cp "$BACKUP_CONF" "$NGINX_CONF" + sudo nginx -s reload exit 1 fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/nginx-switch-port.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/nginx-switch-port.sh (2)
scripts/prod/zero-downtime-deploy.sh (1)
log(17-20)scripts/dev/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)
⏰ 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). (1)
- GitHub Check: build-and-push
🔇 Additional comments (1)
scripts/nginx-switch-port.sh (1)
32-49: 이전 문제 해결됨: sed 패턴이 올바르게 수정됨라인 36의 sed 패턴이 이전 리뷰의 중요 이슈를 해결했습니다.
server 127.0.0.1:패턴을 올바르게 사용하고 있으며, 백업, 검증, 롤백 전략이 탄탄합니다. 무중단 배포를 위한 안전한 업스트림 전환 메커니즘이 잘 구현되었습니다.
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "${timestamp} $@" |
There was a problem hiding this comment.
Shellcheck 경고 수정: 선언과 할당을 분리하고 $@ 인용 처리
SC2155 및 SC2145 경고를 해결해야 합니다:
- Line 10:
local과 할당을 분리하여 date 명령의 반환값 마스킹을 피합니다. - Line 11:
$@를 별도의 인용된 인자로 처리합니다.
다음 diff를 적용하세요:
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.
| 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 10 to 11, split the local
declaration and the date assignment to avoid masking the command substitution
(use `local timestamp` on its own, then assign timestamp=$(date ...) on the next
line) and change the echo to pass the timestamp and the script arguments as
separate, quoted parameters (use echo "${timestamp}" "$@" so each positional
argument is preserved and properly quoted).
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
안녕하세요. 무중단 배포를 위한 PR 잘 보았습니다. Blue/Green 배포 전략을 구현하기 위해 Nginx 설정, 배포 스크립트, 그리고 스프링 부트의 graceful shutdown 설정을 추가하신 점이 인상적입니다. 전체적인 로직은 매우 견고하게 작성되었습니다.
다만, 코드의 유지보수성을 높이고 안정성을 더욱 향상시키기 위해 몇 가지 제안 사항을 리뷰 코멘트로 남겼습니다.
dev와prod환경의 배포 스크립트가 거의 동일하여 중복이 발생합니다. 이를 하나의 스크립트로 통합하는 것을 제안합니다.- Nginx 설정 롤백 로직에서
git checkout을 사용하는 대신, 설정 파일의 백업본을 만들어 사용하는 것이 더 안정적입니다.
자세한 내용은 각 파일의 코멘트를 확인해주세요. 감사합니다.
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| APP_DIR="/home/ubuntu/app" | ||
| PORT_FILE="$APP_DIR/current_port.txt" | ||
| LOG_FILE="$APP_DIR/deploy.log" | ||
| BLUE_PORT=8080 | ||
| GREEN_PORT=8081 | ||
| BLUE_MONITOR_PORT=8083 | ||
| GREEN_MONITOR_PORT=8084 | ||
| MAX_HEALTH_CHECK_RETRIES=60 | ||
| HEALTH_CHECK_INTERVAL=2 | ||
| PROFILE="dev" | ||
| TIMEZONE="Asia/Seoul" | ||
|
|
||
| log() { | ||
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "${timestamp} $@" | tee -a "$LOG_FILE" | ||
| } | ||
|
|
||
| error_exit() { | ||
| log "$1" | ||
| exit 1 | ||
| } | ||
|
|
||
| get_current_port() { | ||
| if [ ! -f "$PORT_FILE" ]; then | ||
| log "Port file not found. Initializing with default port $BLUE_PORT" | ||
| echo "$BLUE_PORT" > "$PORT_FILE" | ||
| echo "$BLUE_PORT" | ||
| else | ||
| cat "$PORT_FILE" | ||
| fi | ||
| } | ||
|
|
||
| get_inactive_port() { | ||
| local current_port=$1 | ||
| if [ "$current_port" -eq "$BLUE_PORT" ]; then | ||
| echo "$GREEN_PORT" | ||
| else | ||
| echo "$BLUE_PORT" | ||
| fi | ||
| } | ||
|
|
||
| get_monitor_port() { | ||
| local app_port=$1 | ||
| if [ "$app_port" -eq "$BLUE_PORT" ]; then | ||
| echo "$BLUE_MONITOR_PORT" | ||
| else | ||
| echo "$GREEN_MONITOR_PORT" | ||
| fi | ||
| } | ||
|
|
||
| is_port_in_use() { | ||
| local port=$1 | ||
| sudo lsof -t -i:$port > /dev/null 2>&1 | ||
| return $? | ||
| } | ||
|
|
||
| kill_process_on_port() { | ||
| local port=$1 | ||
| local pid=$(sudo lsof -t -i:$port 2>/dev/null) | ||
|
|
||
| if [ -z "$pid" ]; then | ||
| log "No process running on port $port" | ||
| return 0 | ||
| fi | ||
|
|
||
| log "Sending graceful shutdown signal to process $pid on port $port" | ||
| sudo kill -15 "$pid" | ||
|
|
||
| local wait_count=0 | ||
| while [ $wait_count -lt 65 ] && is_port_in_use "$port"; do | ||
| sleep 1 | ||
| wait_count=$((wait_count + 1)) | ||
| done | ||
|
|
||
| if is_port_in_use "$port"; then | ||
| log "Process didn't stop gracefully, forcing shutdown" | ||
| sudo kill -9 "$pid" 2>/dev/null || true | ||
| sleep 2 | ||
| fi | ||
|
|
||
| log "Process on port $port stopped successfully" | ||
| } | ||
|
|
||
| health_check() { | ||
| local port=$1 | ||
| local monitor_port=$2 | ||
| local health_url="http://localhost:$monitor_port/monitoring/health" | ||
|
|
||
| log "Starting health check for port $port (monitor: $monitor_port)" | ||
|
|
||
| local retry=1 | ||
| while [ $retry -le $MAX_HEALTH_CHECK_RETRIES ]; do | ||
| local status=$(curl -s -o /dev/null -w "%{http_code}" "$health_url" 2>/dev/null || echo "000") | ||
|
|
||
| log "Health check attempt $retry/$MAX_HEALTH_CHECK_RETRIES - Status: $status" | ||
|
|
||
| if [ "$status" = "200" ]; then | ||
| log "Health check passed!" | ||
| return 0 | ||
| fi | ||
|
|
||
| sleep $HEALTH_CHECK_INTERVAL | ||
| retry=$((retry + 1)) | ||
| done | ||
|
|
||
| log "Health check failed after $MAX_HEALTH_CHECK_RETRIES attempts" | ||
| return 1 | ||
| } | ||
|
|
||
| start_application() { | ||
| local port=$1 | ||
| local monitor_port=$2 | ||
| local staging_jar="$APP_DIR/staging/app.jar" | ||
| local jar_file="$APP_DIR/app-$port.jar" | ||
|
|
||
| if [ ! -f "$staging_jar" ]; then | ||
| error_exit "No JAR file found in staging directory: $staging_jar" | ||
| fi | ||
|
|
||
| log "Copying JAR from staging to $jar_file" | ||
| cp "$staging_jar" "$jar_file" | ||
|
|
||
| log "Starting application on port $port with JAR: $jar_file" | ||
|
|
||
| if is_port_in_use "$port"; then | ||
| log "Port $port is in use, cleaning up..." | ||
| kill_process_on_port "$port" | ||
| fi | ||
|
|
||
| sudo nohup java \ | ||
| -Dspring.profiles.active=$PROFILE,monitor \ | ||
| -Duser.timezone=$TIMEZONE \ | ||
| -Dserver.port=$port \ | ||
| -Dmanagement.server.port=$monitor_port \ | ||
| -Ddd.service=debate-timer \ | ||
| -Ddd.env=$PROFILE \ | ||
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & | ||
|
|
||
| local pid=$! | ||
| log "Application started with PID: $pid" | ||
|
|
||
| sleep 3 | ||
|
|
||
| if ! kill -0 $pid 2>/dev/null; then | ||
| error_exit "Application process died immediately after start. Check logs at $APP_DIR/app-$port.log" | ||
| fi | ||
| } | ||
|
|
||
| switch_nginx_upstream() { | ||
| local new_port=$1 | ||
| local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com" | ||
| 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" | ||
|
|
||
| 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 | ||
| return 1 | ||
| fi | ||
|
|
||
| sudo nginx -s reload | ||
| log "nginx reloaded successfully" | ||
|
|
||
| sleep 2 | ||
| local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000") | ||
| if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then | ||
| log "nginx health check failed after reload (status: $response)" | ||
| return 1 | ||
| fi | ||
|
|
||
| log "nginx is now routing traffic to port $new_port" | ||
| return 0 | ||
| } | ||
|
|
||
| main() { | ||
| local current_port=$(get_current_port) | ||
| local new_port=$(get_inactive_port "$current_port") | ||
| local new_monitor_port=$(get_monitor_port "$new_port") | ||
|
|
||
| log "Current active port: $current_port" | ||
| log "Deploying to port: $new_port" | ||
| log "Monitor port: $new_monitor_port" | ||
|
|
||
| log "Step 1/4: Starting new version on port $new_port" | ||
| start_application "$new_port" "$new_monitor_port" | ||
|
|
||
| log "Step 2/4: Performing health check" | ||
| if ! health_check "$new_port" "$new_monitor_port"; then | ||
| log "Deployment failed: Health check did not pass" | ||
| log "Rolling back: Stopping new version on port $new_port" | ||
| kill_process_on_port "$new_port" | ||
| error_exit "Deployment aborted due to health check failure" | ||
| fi | ||
|
|
||
| log "Step 3/4: Switching nginx to new version" | ||
| if ! switch_nginx_upstream "$new_port"; then | ||
| log "nginx switch failed, rolling back" | ||
| kill_process_on_port "$new_port" | ||
| error_exit "Deployment aborted due to nginx switch failure" | ||
| fi | ||
|
|
||
| log "Step 4/4: Stopping old version on port $current_port" | ||
| kill_process_on_port "$current_port" | ||
|
|
||
| local old_jar="$APP_DIR/app-$current_port.jar" | ||
| if [ -f "$old_jar" ]; then | ||
| log "Removing old JAR file: $old_jar" | ||
| rm -f "$old_jar" | ||
| fi | ||
|
|
||
| echo "$new_port" > "$PORT_FILE" | ||
| log "Updated active port file to $new_port" | ||
|
|
||
| log "Deployment completed successfully!" | ||
| log "Active port: $new_port" | ||
| log "Inactive port: $current_port" | ||
| } | ||
|
|
||
| main "$@" |
There was a problem hiding this comment.
dev와 prod 환경의 배포 스크립트(scripts/dev/zero-downtime-deploy.sh와 scripts/prod/zero-downtime-deploy.sh) 내용이 거의 동일합니다. 이렇게 코드가 중복되면 향후 스크립트 수정 시 두 파일을 모두 변경해야 하므로 실수가 발생할 수 있고 유지보수가 어렵습니다.
이 두 스크립트를 하나의 파일로 통합하고, dev 또는 prod와 같은 환경명을 인자로 받아 처리하도록 리팩토링하는 것을 강력히 권장합니다.
예를 들어, 다음과 같이 스크립트를 실행할 수 있습니다.
./scripts/zero-downtime-deploy.sh dev
./scripts/zero-downtime-deploy.sh prod스크립트 내부에서는 인자로 받은 환경명에 따라 PROFILE 변수와 nginx_conf 경로를 동적으로 설정할 수 있습니다.
#!/bin/bash
set -e
# ... (공통 변수들) ...
# 인자로 환경 받기
if [ -z "$1" ]; then
echo "Usage: $0 <dev|prod>"
exit 1
fi
ENV=$1
PROFILE=$1 # dev or prod
# ... (이후 로직에서 NGINX_CONF_PATH 등을 ENV 기반으로 설정) ...이렇게 변경하면 코드 중복을 제거하고 스크립트를 더 쉽게 관리할 수 있습니다.
scripts/dev/zero-downtime-deploy.sh
Outdated
| git checkout "$nginx_conf" 2>/dev/null || true | ||
| return 1 |
There was a problem hiding this comment.
Nginx 설정 테스트(nginx -t)가 실패했을 때 git checkout을 사용하여 롤백하는 방식은 불안정합니다. EC2 인스턴스의 해당 경로에 .git 디렉토리가 없거나, git 추적 상태가 아닐 수 있어 예기치 않게 동작할 수 있습니다.
nginx-switch-port.sh 스크립트에서처럼, 설정을 변경하기 전에 백업 파일을 만들고 실패 시 해당 백업 파일로 복원하는 것이 훨씬 안정적입니다.
switch_nginx_upstream 함수를 아래와 같이 수정하는 것을 제안합니다.
switch_nginx_upstream() {
local new_port=$1
local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com"
local temp_conf="/tmp/api.dev.debate-timer.com.tmp"
local backup_conf="${nginx_conf}.bak"
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, rolling back."
sudo cp "$backup_conf" "$nginx_conf"
sudo rm "$backup_conf"
return 1
fi
sudo nginx -s reload
log "nginx reloaded successfully"
sleep 2
local response=$(curl -s -o /dev/null -w "%"{http_code}"" "http://localhost/" 2>/dev/null || echo "000")
if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then
log "nginx health check failed after reload (status: $response). Rolling back nginx config."
sudo cp "$backup_conf" "$nginx_conf"
sudo nginx -s reload
sudo rm "$backup_conf"
return 1
fi
log "nginx is now routing traffic to port $new_port"
sudo rm "$backup_conf"
return 0
}이 방식은 git에 대한 의존성을 제거하고 배포 스크립트의 안정성을 높여줍니다.
| 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.
이 스크립트는 Nginx 설정을 수동으로 전환하는 유용한 유틸리티로 보입니다. 하지만 현재 NGINX_CONF 경로가 dev 환경으로 하드코딩되어 있어 prod 환경에서는 사용할 수 없습니다.
스크립트의 재사용성을 높이기 위해, 포트 번호와 함께 환경(dev 또는 prod)도 인자로 받도록 수정하는 것을 고려해보세요.
예시:
# Usage: ./scripts/nginx-switch-port.sh <environment> <port>
./scripts/nginx-switch-port.sh dev 8081이렇게 하면 스크립트 상단의 경로 변수들을 동적으로 설정할 수 있어 dev와 prod 환경 모두에서 동일한 스크립트를 사용할 수 있습니다.
There was a problem hiding this comment.
이거 리뷰 한번 확인해주면 좋을 것 같아요
There was a problem hiding this comment.
이 파일은 실제로 배포시 활용하지 않습니다. 단지 나중에 배포 후 문제가 발생하면 어떻게 롤백을 시킬지 스크립트를 작성한 것입니다.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
scripts/prod/zero-downtime-deploy.sh (2)
1-238: 권장: dev와 prod 배포 스크립트 통합prod 스크립트는 dev 스크립트와 거의 동일하며, 환경별 설정(PROFILE, nginx 경로)만 다릅니다. 코드 중복을 제거하고 단일 스크립트로 통합하여 유지보수성을 향상시키는 것을 권장합니다.
자세한 리팩토링 방안은 dev/zero-downtime-deploy.sh 리뷰 코멘트를 참고하세요.
180-180: curl 형식 문자열 구문 오류Line 180의
"%"{http_code}""형식 문자열에는 괄호 불일치가 있습니다. 올바른 형식은"%{http_code}"입니다. dev 스크립트와 동일하게 수정해야 합니다.- 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/" 2>/dev/null || echo "000")scripts/dev/zero-downtime-deploy.sh (1)
134-141:⚠️ 중대: sudo는 출력 리다이렉션에 영향을 주지 않습니다 (SC2024)Line 141의 리다이렉션(
> "$APP_DIR/app-$port.log" 2>&1)은sudo가 실행되기 전에 셸에서 평가되므로, 로그 파일이 권한 없는 사용자로 생성되어 쓰기 실패할 가능성이 있습니다.전체 Java 명령어를
sudo sh -c로 감싸서 리다이렉션이 root 컨텍스트 내에서 수행되도록 하거나,logging.file.name설정을 통해 애플리케이션에서 로그를 처리하도록 해야 합니다.- sudo nohup java \ + sudo sh -c 'nohup java \ -Dspring.profiles.active=$PROFILE,monitor \ -Duser.timezone=$TIMEZONE \ -Dserver.port=$port \ -Dmanagement.server.port=$monitor_port \ -Ddd.service=debate-timer \ -Ddd.env=$PROFILE \ - -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & + -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &'
🧹 Nitpick comments (1)
scripts/dev/zero-downtime-deploy.sh (1)
1-238: 권장: dev와 prod 배포 스크립트 통합
scripts/dev/zero-downtime-deploy.sh와scripts/prod/zero-downtime-deploy.sh의 코드가 거의 동일합니다(환경별 설정만 다름). 이렇게 중복되면 버그 수정이나 개선 사항을 적용할 때 두 파일을 모두 수정해야 하므로 실수가 발생할 수 있습니다.하나의 스크립트로 통합하고
dev또는prod를 인자로 받아 환경별 설정을 동적으로 적용하도록 리팩토링하는 것을 권장합니다:./scripts/zero-downtime-deploy.sh dev ./scripts/zero-downtime-deploy.sh prod#!/bin/bash set -e # 공통 설정 APP_DIR="/home/ubuntu/app" BLUE_PORT=8080 GREEN_PORT=8081 # ... 기타 공통 변수 ... # 환경 인자 받기 if [ -z "$1" ]; then echo "Usage: $0 <dev|prod>" exit 1 fi ENV=$1 PROFILE=$ENV # 환경별 nginx 설정 경로 if [ "$ENV" = "dev" ]; then NGINX_CONF="/etc/nginx/sites-available/api.dev.debate-timer.com" elif [ "$ENV" = "prod" ]; then NGINX_CONF="/etc/nginx/sites-available/api.prod.debate-timer.com" else echo "Invalid environment: $ENV" exit 1 fi # ... 나머지 함수들 ...
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/Dev_CD.yml(2 hunks)scripts/dev/zero-downtime-deploy.sh(1 hunks)scripts/prod/zero-downtime-deploy.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/prod/zero-downtime-deploy.sh (2)
scripts/dev/zero-downtime-deploy.sh (10)
log(17-20)is_port_in_use(55-59)error_exit(22-25)kill_process_on_port(61-86)get_current_port(27-35)get_inactive_port(37-44)get_monitor_port(46-53)start_application(114-151)health_check(88-112)switch_nginx_upstream(153-192)scripts/nginx-switch-port.sh (1)
log(9-12)
🪛 Shellcheck (0.11.0)
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] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
[warning] 180-180: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 180-180: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 180-180: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 195-195: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 196-196: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 197-197: Declare and assign separately to avoid masking return values.
(SC2155)
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] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
[warning] 180-180: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 180-180: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 180-180: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 195-195: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 196-196: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 197-197: 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)
.github/workflows/Dev_CD.yml (3)
63-63: 배포 아티팩트 경로 변경 승인스테이징 디렉토리로의 경로 변경(
~/app/staging)은 블루-그린 배포 워크플로우에 적절합니다. 이를 통해 기존 활성 버전과 새로운 버전을 명확하게 분리할 수 있습니다.
71-78: 로그 디렉토리 설정 및 권한 설정 승인로그 디렉토리 생성, 소유권 설정, 권한 설정이 올바르게 구성되었습니다.
chmod 755는 사용자가 읽기/쓰기 권한을 가질 수 있도록 합니다.
80-81: 배포 워크플로우 통합 승인별도의 헬스 체크 및 버전 교체 단계를 단일 무중단 배포 단계로 통합한 것이 좋습니다. 이는 배포 프로세스를 더 명확하고 유지보수하기 쉽게 만듭니다.
| sudo nohup java \ | ||
| -Dspring.profiles.active=$PROFILE,monitor \ | ||
| -Duser.timezone=$TIMEZONE \ | ||
| -Dserver.port=$port \ | ||
| -Dmanagement.server.port=$monitor_port \ | ||
| -Ddd.service=debate-timer \ | ||
| -Ddd.env=$PROFILE \ | ||
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & |
There was a problem hiding this comment.
Line 141의 리다이렉션(> "$APP_DIR/app-$port.log" 2>&1)은 sudo가 실행되기 전에 셸에서 평가되므로, 로그 파일이 권한 없는 사용자로 생성되어 쓰기 실패할 가능성이 있습니다.
dev 스크립트와 동일하게 수정해야 합니다:
- sudo nohup java \
+ sudo sh -c 'nohup java \
-Dspring.profiles.active=$PROFILE,monitor \
-Duser.timezone=$TIMEZONE \
-Dserver.port=$port \
-Dmanagement.server.port=$monitor_port \
-Ddd.service=debate-timer \
-Ddd.env=$PROFILE \
- -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &
+ -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &'📝 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.
| sudo nohup java \ | |
| -Dspring.profiles.active=$PROFILE,monitor \ | |
| -Duser.timezone=$TIMEZONE \ | |
| -Dserver.port=$port \ | |
| -Dmanagement.server.port=$monitor_port \ | |
| -Ddd.service=debate-timer \ | |
| -Ddd.env=$PROFILE \ | |
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & | |
| sudo sh -c 'nohup java \ | |
| -Dspring.profiles.active=$PROFILE,monitor \ | |
| -Duser.timezone=$TIMEZONE \ | |
| -Dserver.port=$port \ | |
| -Dmanagement.server.port=$monitor_port \ | |
| -Ddd.service=debate-timer \ | |
| -Ddd.env=$PROFILE \ | |
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &' |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file
(SC2024)
🤖 Prompt for AI Agents
In scripts/prod/zero-downtime-deploy.sh around lines 134 to 141, the output
redirection is evaluated by the calling shell (so the log file may be created by
an unprivileged user), causing write/permission failures; change the invocation
so that the entire nohup/java command including its redirection runs under sudo
(for example wrap the command in sudo bash -c 'nohup java ... >
"$APP_DIR/app-$port.log" 2>&1 &' or use sudo tee in the pipeline) so the log
file is created/written by the elevated user and the process still runs in
background.
|
/gemini review |
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| APP_DIR="/home/ubuntu/app" | ||
| PORT_FILE="$APP_DIR/current_port.txt" | ||
| LOG_FILE="$APP_DIR/deploy.log" | ||
| BLUE_PORT=8080 | ||
| GREEN_PORT=8081 | ||
| BLUE_MONITOR_PORT=8083 | ||
| GREEN_MONITOR_PORT=8084 | ||
| MAX_HEALTH_CHECK_RETRIES=60 | ||
| HEALTH_CHECK_INTERVAL=2 | ||
| PROFILE="dev" | ||
| TIMEZONE="Asia/Seoul" | ||
|
|
||
| log() { | ||
| local timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "${timestamp} $@" | tee -a "$LOG_FILE" | ||
| } | ||
|
|
||
| error_exit() { | ||
| log "$1" | ||
| exit 1 | ||
| } | ||
|
|
||
| get_current_port() { | ||
| if [ ! -f "$PORT_FILE" ]; then | ||
| log "Port file not found. Initializing with default port $BLUE_PORT" | ||
| echo "$BLUE_PORT" > "$PORT_FILE" | ||
| echo "$BLUE_PORT" | ||
| else | ||
| cat "$PORT_FILE" | ||
| fi | ||
| } | ||
|
|
||
| get_inactive_port() { | ||
| local current_port=$1 | ||
| if [ "$current_port" -eq "$BLUE_PORT" ]; then | ||
| echo "$GREEN_PORT" | ||
| else | ||
| echo "$BLUE_PORT" | ||
| fi | ||
| } | ||
|
|
||
| get_monitor_port() { | ||
| local app_port=$1 | ||
| if [ "$app_port" -eq "$BLUE_PORT" ]; then | ||
| echo "$BLUE_MONITOR_PORT" | ||
| else | ||
| echo "$GREEN_MONITOR_PORT" | ||
| fi | ||
| } | ||
|
|
||
| is_port_in_use() { | ||
| local port=$1 | ||
| sudo lsof -t -i:$port > /dev/null 2>&1 | ||
| return $? | ||
| } | ||
|
|
||
| kill_process_on_port() { | ||
| local port=$1 | ||
| local pid=$(sudo lsof -t -i:$port 2>/dev/null) | ||
|
|
||
| if [ -z "$pid" ]; then | ||
| log "No process running on port $port" | ||
| return 0 | ||
| fi | ||
|
|
||
| log "Sending graceful shutdown signal to process $pid on port $port" | ||
| sudo kill -15 "$pid" | ||
|
|
||
| local wait_count=0 | ||
| while [ $wait_count -lt 65 ] && is_port_in_use "$port"; do | ||
| sleep 1 | ||
| wait_count=$((wait_count + 1)) | ||
| done | ||
|
|
||
| if is_port_in_use "$port"; then | ||
| log "Process didn't stop gracefully, forcing shutdown" | ||
| sudo kill -9 "$pid" 2>/dev/null || true | ||
| sleep 2 | ||
| fi | ||
|
|
||
| log "Process on port $port stopped successfully" | ||
| } | ||
|
|
||
| health_check() { | ||
| local port=$1 | ||
| local monitor_port=$2 | ||
| local health_url="http://localhost:$monitor_port/monitoring/health" | ||
|
|
||
| log "Starting health check for port $port (monitor: $monitor_port)" | ||
|
|
||
| local retry=1 | ||
| while [ $retry -le $MAX_HEALTH_CHECK_RETRIES ]; do | ||
| local status=$(curl -s -o /dev/null -w "%{http_code}" "$health_url" 2>/dev/null || echo "000") | ||
|
|
||
| log "Health check attempt $retry/$MAX_HEALTH_CHECK_RETRIES - Status: $status" | ||
|
|
||
| if [ "$status" = "200" ]; then | ||
| log "Health check passed!" | ||
| return 0 | ||
| fi | ||
|
|
||
| sleep $HEALTH_CHECK_INTERVAL | ||
| retry=$((retry + 1)) | ||
| done | ||
|
|
||
| log "Health check failed after $MAX_HEALTH_CHECK_RETRIES attempts" | ||
| return 1 | ||
| } | ||
|
|
||
| start_application() { | ||
| local port=$1 | ||
| local monitor_port=$2 | ||
| local staging_jar="$APP_DIR/staging/app.jar" | ||
| local jar_file="$APP_DIR/app-$port.jar" | ||
|
|
||
| if [ ! -f "$staging_jar" ]; then | ||
| error_exit "No JAR file found in staging directory: $staging_jar" | ||
| fi | ||
|
|
||
| log "Copying JAR from staging to $jar_file" | ||
| cp "$staging_jar" "$jar_file" | ||
|
|
||
| log "Starting application on port $port with JAR: $jar_file" | ||
|
|
||
| if is_port_in_use "$port"; then | ||
| log "Port $port is in use, cleaning up..." | ||
| kill_process_on_port "$port" | ||
| fi | ||
|
|
||
| sudo nohup java \ | ||
| -Dspring.profiles.active=$PROFILE,monitor \ | ||
| -Duser.timezone=$TIMEZONE \ | ||
| -Dserver.port=$port \ | ||
| -Dmanagement.server.port=$monitor_port \ | ||
| -Ddd.service=debate-timer \ | ||
| -Ddd.env=$PROFILE \ | ||
| -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 & | ||
|
|
||
| local pid=$! | ||
| log "Application started with PID: $pid" | ||
|
|
||
| sleep 3 | ||
|
|
||
| if ! kill -0 $pid 2>/dev/null; then | ||
| error_exit "Application process died immediately after start. Check logs at $APP_DIR/app-$port.log" | ||
| fi | ||
| } | ||
|
|
||
| switch_nginx_upstream() { | ||
| local new_port=$1 | ||
| local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com" | ||
| local temp_conf="/tmp/api.dev.debate-timer.com.tmp" | ||
| local backup_conf="${nginx_conf}.bak" | ||
|
|
||
| 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, rolling back." | ||
| sudo cp "$backup_conf" "$nginx_conf" | ||
| sudo rm "$backup_conf" | ||
| return 1 | ||
| fi | ||
|
|
||
| sudo nginx -s reload | ||
| log "nginx reloaded successfully" | ||
|
|
||
| sleep 2 | ||
| local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000") | ||
| if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then | ||
| log "nginx health check failed after reload (status: $response). Rolling back nginx config." | ||
| sudo cp "$backup_conf" "$nginx_conf" | ||
| sudo nginx -s reload | ||
| sudo rm "$backup_conf" | ||
| return 1 | ||
| fi | ||
|
|
||
| log "nginx is now routing traffic to port $new_port" | ||
| sudo rm "$backup_conf" | ||
| return 0 | ||
| } | ||
|
|
||
| main() { | ||
| local current_port=$(get_current_port) | ||
| local new_port=$(get_inactive_port "$current_port") | ||
| local new_monitor_port=$(get_monitor_port "$new_port") | ||
|
|
||
| log "Current active port: $current_port" | ||
| log "Deploying to port: $new_port" | ||
| log "Monitor port: $new_monitor_port" | ||
|
|
||
| log "Step 1/4: Starting new version on port $new_port" | ||
| start_application "$new_port" "$new_monitor_port" | ||
|
|
||
| log "Step 2/4: Performing health check" | ||
| if ! health_check "$new_port" "$new_monitor_port"; then | ||
| log "Deployment failed: Health check did not pass" | ||
| log "Rolling back: Stopping new version on port $new_port" | ||
| kill_process_on_port "$new_port" | ||
| error_exit "Deployment aborted due to health check failure" | ||
| fi | ||
|
|
||
| log "Step 3/4: Switching nginx to new version" | ||
| if ! switch_nginx_upstream "$new_port"; then | ||
| log "nginx switch failed, rolling back" | ||
| kill_process_on_port "$new_port" | ||
| error_exit "Deployment aborted due to nginx switch failure" | ||
| fi | ||
|
|
||
| log "Step 4/4: Stopping old version on port $current_port" | ||
| kill_process_on_port "$current_port" | ||
|
|
||
| local old_jar="$APP_DIR/app-$current_port.jar" | ||
| if [ -f "$old_jar" ]; then | ||
| log "Removing old JAR file: $old_jar" | ||
| rm -f "$old_jar" | ||
| fi | ||
|
|
||
| echo "$new_port" > "$PORT_FILE" | ||
| log "Updated active port file to $new_port" | ||
|
|
||
| log "Deployment completed successfully!" | ||
| log "Active port: $new_port" | ||
| log "Inactive port: $current_port" | ||
| } | ||
|
|
||
| main "$@" |
There was a problem hiding this comment.
scripts/dev/zero-downtime-deploy.sh와 scripts/prod/zero-downtime-deploy.sh 스크립트의 내용이 거의 동일합니다. 이렇게 중복된 코드는 유지보수를 어렵게 만듭니다. 예를 들어, 로직 수정이 필요할 때 두 파일을 모두 수정해야 하는 번거로움이 있습니다.
두 스크립트를 하나로 통합하고, 환경(dev/prod)을 인자로 받아 동적으로 설정을 변경하도록 리팩토링하는 것을 강력히 권장합니다.
예시:
./zero-downtime-deploy.sh dev
이렇게 하면 PROFILE 변수와 nginx_conf 경로를 인자에 따라 설정할 수 있어 코드 중복을 제거하고 유지보수성을 크게 향상시킬 수 있습니다.
| local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000") | ||
| if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then |
There was a problem hiding this comment.
switch_nginx_upstream 함수에서 Nginx 리로드 후 상태 확인을 위해 http://localhost/를 호출하고 있습니다. 만약 루트 경로(/)가 항상 200 OK를 반환하지 않는다면, 이 확인 방법은 신뢰할 수 없습니다. 예를 들어, 애플리케이션이 정상적으로 동작하더라도 루트 경로가 404를 반환하면 배포가 성공했다고 잘못 판단할 수 있습니다.
scripts/nginx-switch-port.sh 스크립트의 55번 줄에서처럼 명시적인 health check 엔드포인트(http://localhost/monitoring/health)를 사용하고, 응답 코드가 200이 아닌 모든 경우를 실패로 처리하는 것이 더 안전하고 정확합니다. 이 변경 사항은 prod 배포 스크립트에도 동일하게 적용해야 합니다.
| local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000") | |
| if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then | |
| local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/monitoring/health" 2>/dev/null || echo "000") | |
| if [ "$response" != "200" ]; then |
| if ($host = api.dev.debate-timer.com) { | ||
| return 308 https://$host$request_uri; | ||
| } # managed by Certbot | ||
|
|
||
| listen 80; | ||
| listen [::]:80; | ||
| server_name api.dev.debate-timer.com; | ||
| return 404; # managed by Certbot |
There was a problem hiding this comment.
Nginx server 블록 내에서 if 지시어 사용은 비효율적일 수 있습니다. server_name 지시어가 이미 api.dev.debate-timer.com으로 들어오는 요청만 이 블록에서 처리하도록 보장하므로, if ($host = ...) 조건문은 불필요합니다. 또한 이 조건문 때문에 33번 줄의 return 404;는 사실상 도달할 수 없는 코드가 됩니다. 아래와 같이 if문 없이 return을 직접 사용하여 코드를 간결하게 개선하는 것을 제안합니다. 이 변경 사항은 nginx/api.prod.debate-timer.com 파일에도 동일하게 적용하는 것이 좋습니다.
listen 80;
listen [::]:80;
server_name api.dev.debate-timer.com;
return 308 https://$host$request_uri; # managed by Certbot
| switch_nginx_upstream() { | ||
| local new_port=$1 | ||
| local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com" | ||
| local temp_conf="/tmp/api.dev.debate-timer.com.tmp" |
There was a problem hiding this comment.
임시 파일 경로로 /tmp/api.dev.debate-timer.com.tmp를 하드코딩하여 사용하고 있습니다. 이 방식은 동시에 여러 배포 프로세스가 실행될 경우 파일 충돌을 일으킬 수 있는 잠재적인 위험이 있습니다.
mktemp 명령어를 사용하여 유니크한 임시 파일을 생성하는 것이 더 안전하고 견고한 방법입니다. mktemp는 예측 불가능한 파일명을 만들어주어 race condition과 같은 보안 문제를 예방합니다. 또한, trap과 함께 사용하면 스크립트 종료 시 임시 파일을 자동으로 정리할 수 있습니다.
# 스크립트 상단에 추가
temp_conf=$(mktemp)
trap 'rm -f "$temp_conf"' EXIT INT TERM이 변경 사항은 prod 배포 스크립트에도 동일하게 적용해야 합니다.
| local temp_conf="/tmp/api.dev.debate-timer.com.tmp" | |
| local temp_conf=$(mktemp) |
| 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 |
There was a problem hiding this comment.
nginx -t 명령어의 표준 에러(stderr)를 /dev/null로 리다이렉션하고 있습니다. 이렇게 하면 Nginx 설정 파일에 문법 오류가 있을 경우, 어떤 오류가 발생했는지 알 수 없어 디버깅이 어려워집니다.
배포 실패 시 원인을 빠르게 파악하기 위해 에러 메시지를 로그 파일에 기록하는 것이 좋습니다. 2>/dev/null 부분을 제거하여 에러가 로그에 남도록 수정하는 것을 제안합니다.
이 변경 사항은 prod 배포 스크립트에도 동일하게 적용해야 합니다.
| if ! sudo nginx -t 2>/dev/null; then | |
| if ! sudo nginx -t; then |
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
비토, 간단한 리뷰 몇가지 남겼습니다! 고생많으셨습니다~
| sudo kill -15 "$pid" | ||
|
|
||
| local wait_count=0 | ||
| while [ $wait_count -lt 65 ] && is_port_in_use "$port"; do |
There was a problem hiding this comment.
wait_count 65초 정도로 대기 시간을 설정한 이유가 있나요?
There was a problem hiding this comment.
큰 이유는 없습니다. shutdown의 시간보다 5초정도 더 여유를 줬습니다
| 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.
이거 리뷰 한번 확인해주면 좋을 것 같아요
| @@ -0,0 +1,238 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
zero-downtime-deploy.sh 에서 다른 점은 spring profile이나 설정 정보등으로 보이는데 파일 분리가 아니라 변경되는 부분만 변수로 넣어주는 방식도 고려해보면 좋을 것 같습니다!
There was a problem hiding this comment.
이건 기존의 스크립트 파일들도 이런 방식으로 작성되었길래 해당 컨벤션을 따라한 거였습니다. 꼭 필요하다고 판단되면 분리해보겠습니다.
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti @unifolio0
조금 걱정되는 부분이 있어 일부 코멘트 남겼습니다.
AI가 코멘트 남긴 부분까지 확인 부탁드립니다. (prod에 치명적인 리뷰도 일부 있는 것 같아요)
There was a problem hiding this comment.
nginx 파일을 전부 여기에서 관리하는 이유가 있을까요?
(일부는 Certbot 측에서 관리하다 보니, Certbot에서 해당 파일을 바꿔버리면 여기 적용 안될 것 같은데;;)
There was a problem hiding this comment.
이건 단지 코드 관리 측면에서 업로드 한 것일 뿐 실제로 깃허브에 있는 이 파일을 사용하지는 않습니다. 제가 직접 ec2에 접속해 수정한 뒤 어떻게 수정한지를 기록한 것입니다.
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
@debate-timer/backend 리뷰에 답변 남겼습니다
There was a problem hiding this comment.
이건 단지 코드 관리 측면에서 업로드 한 것일 뿐 실제로 깃허브에 있는 이 파일을 사용하지는 않습니다. 제가 직접 ec2에 접속해 수정한 뒤 어떻게 수정한지를 기록한 것입니다.
| sudo kill -15 "$pid" | ||
|
|
||
| local wait_count=0 | ||
| while [ $wait_count -lt 65 ] && is_port_in_use "$port"; do |
There was a problem hiding this comment.
큰 이유는 없습니다. shutdown의 시간보다 5초정도 더 여유를 줬습니다
| @@ -0,0 +1,238 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
이건 기존의 스크립트 파일들도 이런 방식으로 작성되었길래 해당 컨벤션을 따라한 거였습니다. 꼭 필요하다고 판단되면 분리해보겠습니다.
| 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.
이 파일은 실제로 배포시 활용하지 않습니다. 단지 나중에 배포 후 문제가 발생하면 어떻게 롤백을 시킬지 스크립트를 작성한 것입니다.
|
/noti @debate-timer/backend 리뷰에 답변 남겼습니다 |
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
비토, 답변 모두 확인했어요. prod 배포는 혹여나 실패할 가능성을 염두해두고 트래픽이 없는 새벽시간 대에 같이 하면 좋을 것 같아요.
3 participants
🚩 연관 이슈
closed #224
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
새 기능
개선 사항
기타
✏️ Tip: You can customize this high-level summary in your review settings.